Skip to content

[FIX] Auto Translate not working on new message template - #27317

Merged
kodiakhq[bot] merged 33 commits into
developfrom
tc-223
Jan 16, 2023
Merged

kodiakhq[bot] merged 33 commits into
developfrom
tc-223

Conversation

@filipemarins

@filipemarins filipemarins commented Nov 22, 2022 •

Copy link
Copy Markdown
Contributor

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

lgtm-com Bot commented Nov 22, 2022

Copy link
Copy Markdown

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 force-pushed the tc-223 branch 2 times, most recently from 244e91a to 6d1f466 Compare November 22, 2022 15:29
@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

@gabriellsh gabriellsh left a comment

Copy link
Copy Markdown
Member

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

Comment thread packages/core-typings/src/IMessage/IMessage.ts Outdated
Comment thread packages/core-typings/src/IMessage/MessageAttachment/MessageAttachmentBase.ts Outdated
Comment thread apps/meteor/client/views/room/MessageList/hooks/useAutoTranslate.ts
Comment thread apps/meteor/client/views/room/MessageList/components/ThreadMessagePreviewBody.tsx Outdated
Comment thread apps/meteor/client/views/room/MessageList/hooks/useMessages.ts Outdated
Comment thread apps/meteor/app/autotranslate/client/lib/actionButton.ts Outdated
Comment thread apps/meteor/app/autotranslate/client/lib/actionButton.ts Outdated
Comment thread apps/meteor/app/autotranslate/client/lib/actionButton.ts Outdated
Comment thread apps/meteor/app/autotranslate/client/lib/actionButton.ts Outdated
Comment thread apps/meteor/app/autotranslate/server/autotranslate.ts Outdated
@codecov

codecov Bot commented Nov 22, 2022 •

Copy link
Copy Markdown

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
Copy Markdown
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

@gabriellsh gabriellsh left a comment

Copy link
Copy Markdown
Member

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
Comment thread apps/meteor/client/components/message/variants/ThreadMessage.tsx Outdated
Comment thread apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx Outdated
Comment thread apps/meteor/client/views/room/MessageList/MessageList.tsx Outdated
Comment thread apps/meteor/client/views/room/MessageList/MessageList.tsx Outdated
Comment thread apps/meteor/client/views/room/MessageList/hooks/useMessages.ts Outdated
@hugocostadev hugocostadev added this to the 6.0.0 milestone Jan 9, 2023
Comment thread apps/meteor/app/autotranslate/client/lib/actionButton.ts Outdated
Comment thread apps/meteor/client/views/room/MessageList/MessageList.tsx Outdated
Comment thread apps/meteor/client/views/room/MessageList/lib/parseMessageTextToAstMarkdown.ts Outdated
Comment thread apps/meteor/client/views/room/MessageList/hooks/useMessages.ts Outdated
Comment thread apps/meteor/client/views/room/MessageList/hooks/useAutoTranslate.ts Outdated
Comment thread apps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx Outdated
Comment thread apps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx Outdated
Comment thread apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx Outdated
@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 done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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