Page MenuHomePhabricator
Differential D12715

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

Authored by inka on Jul 10 2024, 1:32 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 15, 5:39 AM
Unknown Object (File)
Fri, Nov 15, 5:39 AM
Unknown Object (File)
Fri, Nov 15, 5:39 AM
Unknown Object (File)
Fri, Nov 15, 5:37 AM
Unknown Object (File)
Tue, Nov 12, 1:30 PM
Unknown Object (File)
Fri, Nov 8, 10:39 PM
Unknown Object (File)
Fri, Nov 8, 7:14 AM
Unknown Object (File)
Oct 25 2024, 4:45 AM
View All 60 Files
Subscribers
ashoat

Details

Reviewers
tomek
kamil
ashoat
Commits
rCOMM3acce0a16833: [web] Don't allow creating subchannels with users the user should not be able…
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
Branch
inka/fix
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka created this revision.Jul 10 2024, 1:32 AM
Herald added a subscriber: ashoat. · View Herald TranscriptJul 10 2024, 1:32 AM
inka added a parent revision: D12667: [web] Fix creating subchannels in chats in GENESIS.Jul 10 2024, 1:33 AM
Harbormaster completed remote builds in B30259: Diff 42200.Jul 10 2024, 2:12 AM
inka requested review of this revision.Jul 10 2024, 2:12 AM
tomek accepted this revision.Jul 10 2024, 4:38 AM
This revision is now accepted and ready to land.Jul 10 2024, 4:38 AM
inka updated this revision to Diff 42208.Jul 10 2024, 8:57 AM

Rebase

Harbormaster completed remote builds in B30267: Diff 42208.Jul 10 2024, 9:49 AM
ashoat accepted this revision.Jul 10 2024, 11:17 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

inka updated this revision to Diff 42223.Jul 11 2024, 3:42 AM

Memozie

inka added a comment.Jul 11 2024, 3:43 AM

Sorry about this!

Harbormaster completed remote builds in B30281: Diff 42223.Jul 11 2024, 3:58 AM
Closed by commit rCOMM3acce0a16833: [web] Don't allow creating subchannels with users the user should not be able… (authored by inka). · Explain WhyJul 11 2024, 5:17 AM
This revision was automatically updated to reflect the committed changes.
inka added a commit: rCOMM3acce0a16833: [web] Don't allow creating subchannels with users the user should not be able….

Revision Contents
Changeset List

PathSize
web/
settings/
relationship/
7 lines

Diff 42200

View Options

web/settings/relationship/add-users-utils.js

Loading...