[$500] Chat - Reply chat notification email with markdown text does not show it in the app

[lanitochka17]

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.26-1
Reproducible in staging?: Y
Reproducible in production?: Y
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. Open New Expensify app and log in with account A
2. Go to a chat with account B with an email you have access to
3. Send dome messages, but do not read them with the receiver account
4. Go to account B email and look for the chat notification email
5. Reply it with a markdown text
6. Go to the chat again as account A

Expected Result:

Markdown text should be displayed on the chat

Actual Result:

The text is displayed without markdown

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010f8f3d47ee75887e
• Upwork Job ID: 1747717905448091648
• Last Price Increase: 2024-01-24

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~010f8f3d47ee75887e

[melvin-bot]

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

[melvin-bot]

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

[ikevin127]

Proposal

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

Email chat replay with markdown text is displayed without markdown.

What is the root cause of that problem?

The payload that comes from the the email client is not formatted the same as say a comment with markdown is formatted when posted via the composer, this causes the ReportActionItemFragment.js to render the markdown comment as unformatted because:

• whenever any kind of comment is coming from email reply, markdown or not, it is wrapped in <div dir="ltr">whatever</div> which just renders the plain text within the div.

This is the main issue but then we have another types of markdown that it's not parsed correctly via our ExpensiMark parser even though we remove the wrapping div mentioned above - the following will still not be parsed correctly: (found these cases by going line-by-line on our ExpensiMark markdown parser)

• [some link](https://www.google.com) - the email formatting adds a wrapping a tag around the link
• whenever we use newline, the email formatts it by adding a <br /> tag
• [some email]([email protected]) - the email formatting adds @mailto: in front of the email
• quotes - the email formatting replaces > with >
• the email formatting replaces < with <

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

Within RenderCommentHTML.js filter out all the problem additions by the email formatting via a regex map then apply parser.replace which replaces markdown with corresponding html element for any valid markdown.

COMMENT_HTML_REGEX_MAP: {
        '<div dir="ltr">|<\\/div>': '', // the initial wrapping div
        '<a href="(.*)">(.*)<\\/a>': '$1', // when we post [some link](https://www.google.com) the email formatting adds a wrapping a tag around the link
        '<br \\/>': '\n', // when we use newline in comment via email
        '@mailto:': '@', // when we post [some email]([email protected]) // the email formatting adds `@mailto:` in front of the email
        '&gt;': '>', // the email replaces > with &gt;
        '&lt;': '<', // the email replaces < with &lt;
        'mailto:': '', // for the ExpensiMark `autoEmail` case, the email formatting adds mailto: in front of it
    }

This will allow us to post all markdown that we can currently post via the composer, but from an email reply.

Videos

MacOS: Chrome / Safari

Screen.Recording.2024-01-18.at.00.08.07.mov

[zanyrenney]

@thesahindia what do you think about the proposal above, can you review please?

[zanyrenney]

bump @thesahindia

[melvin-bot]

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

[thesahindia]

@ikevin127's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @Beamanator, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Beamanator]

Hmm I believe that proposal could work, but I also think this could be useful to try to "fix" / change in the backend, no? I think this would benefit from some internal investigation to figure out why / if we need this wrapping div, on email responses

[ikevin127]

@Beamanator @thesahindia Actually, while I was testing my fix against our ExpensiMark library which parses markdown and I found out there's more than just the div that we have to remove in order to render the expected markdown in the chat.

This is the regex map I came up with after going line-by-line against our ExpensiMark library source and sending all types of markdown from email reply:

COMMENT_HTML_REGEX_MAP: {
        '<div dir="ltr">|<\\/div>': '', // the initial wrapping div
        '<a href="(.*)">(.*)<\\/a>': '$1', // when we post [some link](https://www.google.com) the email formatting adds a wrapping a tag around the link
        '<br \\/>': '\n', // when we use newline in comment via email
        '@mailto:': '@', // when we post [some email]([email protected]) // the email formatting adds `@mailto:` in front of the email
        '&gt;': '>', // the email replaces > with &gt;
        '&lt;': '<', // the email replaces < with &lt;
        'mailto:': '', // for the ExpensiMark `autoEmail` case, the email formatting adds mailto: in front of it
    }

All these need to be replaced with their correct keys from the map in order to be rendered correctly as expected by the ExpensiMark in the front-end - which will match and align with the markdown we can send from the composer.

Note: this happens regardless, because of how the email client is formatting different markdown. The back-end would basically have to do the exact same thing using this regex map and replace the email formatting.

App/src/pages/home/report/comment/RenderCommentHTML.tsx

Line 11 in ac94dc5

const commentHtml = source === 'email' ? `<email-comment>${html}</email-comment>` : `<comment>${html}</comment>`;

Also a bug was introduced recently where the source is typed as: type OriginalMessageSource = 'Chronos' | 'email' | 'ios' | 'android' | 'web' | '' but what we're actually getting is an object:

{
    "html": "<div dir=\"ltr\"># Heading 1 from email</div>",
    "lastModified": "2024-01-25 01:42:33.973",
    "source": "email"
}

so it becomes source.source.

I accounted for this issue in the PR that is ready to be opened in case we decide to do this from the front-end side 🚀

Updated proposal

• added the newfound details explained above

[zanyrenney]

@Beamanator let us know what you think of the updated proposal with your feedback!

[Beamanator]

Hmm here's my thoughts:

1. If there's more than 1 type of mapping / markdown that's broken, it would be great to fix them all at once (or as many as we can right now) instead of 1 by 1
2. If we want to do ^, I think we should make sure we're clear about what doesn't work correctly (& how to reproduce) so we can be 100% sure the fix DOES work, during testing & after the fix goes live
3. About the type OriginalMessageSource bug, this would be great to comment in the original PR (if you haven't already) since the typescript migration is currently ongoing

Note: this happens regardless, because of how the email client is formatting different markdown. The back-end would basically have to do the exact same thing using this regex map and replace the email formatting.

Hmm so @ikevin127 you're basically saying this mapping you created probably needs to be implemented in the client or backend side, right?

What do you think @thesahindia @ikevin127 ?

[ikevin127]

Fully agree with 1 and 2 🚀

As for 3 I will look into it as I don't think it's a typescript migration issue but something changed in that we're getting an object now instead of a string from the source prop in the RenderCommentHTML.tsx component.

This is most likely coming from a parent up in the stack. 🔍 I will look into it and try to find what changed recently that prompted getting an object instead of a string for the source prop.

Edit: looks like the source object issue is gone (fixed after pulling latest main) - not sure why I it was an object 2 days ago when I tested the implementation - but is's good now, source it's a string of 'email' for email comments.

you're basically saying this mapping you created probably needs to be implemented in the client or backend side, right?

It can be done on either side, but it's less work if we process this in the front-end since the back-end, besides using the regex map to remove / replace the formatting added by the email client, I assume they would also have to format the response sent to the front-end to match the response we get when posting markdown via the composer.

Ultimately, I'm cool with either side handling this - I'll leave it to your discretion 🙌

[zanyrenney]

bump @Beamanator which path forward should we opt for?

[Beamanator]

Thanks for your responses so far @ikevin127 ! One final potential concern I have is: We currently have 3 codebases for our 3 "front ends" - NewDot, OldDot, and OldApp. If we do this mapping in "the front end" won't we have to do the same mapping in all 3 places?

[ikevin127]

This particular issue is targeting ND, beyond that it's above my knowledge.

I guess for the other 2 FE codebases we have to ask the team if we even have any markdown posting capabilities in the chat.

If not, a FE fix can work here. If not then there's no choice then to go with a BE fix.

Edit: looks like we can post markdown in the OD chat too - but I did not check the reply from email for OD (not sure we have the functionality).

[melvin-bot]

@Beamanator @zanyrenney @thesahindia 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!

[zanyrenney]

@Beamanator any chance you can take a look at this one now please?

[melvin-bot]

@Beamanator, @zanyrenney, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Beamanator]

@zanyrenney this isn't part of a VIP or wave, so I haven't been prioritizing it - do you think I really need to get to work on it ASAP? It seems like an edge case, I don't know anyone who replies to emails using markdown 😅

[melvin-bot]

@Beamanator @zanyrenney @thesahindia this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot]

Current assignee @thesahindia is eligible for the Internal assigner, not assigning anyone new.

[zanyrenney]

Fair challenge @Beamanator if we don't think this a priority, let's make it weekly for now then.

[zanyrenney]

is weekly still appropriate? @Beamanator

[Beamanator]

Merp, good question - I honestly think monthly would be a fine priority level, I don't see people responding via email in markdown format often at all - I'm also heading OOO on Friday for 8 days - so we can either look for volunteers to pick this up in the meantime or else I can stay assigned to try to pick this up in March

[zanyrenney]

changing to monthly as per the guideance above from @Beamanator

[zanyrenney]

I think we can close this based on this question from the BZ team analysis:
Is this truly a high enough priority / ROI bug to fix?
Will this affect more than x # of users?
Does it affect users who pay us?
How much of an edge case is it?
If we fix the bug later, in a year or two, will that be OK?