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

🩹 fix crash on updatePrefReminderTime #1130

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

JGreenlee
Copy link
Collaborator

@JGreenlee JGreenlee commented Feb 20, 2024

If user updates the "Time of Day for Reminders", it will store as "Invalid DateTime" and cause a crash.

newTime here is not an ISO string; it is a JavaScript Date object. Attempting to parse it as an ISO string results in "Invalid DateTime" being stored to the user prefs and a crash on the Profile screen

This fix ensures that the selected time will be stored, and "Invalid DateTime" will not get stored.

But if any user had already encountered this, and "Invalid DateTime" was already stored to their profile, they would still not be able to access the Profile screen without it crashing.

So the second change makes sure notifScheduler only attempts to schedule notifications if the datetime is valid.
If not valid, user will see this popup:

After dismissing the popup, they can change their "Time of Day for Reminders" and it will set correctly this time

`newTime` here is not an ISO string; it is a JavaScript Date object.
Attempting to parse it as an ISO string results in "Invalid DateTime" being stored to the user prefs and a crash on the Profile screen
@JGreenlee
Copy link
Collaborator Author

@shankari for merge
@sebastianbarry for visibility

Copy link

codecov bot commented Feb 20, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (dcb6957) 78.45% compared to head (e84e816) 78.37%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1130      +/-   ##
==========================================
- Coverage   78.45%   78.37%   -0.08%     
==========================================
  Files          28       28              
  Lines        1699     1702       +3     
  Branches      364      366       +2     
==========================================
+ Hits         1333     1334       +1     
- Misses        366      368       +2     
Flag Coverage Δ
unit 78.37% <50.00%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
www/js/splash/notifScheduler.ts 98.47% <50.00%> (-1.53%) ⬇️

@shankari shankari merged commit 034bee2 into e-mission:master Feb 20, 2024
6 of 8 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