Page MenuHomePhabricator
Differential D12289

[lib] Introduce `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck`
ClosedPublic
Actions

Authored by atul on Jun 3 2024, 1:18 PM.
Tags
None
Referenced Files
F2831848: D12289.diff
Sat, Sep 28, 1:28 AM
Unknown Object (File)
Wed, Sep 25, 10:49 PM
Unknown Object (File)
Wed, Sep 25, 10:49 PM
Unknown Object (File)
Wed, Sep 25, 10:49 PM
Unknown Object (File)
Mon, Sep 9, 9:07 AM
Unknown Object (File)
Mon, Sep 9, 9:07 AM
Unknown Object (File)
Mon, Sep 9, 9:07 AM
Unknown Object (File)
Thu, Aug 29, 7:53 AM
View All 60 Files
Subscribers
None

Details

Reviewers
ashoat
ginsu
tomek
varun
Commits
rCOMM727b3b565aa0: [lib] Introduce `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck`
Summary

Introduce a variant of threadIsWithBlockedUserOnly that only accepts ThreadInfo and doesn't take skipMemberAdminRoleCheck as an option. This will help us remove ThreadInfo from memberHasAdminPowers.memberInfo argument without that invariant.

There's currently a lot of overlap between threadIsWithBlockedUserOnly and threadIsWithBlockedUserOnlyWithoutAdminRoleCheck. Will pull out the common logic into separate function after sorting out the flow stuff.

Depends on D12288

Test Plan

flow + close reading + some basic logging

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D12289 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Jun 3 2024, 1:18 PM
atul added inline comments.Jun 3 2024, 1:19 PM
lib/shared/thread-utils.js
910–927 ↗(On Diff #40898)

This chunk of logic is shared with threadIsWithBlockedUserOnly.

Will extract into some sort of baseThreadIsWithBlockedUserOnly or similar function after figuring out the flow stuff

atul published this revision for review.Jun 3 2024, 1:19 PM
atul edited the summary of this revision. (Show Details)
atul added a child revision: D12290: [lib] Replace `innerThreadFrozenDueToBlock` with `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck` in `useThreadsWithPermission`.Jun 3 2024, 1:25 PM
Harbormaster completed remote builds in B29397: Diff 40898.Jun 3 2024, 1:35 PM
tomek accepted this revision.Jun 4 2024, 1:27 AM
tomek added inline comments.
lib/shared/thread-utils.js
910–927 ↗(On Diff #40898)

Maybe you can call threadIsWithBlockedUserOnlyWithoutAdminRoleCheck from threadIsWithBlockedUserOnly.

This revision is now accepted and ready to land.Jun 4 2024, 1:27 AM
ashoat added inline comments.Jun 4 2024, 10:48 AM
lib/shared/thread-utils.js
910–927 ↗(On Diff #40898)

I like this idea. To do that, we'd need to extend this new function so it can take LegacyRawThreadInfo | RawThreadInfo as well.

@atul, before landing can you either:

  1. Link the diff where you do this, or
  2. Update this diff to do this?
ashoat added inline comments.Jun 4 2024, 11:43 AM
lib/shared/thread-utils.js
910–927 ↗(On Diff #40898)

Ignore this – @atul's approach in D12295 works too

atul updated this revision to Diff 41226.Jun 11 2024, 2:11 PM

resolve merge conflict

Harbormaster completed remote builds in B29614: Diff 41226.Jun 11 2024, 2:28 PM
atul updated this revision to Diff 41356.Jun 16 2024, 5:10 PM

rebase before landing

atul edited the test plan for this revision. (Show Details)Jun 16 2024, 5:11 PM
This revision was landed with ongoing or failed builds.Jun 16 2024, 5:20 PM
Closed by commit rCOMM727b3b565aa0: [lib] Introduce `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck` (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM727b3b565aa0: [lib] Introduce `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck`.
Harbormaster completed remote builds in B29700: Diff 41356.Jun 16 2024, 5:33 PM

Revision Contents
Changeset List

PathSize
lib/
shared/
37 lines

Diff 41356

View Options

lib/shared/thread-utils.js

Loading...