Page MenuHomePhabricator
Differential D9320

[lib] don't dispatch policy acknowledgment action when user is not logged in
ClosedPublic
Actions

Authored by kamil on Sep 28 2023, 8:27 AM.
Tags
None
Referenced Files
F2110819: D9320.id.diff
Tue, Jun 25, 9:09 PM
F2106764: D9320.id31612.diff
Tue, Jun 25, 8:43 AM
F2106763: D9320.id31530.diff
Tue, Jun 25, 8:43 AM
F2106741: D9320.id.diff
Tue, Jun 25, 8:42 AM
F2106702: D9320.diff
Tue, Jun 25, 8:38 AM
Unknown Object (File)
Sun, Jun 23, 6:48 AM
Unknown Object (File)
Wed, Jun 19, 8:49 PM
Unknown Object (File)
Wed, Jun 19, 8:49 PM
View All 54 Files
Subscribers
ashoat

Details

Reviewers
michal
tomek
Commits
rCOMMbc919fd2511d: [lib] don't dispatch policy acknowledgment action when user is not logged in
Summary

Enhancement to previous diff.
Avoid dispatching forcePolicyAcknowledgmentActionType for not logged-in users.
Added also handling logout in policies reducer.

Depends on D9295

Test Plan

Logic:

  1. Make sure policies still work

The issue we want to fix:

  1. Revert diff D9295
  2. On the login screen call the keyserver endpoint which requires policies
  3. Check if an error was thrown on a keyserver
  4. Make sure the modal is not shown

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil created this revision.Sep 28 2023, 8:27 AM
kamil held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptSep 28 2023, 8:27 AM
Harbormaster completed remote builds in B22899: Diff 31530.Sep 28 2023, 8:45 AM
kamil published this revision for review.Sep 29 2023, 1:38 AM
tomek accepted this revision.Oct 3 2023, 3:53 AM
tomek added inline comments.
lib/utils/action-utils.js
268 ↗(On Diff #31530)

This is a different value than we use in e.g. user-selectors isLoggedIn. Is it ok to use it?

This revision is now accepted and ready to land.Oct 3 2023, 3:53 AM
tomek added a comment.Oct 3 2023, 3:54 AM

Can you expand the test plan a little bit by e.g. checking that a modal doesn't show for logged-out users?

kamil edited the test plan for this revision. (Show Details)Oct 3 2023, 6:38 AM
kamil added a reverted change: D9295: [keyserver] validate policies only for logged in users.
kamil edited the test plan for this revision. (Show Details)Oct 3 2023, 6:40 AM
kamil added a comment.Oct 3 2023, 6:42 AM
In D9320#274599, @tomek wrote:

Can you expand the test plan a little bit by e.g. checking that a modal doesn't show for logged-out users?

Yes, updated test plan

lib/utils/action-utils.js
268 ↗(On Diff #31530)

yes, we only care about the user being different than anonymous

Closed by commit rCOMMbc919fd2511d: [lib] don't dispatch policy acknowledgment action when user is not logged in (authored by kamil). · Explain WhyOct 3 2023, 6:46 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMMbc919fd2511d: [lib] don't dispatch policy acknowledgment action when user is not logged in.
kamil mentioned this in D9295: [keyserver] validate policies only for logged in users.Oct 3 2023, 6:48 AM

Revision Contents
Changeset List

PathSize
lib/
reducers/
11 lines
socket/
2 lines
utils/
2 lines
3 lines

Diff 31612

View Options

lib/reducers/policies-reducer.js

Loading...
View Options

lib/socket/socket.react.js

Loading...
View Options

lib/utils/action-utils.js

Loading...
View Options

lib/utils/call-server-endpoint.js

Loading...