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

SBI Pre-launch Adjustments #343

Merged
merged 6 commits into from
Feb 20, 2025
Merged

SBI Pre-launch Adjustments #343

merged 6 commits into from
Feb 20, 2025

Conversation

cassidysymons
Copy link
Collaborator

@cassidysymons cassidysymons commented Feb 19, 2025

This pull request addresses two issues:

  1. It better handles Null/None values being returned for cheek samples' barcode_meta fields.
  2. It adds a prompt for users who have just edited a cheek sample to take the Skin Scoring App external survey. Given that we have no insight into whether a user has actually completed the app process (by submitting a selfie), we've opted to display this message whenever a cheek sample is edited and the skin scoring app survey is available to the user.

@cassidysymons cassidysymons marked this pull request as ready for review February 19, 2025 18:35
if sample_output['sample_site'] == "Cheek":
# Format date and time to be JavaScript-friendly
if sample_output['barcode_meta'][
'sample_site_last_washed_date'
Copy link
Member

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

Copy link
Collaborator Author

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.

@@ -203,6 +203,11 @@
{{ _('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-nutrition mt-4" role="alert">
Copy link
Member

Choose a reason for hiding this comment

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

Minor but this isn't nutrition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adjusted, and adjusted the other prompt from which this was copied.

@@ -203,6 +203,11 @@
{{ _('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-nutrition 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.') }}
Copy link
Member

Choose a reason for hiding this comment

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

Why prompt rather than redirect?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. To check, does the pop up behave ok on mobile?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.

@cassidysymons
Copy link
Collaborator Author

@wasade Ready for re-review, thanks!

@cassidysymons cassidysymons merged commit 8dcdaf7 into master Feb 20, 2025
3 checks passed
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