Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JN-1467] doc request admin ux #1203

Open
wants to merge 20 commits into
base: development
Choose a base branch
from

Conversation

connorlbark
Copy link
Collaborator

@connorlbark connorlbark commented Nov 12, 2024

DESCRIPTION (include screenshots, and mobile screenshots for participant UX)

Allows creating document requests, viewing the uploaded files, and downloading them.

image

Next up will be the participant UX. For now, creating and editing a document request works like a normal survey. After refining the participant experience, I'll go back and make the admin creation of a document request more streamlined.

TO TEST: (simple manual steps for confirming core behavior -- used for pre-release checks)

Comment on lines 26 to 34
export const getSurveyContentTemplate = (survey: Survey, isScreener: boolean): string => {
if (survey.surveyType === 'OUTREACH' && !isScreener) {
return OUTREACH_SCREENER_TEMPLATE(survey)
}
if (survey.surveyType === 'DOCUMENT_REQUEST') {
return DOC_REQUEST_TEMPLATE(survey)
}
return DEFAULT_TEMPLATE
}
Copy link
Collaborator Author

@connorlbark connorlbark Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to formalize the concept of a survey template without pushing it to the backend. I'm not super happy with isScreener but it's a start. Probably right move would be to make a SurveyTemplateOptions with survey type, out reach type, blurb etc. so that we can fill these templates out more intelligently. I'll investigate on my later PR that will make document request creation streamlined.

@connorlbark connorlbark marked this pull request as ready for review November 12, 2024 17:43
Copy link
Member

@MatthewBemis MatthewBemis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks and works great!

@connorlbark connorlbark marked this pull request as draft November 14, 2024 20:01
Comment on lines 66 to 78
private void validateAnswer(Answer answer) {
answer.inferTypeIfMissing();
if (answer.getFormat() == null) {
answer.setFormat(AnswerFormat.NONE);
}

if (answer.getFormat().equals(AnswerFormat.FILE_NAME)) {
System.out.println(answer.getEnrolleeId());
System.out.println(answer.getStringValue());
participantFileService.findByEnrolleeIdAndFileName(answer.getEnrolleeId(), answer.getStringValue())
.orElseThrow(() -> new IllegalArgumentException("File not found for answer"));
}
}
Copy link
Collaborator Author

@connorlbark connorlbark Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have the new "format" column, this method will enforce the format. For now it's just file name, but if we get, e.g., date or regex formats, then this will also ensure that the string actually conforms to that format.

@@ -31,6 +31,7 @@ public class Answer extends BaseEntity {
private int surveyVersion;
private String viewedLanguage;
private AnswerType answerType;
private AnswerFormat format;
Copy link
Collaborator Author

@connorlbark connorlbark Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After much going back and forth, I think I've finally settled into being comfortable adding participant files as regular survey response answers with the caveat that we have a new "format" column that dictates how to interpret the data that's present (separate from answer type, which tells you the literal type, e.g. string)

As such, the newly created table ParticipantFileSurveyResponse had an unfortuantely lifespan of less than a week 😢

package bio.terra.pearl.core.model.survey;

public enum AnswerFormat {
NONE,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I debated TEXT as the default but that might not always be true (e.g., if it's in the number column). So I felt NONE was more appropriate for a standard "unformatted" answer.

Copy link

sonarcloud bot commented Nov 14, 2024

@connorlbark connorlbark marked this pull request as ready for review November 14, 2024 20:37
Copy link
Member

@MatthewBemis MatthewBemis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, i dig the new changes. i also really like how clean the UX is around attaching the files to the survey in the response view. as more of this code goes through, i'm liking the original choice to model these as surveys more and more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants