Page MenuHomePhabricator
Differential D4711

[lib] Fix `getPotentialMemberItems` to match server logic for adding users to threads in community
ClosedPublic
Actions

Authored by • jacek on Aug 2 2022, 8:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 9, 11:38 AM
Unknown Object (File)
Fri, Nov 8, 8:36 PM
Unknown Object (File)
Fri, Nov 8, 8:36 PM
Unknown Object (File)
Fri, Nov 8, 8:36 PM
Unknown Object (File)
Fri, Nov 8, 8:36 PM
Unknown Object (File)
Fri, Nov 8, 8:36 PM
Unknown Object (File)
Tue, Nov 5, 2:06 AM
Unknown Object (File)
Oct 4 2024, 11:08 PM
View All Files
Subscribers
• abosh
• adrian
ashoat

Details

Reviewers
tomek
atul
ashoat
Commits
rCOMMa21d29460a8c: [lib] Fix `getPotentialMemberItems` to match server logic for adding users to…
Summary

Adjust getPotentialMemberItems logic to match permissions on server side.
Linear issue: https://linear.app/comm/issue/ENG-1200/creating-thread-with-non-friend-user-is-blocked-on-front-end-and

Test Plan

Tested the change by trying to add friends/non-friends to community root/community [open|secret] subchannel/sidebars. It is possible to add any community member to community subchannel, and only friends to community.

Diff Detail

Repository
rCOMM Comm
Branch
jacek/permissions-fix
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• jacek created this revision.Aug 2 2022, 8:28 AM
• jacek held this revision as a draft.
Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptAug 2 2022, 8:28 AM
• jacek added a parent revision: D4710: [keyserver] Fix keyserver to allow adding non-friends from the same community to thread.Aug 2 2022, 8:29 AM
Harbormaster completed remote builds in B11051: Diff 15213.Aug 2 2022, 8:40 AM
• jacek updated this revision to Diff 15251.Aug 3 2022, 4:05 AM

Add "not in parent" notice

Harbormaster completed remote builds in B11077: Diff 15251.Aug 3 2022, 4:17 AM
• jacek published this revision for review.Aug 3 2022, 4:19 AM
ashoat requested changes to this revision.Aug 3 2022, 7:04 AM
ashoat added inline comments.
lib/shared/search-utils.js
35–36 ↗(On Diff #15251)

Seeing this logic duplicated here is concerning. I looked into how we determine this on the keyserver side, and found that it boils down to the function getContainingThreadID in lib.

Can we make sure to use the same codepath here? That function returns an ID instead of a ThreadInfo, but I'm sure there's a way to avoid duplicating the logic.

This revision now requires changes to proceed.Aug 3 2022, 7:04 AM
• jacek updated this revision to Diff 15313.Aug 4 2022, 7:47 AM

use getContainingThreadID

Harbormaster completed remote builds in B11122: Diff 15313.Aug 4 2022, 8:00 AM
ashoat accepted this revision.Aug 5 2022, 9:03 AM

This looks great!!

lib/shared/search-utils.js
158

I don't think the double negation is necessary on parentThreadInfo

This revision is now accepted and ready to land.Aug 5 2022, 9:03 AM
• jacek updated this revision to Diff 15416.Aug 8 2022, 1:19 AM

remove double negation

Harbormaster completed remote builds in B11207: Diff 15416.Aug 8 2022, 1:32 AM
Closed by commit rCOMMa21d29460a8c: [lib] Fix `getPotentialMemberItems` to match server logic for adding users to… (authored by • jacek). · Explain WhyAug 8 2022, 4:32 AM
This revision was automatically updated to reflect the committed changes.
• jacek added a commit: rCOMMa21d29460a8c: [lib] Fix `getPotentialMemberItems` to match server logic for adding users to….

Revision Contents
Changeset List

PathSize
lib/
shared/
106 lines

Diff 15313

View Options

lib/shared/search-utils.js

Loading...