Page MenuHomePhabricator

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

Authored by inka on Wed, Jul 10, 1:32 AM.
Tags
None
Referenced Files
F2318670: D12715.diff
Mon, Jul 22, 2:42 PM
Unknown Object (File)
Mon, Jul 22, 12:58 AM
Unknown Object (File)
Mon, Jul 22, 12:56 AM
Unknown Object (File)
Fri, Jul 19, 9:23 PM
Unknown Object (File)
Fri, Jul 19, 6:16 PM
Unknown Object (File)
Fri, Jul 19, 1:19 PM
Unknown Object (File)
Fri, Jul 19, 9:12 AM
Unknown Object (File)
Fri, Jul 19, 8:53 AM
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.Wed, Jul 10, 2:12 AM
This revision is now accepted and ready to land.Wed, Jul 10, 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