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: Translation for system message #21780

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -2524,6 +2524,9 @@ const CONST = {
SETTINGS_LOUNGE_ACCESS: {
HEADER_IMAGE_ASPECT_RATIO: 0.64,
},
TRANSLATION_KEYS: {
ATTACHMENT: 'common.attachment',
},
};

export default CONST;
4 changes: 2 additions & 2 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,8 @@ function getLastMessageTextForReport(report) {
const lastReportAction = lastReportActions[report.reportID];
let lastMessageTextFromReport = '';

if (ReportUtils.isReportMessageAttachment({text: report.lastMessageText, html: report.lastMessageHtml})) {
lastMessageTextFromReport = `[${Localize.translateLocal('common.attachment')}]`;
if (ReportUtils.isReportMessageAttachment({text: report.lastMessageText, html: report.lastMessageHtml, translationKey: report.lastMessageTranslationKey})) {
lastMessageTextFromReport = `[${Localize.translateLocal(report.lastMessageTranslationKey || 'common.attachment')}]`;
} else if (ReportActionUtils.isReportPreviewAction(lastReportAction)) {
const iouReport = ReportUtils.getReport(ReportActionUtils.getIOUReportIDFromReportActionPreview(lastReportAction));
lastMessageTextFromReport = ReportUtils.getReportPreviewMessage(iouReport, lastReportAction);
Expand Down
18 changes: 12 additions & 6 deletions src/libs/ReportActionsUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,22 +288,28 @@ function getLastVisibleAction(reportID, actionsToMerge = {}) {
/**
* @param {String} reportID
* @param {Object} [actionsToMerge]
* @return {String}
* @return {Object}
*/
function getLastVisibleMessageText(reportID, actionsToMerge = {}) {
function getLastVisibleMessage(reportID, actionsToMerge = {}) {
const lastVisibleAction = getLastVisibleAction(reportID, actionsToMerge);
const message = lodashGet(lastVisibleAction, ['message', 0], {});

if (isReportMessageAttachment(message)) {
return CONST.ATTACHMENT_MESSAGE_TEXT;
return {
lastMessageTranslationKey: CONST.TRANSLATION_KEYS.ATTACHMENT,
};
}

if (isCreatedAction(lastVisibleAction)) {
return '';
return {
lastMessageText: '',
};
}

const messageText = lodashGet(message, 'text', '');
return String(messageText).replace(CONST.REGEX.AFTER_FIRST_LINE_BREAK, '').substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH).trim();
return {
lastMessageText: String(messageText).replace(CONST.REGEX.AFTER_FIRST_LINE_BREAK, '').substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH).trim(),
};
}

/**
Expand Down Expand Up @@ -535,7 +541,7 @@ function isWhisperAction(action) {
export {
getSortedReportActions,
getLastVisibleAction,
getLastVisibleMessageText,
getLastVisibleMessage,
getMostRecentIOURequestActionID,
extractLinksFromMessageHtml,
isDeletedAction,
Expand Down
2 changes: 2 additions & 0 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,7 @@ function buildOptimisticAddCommentReportAction(text, file) {
created: DateUtils.getDBTime(),
message: [
{
translationKey: isAttachment ? CONST.TRANSLATION_KEYS.ATTACHMENT : '',
type: CONST.REPORT.MESSAGE.TYPE.COMMENT,
html: htmlForNewComment,
text: textForNewComment,
Expand Down Expand Up @@ -1571,6 +1572,7 @@ function buildOptimisticChatReport(
isPinned: reportName === CONST.REPORT.WORKSPACE_CHAT_ROOMS.ADMINS,
lastActorEmail: '',
lastActorAccountID: 0,
lastMessageTranslationKey: '',
lastMessageHtml: '',
lastMessageText: null,
lastReadTime: currentTime,
Expand Down
9 changes: 7 additions & 2 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ function addActions(reportID, text = '', file) {

const optimisticReport = {
lastVisibleActionCreated: currentTime,
lastMessageTranslationKey: lodashGet(lastAction, 'message[0].translationKey', ''),
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like that push event should also update this field to update let LHN message after sending an attachment, like this case #22937

Copy link
Member

Choose a reason for hiding this comment

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

Hi all, this PR caused a regression Red dot and [Attachment] shows in LHN even after close error message

This is because we didn't set failure data for lastMessageTranslationKey

lastMessageText: lastCommentText,
lastMessageHtml: lastCommentText,
lastActorEmail: currentUserEmail,
Expand Down Expand Up @@ -835,6 +836,7 @@ function deleteReportComment(reportID, reportAction) {
const reportActionID = reportAction.reportActionID;
const deletedMessage = [
{
translationKey: '',
type: 'COMMENT',
html: '',
text: '',
Expand All @@ -854,13 +856,15 @@ function deleteReportComment(reportID, reportAction) {
// If we are deleting the last visible message, let's find the previous visible one (or set an empty one if there are none) and update the lastMessageText in the LHN.
// Similarly, if we are deleting the last read comment we will want to update the lastVisibleActionCreated to use the previous visible message.
let optimisticReport = {
lastMessageTranslationKey: '',
lastMessageText: '',
lastVisibleActionCreated: '',
};
const lastMessageText = ReportActionsUtils.getLastVisibleMessageText(originalReportID, optimisticReportActions);
if (lastMessageText.length > 0) {
const {lastMessageText = '', lastMessageTranslationKey = ''} = ReportActionsUtils.getLastVisibleMessage(originalReportID, optimisticReportActions);
if (lastMessageText || lastMessageTranslationKey) {
const lastVisibleActionCreated = ReportActionsUtils.getLastVisibleAction(originalReportID, optimisticReportActions).created;
optimisticReport = {
lastMessageTranslationKey,
lastMessageText,
lastVisibleActionCreated,
};
Expand Down Expand Up @@ -1020,6 +1024,7 @@ function editReportComment(reportID, originalReportAction, textForNewComment) {
if (reportActionID === lastVisibleAction.reportActionID) {
const lastMessageText = ReportUtils.formatReportLastMessageText(reportComment);
const optimisticReport = {
lastMessageTranslationKey: '',
lastMessageText,
};
optimisticData.push({
Expand Down
8 changes: 6 additions & 2 deletions src/libs/isReportMessageAttachment.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@ import CONST from '../CONST';
* Check whether a report action is Attachment or not.
* Ignore messages containing [Attachment] as the main content. Attachments are actions with only text as [Attachment].
*
* @param {Object} reportActionMessage report action's message as text and html
* @param {Object} reportActionMessage report action's message as text, html and translationKey
* @returns {Boolean}
*/
export default function isReportMessageAttachment({text, html}) {
export default function isReportMessageAttachment({text, html, translationKey}) {
if (translationKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👋 This was partially responsible for a regression in #24246
It was mainly a back-end problem, the translation key is not set/reset when we change from a message with a translation key to one without, or vice-versa
While this wasn't caused by this PR, leaving a note to myself and others to test more complex test cases (this one included testing with two accounts)

Copy link
Member

Choose a reason for hiding this comment

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

thanks @eVoloshchak

return translationKey === CONST.TRANSLATION_KEYS.ATTACHMENT;
}

if (!text || !html) {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/actions/ReportTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('actions/Report', () => {
actorAccountID: TEST_USER_ACCOUNT_ID,
automatic: false,
avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_3.png',
message: [{type: 'COMMENT', html: 'Testing a comment', text: 'Testing a comment'}],
message: [{type: 'COMMENT', html: 'Testing a comment', text: 'Testing a comment', translationKey: ''}],
person: [{type: 'TEXT', style: 'strong', text: 'Test User'}],
shouldShow: true,
};
Expand Down