Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Show updated relation reply from edited message - v2 #6817

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Sep 16, 2021

Follow-up to #6809


Show updated relation reply from edited message

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"
    }
  }
}

Dev notes

npm test -- test/components/views/elements/ReplyThread-test.js

Here's what your changelog entry will look like:

✨ Features

Preview: https://6142e26957e9c0d62cdb9763--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

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 MadLittleMods requested a review from a team as a code owner September 16, 2021 05:22
@MadLittleMods MadLittleMods added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Sep 16, 2021
@MadLittleMods MadLittleMods changed the title Show updated relation reply from edited message Show updated relation reply from edited message - v2 Sep 16, 2021
Fixes:

```
Attempted to get a component (views.elements.Flair) before a skin has been loaded. This is probably because either: a) Your app has not called sdk.loadSkin(), or b) A component has called getComponent at the root level

      23 |         if (!name) throw new Error(`Invalid component name: ${name}`);
      24 |         if (this.components === null) {
    > 25 |             throw new Error(
         |                   ^
      26 |                 `Attempted to get a component (${name}) before a skin has been loaded.`+
      27 |                 " This is probably because either:"+
      28 |                 " a) Your app has not called sdk.loadSkin(), or"+

      at Skinner.getComponent (src/Skinner.js:25:19)
      at Object.getComponent (src/index.js:29:20)
      at src/utils/replaceableComponent.ts:41:22
      at Object.<anonymous> (src/components/views/elements/Flair.js:65:94)
      at Object.<anonymous> (src/components/views/messages/SenderProfile.tsx:21:1)
      at Object.<anonymous> (src/components/views/rooms/ReplyTile.tsx:23:1)
      at Object.<anonymous> (src/components/views/elements/ReplyThread.tsx:34:1)
      at Object.<anonymous> (test/components/views/elements/ReplyThread-test.js:3:1)
```
@MadLittleMods MadLittleMods force-pushed the madlittlemods/10391-show-relations-in-new-edit-content-v2 branch from 013cdb8 to 9f6a5b8 Compare September 16, 2021 05:40
@MadLittleMods MadLittleMods marked this pull request as draft September 16, 2021 06:11
@MadLittleMods MadLittleMods force-pushed the madlittlemods/10391-show-relations-in-new-edit-content-v2 branch from ed7ce02 to 7d88e4d Compare September 16, 2021 06:17
@MadLittleMods MadLittleMods marked this pull request as ready for review September 16, 2021 06:17
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tests!

@MadLittleMods MadLittleMods merged commit 9c3439a into develop Sep 17, 2021
@MadLittleMods MadLittleMods deleted the madlittlemods/10391-show-relations-in-new-edit-content-v2 branch September 17, 2021 20:18
@MadLittleMods
Copy link
Contributor Author

Thanks for the review @dbkr 🐃

Comment on lines +92 to +97
// We're prefer ev.getContent() over ev.getWireContent() to make sure
// we grab the latest edit with potentially new relations. But we also
// can't just rely on ev.getContent() by itself because historically we
// still show the reply from the original message even though the edit
// event does not include the relation reply.
const mRelatesTo = ev.getContent()['m.relates_to'] || ev.getWireContent()['m.relates_to'];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracing down where this code has gone since this was merged, the codebase has morphed a bit and this code has moved to:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect, we should only ever look at wire content as per the red notice in the spec

image
image

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants