Paths

Table of Contentst

Differential D5197

[lib] Implement `messageContentForServerDB` for `messageTypes.MULTIMEDIA`
ClosedPublic
Actions

Authored by atul on Sep 20 2022, 9:01 AM.

Tags

None

Referenced Files

	F3345752: D5197.diff
	Fri, Nov 22, 6:51 AM

	Unknown Object (File)
	Mon, Nov 18, 10:41 AM

	Unknown Object (File)
	Mon, Nov 11, 2:51 AM

	Unknown Object (File)
	Sun, Nov 10, 12:27 PM

	Unknown Object (File)
	Sat, Nov 9, 12:56 PM

	Unknown Object (File)
	Sat, Nov 2, 1:15 PM

	Unknown Object (File)
	Sat, Nov 2, 1:15 PM

	Unknown Object (File)
	Sat, Nov 2, 1:15 PM

Subscribers

Details

Reviewers

• abosh
tomek
marcin

Commits

rCOMM4597a9a14843: [lib] Implement `messageContentForServerDB` for `messageTypes.MULTIMEDIA`

Summary

The messageContentForServerDB(...) function determines what gets put in the content column of the messages table.

The current content column for media messages is a "stringified" list of uploadIDs. However, the content column is never used after it's been stored in the DB. Rather, messages are joined with corresponding uploads via the container column of the uploads table. Therefore this change isn't "destructive."

Depends on D5195

Test Plan

Rows in the messages and uploads column appear as expected:

Diff Detail

Repository

Branch

arcpatch-D5197 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Sep 20 2022, 9:01 AM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 20 2022, 9:01 AM

atul edited the test plan for this revision. (Show Details)Sep 20 2022, 9:04 AM

atul added 1 blocking reviewer(s): tomek.Sep 20 2022, 9:06 AM

Harbormaster completed remote builds in B12327: Diff 16919.Sep 20 2022, 9:09 AM

atul requested review of this revision.Sep 20 2022, 9:09 AM

atul edited the summary of this revision. (Show Details)Sep 20 2022, 9:38 AM

atul added a parent revision: D5195: [keyserver] Construct `MULTIMEDIA` message media correctly in `rawMessageInfoFromRows(...)`.

atul added a child revision: D5198: [keyserver] Introduce `getMediaMessageServerDBContentsFromMedia(...)`.Sep 20 2022, 9:38 AM

I would reverse these conditions to overall reduce indentation level

EDIT

Never mind, it's cleaner in D5198. I probably would've submitted that first and this diff second

Is it safe for both server and client DB?

Should we have migrations on both platforms that update the values?

This revision is now accepted and ready to land.Sep 21 2022, 6:59 AM

In D5197#152670, @tomek wrote:

Is it safe for both server and client DB?

Should we have migrations on both platforms that update the values?

Yeah it's safe! There aren't any media messages in the database of type 15 (cc @ashoat who checked recently) so we're starting from a clean slate.

There certainly are issues if someone has video messages in their local environment, but that should only affect @abosh and I.

rebase before landing

Harbormaster completed remote builds in B12356: Diff 16964.Sep 21 2022, 2:15 PM

Closed by commit rCOMM4597a9a14843: [lib] Implement `messageContentForServerDB` for `messageTypes.MULTIMEDIA` (authored by atul). · Explain WhySep 22 2022, 6:03 AM

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM4597a9a14843: [lib] Implement `messageContentForServerDB` for `messageTypes.MULTIMEDIA`.

Revision Contents
Changeset List

Path

Size

lib/

shared/

messages/

multimedia-message-spec.js

17 lines

Diff 16964

lib/shared/messages/multimedia-message-spec.js

Loading...