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

Merged
merged 4 commits into from
Sep 17, 2021
Merged

Show updated relation reply from edited message - v2 #6817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
88239fd
Show updated relation reply from edited message
MadLittleMods Sep 16, 2021
3f9ad91
Fix lints
MadLittleMods Sep 16, 2021
9f6a5b8
Fix test failing because skinned sdk not initialized
MadLittleMods Sep 16, 2021
7d88e4d
Add comment explaining why both content methods
MadLittleMods Sep 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/components/views/elements/ReplyThread.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,13 @@ export default class ReplyThread extends React.Component<IProps, IState> {
// could be used here for replies as well... However, the helper
// currently assumes the relation has a `rel_type`, which older replies
// do not, so this block is left as-is for now.
const mRelatesTo = ev.getWireContent()['m.relates_to'];
//
// 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'];
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +92 to +97
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

if (mRelatesTo && mRelatesTo['m.in_reply_to']) {
const mInReplyTo = mRelatesTo['m.in_reply_to'];
if (mInReplyTo && mInReplyTo['event_id']) return mInReplyTo['event_id'];
Expand Down
209 changes: 209 additions & 0 deletions test/components/views/elements/ReplyThread-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
import "../../../skinned-sdk";
import * as testUtils from '../../../test-utils';
import ReplyThread from '../../../../src/components/views/elements/ReplyThread';

describe("ReplyThread", () => {
describe('getParentEventId', () => {
it('retrieves relation reply from unedited event', () => {
const originalEventWithRelation = testUtils.mkEvent({
event: true,
type: "m.room.message",
content: {
"msgtype": "m.text",
"body": "> Reply to this message\n\n foo",
"m.relates_to": {
"m.in_reply_to": {
"event_id": "$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og",
},
},
},
user: "some_other_user",
room: "room_id",
});

expect(ReplyThread.getParentEventId(originalEventWithRelation))
.toStrictEqual('$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og');
});

it('retrieves relation reply from original event when edited', () => {
const originalEventWithRelation = testUtils.mkEvent({
event: true,
type: "m.room.message",
content: {
"msgtype": "m.text",
"body": "> Reply to this message\n\n foo",
"m.relates_to": {
"m.in_reply_to": {
"event_id": "$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og",
},
},
},
user: "some_other_user",
room: "room_id",
});

const editEvent = testUtils.mkEvent({
event: true,
type: "m.room.message",
content: {
"msgtype": "m.text",
"body": "> Reply to this message\n\n * foo bar",
"m.new_content": {
"msgtype": "m.text",
"body": "foo bar",
},
"m.relates_to": {
"rel_type": "m.replace",
"event_id": originalEventWithRelation.event_id,
},
},
user: "some_other_user",
room: "room_id",
});

// The edit replaces the original event
originalEventWithRelation.makeReplaced(editEvent);

// The relation should be pulled from the original event
expect(ReplyThread.getParentEventId(originalEventWithRelation))
.toStrictEqual('$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og');
});

it('retrieves relation reply from edit event when provided', () => {
const originalEvent = testUtils.mkEvent({
event: true,
type: "m.room.message",
content: {
msgtype: "m.text",
body: "foo",
},
user: "some_other_user",
room: "room_id",
});

const editEvent = testUtils.mkEvent({
event: true,
type: "m.room.message",
content: {
"msgtype": "m.text",
"body": "> Reply to this message\n\n * foo bar",
"m.new_content": {
"msgtype": "m.text",
"body": "foo bar",
"m.relates_to": {
"m.in_reply_to": {
"event_id": "$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og",
},
},
},
"m.relates_to": {
"rel_type": "m.replace",
"event_id": originalEvent.event_id,
},
},
user: "some_other_user",
room: "room_id",
});

// The edit replaces the original event
originalEvent.makeReplaced(editEvent);

// The relation should be pulled from the edit event
expect(ReplyThread.getParentEventId(originalEvent))
.toStrictEqual('$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og');
});

it('prefers relation reply from edit event over original event', () => {
const originalEventWithRelation = testUtils.mkEvent({
event: true,
type: "m.room.message",
content: {
"msgtype": "m.text",
"body": "> Reply to this message\n\n foo",
"m.relates_to": {
"m.in_reply_to": {
"event_id": "$111",
},
},
},
user: "some_other_user",
room: "room_id",
});

const editEvent = testUtils.mkEvent({
event: true,
type: "m.room.message",
content: {
"msgtype": "m.text",
"body": "> Reply to this message\n\n * foo bar",
"m.new_content": {
"msgtype": "m.text",
"body": "foo bar",
"m.relates_to": {
"m.in_reply_to": {
"event_id": "$999",
},
},
},
"m.relates_to": {
"rel_type": "m.replace",
"event_id": originalEventWithRelation.event_id,
},
},
user: "some_other_user",
room: "room_id",
});

// The edit replaces the original event
originalEventWithRelation.makeReplaced(editEvent);

// The relation should be pulled from the edit event
expect(ReplyThread.getParentEventId(originalEventWithRelation)).toStrictEqual('$999');
});

it('able to clear relation reply from original event by providing empty relation field', () => {
const originalEventWithRelation = testUtils.mkEvent({
event: true,
type: "m.room.message",
content: {
"msgtype": "m.text",
"body": "> Reply to this message\n\n foo",
"m.relates_to": {
"m.in_reply_to": {
"event_id": "$111",
},
},
},
user: "some_other_user",
room: "room_id",
});

const editEvent = testUtils.mkEvent({
event: true,
type: "m.room.message",
content: {
"msgtype": "m.text",
"body": "> Reply to this message\n\n * foo bar",
"m.new_content": {
"msgtype": "m.text",
"body": "foo bar",
// Clear the relation from the original event
"m.relates_to": {},
},
"m.relates_to": {
"rel_type": "m.replace",
"event_id": originalEventWithRelation.event_id,
},
},
user: "some_other_user",
room: "room_id",
});

// The edit replaces the original event
originalEventWithRelation.makeReplaced(editEvent);

// The relation should be pulled from the edit event
expect(ReplyThread.getParentEventId(originalEventWithRelation)).toStrictEqual(undefined);
});
});
});