Page MenuHomePhabricator

[lib, keyserver, native, web] Remove the cookie field from redux
ClosedPublic

Authored by inka on Jul 18 2023, 12:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jul 2, 12:30 AM
Unknown Object (File)
Mon, Jul 1, 1:56 AM
Unknown Object (File)
Mon, Jul 1, 1:56 AM
Unknown Object (File)
Mon, Jul 1, 1:56 AM
Unknown Object (File)
Mon, Jul 1, 1:56 AM
Unknown Object (File)
Mon, Jul 1, 1:52 AM
Unknown Object (File)
Sun, Jun 30, 6:47 AM
Unknown Object (File)
Sat, Jun 29, 5:39 AM
Subscribers

Details

Summary

https://linear.app/comm/issue/ENG-4377/refactor-cookie-field-in-web-redux
Removing the cookie field from redux. Adding a migration. Undoing a hack in cookieSelector.

Test Plan

ran yarn flow-all. Since cookie was not being used since the previous diff, this should be enough

EDIT:
tested that the cookie was migrated on native:

  • checked out master, opened the app, logged the cookie
  • closed the app
  • checked out this diff, opened the app, logged the cookie

On master, rehyderated keys included cookie. On this diff, rehydrated keys don't include 'cookie', but include the keyserverStore. Checked that cookie that was logged on master is equal to the keyserverStore.keyserverInfos['256'].cookie on this diff

Diff Detail

Repository
rCOMM Comm
Branch
inka/cookies
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Cookie on web has to stay undefined

inka requested review of this revision.Jul 18 2023, 1:25 PM
michal requested changes to this revision.Jul 19 2023, 9:24 AM

Can you amend the test plan with checking if the cookie was migrated correctly?

native/redux/persist.js
651–658

You need to bump the state version (if you aren't doing it in the next diffs)

web/redux/persist.js
97–104

Is the migration on web needed? (and if yes, you also need to bump the stateVersion)

This revision now requires changes to proceed.Jul 19 2023, 9:24 AM

Address review - remove web migration, since cookie is not persisted; bump native state version

This revision is now accepted and ready to land.Jul 20 2023, 2:11 AM