👤 simplify user profile updates; refactor push notif registration - #1210
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1210 +/- ##
==========================================
- Coverage 31.67% 31.10% -0.58%
==========================================
Files 124 125 +1
Lines 4792 4816 +24
Branches 1132 1147 +15
==========================================
- Hits 1518 1498 -20
- Misses 3272 3308 +36
- Partials 2 10 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Before this change, we would make 2 calls to `profile/update` every time the UI is initialized: one for push notification registration and another to store device settings. We also had a lot of unecessary complexity in all these places (event listeners everywhere!) All we really need to do on UI init, iff onboarding is done: 1. call `profile/get` on the server to get the current user profile 2. perform push notif registration 3. get current device settings 4. based on the results of the above, determine what fields need to be updated and call `profile/update` on server if needed 1, 2, and 3 can be executed in any order/ in parallel (with Promise.all / Promise.allSettled). Added a file userProfile to handle this. The added benefit of this is that we have access to the value of the current profile. We can keep it as state in AppContext allowing it to be used anywhere in the UI (this can simplify how custom labels are retrieved + used; and it may be useful for users to know their "last synced ts", for example) When the profile is updated from the UI, we have to keep the the local state in sync so instead of directly calling commHelper updateUser directly, we expose `updateUserProfile` from AppContext In removing the complexity, I moved remoteNotifyHandler into pushNotifySettings Updated tests for all changes
TimelineContext loadTimelineEntries retrieves the pipeline range, calls updateAllUnprocessedInputs for that range, and then sets the pipeline range as state via setPipelineRange. Then in LabelTab, we have a useEffect: when pipelineRange changes, we call updateAllUnprocessedInputs (again!) setPipelineRange is not called from anywhere else, meaning that this duplicate scenario is the time this useEffect gets called. We can remove it. tested recording new labels, refreshing the page, and changing to different dates, and labels still get matched properly
JGreenlee
force-pushed
the
refactor-profile-push
branch
from
August 4, 2025 15:22
0ce83af to
9982d14
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
👤 simplify user profile updates; refactor push notif registration
Before this change, we would make 2 calls to
profile/updateevery time the UI is initialized: one for push notification registration and another to store device settings.We also had a lot of unecessary complexity in all these places (event listeners everywhere!)
All we really need to do on UI init, iff onboarding is done:
profile/geton the server to get the current user profileprofile/updateon server if needed1, 2, and 3 can be executed in any order/ in parallel (with Promise.all / Promise.allSettled). Added a file userProfile to handle this.
The added benefit of this is that we have access to the value of the current profile. We can keep it as state in AppContext allowing it to be used anywhere in the UI (this can simplify how custom labels are retrieved + used; and it may be useful for users to know their "last synced ts", for example)
When the profile is updated from the UI, we have to keep the the local state in sync so instead of directly calling commHelper updateUser directly, we expose
updateUserProfilefrom AppContextIn removing the complexity, I moved remoteNotifyHandler into pushNotifySettings
Updated tests for all changes
prevent duplicate updateAllUnprocessedInputs call
TimelineContext loadTimelineEntries retrieves the pipeline range, calls updateAllUnprocessedInputs for that range, and then sets the pipeline range as state via setPipelineRange.
Then in LabelTab, we have a useEffect: when pipelineRange changes, we call updateAllUnprocessedInputs (again!)
setPipelineRange is not called from anywhere else, meaning that this duplicate scenario is the time this useEffect gets called. We can remove it.
tested recording new labels, refreshing the page, and changing to different dates, and labels still get matched properly