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

A few component adjustments (mostly Asthma related). #217

Merged
merged 11 commits into from
Mar 6, 2024
Merged

Conversation

greinard
Copy link
Collaborator

@greinard greinard commented Mar 5, 2024

Overview

This branch contains three bug fixes.

  1. The Asthma Action Plan Manager now waits a half second before loading the action plan to give any survey submissions the necessary time to update the task run UUID custom field appropriately. On Android, we noticed that, occasionally, the custom field was not yet updated when the component attempted to reload the AAP.

  2. The HR and SpO2 charts on the Asthma Heart and Lung view have been updated to round all values to the nearest whole number rather than displaying decimal values.

  3. The "no data" state for the recent daily data bar chart has been updated to handle larger font sizes.

Security

REMINDER: All file contents are public.

  • I have ensured no secure credentials or sensitive information remain in code, metadata, comments, etc.
    • Please verify that you double checked that .storybook/preview.js does not contain your participant access key details.
    • There are no temporary testing changes committed such as API base URLs, access tokens, print/log statements, etc.
  • These changes do not introduce any security risks, or any such risks have been properly mitigated.

Describe briefly what security risks you considered, why they don't apply, or how they've been mitigated.

Checklist

Testing

Documentation

n/a

@@ -58,7 +58,7 @@ export default function (props: AsthmaActionPlanManagerProps) {
setDeviceInfo(deviceInfo);
});

loadActionPlan();
setTimeout(loadActionPlan, 500);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel slightly obligated to explore this a bit, since I think we all know that setTimeout as a solution to a problem always feels a little awkward since it doesn't really guarantee anything. In the PR Description you mention a custom field that's not yet updated - how is that update occuring? Being a little less familiar with the overall Asthma flow and only looking at the widgets in the storybook it's not very clear.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, for sure. I am not real fond of it either, but I'm not sure how else to solve it.

Here is what appears to be happening on Android:

  1. User visits a view that contains the AsthmaActionPlanManager.
  2. AsthmaActionPlanManager queries for the participant and finds an empty AAPTaskRunUUID custom field. It displays the "Tap to add photo" button.
  3. User taps to add a photo and the image capture survey gets launched.
  4. User completes the image capture survey, which has a web view step that populates the AAPTaskRunUUID custom field with the task run UUID for that survey submission.
  5. The AsthmaActionPlanManager comes back to the foreground and re-queries for the participant to check for a new value in the AAPTaskRunUUID custom field.
  6. About 50% of the time during my testing, the AAPTaskRunUUID custom field was not yet populated when the AsthmaActionPlanManager returned to the foreground and the reload occurred.

I assumed the survey submission loading indicator would remain in the foreground until all custom fields were updated as part of the survey submission, but some part of it must be asynchronous, or the survey submission loading indicator screen on Android must be closing before it completes.

Adding this slight delay to the loading seems to allow enough time to pass for the custom field to always be populated.

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'm going to tinker around with using the surveyDidFinish event instead of the applicationDidBecomeVisible event to see if that behaves more like I want it to. If so, I'll revert this change. 😃

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ajmenca - Thanks for the nudge on this. It has the desired behavior when I use the surveyDidFinish event instead. I was able to remove the setTimeout(...).

Copy link
Contributor

Choose a reason for hiding this comment

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

@greinard Can you run this scenario by @hallc and @dyerjo ... applicationDidBecomeVisible should work reliably for this

Copy link
Member

Choose a reason for hiding this comment

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

For the record, here's how it works in the iOS app. First, the app uploads the survey task result. After completing that API request, it triggers a few events simultaneously and asynchronously:

  • Uploading any auxiliary files (e.g. Active Task data files)
  • Refresh the app UI
  • Post surveyDidFinish and applicationDidBecomeVisible JS SDK events

The web content receives those SDK events ±10 milliseconds after the survey task upload API request completes. surveyDidFinish is received first, and immediately afterward (less than a millisecond) applicationDidBecomeVisible.

@greinard greinard merged commit d80e7c0 into main Mar 6, 2024
@greinard greinard deleted the AsthmaUpdates branch March 6, 2024 20:12
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.

4 participants