Page MenuHomePhabricator

[lib] implement checking if user supports thick thread
ClosedPublic

Authored by kamil on Sep 26 2024, 3:59 AM.
Tags
None
Referenced Files
F3256411: D13489.diff
Fri, Nov 15, 8:47 PM
Unknown Object (File)
Sun, Nov 10, 4:18 PM
Unknown Object (File)
Fri, Nov 8, 3:01 AM
Unknown Object (File)
Fri, Nov 8, 3:01 AM
Unknown Object (File)
Fri, Nov 8, 3:01 AM
Unknown Object (File)
Fri, Nov 8, 3:01 AM
Unknown Object (File)
Fri, Nov 8, 3:01 AM
Unknown Object (File)
Fri, Nov 8, 3:01 AM
Subscribers
None

Details

Summary

ENG-9394

Depends on D13488

Test Plan

Search for users and check on the icon:

image.png (2×816 px, 263 KB)

Made sure that users which have Local DM are registered to Identity, other users are either reserved usernames or only from my local keyserver

Diff Detail

Repository
rCOMM Comm
Branch
publish-personal
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Sep 26 2024, 6:47 AM
kamil added inline comments.
lib/actions/user-actions.js
1381–1392 ↗(On Diff #44601)

I had to move it because of the dependency cycle

lib/hooks/thread-search-hooks.js
54–59 ↗(On Diff #44601)

this is called when usingCommServicesAccessToken = false so we're not able to use thick threads anyway

lib/shared/thread-utils.js
1713–1725 ↗(On Diff #44601)

Not the cleanest solution but reduce amount of required changes

lib/hooks/user-identities-hooks.js
24 ↗(On Diff #44601)

I feel like this would have been much easier to review if it had been split up more

lib/actions/user-actions.js
1381–1392 ↗(On Diff #44601)

This should've been a separate diff

lib/hooks/thread-search-hooks.js
90 ↗(On Diff #44601)

Invert condition

lib/hooks/user-identities-hooks.js
11–22 ↗(On Diff #44601)

Combining this here feels risky for dependency cycles. I think that separating this out into its own file is the best way to avoid getting the dependency cycle again

26 ↗(On Diff #44601)

I'd consider returning a set here... it feels like that would be more convenient for callers than an array

37 ↗(On Diff #44601)
lib/shared/thread-utils.js
1714 ↗(On Diff #44601)
This revision is now accepted and ready to land.Sep 26 2024, 2:18 PM
This revision was landed with ongoing or failed builds.Sep 30 2024, 5:34 AM
This revision was automatically updated to reflect the committed changes.