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 timeline range issues #1183

Merged
merged 5 commits into from
Oct 9, 2024

Conversation

JGreenlee
Copy link
Collaborator

@JGreenlee JGreenlee commented Oct 8, 2024

on timeline refresh, recompute "past week" date range

Within one app session, the constants TODAY_DATE and INITIAL_DATE_RANGE do not change. On app resume, we call refreshTimeline which sets dateRange to INITIAL_DATE_RANGE. But this is a problem if the app stays running for multiple days because INITIAL_DATE_RANGE is old.

Instead we need to recompute the date range, using an up-to-date "today" date. Thus, TODAY_DATE and INITIAL_DATE_RANGE are replaced with functions: getTodayDate and getPastWeekDateRange.


fix errors on pipelineRange with null start_ts and end_ts

For a new user where the pipeline has not yet processed, pipelineRange will be { start_ts: null, end_ts: null }.
We should handle this appropriately:
-in TimelineScrollList, pipelineEndDate should be undefined instead of trying to call DateTime.fromSeconds with null
-loadDateRange should return early and do nothing
-The early return from fetchTripsInRange should return empty arrays, not void


fix re-prompting of notifications permission after denied

Within one app session, the constants TODAY_DATE and INITIAL_DATE_RANGE do not change. On app resume, we call refreshTimeline which sets dateRange to INITIAL_DATE_RANGE. But this is a problem if the app stays running for multiple days because INITIAL_DATE_RANGE is old.

Instead we need to recompute the date range, using an up-to-date "today" date. Thus, TODAY_DATE and INITIAL_DATE_RANGE are replaced with functions: getTodayDate and getPastWeekDateRange.
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

Attention: Patch coverage is 26.08696% with 17 lines in your changes missing coverage. Please review.

Project coverage is 29.44%. Comparing base (039f647) to head (6d1fcad).
Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
www/js/usePermissionStatus.ts 0.00% 10 Missing ⚠️
www/js/TimelineContext.ts 50.00% 6 Missing ⚠️
www/js/diary/list/TimelineScrollList.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1183      +/-   ##
==========================================
+ Coverage   29.42%   29.44%   +0.01%     
==========================================
  Files         122      122              
  Lines        4910     4915       +5     
  Branches     1080     1080              
==========================================
+ Hits         1445     1447       +2     
- Misses       3465     3468       +3     
Flag Coverage Δ
unit 29.44% <26.08%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
www/js/diary/list/TimelineScrollList.tsx 0.00% <0.00%> (ø)
www/js/TimelineContext.ts 51.80% <50.00%> (+0.27%) ⬆️
www/js/usePermissionStatus.ts 0.00% <0.00%> (ø)

For a new user where the pipeline has not yet processed, pipelineRange will be `{ start_ts: null, end_ts: null }`.
We should handle this appropriately:
-in TimelineScrollList, pipelineEndDate should be undefined instead of trying to call DateTime.fromSeconds with null
-loadDateRange should return early and do nothing
-The early return from fetchTripsInRange should return empty arrays, not void
@JGreenlee JGreenlee changed the title 🐛 On timeline refresh, recompute "past week" date range 🐛 Fix timeline range issues Oct 8, 2024
@shankari shankari merged commit 655f843 into e-mission:master Oct 9, 2024
7 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
Status: Tasks completed
Development

Successfully merging this pull request may close these issues.

2 participants