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
F3525541: D9320.diff
Mon, Dec 23, 5:34 PM
Unknown Object (File)
Sat, Nov 30, 3:25 PM
Unknown Object (File)
Fri, Nov 29, 2:27 PM
Unknown Object (File)
Nov 20 2024, 5:10 PM
Unknown Object (File)
Nov 20 2024, 5:10 PM
Unknown Object (File)
Nov 20 2024, 5:10 PM
Unknown Object (File)
Nov 20 2024, 5:08 PM
Unknown Object (File)
Nov 11 2024, 4:49 AM
View All 89 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
Branch
fix-prompt
Lint
No Lint Coverage
Unit
No Test Coverage

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

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

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 31530

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...