[HOLD for payment 2024-07-24] [$250] Room - Room mention is not copied to clipboard and editor is empty when editing room mention

[izarutskaya]

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: 1.4.63-0 Reproducible in staging?: Y Reproducible in production?: N Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

Precondition:

• User has created a few rooms linked to the same workspace.

1. Go to staging.new.expensify.com
2. Go to any room from the precondition.
3. Type # and select a room.
4. Send the message.
5. Right click on the message > Copy to clipboard.
6. Paste the content.
7. Right click on the message > Edit comment.

Expected Result:

In Step 5, the room in the message will be copied. In Step 7. the room in the editor will be preserved.

Actual Result:

In Step 5, the room in the message is not copied. In Step 7. the room in the editor is missing.

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/d46d3c24-9fcd-430f-b603-c81d915f9d8e

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c98cde11a4c25f4e
• Upwork Job ID: 1780934156183453696
• Last Price Increase: 2024-07-10

Issue Owner

Current Issue Owner: @JmillsExpensify

[melvin-bot[bot]]

Triggered auto assignment to @JmillsExpensify (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.

[melvin-bot[bot]]

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

[github-actions[bot]]

:wave: 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.

[izarutskaya]

@JmillsExpensify 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.

[izarutskaya]

We think this issue might be related to the #collect project.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01c98cde11a4c25f4e

[melvin-bot[bot]]

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

[lakchote]

Not a blocker - it's an edge case that only concer room mentions, no core customer functionality is broken.

[ahmedGaber93]

Proposal

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

Room - Room mention is not copied to clipboard and editor is empty when editing room mention

What is the root cause of that problem?

we don't handle mention-report in ContextMenuActions.tsx after copy to clipboard pressed, we only handle mention-user https://github.com/Expensify/App/blob/9b839f4f18d724dba1ed17ed5967f7e033781428/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L395-L398

before sending mention report text to backend we format it to be like this <mention-report>#reportName<mention-report/> but the message html come from backend with this format <mention-report reportID="1234" /> and this format will return empty text after parsing it to text before add it to clipboard here

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

we need to handle mention-report in ContextMenuActions.tsx with the same way we used it in mention-user.

to copy the orignal message we need to catch reportID from <mention-report reportID="1234" />, and get reportName then revert it to orignal format before send <mention-report>#reportName<mention-report/>.

for that we need to add this extra replace here

.replace(/<mention-report reportID="(\d*)"\/>/gi, (match, reportID): string => {
   const report = getReport(reportID);

   return `<mention-report>${report?.reportName ?? report?.displayName}<mention-report/>`;
})

and setClipboardMessage will parse the orignal message html and get the massage text and add it to Clipboard https://github.com/Expensify/App/blob/9b839f4f18d724dba1ed17ed5967f7e033781428/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L43-L51

What alternative solutions did you explore? (Optional)

[lakchote]

@JmillsExpensify I'll be OOO next week until April 29th. Feel free to assign another internal engineer to review if needed.

[Santhosh-Sellavel]

Roomname is copied successfully, but the editor is still empty while editing the room mention

https://github.com/Expensify/App/assets/85645967/0afe9bc1-833d-4239-ba2e-c662695a660e

[JmillsExpensify]

Interesting, let's keep this open then

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[ahmedGaber93]

@Santhosh-Sellavel room name will be copied successfully only with optimistic data like when user offline, but messages with backend data will produce the issue.

https://github.com/Expensify/App/assets/41129870/ebc47231-b354-4e78-802c-0b7af9eccd24

[Santhosh-Sellavel]

but messages with backend data will produce the issue. That's address by your proposal @ahmedGaber93

Now here we wait for

the editor is still empty while editing the room mention

[lakchote]

What's the latest on here @Santhosh-Sellavel @ahmedGaber93?

[ahmedGaber93]

@lakchote My proposal here should fix copy to clipboard issue, but @Santhosh-Sellavel mention here https://github.com/Expensify/App/issues/40403#issuecomment-2068185779 another case has the same root cause which not covered by my proposal.

The root cause is the backend data come with mention based-reportID format <mention-report reportID="1234" />, and ExpensiMark in replace method not have any rule to replace it with mention based-name <mention-report>#romName</mention-report>.

I think it is difficult to make ExpensiMark replace mention based-reportID to mention based-name, because we need to the reports data from onyx, and pass it to ExpensiMark seems unclean.

So I think we can apply the same fix in my proposal to everywhere we use report mention (BE handled it now in comments only), by create a new method replaceMentionReport(html)

function replaceMentionReport(html) {
  return html.replace(/<mention-report reportID="(\d*)"\/>/gi, (match, reportID): string => {
     const report = getReport(reportID);
     return `<mention-report>${report?.reportName ?? report?.displayName}<mention-report/>`;
  })
}

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@JmillsExpensify @lakchote @Santhosh-Sellavel this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[Santhosh-Sellavel]

The root cause is the backend data come with mention based-reportID format <mention-report reportID="1234" />, and ExpensiMark in replace method not have any rule to replace it with mention based-name <mention-report>#romName</mention-report>.

If so then we should add/update rule there

I think it is difficult to make ExpensiMark replace mention based-reportID to mention based-name, because we need to the reports data from onyx, and pass it to ExpensiMark seems unclean.

But yes I agree, that seems weird.

[Santhosh-Sellavel]

@ahmedGaber93 replacing everywhere seems weird too. How many places should we replace this? Can you list down?

[ahmedGaber93]

If so then we should add/update rule there

@Santhosh-Sellavel After dig in ExpensiMark.js I found we already have parameter extra which have accountIDToName and reportIdToName in htmlToMarkdown and htmlToText and will replace them here, but I can't found any usage for this parameter in the App, also when trying to use it, it not works, but I will try again.

[ahmedGaber93]

How many places should we replace this? Can you list down?

I think two places only edit action and copy to clipboard because BE replace report mention in comments only.

[ahmedGaber93]

when trying to use it, it not works, but I will try again.

@Santhosh-Sellavel After some investigation, I found why it not works (see step 3, 4), and these are the changes that work with me.

1. in src/libs/ReportUtils.ts add new method getReportIdToName

   function getReportIdToName(): Record<string, string> | undefined {
   const reportIdToName: Record<string, string> = {};
   Object.values(allReports ?? {}).forEach((report: OnyxEntry<Report>) => {
       if (report?.reportID && report?.reportName){
           reportIdToName[report?.reportID] = report?.reportName;
       }
   });
   return reportIdToName;
   }

2. in setClipboardMessage pass reportIdToName to ExpensiMark

   function setClipboardMessage(content: string) {
   const parser = new ExpensiMark();
   const reportIdToName = ReportUtils.getReportIdToName();
   if (!Clipboard.canSetHtml()) {
       Clipboard.setString(parser.htmlToMarkdown(content, {reportIdToName}));
   } else {
       const plainText = parser.htmlToText(content, {reportIdToName}); // this now return #roomName
       Clipboard.setHtml(content, plainText);
   }
   }

3. in setClipboardMessage above we fix the plainText but when we copy inside composer in web we check for html (content) first if found we copy it not plainText see here.

   Clipboard.setHtml(content, plainText);
   // we now copy this values Clipboard.setHtml('<mention-report reportID="1234" />', '#roomName')
   
   // paste in composer will paste content `<mention-report reportID="1234" />` return empty text and still need to fix
   // past outside app will past plainText `#roomName` fixed

To fix content above, we need to pass reportIdToName in handlePastedHTML to be parser.htmlToMarkdown(html, {reportIdToName}).

4. we still have a problem when I copy <mention-report reportID="1234" /> as HTML to clipboard, the paste value here paste with different format <mention-report reportid="1234"></mention-report> (<openTag></closeTag> not <closedTag />), and this format not match rule reportMentions in ExpensiMark so step 3 above will failed.

So we need to edit reportMentions here to match <closedTag /> and <openTag></closeTag>

// regex: /<mention-report reportID="(\d+)" *\/>/gi,
regex: /<mention-report reportID="(\d+)"(?: *\/>|><\/mention-report>)/gi,

https://github.com/Expensify/App/assets/41129870/78f37c5f-9cbd-43a3-8d61-9288f51cc594

[lakchote]

@Santhosh-Sellavel can you take a look at recent comments from @ahmedGaber93 please?

[eh2077]

@lakchote We can put this on hold as discussed here https://github.com/Expensify/App/issues/40477#issuecomment-2093804873. We can have a holistic solution to fix them together.

[marcaaron]

@ahmedGaber93 do you think you could drop into the other issue a Problem & Solution statement that summarizes both of these very similar issues and present the holistic plan to fix them?

[ahmedGaber93]

@ahmedGaber93 do you think you could drop into https://github.com/Expensify/App/issues/40477#issuecomment-2093804873 a Problem & Solution statement that summarizes both of these very similar issues and present the holistic plan to fix them?

@marcaaron Yes I agree, these are two implementations for one feature, and fixing them together will facilitate the process of testing and discover the possible bugs and fix it.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@JmillsExpensify, @lakchote, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

[eh2077]

I added a summary here https://github.com/Expensify/App/issues/40477#issuecomment-2104490261

[melvin-bot[bot]]

@JmillsExpensify, @lakchote, @Santhosh-Sellavel Still overdue 6 days?! Let's take care of this!

[eh2077]

We have updates on the other issue.

[JmillsExpensify]

Thanks, yes updated on the other issue.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@JmillsExpensify @lakchote @Santhosh-Sellavel this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[JmillsExpensify]

Based on the above discussion, should we close this issue or leave it open?

[lakchote]

Based on the above discussion, should we close this issue or leave it open?

Looking at latest @eh2077's comment, we could put this on hold for https://github.com/Expensify/App/pull/40565 to keep track of the issue, and revisit once it's merged?

[JmillsExpensify]

Fine putting it on Hold for now. That PR is being actively worked on.

[JmillsExpensify]

Moved to weekly given the hold

[JmillsExpensify]

Still on hold.

[JmillsExpensify]

Looks like the related issue is now merged, so removing the hold.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[lakchote]

@eh2077 the related PR https://github.com/Expensify/App/pull/40565 that we were waiting for was merged, but it did not solve the issue.

[eh2077]

@lakchote Yes, I pinged @ahmedGaber93 here https://github.com/Expensify/App/issues/40477#issuecomment-2199950787. I think it still makes sense to solve it together with https://github.com/Expensify/App/issues/40477

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[eh2077]

We're fixing this together with https://github.com/Expensify/App/issues/40477 - waiting for the PR from @ahmedGaber93

[JmillsExpensify]

Looks like we're waiting on the related App PR, though I'll go ahead and keep this issue as is because it shouldn't be too much longer.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸