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
F5117690: D9320.id.diff
Wed, Apr 2, 8:01 PM
F5114236: D9320.diff
Wed, Apr 2, 7:43 AM
Unknown Object (File)
Mon, Mar 10, 5:22 AM
Unknown Object (File)
Sat, Mar 8, 9:51 AM
Unknown Object (File)
Feb 25 2025, 9:58 PM
Unknown Object (File)
Feb 24 2025, 2:24 AM
Unknown Object (File)
Feb 15 2025, 2:24 AM
Unknown Object (File)
Feb 11 2025, 3:03 PM
View All 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

Show All 10 Lines
div.messageActionContainer { div.messageActionContainer {
display: flex; display: flex;
flex-direction: row; flex-direction: row;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
background-color: var(--message-action-tooltip-bg); background-color: var(--message-action-tooltip-bg);
border-radius: 8px; border-radius: 8px;
width: fit-content; width: fit-content;
box-shadow: var(--message-action-tooltip-bg) 0 2px 8px;
} }
div.messageActionButtons { div.messageActionButtons {
display: flex; display: flex;
font-size: 16px; font-size: 16px;
} }
div.messageActionButtons svg { div.messageActionButtons svg {
padding: 10px 6px 6px; padding: 10px 6px 6px;
Show All 17 Linesdiv.messageTooltipLabel {
flex-direction: row; flex-direction: row;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
background-color: var(--message-action-tooltip-bg); background-color: var(--message-action-tooltip-bg);
color: var(--tool-tip-color); color: var(--tool-tip-color);
border-radius: 8px; border-radius: 8px;
overflow: auto; overflow: auto;
white-space: nowrap; white-space: nowrap;
box-shadow: var(--message-action-tooltip-bg) 0 2px 8px;
} }
div.leftTooltipAlign { div.leftTooltipAlign {
align-items: flex-start; align-items: flex-start;
} }
div.centerTooltipAlign { div.centerTooltipAlign {
align-items: center; align-items: center;
Show All 10 Lines
View Options

lib/socket/socket.react.js

Show All 10 Lines
div.messageActionContainer { div.messageActionContainer {
display: flex; display: flex;
flex-direction: row; flex-direction: row;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
background-color: var(--message-action-tooltip-bg); background-color: var(--message-action-tooltip-bg);
border-radius: 8px; border-radius: 8px;
width: fit-content; width: fit-content;
box-shadow: var(--message-action-tooltip-bg) 0 2px 8px;
} }
div.messageActionButtons { div.messageActionButtons {
display: flex; display: flex;
font-size: 16px; font-size: 16px;
} }
div.messageActionButtons svg { div.messageActionButtons svg {
padding: 10px 6px 6px; padding: 10px 6px 6px;
Show All 17 Linesdiv.messageTooltipLabel {
flex-direction: row; flex-direction: row;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
background-color: var(--message-action-tooltip-bg); background-color: var(--message-action-tooltip-bg);
color: var(--tool-tip-color); color: var(--tool-tip-color);
border-radius: 8px; border-radius: 8px;
overflow: auto; overflow: auto;
white-space: nowrap; white-space: nowrap;
box-shadow: var(--message-action-tooltip-bg) 0 2px 8px;
} }
div.leftTooltipAlign { div.leftTooltipAlign {
align-items: flex-start; align-items: flex-start;
} }
div.centerTooltipAlign { div.centerTooltipAlign {
align-items: center; align-items: center;
Show All 10 Lines
View Options

lib/utils/action-utils.js

Show All 10 Lines
div.messageActionContainer { div.messageActionContainer {
display: flex; display: flex;
flex-direction: row; flex-direction: row;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
background-color: var(--message-action-tooltip-bg); background-color: var(--message-action-tooltip-bg);
border-radius: 8px; border-radius: 8px;
width: fit-content; width: fit-content;
box-shadow: var(--message-action-tooltip-bg) 0 2px 8px;
} }
div.messageActionButtons { div.messageActionButtons {
display: flex; display: flex;
font-size: 16px; font-size: 16px;
} }
div.messageActionButtons svg { div.messageActionButtons svg {
padding: 10px 6px 6px; padding: 10px 6px 6px;
Show All 17 Linesdiv.messageTooltipLabel {
flex-direction: row; flex-direction: row;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
background-color: var(--message-action-tooltip-bg); background-color: var(--message-action-tooltip-bg);
color: var(--tool-tip-color); color: var(--tool-tip-color);
border-radius: 8px; border-radius: 8px;
overflow: auto; overflow: auto;
white-space: nowrap; white-space: nowrap;
box-shadow: var(--message-action-tooltip-bg) 0 2px 8px;
} }
div.leftTooltipAlign { div.leftTooltipAlign {
align-items: flex-start; align-items: flex-start;
} }
div.centerTooltipAlign { div.centerTooltipAlign {
align-items: center; align-items: center;
Show All 10 Lines
View Options

lib/utils/call-server-endpoint.js

Show All 10 Lines
div.messageActionContainer { div.messageActionContainer {
display: flex; display: flex;
flex-direction: row; flex-direction: row;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
background-color: var(--message-action-tooltip-bg); background-color: var(--message-action-tooltip-bg);
border-radius: 8px; border-radius: 8px;
width: fit-content; width: fit-content;
box-shadow: var(--message-action-tooltip-bg) 0 2px 8px;
} }
div.messageActionButtons { div.messageActionButtons {
display: flex; display: flex;
font-size: 16px; font-size: 16px;
} }
div.messageActionButtons svg { div.messageActionButtons svg {
padding: 10px 6px 6px; padding: 10px 6px 6px;
Show All 17 Linesdiv.messageTooltipLabel {
flex-direction: row; flex-direction: row;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
background-color: var(--message-action-tooltip-bg); background-color: var(--message-action-tooltip-bg);
color: var(--tool-tip-color); color: var(--tool-tip-color);
border-radius: 8px; border-radius: 8px;
overflow: auto; overflow: auto;
white-space: nowrap; white-space: nowrap;
box-shadow: var(--message-action-tooltip-bg) 0 2px 8px;
} }
div.leftTooltipAlign { div.leftTooltipAlign {
align-items: flex-start; align-items: flex-start;
} }
div.centerTooltipAlign { div.centerTooltipAlign {
align-items: center; align-items: center;
Show All 10 Lines