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

[$250] Task – Hidden assignee appears in LHN when assign task without assignee to yourself #44205

Closed
3 of 6 tasks
m-natarajan opened this issue Jun 22, 2024 · 19 comments
Closed
3 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@m-natarajan
Copy link

m-natarajan commented Jun 22, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.1-0
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail: n/a
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 https://staging.new.expensify.com/
  2. Log in
  3. Open any chat
  4. Create a task without assignee
  5. Open task details and assign it to yourself
  6. Find task in LHN

Expected Result:

Assignee name appears in LHN

Actual Result:

Hidden assignee appears in LHN

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

Bug6521452_1719056664237.Hidden.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~012e2bf84563d514ee
  • Upwork Job ID: 1804545733497236627
  • Last Price Increase: 2024-06-22
Issue OwnerCurrent Issue Owner: @rushatgabhane
@m-natarajan m-natarajan added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels Jun 22, 2024
Copy link

melvin-bot bot commented Jun 22, 2024

Triggered auto assignment to @puneetlath (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

Copy link
Contributor

👋 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.

@melvin-bot melvin-bot bot added the Daily KSv2 label Jun 22, 2024
@melvin-bot melvin-bot bot removed the Hourly KSv2 label Jun 22, 2024
Copy link

melvin-bot bot commented Jun 22, 2024

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

@m-natarajan
Copy link
Author

@carlosmiceli 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

@m-natarajan
Copy link
Author

We think that this bug might be related to #vip-vsb

@carlosmiceli carlosmiceli added External Added to denote the issue can be worked on by a contributor and removed DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jun 22, 2024
Copy link

melvin-bot bot commented Jun 22, 2024

Job added to Upwork: https://www.upwork.com/jobs/~012e2bf84563d514ee

@melvin-bot melvin-bot bot changed the title Task – Hidden assignee appears in LHN when assign task without assignee to yourself [$250] Task – Hidden assignee appears in LHN when assign task without assignee to yourself Jun 22, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 22, 2024
Copy link

melvin-bot bot commented Jun 22, 2024

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

@robert-westenberger
Copy link

Proposal

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

Instead of the task assignee's name appearing in the LHN, @Hidden shows instead.

What is the root cause of that problem?

The piece of text is rendered by this line in OptionRowLHN

{parseHtmlToText(optionItem.alternateText)}

optionItem is a prop from OptionRowLHNData, building it via SidebarUtils.getOptionData

const item = SidebarUtils.getOptionData({
report: fullReport,
reportActions,
personalDetails,
preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT,
policy,
parentReportAction,
hasViolations: !!shouldDisplayViolations,
});

That function sets the alternateText attribute using TaskUtils.getTaskReportActionMessage, accessing the text attribute on it's return value

result.alternateText = ReportUtils.formatReportLastMessageText(TaskUtils.getTaskReportActionMessage(lastAction).text);

All that does is call getReportActionText from ReportActionUtils.ts

text: getReportActionText(action),

That function passes the report action's message.html (or message[0].html if message is an array) to the Expensimark parser's htmlToText method.

function getReportActionHtml(reportAction: PartialReportAction): string {
return getReportActionMessage(reportAction)?.html ?? '';
}
function getReportActionText(reportAction: PartialReportAction): string {
const html = getReportActionHtml(reportAction);
const parser = new ExpensiMark();
return html ? parser.htmlToText(html) : '';
}

That method uses this userMention rule, which relies upon an extras value to be passed to it containing a accountID -> name map to inject names into parsed text. If one isn't found, the string "@hidden" is injected.

https://github.com/Expensify/expensify-common/blob/e6fe884e56d64df8ab2157d92fe2de6b1f6a91d5/lib/ExpensiMark.ts#L732-L743

So we're actually calling ExpensiMark's parseHtmlToText twice.

The first time it's called we don't pass it the accountID -> name map

return html ? parser.htmlToText(html) : '';

When we render the OptionRowLHN component, the component receives the already parsed text, but the component is calling parseHtmlToText itself..

{parseHtmlToText(optionItem.alternateText)}

But this one is importing it from the OnyxAwareParser

import {parseHtmlToText} from '@libs/OnyxAwareParser';

The onyx aware parser also uses ExpensiMark's htmlToText method, but it wraps that method and passes the accountID -> name map for the parser to use and inject the name with.

return parser.htmlToText(html, {reportIDToName: reportIDToName ?? reportIDToNameMap, accountIDToName: accountIDToName ?? accountIDToNameMap, cacheVideoAttributes});

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

We can change this line

result.alternateText = ReportUtils.formatReportLastMessageText(TaskUtils.getTaskReportActionMessage(lastAction).text);

to instead return the raw HTML instead of the parsed text

result.alternateText = ReportUtils.formatReportLastMessageText(TaskUtils.getTaskReportActionMessage(lastAction).html);

This is fine because the only place the alternateText is ultimately used, it gets parsed anyway.. but during that parse, the parser gets the accountID -> name map it needs to inject the username into the parsed piece of text from the onyx store.

What alternative solutions did you explore? (Optional)

With my proposal, the expensimark parser is still called twice, pointlessly.... we just aren't accessing the parse result anymore from SidebarUtils.getOptionData. Currently looking into a way to see if we can remove that redundant call to parser.htmlToText

@tsa321
Copy link
Contributor

tsa321 commented Jun 23, 2024

Proposal

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

Task – assigned to @hidden when last message is edited assignee.

What is the root cause of that problem?

In PR : #40168
In the newly created function:

function getReportActionText(reportAction: PartialReportAction): string {
const html = getReportActionHtml(reportAction);
const parser = new ExpensiMark();
return html ? parser.htmlToText(html) : '';
}

we don't use OnyxAwareParser:

function parseHtmlToText(
html: string,
reportIDToName?: Record<string, string>,
accountIDToName?: Record<string, string>,
cacheVideoAttributes?: (videoSource: string, videoAttrs: string) => void,
): string {
return parser.htmlToText(html, {reportIDToName: reportIDToName ?? reportIDToNameMap, accountIDToName: accountIDToName ?? accountIDToNameMap, cacheVideoAttributes});
}

So the parsed text from original html message, will fails to insert the mentioned user name.
Another problem is when user change the assignee, the optimistic data from :

html: `assigned to <mention-user accountID=${assigneeAccountID}></mention-user>`,

doesn't use new tag for metioned user. This will make parser fails to parse the mentioned user and and the mentioned user will be empty string.

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

We should use the OnyxAwareParser parserHtmlToText in the getReportActionText above. and if we want to modify the mentioned user account into user name, we can pass parameter to the parseHtmlToText, wheter we want username or user email account. Also for optimization: we could cache the text result of parseHtmlToText in an object with keyreportID_ reportActionID_lastModifiedTime.
As for the optimistic report action problem(the assignee is blank): we should change the format into :

html:  `assigned to <mention-user accountID="${assigneeAccountID}"\/\>`;

Copy link

melvin-bot bot commented Jun 25, 2024

@carlosmiceli, @puneetlath, @rushatgabhane Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Jun 25, 2024
@carlosmiceli
Copy link
Contributor

Still waiting for proposal reviews by @rushatgabhane.

@rushatgabhane
Copy link
Member

@robert-westenberger's proposal LGTM
🎀 👀 🎀

@melvin-bot melvin-bot bot removed the Overdue label Jun 26, 2024
Copy link

melvin-bot bot commented Jun 26, 2024

Current assignees @puneetlath and @carlosmiceli are eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 26, 2024
Copy link

melvin-bot bot commented Jun 26, 2024

📣 @robert-westenberger You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@robert-westenberger
Copy link

@carlosmiceli @rushatgabhane hey carlos and rushat, how do i get invited to the expensify open source slack channel? I emailed contributors@expensify.com with the subject Slack Channel Invites as described here but I never got any response.

@robert-westenberger
Copy link

Also @carlosmiceli @rushatgabhane this issue is no longer reproducible. It looks like it was fixed with this PR #44365

@carlosmiceli
Copy link
Contributor

@puneetlath this would be good to close then, right?

@puneetlath
Copy link
Contributor

Sounds good, let's close it.

Copy link

melvin-bot bot commented Jun 28, 2024

@carlosmiceli @puneetlath Be sure to fill out the Contact List!

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. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

6 participants