Skip to content

fix: Don't allow OTR message editing #35600

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

Merged
merged 15 commits into from
May 13, 2025
Merged

fix: Don't allow OTR message editing #35600

merged 15 commits into from
May 13, 2025

Conversation

yash-rajpal
Copy link
Member

@yash-rajpal yash-rajpal commented Mar 24, 2025

Proposed changes (including videos or screenshots)

We currently don't support the editing of OTR messages, but still show the option in UI, this PR blocks this option in UI.

Issue(s)

Steps to test or reproduce

  • Start an OTR session
  • Send some OTR messages
  • Try to edit OTR messages

Further comments

SUP-701


This pull request addresses the issue of editing Off-The-Record (OTR) messages in the Rocket.Chat application. The changes include:

  1. Adding a changeset to block the edit message action for OTR messages.
  2. Modifying the ChatMessages.ts file by changing the method call from findLastOwnMessage to findPreviousOwnMessage, which corrects the logic for navigating through messages.
  3. Updating the useEditMessageAction.ts file to include checks for OTR messages in the edit message functionality.
  4. Removing two message-related methods from ChatAPI.ts and altering the findPreviousOwnMessage method to make the message parameter optional.
  5. Enhancing the chat data API in data.ts by adding checks to prevent editing OTR messages and adjusting the logic for finding the previous editable message authored by the current user.

These changes ensure that OTR messages cannot be edited, maintaining the integrity and privacy of such communications.

Copy link
Contributor

dionisio-bot bot commented Mar 24, 2025

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented Mar 24, 2025

🦋 Changeset detected

Latest commit: cfec8a1

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

This PR includes changesets to release 36 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts 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/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/ui-voip Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/instance-status 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

Copy link
Contributor

github-actions bot commented Mar 24, 2025

PR Preview Action v1.6.1

🚀 View preview at
https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-35600/

Built to branch gh-pages at 2025-05-13 10:31 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Copy link

codecov bot commented Mar 24, 2025

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 64.72%. Comparing base (df48946) to head (cfec8a1).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #35600      +/-   ##
===========================================
+ Coverage    64.67%   64.72%   +0.04%     
===========================================
  Files         3244     3242       -2     
  Lines        95394    95179     -215     
  Branches     17858    17817      -41     
===========================================
- Hits         61695    61601      -94     
+ Misses       30805    30733      -72     
+ Partials      2894     2845      -49     
Flag Coverage Δ
e2e 58.25% <87.50%> (+0.05%) ⬆️
unit 71.91% <100.00%> (-0.05%) ⬇️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yash-rajpal yash-rajpal marked this pull request as ready for review March 24, 2025 17:52
@yash-rajpal yash-rajpal requested a review from a team as a code owner March 24, 2025 17:52
@yash-rajpal yash-rajpal added this to the 7.6.0 milestone Mar 24, 2025
Copy link

kodus-ai bot commented Apr 15, 2025

Kody Review Complete

Great news! 🎉
No issues were found that match your current review configurations.

Keep up the excellent work! 🚀

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Security
Code Style
Kody Rules
Refactoring
Error Handling
Maintainability
Potential Issues
Documentation And Comments
Performance And Optimization
Breaking Changes

Access your configuration settings here.

@yash-rajpal yash-rajpal requested a review from gabriellsh April 15, 2025 13:28
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.

Can we add tests to ensure this?

@yash-rajpal
Copy link
Member Author

OTR is a beta feature, we don't fully support it, so we shouldn't invest time and effort writing e2e tests for a beta feature.

gabriellsh
gabriellsh previously approved these changes Apr 17, 2025
Copy link

kodus-ai bot commented Apr 17, 2025

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Security
Code Style
Kody Rules
Refactoring
Error Handling
Maintainability
Potential Issues
Documentation And Comments
Performance And Optimization
Breaking Changes

Access your configuration settings here.

@ggazzo
Copy link
Member

ggazzo commented Apr 17, 2025

OTR is a beta feature, we don't fully support it, so we shouldn't invest time and effort writing e2e tests for a beta feature.

I think we will never leave beta if we don't have tests, so if it's worth the effort to fix it, it's worth the effort to test it. ;)

Copy link
Member

@scuciatto scuciatto left a comment

Choose a reason for hiding this comment

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

Although there are ongoing discussions about deprecating this feature, no final decision has been made yet, so it’s still worthwhile to include tests for it.

@scuciatto scuciatto modified the milestones: 7.6.0, 7.7.0 Apr 21, 2025
@dougfabris dougfabris requested a review from scuciatto May 8, 2025 13:42
@yash-rajpal yash-rajpal dismissed stale reviews from MarcosSpessatto and gabriellsh via fce3763 May 8, 2025 14:52
Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

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

Looks good to me!
Approving on behalf of frontend

@dougfabris dougfabris added the stat: QA assured Means it has been tested and approved by a company insider label May 12, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label May 12, 2025
@kodiakhq kodiakhq bot merged commit eb3d03f into develop May 13, 2025
48 checks passed
@kodiakhq kodiakhq bot deleted the fix/otr-message-editing branch May 13, 2025 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA assured Means it has been tested and approved by a company insider 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