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: Message mentions displayed in the UI are outdated after changing username #30774

Closed
wants to merge 26 commits into from

Conversation

heitortanoue
Copy link
Contributor

@heitortanoue heitortanoue commented Oct 26, 2023

Proposed changes (including videos or screenshots)

Reparse the markdown (md) field when updating messages that mention the username changed.

Issue(s)

Steps to test or reproduce

  • Send a message mentioning a user.
  • Change the user's username
  • Check if the mentioned message is still working
    (test also the DISABLE_MESSAGE_PARSER env config to see if it is correctly disabling the parsing)

Further comments

SUP-378

@changeset-bot
Copy link

changeset-bot bot commented Oct 26, 2023

🦋 Changeset detected

Latest commit: 1d3a745

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/model-typings Patch
@rocket.chat/models Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/omnichannel-services Patch
rocketchat-services Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/instance-status Patch
@rocket.chat/presence Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/api-client Patch
@rocket.chat/license Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/web-ui-registration Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (1b486a1) 49.57% compared to head (1d3a745) 76.72%.
Report is 7 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop   #30774       +/-   ##
============================================
+ Coverage    49.57%   76.72%   +27.14%     
============================================
  Files         3315      277     -3038     
  Lines        81425     8963    -72462     
  Branches     16707     1695    -15012     
============================================
- Hits         40369     6877    -33492     
+ Misses       36338     1662    -34676     
+ Partials      4718      424     -4294     
Flag Coverage Δ
e2e ?
e2e-api ?
unit 76.72% <66.66%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@heitortanoue heitortanoue marked this pull request as ready for review October 31, 2023 14:31
@heitortanoue heitortanoue requested review from a team as code owners October 31, 2023 14:31
yash-rajpal
yash-rajpal previously approved these changes Oct 31, 2023
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Nov 3, 2023
@scuciatto scuciatto added this to the 6.6.0 milestone Nov 3, 2023
MarcosSpessatto
MarcosSpessatto previously approved these changes Nov 3, 2023
Copy link
Member

@MarcosSpessatto MarcosSpessatto left a comment

Choose a reason for hiding this comment

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

@heitortanoue Can we add some tests to cover this case and prevent that from happening in the future? We can create it in a separate PR if you prefer.

@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Nov 20, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Nov 21, 2023
Copy link
Contributor

@matheusbsilva137 matheusbsilva137 left a comment

Choose a reason for hiding this comment

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

Code for this approach LGTM. We just gotta check if Architecture thinks we can move on with this one or if they have any other suggestion

@casalsgh casalsgh requested a review from a team December 20, 2023 14:11
@heitortanoue heitortanoue marked this pull request as ready for review December 29, 2023 20:04
Copy link
Contributor

@matheusbsilva137 matheusbsilva137 left a comment

Choose a reason for hiding this comment

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

Code LGTM. We just need to check if this approach looks fine to Architecture

@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Jan 11, 2024
@scuciatto scuciatto closed this Jan 16, 2024
@debdutdeb debdutdeb deleted the fix/refresh-md-on-username-change branch January 16, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad: team-collab stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants