-
Notifications
You must be signed in to change notification settings - Fork 17
SBI Pre-launch Adjustments #343
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
Changes from all commits
e4411c6
13e912c
12cf46b
aa1ecf6
8f4593d
33e6a1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -199,10 +199,15 @@ | |
{% else %} | ||
<div class="container profile-container"> | ||
{% if prompt_survey_update %} | ||
<div class="alert alert-primary alert-nutrition mt-4" role="alert"> | ||
<div class="alert alert-primary alert-kits mt-4" role="alert"> | ||
{{ _('Thank you for logging your sample information. It looks like you haven\'t updated your profile recently. Please review') }} <a href="/accounts/{{account_id}}/sources/{{source_id}}/take_survey?survey_template_id={{prompt_survey_id}}">{{ _('your survey responses') }}</a> {{ _('to ensure they\'re as current and complete as possible.') }} | ||
</div> | ||
{% endif %} | ||
{% if prompt_skin_app %} | ||
<div class="alert alert-primary alert-kits mt-4" role="alert"> | ||
{{ _('You now have access to the Skin Scoring App survey. Please return to the') }} <a href="/accounts/{{account_id}}/sources/{{source_id}}/take_survey?survey_template_id={{prompt_survey_id}}">{{ _('My Profile tab') }}</a> {{ _('to complete it.') }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why prompt rather than redirect? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accessing the Skin Scoring App isn't a simple redirect to an external site. It requires the user to open a pop-up on our side with instructions, hit a button to be allocated credentials, then hit a button to open the external site. As such, prompting the user to access it feels cleaner from a UX perspective than redirecting them to the My Profile tab with the pop-up already opened. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. To check, does the pop up behave ok on mobile? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the formatting is responsive and I've tested it using both Firefox and Chrome's developer-mode settings for a handful of different screen sizes. |
||
</div> | ||
{% endif %} | ||
{% if profile_has_samples and (need_reconsent_data or need_reconsent_biospecimen) %} | ||
<div id="consent_alert" class="alert alert-consent mt-4" role="alert"> | ||
{{ _('To update your existing samples or contribute new samples, please review the following:') }}<br /> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure whether these keys are guaranteed to exist as the private API can return an empty dict? If they are not assured to exist then they could they be accessed through
get
?https://github.com/biocore/microsetta-private-api/blob/d9653bc1059957f8afab23c4ca804810c365255b/microsetta_private_api/repo/sample_repo.py#L423
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All three keys should exist for any cheek sample, and their absence would indicate an unexpected state. However, using
get
doesn't hurt anything and mitigates the minimal potential for an error.