Page MenuHomePhabricator

[web] Don't allow creating subchannels with users the user should not be able to add
ClosedPublic

Authored by inka on Jul 10 2024, 1:32 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 25, 4:45 AM
Unknown Object (File)
Tue, Oct 22, 9:29 PM
Unknown Object (File)
Tue, Oct 15, 4:17 PM
Unknown Object (File)
Tue, Oct 15, 4:16 PM
Unknown Object (File)
Tue, Oct 15, 4:16 PM
Unknown Object (File)
Tue, Oct 15, 4:14 PM
Unknown Object (File)
Sep 25 2024, 8:25 PM
Unknown Object (File)
Sep 25 2024, 8:25 PM
Subscribers

Details

Summary

issue: ENG-8710
The user should not be able to add some users to some chats. On native we handle this in the UI by showing a popup if the user wants to add someone they cannot.
On web we decided that for now we will just not display those users. Handlig tih siwth a popup is tracked in ENG-8798

Property alert is set only when the user cannot be added to the thread. There are four cases. They can be found in lib/shared/search-utils.js in usePotentialMemberItems.

Test Plan

Tested that in all four cses the users don't show up in search results on web anymore.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Jul 10 2024, 2:12 AM
This revision is now accepted and ready to land.Jul 10 2024, 4:38 AM

This diff and the parent diff are missing React.useMemos in critical places

Feedback to both @inka and @tomek: please be more thoughtful about this going forward. Skipping a useMemo can cause huge performance regressions and can even result in the app crashing

If you disagree or this feedback doesn't make sense, please re-request review

web/settings/relationship/add-users-utils.js
224 ↗(On Diff #42208)

You are creating a new array on every invocation here, which will force useENSNames to constantly re-query for ENS names

Please wrap this in a React.useMemo

228–229 ↗(On Diff #42208)

This also needs to be wrapped in a React.useMemo