Page MenuHomePhabricator

[lib] Stop removing policies on keyserver logout
ClosedPublic

Authored by inka on Jan 16 2024, 2:48 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 6, 1:15 PM
Unknown Object (File)
Tue, Nov 5, 6:59 PM
Unknown Object (File)
Tue, Nov 5, 4:04 PM
Unknown Object (File)
Tue, Nov 5, 3:59 PM
Unknown Object (File)
Tue, Nov 5, 3:53 PM
Unknown Object (File)
Tue, Nov 5, 1:24 PM
Unknown Object (File)
Oct 16 2024, 1:56 AM
Unknown Object (File)
Oct 16 2024, 1:56 AM
Subscribers

Details

Summary

issue: ENG-5824
We get policies from Ashoat's keyserver for the time being, but I think it makes sense to remove them only on identity logout

Test Plan

Tested that the policies still get removed if the user is not using CSAT and logs out

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 16 2024, 2:59 AM
Harbormaster failed remote builds in B25789: Diff 35657!
inka requested review of this revision.Jan 16 2024, 6:10 AM

but I think it makes sense to remove them only on identity logout

Something I'm a bit confused about... here it seems like we're planning to keep using logOutActionTypes for logout. But in this diff, you seem to imply that there will be a new action added later for identity logout. Can you clarify the scenario where we'll want to use that identity logout action?

For user-initiated logout, I just proposed having a single set of logout actions to handle combined keyserver and identity logout here. If we take that approach, should we keep logOutActionTypes for that combined keyserver/identity logout? If we take that approach, this diff won't make sense.

PS – there are no reviewers here

inka added a subscriber: varun.

PS – there are no reviewers here

Thank you

For user-initiated logout, I just proposed having a single set of logout actions to handle combined keyserver and identity logout here. If we take that approach, should we keep logOutActionTypes for that combined keyserver/identity logout? If we take that approach, this diff won't make sense.

My understanding from talking to @varun was that we will have a separate logout from identity action.
We started discussing this in many places, I propose to continue in ENG-6424.

This change would still make sense for deleteKeyserverAccountActionTypes though. When a user removes a keyserver from their keyserver list, we don't want to remove the policies store.

inka planned changes to this revision.Jan 17 2024, 8:18 AM

We decided to have a single action for logging out of keyservers and identity.

This change would still make sense for deleteKeyserverAccountActionTypes though. When a user removes a keyserver from their keyserver list, we don't want to remove the policies store.

I don't follow why we should be clearing the policies store on this action when !usingCommServicesAccessToken. It feels like the behavior of this single action is being overloaded to mean two completely different things depending on usingCommServicesAccessToken. This is very confusing... we should have two different actions here, I think.

lib/reducers/policies-reducer.js
56–59 ↗(On Diff #35775)

Same question as here

Merge identity delete account with keyserver delete account

This revision is now accepted and ready to land.Jan 25 2024, 6:15 AM