Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): Timeline::send_reply correctly sets up m.mentions #3341

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Apr 18, 2024

In #2691, I suppose
the way add_mentions is computed is… wrong. AddMentions is used to
automatically infer the m.mentions of the reply event based on the
replied event. The way it was computed was based on the reply event
mentions, which seems wrong: if the reply contains mentions, then the
sender should be part of it? Nah. That's a bug. We want the reply event
to automatically mention the sender of the replied event if and only
if it's not the same as the current user, i.e. the sender of the reply
event.

This patch fixes the add_mentions calculation. This patch also updates
a test and adds another test to ensure that m.mentions is correctly
defined when replying to an event.


@Hywan Hywan requested a review from a team as a code owner April 18, 2024 15:14
@Hywan Hywan requested review from bnjbvr and removed request for a team April 18, 2024 15:14
@Hywan Hywan force-pushed the fix-3314 branch 2 times, most recently from ee3d97d to 09496a0 Compare April 18, 2024 15:23
Copy link
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

Thanks, that's a sweet test!

crates/matrix-sdk-ui/src/timeline/mod.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-ui/src/timeline/mod.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-ui/src/timeline/mod.rs Outdated Show resolved Hide resolved
In matrix-org#2691, I suppose
the way `add_mentions` is computed is… wrong. `AddMentions` is used to
automatically infer the `m.mentions` of the reply event based on the
replied event. The way it was computed was based on the reply event
`mentions`, which seems wrong: if the reply contains mentions, then the
sender should be part of it? Nah. That's a bug. We want the reply event
to automatically mention the sender of the replied event if and only
if it's not the same as the current user, i.e. the sender of the reply
event.

This patch fixes the `add_mentions` calculation. This patch also updates
a test and adds another test to ensure that `m.mentions` is correctly
defined when replying to an event.
Copy link

codecov bot commented Apr 23, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 83.58%. Comparing base (f7329c7) to head (97ce574).
Report is 26 commits behind head on main.

Files Patch % Lines
crates/matrix-sdk-ui/src/timeline/mod.rs 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3341      +/-   ##
==========================================
- Coverage   83.58%   83.58%   -0.01%     
==========================================
  Files         240      240              
  Lines       24841    24843       +2     
==========================================
+ Hits        20764    20765       +1     
- Misses       4077     4078       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Hywan Hywan merged commit 5e347ce into matrix-org:main Apr 23, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When user reply to their message, the reply mentions the current user
2 participants