Show updated relation reply from edited message #6809

MadLittleMods · 2021-09-14T21:11:45Z

Show updated relation reply from edited message

Part of Message editing: attach message as reply to other message via Edit form element-hq/element-meta#1047
My goal is to use this for https://gitlab.com/gitterHQ/webapp/-/merge_requests/2229#note_659864130 so I can import all messages in one pass, and then in second pass, update the messages with their threaded conversation relations.
Related MSC discussion: MSC2676: Message editing matrix-spec-proposals#2676 (comment)

When m.relates_to -> m.in_reply_to is provided in m.new_content
for an edited message, use the updated reply.

Before	After

ex.

{
  "type": "m.room.message",
  "content": {
    "body": " * foo bar",
    "msgtype": "m.text",
    "m.new_content": {
      "body": "foo bar",
      "msgtype": "m.text",
      "m.relates_to": {
        "m.in_reply_to": {
          "event_id": "$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og"
        }
      }
    },
    "m.relates_to": {
      "rel_type": "m.replace",
      "event_id": "$lX9MRe9ZTFOOvnU8PRVbvr1wqGtYvNQ1rSot-iUTN5k"
    }
  }
}

Here's what your changelog entry will look like:

✨ Features

Show updated relation reply from edited message (#6809). Contributed by MadLittleMods.

Preview: https://61421f65dde597075b63cc01--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

Part of https://github.com/vector-im/element-web/issues/10391#issuecomment-906131724 When `m.relates_to` -> `m.in_reply_to` is provided in `m.new_content` for an edited message, use the updated reply. ex. ```json { "type": "m.room.message", "content": { "body": " * foo bar", "msgtype": "m.text", "m.new_content": { "body": "foo bar", "msgtype": "m.text", "m.relates_to": { "m.in_reply_to": { "event_id": "$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og" } } }, "m.relates_to": { "rel_type": "m.replace", "event_id": "$lX9MRe9ZTFOOvnU8PRVbvr1wqGtYvNQ1rSot-iUTN5k" } } } ```

MadLittleMods · 2021-09-14T21:23:36Z

src/components/views/elements/ReplyThread.tsx

+        //
+        // We're using ev.getContent() over ev.getWireContent() to make sure
+        // we grab the latest edit with potentially new relations.
+        const mRelatesTo = ev.getContent()['m.relates_to'];


This works because event.getContent() is smart enough to pull from the latest replacing event (message edit), this._replacingEvent.getContent()["m.new_content"] .

We probably want to also update matrix-js-sdk event.getRelation() to pull from getContent() as well, src/models/event.ts#L1077-L1087. I can create a follow-up PR after this is deemed acceptable 🙇

Updating the js-sdk makes sense, though I'm a bit concerned that the impact could be greater than we anticipate. Until it's reported to cause bugs, we should leave it alone I think.

I know what you mean 🤔

I feel like we may be leaving other clients in the dust if we don't change it though. Someone making a client, bridge, etc just falls into the pitfall of not supporting this use case.

We do have a breaking change queued up for the js-sdk's next release, so squeezing it in there would be the safest bet I think.

I guess I'll skip on adjusting it there because trying to make the change and I run into the rel_type and event_id constraint which isn't present for m.in_repy_to and not sure of the downstream effects.

turt2live

Thanks!

turt2live · 2021-09-15T15:21:51Z

src/components/views/elements/ReplyThread.tsx

+        //
+        // We're using ev.getContent() over ev.getWireContent() to make sure
+        // we grab the latest edit with potentially new relations.
+        const mRelatesTo = ev.getContent()['m.relates_to'];


Updating the js-sdk makes sense, though I'm a bit concerned that the impact could be greater than we anticipate. Until it's reported to cause bugs, we should leave it alone I think.

turt2live · 2021-09-15T15:22:55Z

I think the CI is unhappy because of things outside of your control: merging develop back into this branch should fix it.

…-edit-content

MadLittleMods · 2021-09-15T16:50:31Z

Thanks for the review @turt2live 🦐 Happy to see this use case supported now!

MadLittleMods · 2021-09-15T19:57:45Z

@turt2live After pondering about element-hq/element-web#19062, I came up with a previous use case that is now broken.

If you reply to a message originally, then edit the message, the reply disappears. This worked previously because we always looked at the original message. Will need to think of something smarter here. Would you prefer to revert in the mean-time?

turt2live · 2021-09-15T20:34:24Z

that does sound not-great :(

reverting is probably the best course of action for now, at least until we can figure out how all these systems are meant to work together (iirc edits and reply fallbacks in particular are largely undefined spec and MSC wise). Are you able to take a look at what might be going sideways?

This reverts commit eaab8e1.

MadLittleMods · 2021-09-16T02:12:38Z

Are you able to take a look at what might be going sideways?

Yes, I'll write some tests around this too so we don't regress what we know 👍

MadLittleMods · 2021-09-16T05:47:20Z

v2 created at #6817 ⏩

…nd-quoted-messages * origin/develop: (80 commits) Revert "Show updated relation reply from edited message (#6809)" Show updated relation reply from edited message (#6809) yarn upgrade Pin react too... Pin typescript/types version Fix checks to show prompt to start new chats Fix comment lint fix tests Update comment reflecting tracking scheme via analytics ID Remove all room data from tracking Fix import, convert event type to constant fix lockfile Remove replies from hidden events when shown with `messages.ViewSourceEvent` (#6796) Convert StatusMessageContextMenu to TS Convert GenericTextContextMenu to TS Convert GenericElementContextMenu to TS Convert MemberStatusMessageAvatar to TS More lint lint ...

Palid · 2021-09-17T14:14:58Z

@turt2live After pondering about vector-im/element-web#19062, I came up with a previous use case that is now broken.

If you reply to a message originally, then edit the message, the reply disappears. This worked previously because we always looked at the original message. Will need to think of something smarter here. Would you prefer to revert in the mean-time?

Oh, and I wondered why the threaded things are disappearing yesterday and couldn't figure out what's going on! 😄

MadLittleMods requested a review from a team as a code owner September 14, 2021 21:11

MadLittleMods added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Sep 14, 2021

MadLittleMods marked this pull request as draft September 14, 2021 21:11

Remove debug logs

faa80bd

MadLittleMods commented Sep 14, 2021

View reviewed changes

MadLittleMods changed the title ~~Draft: Show updated relation replies from edited message~~ Show updated relation replies from edited message Sep 14, 2021

MadLittleMods marked this pull request as ready for review September 14, 2021 21:29

MadLittleMods mentioned this pull request Sep 14, 2021

Message editing: attach message as reply to other message via Edit form element-hq/element-meta#1047

Open

MadLittleMods changed the title ~~Show updated relation replies from edited message~~ Show updated relation reply from edited message Sep 15, 2021

turt2live approved these changes Sep 15, 2021

View reviewed changes

Merge branch 'develop' into madlittlemods/10391-show-relations-in-new…

235fdfa

…-edit-content

MadLittleMods merged commit eaab8e1 into develop Sep 15, 2021

MadLittleMods deleted the madlittlemods/10391-show-relations-in-new-edit-content branch September 15, 2021 16:49

turt2live added a commit that referenced this pull request Sep 15, 2021

Revert "Show updated relation reply from edited message (#6809)"

a103a2c

This reverts commit eaab8e1.

turt2live mentioned this pull request Sep 15, 2021

Revert "Show updated relation reply from edited message" #6816

Merged

MadLittleMods mentioned this pull request Sep 16, 2021

Show updated relation reply from edited message - v2 #6817

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show updated relation reply from edited message #6809

Show updated relation reply from edited message #6809

MadLittleMods commented Sep 14, 2021 •

edited by github-actions bot

Loading

MadLittleMods Sep 14, 2021 •

edited

Loading

turt2live Sep 15, 2021

MadLittleMods Sep 15, 2021

turt2live Sep 15, 2021

MadLittleMods Sep 18, 2021

turt2live left a comment

turt2live Sep 15, 2021

turt2live commented Sep 15, 2021

MadLittleMods commented Sep 15, 2021

MadLittleMods commented Sep 15, 2021 •

edited

Loading

turt2live commented Sep 15, 2021

MadLittleMods commented Sep 16, 2021

MadLittleMods commented Sep 16, 2021

Palid commented Sep 17, 2021

Show updated relation reply from edited message #6809

Show updated relation reply from edited message #6809

Conversation

MadLittleMods commented Sep 14, 2021 • edited by github-actions bot Loading

✨ Features

MadLittleMods Sep 14, 2021 • edited Loading

Choose a reason for hiding this comment

turt2live Sep 15, 2021

Choose a reason for hiding this comment

MadLittleMods Sep 15, 2021

Choose a reason for hiding this comment

turt2live Sep 15, 2021

Choose a reason for hiding this comment

MadLittleMods Sep 18, 2021

Choose a reason for hiding this comment

turt2live left a comment

Choose a reason for hiding this comment

turt2live Sep 15, 2021

Choose a reason for hiding this comment

turt2live commented Sep 15, 2021

MadLittleMods commented Sep 15, 2021

MadLittleMods commented Sep 15, 2021 • edited Loading

turt2live commented Sep 15, 2021

MadLittleMods commented Sep 16, 2021

MadLittleMods commented Sep 16, 2021

Palid commented Sep 17, 2021

MadLittleMods commented Sep 14, 2021 •

edited by github-actions bot

Loading

MadLittleMods Sep 14, 2021 •

edited

Loading

MadLittleMods commented Sep 15, 2021 •

edited

Loading