Page MenuHomePhabricator
Differential D11601

[native/web] introduce migration to update notifPermissionAlertInfo to alertStore in redux
ClosedPublic
Actions

Authored by ginsu on Apr 9 2024, 12:18 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 21, 1:03 AM
Unknown Object (File)
Apr 17 2024, 8:30 AM
Unknown Object (File)
Apr 16 2024, 7:49 AM
Unknown Object (File)
Apr 15 2024, 1:57 PM
Unknown Object (File)
Apr 15 2024, 11:03 AM
Unknown Object (File)
Apr 14 2024, 5:48 PM
Unknown Object (File)
Apr 13 2024, 4:44 PM
Unknown Object (File)
Apr 13 2024, 2:25 PM
View All 14 Files
Subscribers
ashoat
kamil
tomek

Details

Reviewers
atul
inka
kamil
Commits
rCOMMdbc17db48d23: [native/web] introduce migration to update notifPermissionAlertInfo to…
Summary

This diff introduces a migration to update notifPermissionAlertInfo to alertStore in redux. After this migration all the alerts regarding rehydration warning no longer appear

Adding @kamil as an extra reviewer to make sure my changes to persistWhitelist make sense

Linear task: https://linear.app/comm/issue/ENG-7624/introduce-new-migration-to-update-notifpermissionalertinfo-to

Depends on D11600

Test Plan

The rehydration alerts no longer appear + confirmed that the notif permission alert works as expected

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Apr 9 2024, 12:18 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptApr 9 2024, 12:18 PM
ginsu edited the summary of this revision. (Show Details)Apr 9 2024, 12:19 PM
ginsu added reviewers: atul, inka.
ginsu added a parent revision: D11600: [lib/native/web] refactor recordNotifPermissionAlertActionType to recordAlertActionType.
ginsu added a reviewer: kamil.
ginsu edited the summary of this revision. (Show Details)
ginsu added inline comments.Apr 9 2024, 12:24 PM
web/redux/persist.js
51 ↗(On Diff #38956)

@kamil wondering if this change here is okay/wondering if there are other things I need to do to change notifPermissionAlertInfo => alertStore. I don't see anymore rehydration errors on native + everything looks good when I open up the redux dev tools, but just wanted to double check with you and see what you think!

Harbormaster completed remote builds in B28101: Diff 38956.Apr 9 2024, 12:35 PM
ginsu requested review of this revision.Apr 9 2024, 12:35 PM
ginsu added a child revision: D11613: [lib/native] introduce connect farcaster alert handler.Apr 9 2024, 10:37 PM
kamil resigned from this revision.Apr 10 2024, 5:54 AM

Not familiar with this project so letting others review, persistWhitelist looks good

web/redux/persist.js
51 ↗(On Diff #38956)

this seems fine

atul requested changes to this revision.Apr 10 2024, 10:41 AM

Might be missing something, but don't we also need to update default states? For example in defaultWebState?

This revision now requires changes to proceed.Apr 10 2024, 10:41 AM
atul accepted this revision.EditedApr 10 2024, 10:43 AM

{F1510339}

jk, you already handled this in stack

This revision is now accepted and ready to land.Apr 10 2024, 10:43 AM
ginsu updated this revision to Diff 39014.Apr 10 2024, 12:37 PM
ginsu added a subscriber: kamil.

rebase before landing

Harbormaster completed remote builds in B28146: Diff 39014.Apr 10 2024, 1:20 PM
Closed by commit rCOMMdbc17db48d23: [native/web] introduce migration to update notifPermissionAlertInfo to… (authored by ginsu). · Explain WhyApr 10 2024, 1:32 PM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMMdbc17db48d23: [native/web] introduce migration to update notifPermissionAlertInfo to….

Revision Contents
Changeset List

PathSize
native/
redux/
15 lines
web/
redux/
16 lines

Diff 39014

View Options

native/redux/persist.js

Loading...
View Options

web/redux/persist.js

Loading...