[keyserver] Add function for fetching RawMessageInfo for rows and their related messages
ClosedPublic
Actions

Authored by inka on Apr 24 2023, 2:11 AM.

Details

Reviewers

kamil
bartek
ashoat

Commits

rCOMM8930c29e5400: [keyserver] Add function for fetching RawMessageInfo for rows and their related…

Summary

issue: https://linear.app/comm/issue/ENG-3689/implement-fetching-edits-sidebar-source-and-reactions
This function takes rows from messages table, and returns RawMessageInfo for those rows and messages related, ie edits, reactions and sidebar_sources.
For sidebar_source, edit_message and reaction messages there are no sidebar_source, edit_message and reaction messages (user cannot react to a reaction message etc). So those messages are not passed to
fetchRelatedMessages function.

Test Plan

Called the function on some rows, checked that all related messages are returned (and the messages from the rows).

Diff Detail

Repository

rCOMM Comm

Branch

inka/search_backend

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

inka created this revision.Apr 24 2023, 2:11 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptApr 24 2023, 2:11 AM

inka added a parent revision: D7562: [keyserver] Add function to fetch reactions/edits/sidebar_source for messages.Apr 24 2023, 2:11 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 24 2023, 2:25 AM

Harbormaster failed remote builds in B18683: Diff 25589!

inka added a child revision: D7585: [keyserver] Add function for fetching messages matching search query.Apr 24 2023, 3:25 AM

Rebase

Harbormaster completed remote builds in B18729: Diff 25651.Apr 25 2023, 4:15 AM

inka requested review of this revision.Apr 25 2023, 4:15 AM

ashoat added inline comments.Apr 25 2023, 4:34 AM

keyserver/src/fetchers/message-fetchers.js
750 ↗	(On Diff #25651)	Realizing this error message should be updated to mention `TOGGLE_PIN` (Please adjust to keep under 80 chars)
900–902 ↗	(On Diff #25651)	Since we use this also on line 747, can we factor it out into a function in `lib/types/message-types.js`? (Also look for other cases where it should be used in the codebase)

bartek added inline comments.Apr 26 2023, 5:53 AM

keyserver/src/fetchers/message-fetchers.js
750 ↗	(On Diff #25651)	Or maybe make this error message more generic to avoid having to update the message if we added other message types in the future. The extracted function name of this check (comment below on L900) could potentially be used.

inka mentioned this in D7632: [keyserver, lib] extract checking if message is an edit, reaction or sidebar_source.Apr 26 2023, 7:06 AM

inka added a parent revision: D7632: [keyserver, lib] extract checking if message is an edit, reaction or sidebar_source.Apr 26 2023, 7:07 AM

inka added inline comments.Apr 26 2023, 7:11 AM

keyserver/src/fetchers/message-fetchers.js
900–902 ↗	(On Diff #25651)	I added D7632 to address this, with an explanation why I did it the way I did it, but honestly I think it's worse. I will change this if D7632 is accepted.

inka added inline comments.Apr 27 2023, 7:08 AM

keyserver/src/fetchers/message-fetchers.js
900–902 ↗	(On Diff #25651)	@bartek showed me a better way of doing that (in D7632), and now I think it's fine. I will still wait for D7632 to be accepted, in case the reviewers don't like the functions name or something