Page MenuHomePhabricator
Differential D12290

[lib] Replace `innerThreadFrozenDueToBlock` with `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck` in `useThreadsWithPermission`
ClosedPublic
Actions

Authored by atul on Mon, Jun 3, 1:25 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jun 21, 6:23 AM
Unknown Object (File)
Fri, Jun 21, 6:23 AM
Unknown Object (File)
Thu, Jun 20, 6:26 AM
Unknown Object (File)
Thu, Jun 20, 4:55 AM
Unknown Object (File)
Fri, Jun 14, 11:59 PM
Unknown Object (File)
Fri, Jun 14, 5:25 PM
Unknown Object (File)
Mon, Jun 10, 5:02 AM
Unknown Object (File)
Mon, Jun 10, 4:59 AM
View All 15 Files
Subscribers
None

Details

Reviewers
ashoat
ginsu
tomek
varun
Commits
rCOMM940b2ff9147f: [lib] Replace `innerThreadFrozenDueToBlock` with…
Summary

This allows us to narrow things as threadIsWithBlockedUserOnlyWithoutAdminRoleCheck only accepts ThreadInfo and will help us remove ThreadInfo from memberHasAdminPowers.memberInfo argument without that invariant.

Depends on D12289

Test Plan

flow + close reading

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul created this revision.Mon, Jun 3, 1:25 PM
atul published this revision for review.Mon, Jun 3, 1:27 PM
atul added inline comments.
lib/shared/thread-utils.js
179 ↗(On Diff #40899)

What was set to true here was skipMemberAdminRoleCheck option. The checkOnlyViewerBlock option was set within innerThreadFrozenDueToBlock.

179 ↗(On Diff #40899)

What is set to false here is the checkOnlyViewerBlock option, there's no skipMemberAdminRoleCheck option when calling threadIsWithBlockedUserOnlyWithoutAdminRoleCheck because the whole point is that we only accept ThreadInfo and handle admin role check separately.

Harbormaster completed remote builds in B29398: Diff 40899.Mon, Jun 3, 1:43 PM
atul added a child revision: D12292: [lib] Narrow types in `memberHasAdminPowers` call stack to remove `invariant`.Mon, Jun 3, 1:43 PM
tomek accepted this revision.Tue, Jun 4, 1:51 AM
tomek added inline comments.
lib/shared/thread-utils.js
179 ↗(On Diff #40899)

Can we introduce an object param instead of multiple params with an unobvious meaning?

This revision is now accepted and ready to land.Tue, Jun 4, 1:51 AM
atul added inline comments.Sun, Jun 16, 5:21 PM
lib/shared/thread-utils.js
179 ↗(On Diff #40899)

We actually introduced and un-introduced object param in this stack, there's just one option now.

atul edited the test plan for this revision. (Show Details)Sun, Jun 16, 5:24 PM
atul updated this revision to Diff 41358.Sun, Jun 16, 5:24 PM

rebase and land

This revision was landed with ongoing or failed builds.Sun, Jun 16, 5:35 PM
Closed by commit rCOMM940b2ff9147f: [lib] Replace `innerThreadFrozenDueToBlock` with… (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM940b2ff9147f: [lib] Replace `innerThreadFrozenDueToBlock` with….
Harbormaster completed remote builds in B29701: Diff 41358.Sun, Jun 16, 5:41 PM

Revision Contents
Changeset List

PathSize
lib/
shared/
4 lines

Diff 41359

View Options

lib/shared/thread-utils.js

Loading...