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] Auto Translate not working on new message template #27317

Merged
merged 33 commits into from
Jan 16, 2023
Merged

Conversation

filipemarins
Copy link
Contributor

@filipemarins filipemarins commented Nov 22, 2022

Proposed changes (including videos or screenshots)

  • Fix auto translate not working on new message template;
  • Fix a bug that the auto-translate icon was showing for users who didn't have enabled.
  • Show translated quote messages;
  • Remove translation provider for end users.
  • Translate the ThreadMessagePreview component

Issue(s)

closes #27567

Steps to test or reproduce

Further comments

I had to move useAutoTranslate and useKatex to a separate hook cause on MessageList component the context isn't fulfilled yet.

Quote attachment
Screenshot 2022-11-22 at 11 07 34

Multilevel quote attachments get translated as well:
Screenshot 2022-11-22 at 11 13 21

But if you try to translate a message that wasn't translated yet and has multiple levels of quote message it will only translate the first quote message...

ThreadMessagePreview component translated

Screenshot 2022-11-23 at 14 32 32

@lgtm-com
Copy link

lgtm-com bot commented Nov 22, 2022

This pull request introduces 1 alert when merging b90ea63 into b816803 - view on LGTM.com

new alerts:

  • 1 for Syntax error

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@filipemarins filipemarins marked this pull request as ready for review November 22, 2022 15:29
@filipemarins filipemarins requested review from a team as code owners November 22, 2022 15:29
Copy link
Member

@gabriellsh gabriellsh left a comment

Choose a reason for hiding this comment

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

When sending an attachment, the description was not translated in the main room, but was in the thread.
image

packages/core-typings/src/IMessage/IMessage.ts Outdated Show resolved Hide resolved
apps/meteor/app/autotranslate/client/lib/actionButton.ts Outdated Show resolved Hide resolved
apps/meteor/app/autotranslate/client/lib/actionButton.ts Outdated Show resolved Hide resolved
apps/meteor/app/autotranslate/client/lib/actionButton.ts Outdated Show resolved Hide resolved
apps/meteor/app/autotranslate/client/lib/actionButton.ts Outdated Show resolved Hide resolved
apps/meteor/app/autotranslate/server/autotranslate.ts Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Merging #27317 (f0c0b16) into develop (69c7248) will decrease coverage by 0.02%.
The diff coverage is 47.76%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #27317      +/-   ##
===========================================
- Coverage    42.90%   42.88%   -0.03%     
===========================================
  Files          814      817       +3     
  Lines        17199    17234      +35     
  Branches      1923     1934      +11     
===========================================
+ Hits          7380     7391      +11     
- Misses        9563     9579      +16     
- Partials       256      264       +8     
Flag Coverage Δ
e2e 42.88% <47.76%> (-0.03%) ⬇️

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

@filipemarins
Copy link
Contributor Author

When sending an attachment, the description was not translated in the main room, but was in the thread. image

Inside the thread, we are having the wrong behavior, we should not translate our own message. I was only focused on the main message list but nice catch, I will fix it on the thread.

gabriellsh
gabriellsh previously approved these changes Nov 24, 2022
Copy link
Member

@gabriellsh gabriellsh left a comment

Choose a reason for hiding this comment

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

Since this is a very large PR on an important feature, I'll ask for an additional review.

hugocostadev
hugocostadev previously approved these changes Dec 7, 2022
@hugocostadev hugocostadev added this to the 6.0.0 milestone Jan 9, 2023
@casalsgh casalsgh requested review from a team January 10, 2023 18:06
tassoevan
tassoevan previously approved these changes Jan 12, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: conflict labels Jan 16, 2023
@kodiakhq kodiakhq bot merged commit c420e99 into develop Jan 16, 2023
@kodiakhq kodiakhq bot deleted the tc-223 branch January 16, 2023 18:38
gabriellsh added a commit that referenced this pull request Jan 16, 2023
…ixSearch

* 'develop' of github.com:RocketChat/Rocket.Chat:
  Chore: Show different labels based on the call originator on direct calls (#27729)
  Chore: Change bundle tags color and refactor app details page header styles (#27293)
  [FIX] Auto Translate not working on new message template (#27317)
  Chore: Custom Sounds Empty State (#27632)
  [IMPROVE] Require acceptance when setting new E2E Encryption key for another user (#27556)
  Regression: Broken room and message composer events (#27754)
  [FIX] Missing placeholders from encrypted channel preview messages now with expected behavior (#27699)
  [IMPROVE] SAML and OAuth role sync to support id and name (#27405)
  Chore: add e2e test coverage for Video Conference (#27075)
  Chore: Invites Empty State (#27631)
  [FIX] Marketplace app status initially disabled (#27330)
  Regression: Missing contexts on contextual bar (#27734)
gabriellsh added a commit that referenced this pull request Jan 17, 2023
* matrixSearch:
  First iteration
  Chore: Show different labels based on the call originator on direct calls (#27729)
  Chore: Change bundle tags color and refactor app details page header styles (#27293)
  [FIX] Auto Translate not working on new message template (#27317)
 the commit.
@sampaiodiego sampaiodiego mentioned this pull request Feb 17, 2023
@sampaiodiego sampaiodiego mentioned this pull request Mar 9, 2023
13 tasks
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.

Bold font in the description
4 participants