Page MenuHomePhabricator
Differential D9543

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

Authored by rohan on Oct 19 2023, 10:13 AM.
Tags
None
Referenced Files
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)
Tue, Nov 19, 11:37 PM
Unknown Object (File)
Thu, Nov 14, 8:27 PM
Unknown Object (File)
Thu, Nov 7, 3:54 PM
View All 74 Files
Subscribers
ashoat
tomek
will

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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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

rohan updated this revision to Diff 32228.Oct 19 2023, 12:11 PM

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

Harbormaster completed remote builds in B23381: Diff 32228.Oct 19 2023, 12:29 PM
rohan added a child revision: D9544: [web/native] Prevent text/multimedia from being pinned if they begin a sidebar.Oct 19 2023, 12:49 PM
atul accepted this revision.Oct 20 2023, 12:07 PM
This revision is now accepted and ready to land.Oct 20 2023, 12:07 PM
Closed by commit rCOMM3f98322f60fe: [keyserver/lib] Restrict the derivedMessages parameter from being passed to… (authored by rohan). · Explain WhyOct 20 2023, 12:19 PM
This revision was automatically updated to reflect the committed changes.
rohan added a commit: rCOMM3f98322f60fe: [keyserver/lib] Restrict the derivedMessages parameter from being passed to….

Revision Contents
Changeset List

PathSize
keyserver/
src/
fetchers/
37 lines
lib/
shared/
messages/
2 lines
3 lines

Diff 32291

View Options

keyserver/src/fetchers/message-fetchers.js

Loading...
View Options

lib/shared/messages/message-spec.js

Loading...
View Options

lib/shared/messages/toggle-pin-message-spec.js

Loading...