Page MenuHomePhabricator
Differential D12289

[lib] Introduce `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck`
ClosedPublic
Actions

Authored by atul on Jun 3 2024, 1:18 PM.
Tags
None
Referenced Files
F3377088: D12289.id40898.diff
Wed, Nov 27, 3:40 AM
F3376564: D12289.diff
Wed, Nov 27, 1:04 AM
Unknown Object (File)
Sat, Nov 23, 11:42 AM
Unknown Object (File)
Tue, Nov 19, 1:26 AM
Unknown Object (File)
Sun, Nov 10, 8:48 PM
Unknown Object (File)
Sun, Nov 10, 5:59 PM
Unknown Object (File)
Sun, Nov 10, 3:12 PM
Unknown Object (File)
Fri, Nov 8, 1:52 PM
View All 80 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
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

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

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

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

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/
40 lines

Diff 40898

View Options

lib/shared/thread-utils.js

Loading...