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

Ensure meal is only saved once #248

Merged
merged 1 commit into from
Jun 1, 2024
Merged

Conversation

MikePlante1
Copy link
Contributor

  • Disables the Save and continue button after it's pressed until the AddCarbs module is reopened.
  • Renames saved flag to noteSaved for clarity.
  • Addresses Duplicated carb entries #175

Disables `Save and continue` button once pressed by utilizing new `mealSaved` flag.
Renames `saved` to `noteSaved`.
Copy link
Contributor

@bjornoleh bjornoleh left a comment

Choose a reason for hiding this comment

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

This looks great, and seems to do what it says.

There is no noticeable impact on the workflow for normal carb entries, and the mealSaved flag should be set to true as soon as the "Save and continue" button is pressed, and should block a second entry if the app is hanging.

I also tested changing @State var mealSaved from false to true to confirm that this state would disable the "Save and continue" button. This disabled the button as expected.

LGTM!

@bjornoleh bjornoleh linked an issue Jun 1, 2024 that may be closed by this pull request
@aug0211
Copy link
Contributor

aug0211 commented Jun 1, 2024

  • Multiple days of testing in vitro and in vivo, 0 issues encountered
  • Button grays out as expected immediately on tap
  • Attempted to recreate instigating issue a number of times in vivo with a looping pump (and CGM) connected by entering carbs during loop cycle
  • 0 instances resulted in duplicate carbs, despite my best efforts to break it
  • 0 other issues observed over the course of multiple meal entries (both in vitro and in vivo)

Copy link
Contributor

@aug0211 aug0211 left a comment

Choose a reason for hiding this comment

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

See comments, tested in vitro and in vivo with 0 issue and was unable to reproduce initial issue (which is a good thing in this case).

Copy link
Contributor

@dnzxy dnzxy left a comment

Choose a reason for hiding this comment

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

Basing approval on code review only. LGTM. Precise, intentional changes. We have two test accounts by experienced testers.

@dnzxy
Copy link
Contributor

dnzxy commented Jun 1, 2024

Merging with 2 testers (in-vitro and in-vivo) and 3 approvals.

@MikePlante1
Copy link
Contributor Author

From the code, I'm pretty confident the button won't be able to be pressed twice, but I've still been spamming the "save" button every time I log meals the past few days and haven't had it trigger twice, even when purposely entering during a loop cycle.

@dnzxy dnzxy merged commit 14c7643 into nightscout:dev Jun 1, 2024
1 check passed
@MikePlante1 MikePlante1 deleted the pause_add_carbs branch June 1, 2024 15:00
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.

Duplicated carb entries
4 participants