issue: ENG-8801
It should not be possible to create a subchannel in a chat under GENESIS with users the current user is not friends with, even if they are not in the parent chat
It should not be possible to add a user that is not a friend of the current user, even if they are not in the parent chat, to a subchannel of a chat under GENESIS
To see the full explanation of what behaviour we want in all cases, please see this comment
Details
Details
I tested the behaviour of keyserver by removing filtering from UI and allowing it to display all users.
Tested that it is not possible to create a subchannel in GENESIS subchannel that would contain users that are not in parent chat, unless they are our friends
Tested that it is not possible to create a chat through ChatThreadComposer with non-friend (even if UI allows it), or create direct GENESIS subchannels with non-friends (from admin)
Tested that it IS possible to create direct GENESIS subchannels with friends (from admin)
Tested that it IS possible to create subchannel in GENESIS subchannel with members of the parent even if they are not our friends.
Tested that it IS possible to create to create a subchannel in a different community with users that are in this community but we are not friends with.
Tested that it IS NOT possible to create a subchannel in a different community with users that are not in this community and we are not friends with.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| keyserver/src/fetchers/thread-fetchers.js | ||
|---|---|---|
| 396 ↗ | (On Diff #42234) | For chats under GENESIS we want:
|
| keyserver/src/fetchers/thread-permission-fetchers.js | ||
| 374–379 ↗ | (On Diff #42234) | For chats in different communities than GENESIS we require that the user we are trying to add is a member of that community |
Comment Actions
I'd like to avoid querying determineThreadAncestry twice
| keyserver/src/creators/thread-creator.js | ||
|---|---|---|
| 165–169 ↗ | (On Diff #42234) | The way you've built this, we end up querying determineThreadAncestry twice. Instead, can we keep the old await determineThreadAncestryPromise call here, and pass the result to the new function? |
| keyserver/src/fetchers/thread-fetchers.js | ||
| 396 ↗ | (On Diff #42234) | Hmm... realized we didn't consider a depth 3 subchannel of GENESIS in my Linear comment here: GENESIS -> depth 1 -> depth 2 -> depth 3 This logic will lead to the child of depth 3 have a containing thread of depth 2. But in the mental model of "we're treating subchannels of GENESIS like a community", the containing thread should arguably be depth 1. (However, there's an exception for sidebars, which should have the containing thread set to the parent. See the logic in getContainingThreadID.) I think to fix this we'd need to pass in the parentContainingThreadID here, which might not be worth it. I don't feel super strongly about this, and if it's too complicated to make this change then we can probably leave it as-is |
| keyserver/src/fetchers/thread-fetchers.js | ||
|---|---|---|
| 396 ↗ | (On Diff #42234) | Initially I have spent some time trying to implement this lake that, but it seemed like a lot of extra computation. We would have to get all thread infos and traverse them upwards to get the parentContainingThreadID. So we would have to either call the db or keep this in keyservers memory. |
| keyserver/src/creators/thread-creator.js | ||
|---|---|---|
| 166 ↗ | (On Diff #42253) | Can we rename to containingThreadIDForPossibleMemberResolution? |
| keyserver/src/fetchers/thread-fetchers.js | ||
| 396 ↗ | (On Diff #42234) | Thanks for trying! |
| keyserver/src/updaters/thread-updaters.js | ||
| 590 ↗ | (On Diff #42253) | Can we rename to containingThreadIDForPossibleMemberResolution? |