Page MenuHomePhabricator

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

Authored by atul on Tue, May 7, 9:21 AM.
Tags
None
Referenced Files
F1886329: D11924.id40188.diff
Tue, May 28, 10:42 AM
Unknown Object (File)
Mon, May 27, 12:46 PM
Unknown Object (File)
Mon, May 27, 9:34 AM
Unknown Object (File)
Thu, May 23, 5:33 PM
Unknown Object (File)
Sat, May 18, 4:46 AM
Unknown Object (File)
Tue, May 14, 7:22 PM
Unknown Object (File)
Mon, May 13, 11:41 PM
Unknown Object (File)
Sun, May 12, 4:31 PM
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul published this revision for review.Tue, May 7, 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.Tue, May 7, 9:54 AM
atul marked an inline comment as done.Tue, May 14, 2:10 PM

address feedback and land