Page MenuHomePhabricator

[lib] Introduce `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck`
ClosedPublic

Authored by atul on Jun 3 2024, 1:18 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 24, 5:49 PM
Unknown Object (File)
Wed, Oct 23, 7:48 AM
Unknown Object (File)
Wed, Oct 23, 5:34 AM
Unknown Object (File)
Tue, Oct 22, 5:02 PM
Unknown Object (File)
Mon, Oct 21, 4:07 AM
Unknown Object (File)
Mon, Oct 21, 1:44 AM
Unknown Object (File)
Tue, Oct 15, 9:50 PM
Unknown Object (File)
Thu, Oct 10, 10:53 PM
Subscribers
None

Details

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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)
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
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?
lib/shared/thread-utils.js
910–927 ↗(On Diff #40898)

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

This revision was landed with ongoing or failed builds.Jun 16 2024, 5:20 PM
This revision was automatically updated to reflect the committed changes.