Page MenuHomePhabricator
Differential D12289

[lib] Introduce `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck`
ClosedPublic
Actions

Authored by atul on Mon, Jun 3, 1:18 PM.
Tags
None
Referenced Files
F2125093: D12289.id41357.diff
Thu, Jun 27, 12:21 AM
Unknown Object (File)
Fri, Jun 21, 11:02 AM
Unknown Object (File)
Fri, Jun 21, 9:29 AM
Unknown Object (File)
Fri, Jun 21, 5:51 AM
Unknown Object (File)
Fri, Jun 21, 5:51 AM
Unknown Object (File)
Fri, Jun 21, 5:51 AM
Unknown Object (File)
Thu, Jun 20, 11:37 PM
Unknown Object (File)
Thu, Jun 20, 7:30 AM
View All 19 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
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Mon, Jun 3, 1:18 PM
atul added inline comments.Mon, Jun 3, 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.Mon, Jun 3, 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`.Mon, Jun 3, 1:25 PM
Harbormaster completed remote builds in B29397: Diff 40898.Mon, Jun 3, 1:35 PM
tomek accepted this revision.Tue, Jun 4, 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.Tue, Jun 4, 1:27 AM
ashoat added inline comments.Tue, Jun 4, 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.Tue, Jun 4, 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.Tue, Jun 11, 2:11 PM

resolve merge conflict

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

rebase before landing

atul edited the test plan for this revision. (Show Details)Sun, Jun 16, 5:11 PM
This revision was landed with ongoing or failed builds.Sun, Jun 16, 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.Sun, Jun 16, 5:33 PM

Revision Contents
Changeset List

PathSize
lib/
shared/
37 lines

Diff 41226

View Options

lib/shared/thread-utils.js

Loading...