-
-
Notifications
You must be signed in to change notification settings - Fork 8
[WEB-4330] - Missing Pump Settings in PDF #587
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
Conversation
src/utils/DataUtil.js
Outdated
| } else { | ||
| // Clock drift on user's device may cause pumpSettings datum to have LATER timestamp than upload | ||
| // datum. The maximum time devation that Uploader allows between user device and tidepool server | ||
| // time is 15 minutes, so we search 15 minutes into the future for the pumpSettings datum |
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.
Please examine this comment for interpretable-ness .. I struggled to phrase this in a summarized but still comprehensible way
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.
@henry-tp I'm fine with the comment content. Copilot did point out a typo that could be fixed, but I don't have any strong opinion about the phrasing.
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.
Pull request overview
Adjusts pumpSettings selection for the “latest pump upload” to tolerate small clock drift, preventing missing pump settings in generated PDFs.
Changes:
- Allow selecting
pumpSettingsup to 15 minutes afterlatestPumpUpload.timefor non-continuous uploads. - Update/add unit tests to cover the >15 minute exclusion and <=15 minute inclusion behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/utils/DataUtil.js |
Relaxes pumpSettings timestamp filtering by adding a 15-minute tolerance window. |
test/utils/DataUtil.test.js |
Updates and adds tests validating pumpSettings selection with the new tolerance logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ol-org/viz into WEB-4330-missing-settings
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ol-org/viz into WEB-4330-missing-settings
krystophv
left a comment
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.
LGTM 🎸
WEB-4330
Blip: tidepool-org/blip#1853