Page MenuHomePhabricator

[lib] Refactor fetchDerivedMessages to call isInvalidSidebarSource or isInvalidPinSource
ClosedPublic

Authored by rohan on Oct 19 2023, 6:39 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 4:42 PM
Unknown Object (File)
Wed, Nov 20, 4:38 PM
Unknown Object (File)
Wed, Nov 20, 4:38 PM
Unknown Object (File)
Wed, Nov 20, 4:38 PM
Unknown Object (File)
Wed, Nov 20, 4:37 PM
Unknown Object (File)
Wed, Nov 20, 4:18 PM
Unknown Object (File)
Thu, Nov 14, 7:31 PM
Unknown Object (File)
Oct 19 2024, 4:19 AM
Subscribers

Details

Summary

Now that we have both isInvalidSidebarSource and isInvalidPinSource, it makes sense to make the checks in fetchDerivedMessages more specific. Rather than simply filtering out message types generically, we'll handle it more specifically with 1) checking the message type and 2) calling either isInvalidSidebarSource or isInvalidPinSource accordingly.

Once again, there's a FlowFixMe for the same reason as in D9537 -- Flow doesn't refine the types if I don't explicitly invariant on all of the unexpected message types, and that list can either get long or just generally defeats the purpose of moving the logic into message specs to have one single source of truth.

Resolves https://linear.app/comm/issue/ENG-5218/refactor-fetchderivedmessages-to-call-isinvalidsidebarsource-or

Depends on D9537

Test Plan

yarn workspace lib test and repeated the same test plan as D9537 to ensure usage of the app doesn't trigger unexpected invariants

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

rohan held this revision as a draft.
rohan added inline comments.
keyserver/src/fetchers/message-fetchers.js
773 ↗(On Diff #32196)

Once again, there's a FlowFixMe for the same reason as in D9537 -- Flow doesn't refine the types if I don't explicitly invariant on all of the unexpected message types, and that list can either get long or just generally defeats the purpose of moving the logic into message specs to have one single source of truth.

keyserver/src/fetchers/message-fetchers.js
760 ↗(On Diff #32196)

Note - should fix this concatenation

rohan published this revision for review.Oct 19 2023, 7:49 AM
atul added inline comments.
keyserver/src/fetchers/message-fetchers.js
760 ↗(On Diff #32196)

You plan on fixing this before landing, right?

773 ↗(On Diff #32196)

Let's include a concise version of this comment next to the $FlowFixMe so the reasoning is more clear.

This revision is now accepted and ready to land.Oct 19 2023, 11:37 AM

Fix concatenation and add comment