[web] Create a function that reassigns notifs session
ClosedPublic
Actions

Authored by tomek on Apr 12 2024, 11:13 AM.

Details

Reviewers

kamil
marcin
varun

Commits

rCOMMd6620dd49be4: [web] Create a function that reassigns notifs session

Summary

This function determines where a session were stored, where it should be placed, and moves it from one to another.

https://linear.app/comm/issue/ENG-7688/introduce-a-function-that-moves-a-notifs-session

Depends on D11647

Test Plan

Tested in combination with the next diff: open the web app, login, send a message from another client - a notif should be displayed (without this stack we can only see a notif with a warning message). Also, verify that the reassignment actually occurs by adding some console logs.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Apr 12 2024, 11:13 AM

Herald added a subscriber: ashoat. · View Herald TranscriptApr 12 2024, 11:13 AM

tomek added a child revision: D11649: [web] Reassign the notifs session.Apr 12 2024, 11:17 AM

Harbormaster completed remote builds in B28192: Diff 39082.Apr 12 2024, 11:36 AM

tomek requested review of this revision.Apr 12 2024, 11:36 AM

marcin accepted this revision.Apr 15 2024, 12:26 AM

marcin added inline comments.

web/shared-worker/worker/worker-crypto.js
368 ↗	(On Diff #39082)	Those two promises look like they could run simultaneously. However if the app crashes while those two promises run in parallel we might not be able to recover from it and retry item reassignment but if we separate those two steps we can retry item reassignment. Do I understand correctly that keeping those promises sequential was intentional?

This revision is now accepted and ready to land.Apr 15 2024, 12:26 AM

Rebase

web/shared-worker/worker/worker-crypto.js
368 ↗	(On Diff #39082)	Yes, it is safer to first set and then delete. But I'm not sure if we can recover from this currently - we probably would need to introduce some retry logic.