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

[HOLD #374470][$500] Task system message is not translated to Spanish #37943

Closed
6 tasks done
kavimuru opened this issue Mar 8, 2024 · 26 comments
Closed
6 tasks done

[HOLD #374470][$500] Task system message is not translated to Spanish #37943

kavimuru opened this issue Mar 8, 2024 · 26 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Monthly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Mar 8, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.4.49-0
Reproducible in staging?: y
Reproducible in production?: no, new feature
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: applause internal team
Slack conversation:

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to any chat.
  3. Create a task.
  4. Go to task report.
  5. Edit task report (title, description, assignee).
  6. Go to profile settings.
  7. Go to Preferences > Language > Spanish.
  8. Return to task report.

Expected Result:

The system message for task edit will be translated to Spanish.

Actual Result:

The system message for task edit remains in English when app language is Spanish.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6406331_1709860119344.bandicam_2024-03-08_09-05-36-163.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016868ca83c5742cfd
  • Upwork Job ID: 1766053598621216768
  • Last Price Increase: 2024-03-08
@kavimuru kavimuru added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 8, 2024
Copy link

melvin-bot bot commented Mar 8, 2024

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Mar 8, 2024
Copy link
Contributor

github-actions bot commented Mar 8, 2024

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@kavimuru
Copy link
Author

kavimuru commented Mar 8, 2024

@isabelastisser FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

Copy link

melvin-bot bot commented Mar 8, 2024

Triggered auto assignment to @robertjchen (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@kavimuru
Copy link
Author

kavimuru commented Mar 8, 2024

We think this bug might be related to #vip-vsb , cc @quinthar

@robertjchen robertjchen added Weekly KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Mar 8, 2024
@robertjchen
Copy link
Contributor

Not a deploy blocker, and this specific issue has been present since 3 months ago as of https://github.com/Expensify/Auth/pull/9451

Ideally system messages should be translated as well, but due to how they're formed on the backend it is not the case today.

@dragnoir
Copy link
Contributor

dragnoir commented Mar 8, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Edit message does not have translation

What is the root cause of that problem?

on TaskUtils, we are turning back translation for all cases exept for REPORT.ACTIONS.TYPE.TASKEDITED

case CONST.REPORT.ACTIONS.TYPE.TASKREOPENED:
return {text: Localize.translateLocal('task.messages.reopened')};
case CONST.REPORT.ACTIONS.TYPE.TASKEDITED:
return {
text: action?.message?.[0].text ?? '',
html: action?.message?.[0].html,
};

What changes do you think we should make in order to solve the problem?

1- Create a function identifyUpdateType that take the action?.message?.[0].text as input and extract the tape of the edit.

function identifyUpdateType(text: string, html: string) {
    let type = 'unknownUpdate';
    let prefix = '';

    if (text.startsWith('updated the task title to ')) {
        type = 'taskUpdate';
        prefix = 'updated the task title to ';
    } else if (text.startsWith('updated the description to ')) {
        type = 'descriptionUpdate';
        prefix = 'updated the description to ';
    } else if (text.startsWith('assigned to ')) {
        type = 'assigneeUpdate';
        prefix = 'assigned to ';
    }

    // Extracting the substring from the text and html inputs after the found prefix
    const textOutput = prefix.length > 0 ? text.substring(prefix.length) : '';
    const htmlOutput = prefix.length > 0 ? html.substring(html.indexOf(prefix) + prefix.length) : '';

    return {text: textOutput, html: htmlOutput, type};
}

2- Depending of this new function and we now know the type of the edit we can adjust getTaskReportActionMessage

function getTaskReportActionMessage(action: OnyxEntry<ReportAction>): Pick<Message, 'text' | 'html'> {
    console.log('action name: ', action?.actionName);

    const taskeditedType = identifyUpdateType(action?.message?.[0].text ?? '', action?.message?.[0].html ?? '');

    switch (action?.actionName) {
        case CONST.REPORT.ACTIONS.TYPE.TASKCOMPLETED:
            return {text: Localize.translateLocal('task.messages.completed')};
        case CONST.REPORT.ACTIONS.TYPE.TASKCANCELLED:
            return {text: Localize.translateLocal('task.messages.canceled')};
        case CONST.REPORT.ACTIONS.TYPE.TASKREOPENED:
            return {text: Localize.translateLocal('task.messages.reopened')};
        case CONST.REPORT.ACTIONS.TYPE.TASKEDITED:
            switch (taskeditedType.type) {
                case 'taskUpdate':
                    return {
                        text: `${Localize.translateLocal('task.messages.taskUpdate')} ${taskeditedType.text}` ?? '',
                        html: `${Localize.translateLocal('task.messages.taskUpdate')} ${taskeditedType.html}` ?? '',
                    };
                case 'descriptionUpdate':
                    return {
                        text: `${Localize.translateLocal('task.messages.descriptionUpdate')} ${taskeditedType.text}` ?? '',
                        html: `${Localize.translateLocal('task.messages.descriptionUpdate')} ${taskeditedType.html}` ?? '',
                    };
                case 'assigneeUpdate':
                    return {
                        text: `${Localize.translateLocal('task.messages.assigneeUpdate')} ${taskeditedType.text}` ?? '',
                        html: `${Localize.translateLocal('task.messages.assigneeUpdate')} ${taskeditedType.html}` ?? '',
                    };
                default:
                    return {text: Localize.translateLocal('task.task')};
            }
        default:
            return {text: Localize.translateLocal('task.task')};
    }
}

For sure we need to add the language variables for English and Spanish.

POC video:

20240308_152119.mp4

@robertjchen robertjchen added the External Added to denote the issue can be worked on by a contributor label Mar 8, 2024
Copy link

melvin-bot bot commented Mar 8, 2024

Job added to Upwork: https://www.upwork.com/jobs/~016868ca83c5742cfd

@melvin-bot melvin-bot bot changed the title Task system message is not translated to Spanish [$500] Task system message is not translated to Spanish Mar 8, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 8, 2024
Copy link

melvin-bot bot commented Mar 8, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @hungvu193 (External)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Mar 8, 2024
@ghost
Copy link

ghost commented Mar 8, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Task system message is not translated to Spanish

What is the root cause of that problem?

Over here :

case CONST.REPORT.ACTIONS.TYPE.TASKEDITED:
return {
text: action?.message?.[0].text ?? '',
html: action?.message?.[0].html,
};

and also over here :

App/src/libs/ReportUtils.ts

Lines 3750 to 3756 in 86a9904

message: [
{
type: CONST.REPORT.MESSAGE.TYPE.COMMENT,
text: `assigned to ${getDisplayNameForParticipant(assigneeAccountID)}`,
html: `assigned to <mention-user accountID=${assigneeAccountID}></mention-user>`,
},
],

Here -

App/src/libs/ReportUtils.ts

Lines 3711 to 3716 in 86a9904

let changelog = 'edited this task';
if (field && value) {
changelog = `updated the ${field} to ${value}`;
} else if (field) {
changelog = `removed the ${field}`;
}

Also we are hard coding the text like updated the ${field} to ${value}; that's why even we are changing the task title it is not getting updated to Spanish language.

we need to add translateLocal function to translate task messages like this :

case CONST.REPORT.ACTIONS.TYPE.TASKCOMPLETED:
return {text: Localize.translateLocal('task.messages.completed')};
case CONST.REPORT.ACTIONS.TYPE.TASKCANCELLED:
return {text: Localize.translateLocal('task.messages.canceled')};
case CONST.REPORT.ACTIONS.TYPE.TASKREOPENED:
return {text: Localize.translateLocal('task.messages.reopened')};
case CONST.REPORT.ACTIONS.TYPE.TASKEDITED:

Also, here :

App/src/languages/es.ts

Lines 2045 to 2064 in 86a9904

task: {
task: 'Tarea',
title: 'Título',
description: 'Descripción',
assignee: 'Usuario asignado',
completed: 'Completada',
messages: {
created: ({title}: TaskCreatedActionParams) => `tarea para ${title}`,
completed: 'marcada como completa',
canceled: 'tarea eliminada',
reopened: 'marcada como incompleta',
error: 'No tiene permiso para realizar la acción solicitada.',
},
markAsComplete: 'Marcar como completada',
markAsIncomplete: 'Marcar como incompleta',
assigneeError: 'Hubo un error al asignar esta tarea, inténtalo con otro usuario.',
genericCreateTaskFailureMessage: 'Error inesperado al crear el tarea, por favor, inténtalo más tarde.',
deleteTask: 'Eliminar tarea',
deleteConfirmation: '¿Estás seguro de que quieres eliminar esta tarea?',
},

we don't have keys and values for task system.

What changes do you think we should make in order to solve the problem?

we need to add it over here :

App/src/languages/es.ts

Lines 2045 to 2064 in 86a9904

task: {
task: 'Tarea',
title: 'Título',
description: 'Descripción',
assignee: 'Usuario asignado',
completed: 'Completada',
messages: {
created: ({title}: TaskCreatedActionParams) => `tarea para ${title}`,
completed: 'marcada como completa',
canceled: 'tarea eliminada',
reopened: 'marcada como incompleta',
error: 'No tiene permiso para realizar la acción solicitada.',
},
markAsComplete: 'Marcar como completada',
markAsIncomplete: 'Marcar como incompleta',
assigneeError: 'Hubo un error al asignar esta tarea, inténtalo con otro usuario.',
genericCreateTaskFailureMessage: 'Error inesperado al crear el tarea, por favor, inténtalo más tarde.',
deleteTask: 'Eliminar tarea',
deleteConfirmation: '¿Estás seguro de que quieres eliminar esta tarea?',
},

i.e. for both en.ts file and es.ts we need to add key and value pair for task system messages rather than hard coding it like this :
text: `assigned to ${getDisplayNameForParticipant(assigneeAccountID)}`,

What alternative solutions did you explore? (Optional)

N/A

@hungvu193
Copy link
Contributor

Not a deploy blocker, and this specific issue has been present since 3 months ago as of https://github.com/Expensify/Auth/pull/9451

Ideally system messages should be translated as well, but due to how they're formed on the backend it is not the case today.

@robertjchen I assume that our BE also need to update right? Similar to modified expense messages, we need to know which field is currently edited. Is there any BE's PR that linked to this issue and should we hold this issue on that?

@ghost
Copy link

ghost commented Mar 8, 2024

I have tried to implement @dragnoir's solution and it is throwing error i.e. this

Screenshot 2024-03-08 at 5 05 32 PM

@dragnoir
Copy link
Contributor

dragnoir commented Mar 8, 2024

@paultsimura my solution does not require BE update. Can you pls explain why you require BE changes?

@hungvu193
Copy link
Contributor

@thienlnam @hungvu193 is it possible that we could take it as a follow-up from our PR?

For the context: I implemented these "TASKEDITED actions" feature and had the FE localization code fully ready in the PR: a86a2f7 (we can consider it as a proposal along with #36768 (comment) and #36768 (comment)).

But since it requires BE changes, it was decided to release without the localization part and revert the localization commit for the release.

Sounds good to me. Can you post a proposal please? Assuming you have everything on the BE that you need.

@hungvu193
Copy link
Contributor

hungvu193 commented Mar 8, 2024

@paultsimura my solution does not require BE update. Can you pls explain why you require BE changes?

We need to know which field is editing, because when we update description, title, assignee the action type will be "TASKEDITED", the message will be "Updated fieldType to fieldValue". Your solution will return the same fieldType for all these editing actions above (description, title, assignee)

@dragnoir
Copy link
Contributor

dragnoir commented Mar 8, 2024

@hungvu193 proposal updates with new POC video

@paultsimura
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

The TASKEDITED actions are not localized

What is the root cause of that problem?

They were implemented without localization as a POC.

What changes do you think we should make in order to solve the problem?

First, we need to change the way we build these actions:

We should implement these events similar to the MODIFIEDEXPENSE actions using the originalMessage with the data that was modified in the action:

type OriginalMessageTaskEdited = {
    actionName: typeof CONST.REPORT.ACTIONS.TYPE.TASKEDITED;
    originalMessage: {
        oldTitle?: string;
        title?: string;
        oldDescription?: string;
        description?: string;
        oldAssigneeAccountID?: number;
        assigneeAccountID?: number;
    };
};

Since we allow modifying only a single field per action at the moment, each action will contain a single pair of oldValue/value (e.g. oldTitle, title).

For this, we should implement the following logic:

function getTaskEditedOriginalMessage(oldTask: OnyxEntry<Report>, taskChanges: Partial<Task>): OriginalMessageTaskEdited['originalMessage'] {
    const originalMessage: OriginalMessageTaskEdited['originalMessage'] = {};

    if ('title' in taskChanges) {
        originalMessage.oldTitle = oldTask?.reportName;
        originalMessage.title = taskChanges?.title;
    }
    if ('description' in taskChanges) {
        originalMessage.oldDescription = oldTask?.description;
        originalMessage.description = taskChanges?.description;
    }
    if ('assigneeAccountID' in taskChanges) {
        originalMessage.oldAssigneeAccountID = getTaskAssigneeAccountID(oldTask);
        originalMessage.assigneeAccountID = taskChanges?.assigneeAccountID;
    }

    return originalMessage;
}

We should pass this originalMessage when building the optimistic TASKEDITED action.
Also, we need to make changes on the BE to return the same format of the actions.

Second, based on this originalMessage, we should implement localization similar to MODIFIEDEXPENSE case:

function buildMessageFragmentForValue(newValue: string | undefined, valueName: string): string {
    const fieldName = valueName.toLowerCase();

    if (!newValue) {
        return Localize.translateLocal('task.messages.removedField', {fieldName});
    }

    return Localize.translateLocal('task.messages.updatedField', {fieldName, newValueToDisplay: newValue});
}

function getTaskEditedMessage(reportAction: OnyxEntry<ReportAction>) {
    if (reportAction?.actionName !== CONST.REPORT.ACTIONS.TYPE.TASKEDITED) {
        return {text: ''};
    }

    const originalMessage = reportAction?.originalMessage as OriginalMessageTaskEdited['originalMessage'] | undefined;
    if (!originalMessage) {
        return {
            text: reportAction?.message?.[0].text ?? '',
            html: reportAction?.message?.[0].html,
        };
    }

    const hasModifiedTitle = 'title' in originalMessage;
    if (hasModifiedTitle) {
        return {
            text: buildMessageFragmentForValue(originalMessage.title, Localize.translateLocal('task.title')),
        };
    }

    ....
}

We should add the corresponding copies as well:

updatedField: ({fieldName, newValueToDisplay}: UpdatedTaskFieldParams) => `updated the ${fieldName} to "${newValueToDisplay}"`,
removedField: ({fieldName}: RemovedTaskFieldParams) => `removed the ${fieldName}`,
updatedAssignee: ({assignee}: UpdatedTaskAssigneeParams) => `assigned to ${assignee}`,

This was already implemented in my PR here, but reverted due to urgency and pending BE changes.

What alternative solutions did you explore? (Optional)

@Victor-Nyagudi
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Task system messges are not translated to Spanish.

What is the root cause of that problem?

When offline, the system messages are hardcoded in English in ReportUtils.

App/src/libs/ReportUtils.ts

Lines 3719 to 3724 in a2c24e1

let changelog = 'edited this task';
if (field && value) {
changelog = `updated the ${field} to ${value}`;
} else if (field) {
changelog = `removed the ${field}`;
}

While online, the entire message is bundled up into one e.g. "updated the description to Test description", instead of being broken up into the system message i.e. "updated the description to" and the value the field is updated to i.e. "Test description".

This makes it harder to translate the system message portion, so if at all any localization were to be done to the message from the back end, it would translate both the system message and the new value of the updated field.

This is possibly why edited task actions are not localized yet while other task actions are.

case CONST.REPORT.ACTIONS.TYPE.TASKCOMPLETED:
return {text: Localize.translateLocal('task.messages.completed')};
case CONST.REPORT.ACTIONS.TYPE.TASKCANCELLED:
return {text: Localize.translateLocal('task.messages.canceled')};
case CONST.REPORT.ACTIONS.TYPE.TASKREOPENED:
return {text: Localize.translateLocal('task.messages.reopened')};
case CONST.REPORT.ACTIONS.TYPE.TASKEDITED:
return {
text: action?.message?.[0].text ?? '',
html: action?.message?.[0].html,
};

What changes do you think we should make in order to solve the problem?

I propose using the Localize.translateLocal() method in ReportUtils for both buildOptimisticEditedTaskFieldReportAction() and buildOptimisticChangedTaskAssigneeReportAction() instead of hardcoding the text in English to handle the offline scenario.

// These are just examples. The correct Spanish text will be used after verification
let changelog = Localize.translateLocal('common.editedTask');
   if (field && value) {
       changelog = `${Localize.translateLocal('common.updatedFieldTo')} ${value}`;
    } else if (field) {
        changelog = `${Localize.translateLocal('common.removedThe')} ${field}`;
}

//... This is in 'buildOptimisticChangedTaskAssigneeReportAction'
message: [
    {
        type: CONST.REPORT.MESSAGE.TYPE.COMMENT,
        text: `${Localize.translateLocal('common.assignedTo')} ${getDisplayNameForParticipant(assigneeAccountID)}`,
        html: `${Localize.translateLocal('common.assignedTo')} <mention-user accountID=${assigneeAccountID}></mention-user>`,
    },
],

When online, we can have the backend return two messages inside the message array - the first being the system message, and the second the updated value.

With these in hand, we can translate just the system message's html property inside getTaskReportActionMessage since this is the property used in TaskAction to show system messages.

case CONST.REPORT.ACTIONS.TYPE.TASKEDITED:
    return {
      text: action?.message?.[1].text ? `${action?.message?.[0].text} ${action?.message?.[1].text}` : '',
      html: action?.message?.[1].html ? `${Localize.translateLocal(action?.message?.[0].html)} ${action?.message?.[1].html}` : undefined,
};

What alternative solutions did you explore? (Optional)

None so far.

@thienlnam thienlnam assigned thienlnam and unassigned robertjchen Mar 8, 2024
@thienlnam thienlnam added Monthly KSv2 and removed External Added to denote the issue can be worked on by a contributor Daily KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors labels Mar 8, 2024
@thienlnam
Copy link
Contributor

As mentioned earlier - we were aware of this when we added the edit task system messages (PR). Considering this requires some additional BE changes and is a feature we're not actively prioritizing, going to put this on hold for now

@thienlnam thienlnam changed the title [$500] Task system message is not translated to Spanish [HOLD https://github.com/Expensify/Expensify/issues/374470][$500] Task system message is not translated to Spanish Mar 8, 2024
@thienlnam thienlnam changed the title [HOLD https://github.com/Expensify/Expensify/issues/374470][$500] Task system message is not translated to Spanish [HOLD #374470][$500] Task system message is not translated to Spanish Mar 8, 2024
@thienlnam thienlnam moved this to FUTURE in [#whatsnext] #vip-vsb Mar 8, 2024
Copy link

melvin-bot bot commented Mar 10, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@paultsimura
Copy link
Contributor

False alert ☝️

@melvin-bot melvin-bot bot added the Overdue label Apr 9, 2024
@isabelastisser
Copy link
Contributor

On hold, not a priority.

@melvin-bot melvin-bot bot removed the Overdue label Apr 10, 2024
@melvin-bot melvin-bot bot added the Overdue label May 13, 2024
@isabelastisser
Copy link
Contributor

Still on hold.

@melvin-bot melvin-bot bot removed the Overdue label May 13, 2024
@melvin-bot melvin-bot bot added the Overdue label Jun 13, 2024
@thienlnam
Copy link
Contributor

Since this is not a priority (#37943 (comment)) - going to close for now

@melvin-bot melvin-bot bot removed the Overdue label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Monthly KSv2
Projects
No open projects
Development

No branches or pull requests

8 participants