[keyserver/lib] Restrict the derivedMessages parameter from being passed to rawMessageInfoFromServerDBRow
ClosedPublic
Actions

Authored by rohan on Oct 19 2023, 10:13 AM.

Details

Reviewers

atul
ginsu

Commits

rCOMM3f98322f60fe: [keyserver/lib] Restrict the derivedMessages parameter from being passed to…

Summary

This should hopefully be the last step of ENG-4849. Now that we have a function parseDerivedMessages defined only in the message specs that will need to support derived messages (SIDEBAR_SOURCE and TOGGLE_PIN), we want to update the callsites of rawMessageInfoFromServerDBRow in message-fetchers to only pass in derivedMessages if necessary. This means for rawMessageInfoFromServerDBRow to receive derivedMessages, the message spec must have a parseDerivedMessages function in it.

Regarding the callsites, although we definitely don't need to pass in derivedMessages for the multimedia spec (I checked that it's not destructred), just to have it resistant to change I've only excluded it if parseDerivedMessages is not defined. For all other message specs, the same logic applies as well.

Resolves https://linear.app/comm/issue/ENG-5320/restrict-the-derivedmessages-parameter-from-being-passed-to

Depends on D9542

Test Plan

Ran flow and manually tested the web app to confirm that there are no regressions with this change

Diff Detail

Repository

rCOMM Comm

Branch

ENG-4849

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

rohan created this revision.Oct 19 2023, 10:13 AM

Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptOct 19 2023, 10:13 AM

Harbormaster completed remote builds in B23374: Diff 32221.Oct 19 2023, 10:33 AM

rohan requested review of this revision.Oct 19 2023, 10:33 AM

rohan added inline comments.Oct 19 2023, 10:37 AM

lib/shared/messages/toggle-pin-message-spec.js
52 ↗	(On Diff #32221)	Same `invariant` as sidebar-source-message-spec

ashoat added inline comments.Oct 19 2023, 11:54 AM

keyserver/src/fetchers/message-fetchers.js
249 ↗	(On Diff #32221)	Won't this always be true? If it's not specified it will be `undefined`, and `undefined !== null` so this would be `true`

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

keyserver/src/fetchers/message-fetchers.js
249 ↗	(On Diff #32221)	Yeah you're right, oversight on my part

Check undefined instead of null. Confirmed that previously requiresDerivedMessages was always true regardless of the type, but now it's only true for TOGGLE_PIN and SIDEBAR_SOURCE