Page MenuHomePhabricator

[lib] Introduce `useThreadsWithPermission` to filter `ThreadInfo`s
ClosedPublic

Authored by atul on May 7 2024, 9:21 AM.
Tags
None
Referenced Files
F3154454: D11924.diff
Tue, Nov 5, 8:39 AM
Unknown Object (File)
Sat, Oct 26, 6:50 PM
Unknown Object (File)
Tue, Oct 22, 1:15 PM
Unknown Object (File)
Tue, Oct 22, 9:03 AM
Unknown Object (File)
Tue, Oct 22, 8:34 AM
Unknown Object (File)
Mon, Oct 7, 5:27 PM
Unknown Object (File)
Mon, Oct 7, 1:40 PM
Unknown Object (File)
Mon, Oct 7, 7:14 AM
Subscribers
None

Details

Summary

There are places where we want to check which ThreadInfos in a collection have a given ThreadPermission. In these situations, we call threadHasPermission in some sort of for-loop or in the body of a .filter. See onScreenEntryEditableThreadInfos below as an example:

ef4a70.png (476×1 px, 98 KB)

Now that we're moving from threadHasPermission to useThreadHasPermission, we have the issue where calling useThreadHasPermission within a for-loop or .filter for each ThreadInfo would violate the rules of hooks. Instead, we introduce a new hook that takes in a $ReadOnlyArray<ThreadInfo> and filters out ThreadInfos that don't have a given ThreadPermission.

The original implementation of useThreadsWithPermission was 90% the same as useThreadHasPermission, so it made sense to consume useThreadsWithPermission within useThreadHasPermission to reduce duplication. Also considered pulling logic out into helper function, but thought this approach was cleaner.


Depends on D11916

Test Plan

flow + close reading + some log statements as sanity checks for now

Put log statements in both useThreadHasPermission and useThreadsWithPermission to ensure that return values were as expected. Specifically that return values of useThreadHasPermission remained the same.

Diff Detail

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

Event Timeline

atul published this revision for review.May 7 2024, 9:22 AM
atul edited the summary of this revision. (Show Details)
atul edited the summary of this revision. (Show Details)
ashoat added inline comments.
lib/shared/thread-utils.js
182–185 ↗(On Diff #39897)

Can we extract this to its own line? Similar to how we want await to be visible, I think it's good for hooks to be as well

This revision is now accepted and ready to land.May 7 2024, 9:54 AM
atul marked an inline comment as done.May 14 2024, 2:10 PM

address feedback and land