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
Unknown Object (File)
Wed, Apr 2, 8:01 PM
Unknown Object (File)
Wed, Apr 2, 7:43 AM
Unknown Object (File)
Mon, Mar 10, 5:22 AM
Unknown Object (File)
Mar 8 2025, 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 First 20 Lines • Show All 226 Lines • ▼ Show 20 Lines const row = [
description, description,
viewer.userID, viewer.userID,
time, time,
color, color,
parentThreadID, parentThreadID,
threadAncestry.containingThreadID, threadAncestry.containingThreadID,
threadAncestry.community, threadAncestry.community,
threadAncestry.depth, threadAncestry.depth,
newRoles.default.id,
sourceMessageID, sourceMessageID,
]; ];
let existingThreadQuery = null; let existingThreadQuery = null;
if (threadType === threadTypes.PERSONAL) { if (threadType === threadTypes.PERSONAL) {
const otherMemberID = initialMemberIDs?.[0]; const otherMemberID = initialMemberIDs?.[0];
invariant( invariant(
otherMemberID, otherMemberID,
'Other member id should be set for a PERSONAL thread', 'Other member id should be set for a PERSONAL thread',
); );
existingThreadQuery = personalThreadQuery(viewer.userID, otherMemberID); existingThreadQuery = personalThreadQuery(viewer.userID, otherMemberID);
} else if (sourceMessageID) { } else if (sourceMessageID) {
existingThreadQuery = SQL` existingThreadQuery = SQL`
SELECT t.id SELECT t.id
FROM threads t FROM threads t
WHERE t.source_message = ${sourceMessageID} WHERE t.source_message = ${sourceMessageID}
`; `;
} }
if (existingThreadQuery) { if (existingThreadQuery) {
const query = SQL` const query = SQL`
INSERT INTO threads(id, type, name, description, creator, creation_time, INSERT INTO threads(id, type, name, description, creator, creation_time,
color, parent_thread_id, containing_thread_id, community, depth, color, parent_thread_id, containing_thread_id, community, depth,
default_role, source_message) source_message)
SELECT ${row} SELECT ${row}
WHERE NOT EXISTS (`; WHERE NOT EXISTS (`;
query.append(existingThreadQuery).append(SQL`)`); query.append(existingThreadQuery).append(SQL`)`);
const [result] = await dbQuery(query); const [result] = await dbQuery(query);
if (result.affectedRows === 0) { if (result.affectedRows === 0) {
const deleteRoles = SQL` const deleteRoles = SQL`
DELETE FROM roles DELETE FROM roles
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines if (result.affectedRows === 0) {
userInfos, userInfos,
newMessageInfos, newMessageInfos,
}; };
} }
} else { } else {
const query = SQL` const query = SQL`
INSERT INTO threads(id, type, name, description, creator, creation_time, INSERT INTO threads(id, type, name, description, creator, creation_time,
color, parent_thread_id, containing_thread_id, community, depth, color, parent_thread_id, containing_thread_id, community, depth,
default_role, source_message) source_message)
VALUES ${[row]} VALUES ${[row]}
`; `;
await dbQuery(query); await dbQuery(query);
} }
let initialMemberPromise; let initialMemberPromise;
if (initialMemberIDs) { if (initialMemberIDs) {
initialMemberPromise = changeRole(id, initialMemberIDs, null, { initialMemberPromise = changeRole(id, initialMemberIDs, null, {
▲ Show 20 Lines • Show All 178 Lines • Show Last 20 Lines
View Options

lib/socket/socket.react.js

Show First 20 Lines • Show All 226 Lines • ▼ Show 20 Lines const row = [
description, description,
viewer.userID, viewer.userID,
time, time,
color, color,
parentThreadID, parentThreadID,
threadAncestry.containingThreadID, threadAncestry.containingThreadID,
threadAncestry.community, threadAncestry.community,
threadAncestry.depth, threadAncestry.depth,
newRoles.default.id,
sourceMessageID, sourceMessageID,
]; ];
let existingThreadQuery = null; let existingThreadQuery = null;
if (threadType === threadTypes.PERSONAL) { if (threadType === threadTypes.PERSONAL) {
const otherMemberID = initialMemberIDs?.[0]; const otherMemberID = initialMemberIDs?.[0];
invariant( invariant(
otherMemberID, otherMemberID,
'Other member id should be set for a PERSONAL thread', 'Other member id should be set for a PERSONAL thread',
); );
existingThreadQuery = personalThreadQuery(viewer.userID, otherMemberID); existingThreadQuery = personalThreadQuery(viewer.userID, otherMemberID);
} else if (sourceMessageID) { } else if (sourceMessageID) {
existingThreadQuery = SQL` existingThreadQuery = SQL`
SELECT t.id SELECT t.id
FROM threads t FROM threads t
WHERE t.source_message = ${sourceMessageID} WHERE t.source_message = ${sourceMessageID}
`; `;
} }
if (existingThreadQuery) { if (existingThreadQuery) {
const query = SQL` const query = SQL`
INSERT INTO threads(id, type, name, description, creator, creation_time, INSERT INTO threads(id, type, name, description, creator, creation_time,
color, parent_thread_id, containing_thread_id, community, depth, color, parent_thread_id, containing_thread_id, community, depth,
default_role, source_message) source_message)
SELECT ${row} SELECT ${row}
WHERE NOT EXISTS (`; WHERE NOT EXISTS (`;
query.append(existingThreadQuery).append(SQL`)`); query.append(existingThreadQuery).append(SQL`)`);
const [result] = await dbQuery(query); const [result] = await dbQuery(query);
if (result.affectedRows === 0) { if (result.affectedRows === 0) {
const deleteRoles = SQL` const deleteRoles = SQL`
DELETE FROM roles DELETE FROM roles
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines if (result.affectedRows === 0) {
userInfos, userInfos,
newMessageInfos, newMessageInfos,
}; };
} }
} else { } else {
const query = SQL` const query = SQL`
INSERT INTO threads(id, type, name, description, creator, creation_time, INSERT INTO threads(id, type, name, description, creator, creation_time,
color, parent_thread_id, containing_thread_id, community, depth, color, parent_thread_id, containing_thread_id, community, depth,
default_role, source_message) source_message)
VALUES ${[row]} VALUES ${[row]}
`; `;
await dbQuery(query); await dbQuery(query);
} }
let initialMemberPromise; let initialMemberPromise;
if (initialMemberIDs) { if (initialMemberIDs) {
initialMemberPromise = changeRole(id, initialMemberIDs, null, { initialMemberPromise = changeRole(id, initialMemberIDs, null, {
▲ Show 20 Lines • Show All 178 Lines • Show Last 20 Lines
View Options

lib/utils/action-utils.js

Show First 20 Lines • Show All 226 Lines • ▼ Show 20 Lines const row = [
description, description,
viewer.userID, viewer.userID,
time, time,
color, color,
parentThreadID, parentThreadID,
threadAncestry.containingThreadID, threadAncestry.containingThreadID,
threadAncestry.community, threadAncestry.community,
threadAncestry.depth, threadAncestry.depth,
newRoles.default.id,
sourceMessageID, sourceMessageID,
]; ];
let existingThreadQuery = null; let existingThreadQuery = null;
if (threadType === threadTypes.PERSONAL) { if (threadType === threadTypes.PERSONAL) {
const otherMemberID = initialMemberIDs?.[0]; const otherMemberID = initialMemberIDs?.[0];
invariant( invariant(
otherMemberID, otherMemberID,
'Other member id should be set for a PERSONAL thread', 'Other member id should be set for a PERSONAL thread',
); );
existingThreadQuery = personalThreadQuery(viewer.userID, otherMemberID); existingThreadQuery = personalThreadQuery(viewer.userID, otherMemberID);
} else if (sourceMessageID) { } else if (sourceMessageID) {
existingThreadQuery = SQL` existingThreadQuery = SQL`
SELECT t.id SELECT t.id
FROM threads t FROM threads t
WHERE t.source_message = ${sourceMessageID} WHERE t.source_message = ${sourceMessageID}
`; `;
} }
if (existingThreadQuery) { if (existingThreadQuery) {
const query = SQL` const query = SQL`
INSERT INTO threads(id, type, name, description, creator, creation_time, INSERT INTO threads(id, type, name, description, creator, creation_time,
color, parent_thread_id, containing_thread_id, community, depth, color, parent_thread_id, containing_thread_id, community, depth,
default_role, source_message) source_message)
SELECT ${row} SELECT ${row}
WHERE NOT EXISTS (`; WHERE NOT EXISTS (`;
query.append(existingThreadQuery).append(SQL`)`); query.append(existingThreadQuery).append(SQL`)`);
const [result] = await dbQuery(query); const [result] = await dbQuery(query);
if (result.affectedRows === 0) { if (result.affectedRows === 0) {
const deleteRoles = SQL` const deleteRoles = SQL`
DELETE FROM roles DELETE FROM roles
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines if (result.affectedRows === 0) {
userInfos, userInfos,
newMessageInfos, newMessageInfos,
}; };
} }
} else { } else {
const query = SQL` const query = SQL`
INSERT INTO threads(id, type, name, description, creator, creation_time, INSERT INTO threads(id, type, name, description, creator, creation_time,
color, parent_thread_id, containing_thread_id, community, depth, color, parent_thread_id, containing_thread_id, community, depth,
default_role, source_message) source_message)
VALUES ${[row]} VALUES ${[row]}
`; `;
await dbQuery(query); await dbQuery(query);
} }
let initialMemberPromise; let initialMemberPromise;
if (initialMemberIDs) { if (initialMemberIDs) {
initialMemberPromise = changeRole(id, initialMemberIDs, null, { initialMemberPromise = changeRole(id, initialMemberIDs, null, {
▲ Show 20 Lines • Show All 178 Lines • Show Last 20 Lines
View Options

lib/utils/call-server-endpoint.js

Show First 20 Lines • Show All 226 Lines • ▼ Show 20 Lines const row = [
description, description,
viewer.userID, viewer.userID,
time, time,
color, color,
parentThreadID, parentThreadID,
threadAncestry.containingThreadID, threadAncestry.containingThreadID,
threadAncestry.community, threadAncestry.community,
threadAncestry.depth, threadAncestry.depth,
newRoles.default.id,
sourceMessageID, sourceMessageID,
]; ];
let existingThreadQuery = null; let existingThreadQuery = null;
if (threadType === threadTypes.PERSONAL) { if (threadType === threadTypes.PERSONAL) {
const otherMemberID = initialMemberIDs?.[0]; const otherMemberID = initialMemberIDs?.[0];
invariant( invariant(
otherMemberID, otherMemberID,
'Other member id should be set for a PERSONAL thread', 'Other member id should be set for a PERSONAL thread',
); );
existingThreadQuery = personalThreadQuery(viewer.userID, otherMemberID); existingThreadQuery = personalThreadQuery(viewer.userID, otherMemberID);
} else if (sourceMessageID) { } else if (sourceMessageID) {
existingThreadQuery = SQL` existingThreadQuery = SQL`
SELECT t.id SELECT t.id
FROM threads t FROM threads t
WHERE t.source_message = ${sourceMessageID} WHERE t.source_message = ${sourceMessageID}
`; `;
} }
if (existingThreadQuery) { if (existingThreadQuery) {
const query = SQL` const query = SQL`
INSERT INTO threads(id, type, name, description, creator, creation_time, INSERT INTO threads(id, type, name, description, creator, creation_time,
color, parent_thread_id, containing_thread_id, community, depth, color, parent_thread_id, containing_thread_id, community, depth,
default_role, source_message) source_message)
SELECT ${row} SELECT ${row}
WHERE NOT EXISTS (`; WHERE NOT EXISTS (`;
query.append(existingThreadQuery).append(SQL`)`); query.append(existingThreadQuery).append(SQL`)`);
const [result] = await dbQuery(query); const [result] = await dbQuery(query);
if (result.affectedRows === 0) { if (result.affectedRows === 0) {
const deleteRoles = SQL` const deleteRoles = SQL`
DELETE FROM roles DELETE FROM roles
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines if (result.affectedRows === 0) {
userInfos, userInfos,
newMessageInfos, newMessageInfos,
}; };
} }
} else { } else {
const query = SQL` const query = SQL`
INSERT INTO threads(id, type, name, description, creator, creation_time, INSERT INTO threads(id, type, name, description, creator, creation_time,
color, parent_thread_id, containing_thread_id, community, depth, color, parent_thread_id, containing_thread_id, community, depth,
default_role, source_message) source_message)
VALUES ${[row]} VALUES ${[row]}
`; `;
await dbQuery(query); await dbQuery(query);
} }
let initialMemberPromise; let initialMemberPromise;
if (initialMemberIDs) { if (initialMemberIDs) {
initialMemberPromise = changeRole(id, initialMemberIDs, null, { initialMemberPromise = changeRole(id, initialMemberIDs, null, {
▲ Show 20 Lines • Show All 178 Lines • Show Last 20 Lines