[keyserver/lib] Introduce a parseDerivedMessages function in the message specs
ClosedPublic
Actions

Authored by rohan on Oct 19 2023, 9:16 AM.

Details

Reviewers

atul
ginsu

Commits

rCOMMad06bcbd05f1: [keyserver/lib] Introduce a parseDerivedMessages function in the message specs

Summary

The last step of ENG-4849 is to simplify the derived messages logic in fetchDerivedMessages. Ideally, rather than handling parsing out the JSON object in the function itself, we'll introduce a function in specific message specs (only SIDEBAR_SOURCE and TOGGLE_PIN since those are the ones that are involved here) that, if defined, contains the logic for parsing in itself. This once again moves the source of truth to the message specs. The next diff will involve updating the callsites of rawMessageInfoFromServerDBRow to only pass in derivedMessages as a param if this function is defined in the message spec.

Resolves https://linear.app/comm/issue/ENG-5220/introduce-a-requiresderivedmessages-function-in-the-message-specs

Depends on D9538

Test Plan

Logged out requiredIDs before and after this change when loading Comm and confirmed that the Sets were the same containing the same message IDs

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Oct 19 2023, 9:16 AM

rohan held this revision as a draft.

Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptOct 19 2023, 9:16 AM

Destructure parseDerivedMessages

Harbormaster completed remote builds in B23372: Diff 32218.Oct 19 2023, 9:35 AM

Harbormaster completed remote builds in B23373: Diff 32220.Oct 19 2023, 9:37 AM

rohan published this revision for review.Oct 19 2023, 9:59 AM

rohan added a child revision: D9543: [keyserver/lib] Restrict the derivedMessages parameter from being passed to rawMessageInfoFromServerDBRow.Oct 19 2023, 10:13 AM

Good to land as is

keyserver/src/fetchers/message-fetchers.js
724–728 ↗	(On Diff #32220)	You could do something like messageSpecs[row.type]?.parseDerivedMessages?.(row, requiredIDs); or maybe more realistically const { parseDerivedMessages } = messageSpecs[row.type]; parseDerivedMessages?.(row, requiredIDs); Whatever you prefer though, totally fine as is
lib/shared/messages/sidebar-source-message-spec.js
179–182 ↗	(On Diff #32220)	Should we maybe do some validation/error-handling here?

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

rohan added inline comments.Oct 19 2023, 12:05 PM

lib/shared/messages/sidebar-source-message-spec.js
179–182 ↗	(On Diff #32220)	We could do, I just matched the same behavior that exists on `master` currently. We haven't had any issues with the parsing so far. I could at least catch the error and console log it