Details

Reviewers

michal
inka
ginsu

Commits

rCOMM0234a2396b70: [lib][native] Displaying edited messages from database

Summary

Now edited messages are visible in the chats. Currently, it's only possible to edit the message (send a new message with edit_message type) from the database. Editing message
which is source of sidebar currently isn't visible in the sidebar view.

Test Plan

added edited message to the database, checked if it shows correctly,
edited several times the same message, checked if the latest one is shown,
edited a few messages in one thread, checked if the messages are updated correctly,
edited a few messages in a few threads, checked if the messages are correctly updated

Screenshot 2023-03-07 at 11.01.26.png (1×2 px, 782 KB)

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• kuba created this revision.Mar 7 2023, 1:44 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptMar 7 2023, 1:44 AM

• kuba added a parent revision: D6964: [native] Added new editing messages type to MessageSpecs in C++.Mar 7 2023, 1:44 AM

• kuba added a child revision: D6966: [lib] Add 'hasBeenEdited' status to messages.Mar 7 2023, 1:48 AM

Harbormaster completed remote builds in B17138: Diff 23478.Mar 7 2023, 1:59 AM

• kuba requested review of this revision.Mar 7 2023, 1:59 AM

• kuba edited the test plan for this revision. (Show Details)Mar 7 2023, 2:01 AM

ashoat added inline comments.Mar 7 2023, 6:07 AM

lib/selectors/chat-selectors.js
403 ↗	(On Diff #23478)	See comment in D6966 about read-only types
lib/types/messages/text.js
31 ↗	(On Diff #23478)	Make this read-only

• kuba added a reviewer: ginsu.Mar 7 2023, 8:11 AM

Responded to comments, made read-only 'text' property

Harbormaster completed remote builds in B17180: Diff 23528.Mar 8 2023, 3:15 AM

Looks good to me, but I haven't worked with this code so it would be nice if someone else could also take a look.

This revision is now accepted and ready to land.Mar 13 2023, 5:59 AM

ashoat added 1 blocking reviewer(s): ginsu.Mar 13 2023, 9:47 AM

This revision now requires review to proceed.Mar 13 2023, 9:47 AM

accepting with one question inline

lib/selectors/chat-selectors.js
405	Could we simply this condition to this? Or are we concerned about `newText` possibly being an empty string?

This revision is now accepted and ready to land.Mar 16 2023, 5:33 PM

Simplified condition

• kuba retitled this revision from [lib][native] Displaying edited messages from database to [lib] Displaying edited messages from database.Mar 22 2023, 5:23 AM

Revert the changes

lib/selectors/chat-selectors.js
405	I changed it, but then after more thought, I believe, that it should stay like this. If the `newText` is empty we would prefer to display an even empty message, than the original content (that is what the user would probably want). I revert the changes. We can consider preventing users from sending empty edit messages. But in the future, it would be easy to add deleting messages by sending edit messages with empty text. We would just not display them at all.

Harbormaster completed remote builds in B17513: Diff 23981.Mar 22 2023, 5:29 AM

Harbormaster completed remote builds in B17514: Diff 23982.Mar 22 2023, 5:38 AM

We can consider preventing users from sending empty edit messages.

We should do this. We don't allow empty messages for messageTypes.TEXT. Can you create a follow-up Linear task for this before landing?

In D6965#212104, @ashoat wrote:

We can consider preventing users from sending empty edit messages.

We should do this. We don't allow empty messages for messageTypes.TEXT. Can you create a follow-up Linear task for this before landing?

Done: https://linear.app/comm/issue/ENG-3395/prevent-empty-messages-in-message-editing

Closed by commit rCOMM0234a2396b70: [lib][native] Displaying edited messages from database (authored by kosmydel <kukubaczek@gmail.com>). · Explain WhyMar 30 2023, 7:36 AM

This revision was automatically updated to reflect the committed changes.

kosmydel <kukubaczek@gmail.com> added a commit: rCOMM0234a2396b70: [lib][native] Displaying edited messages from database.