[$250] [Guided Setup] Fix subject lines in emails sent upon onboarding modal completion

[MitchExpensify]

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: v1.4.60-13
Reproducible in staging?: yes
Reproducible in production?: no
If this was caught during regression testing, add the test name, ID and link from TestRail: na
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: [email protected]
Slack conversation: Internal: https://expensify.slack.com/archives/C036QM0SLJK/p1712608278412419

Action Performed:

1. Sign up to staging.new.expensify.com
2. Go to staging.new.expensify.com/onboarding
3. Enter first name and last name go, click "Continue" on the onboarding modal
4. Click any of these options:

• Manage my team’s expenses
• Track business spend for taxes
• Track and budget personal spend

5. Click Continue
6. Check your email

Expected Result:

The custom subject line should not contain "'" wherever a "'" should be.

e.g:

• Track business spend for taxes: "Welcome to Expensify, let's start tracking your expenses!"
• Manage my team’s expenses: "Welcome to Expensify, let's start managing your team's expenses!"
• Track and budget personal spend: "Welcome to Expensify, let's start tracking your expenses!"

Actual Result:

Custom subject lines contain erroneous characters wherever a "'" should be:

• Track business spend for taxes: "Welcome to Expensify, let's start tracking your expenses!"
• Manage my team’s expenses: "Welcome to Expensify, let's start managing your team's expenses!"
• Track and budget personal spend: "Welcome to Expensify, let's start tracking your expenses!"

Workaround:

Ignore

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01697f68b79fde14d3
• Upwork Job ID: 1778159532698587136
• Last Price Increase: 2024-04-10
• Automatic offers:
• rayane-djouah | Contributor | 0

[melvin-bot]

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

[filip-solecki]

Hi! I am Filip from SWM an expert agency and I'd like to work on this issue!

[brkdavis]

Proposal

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

Single quotes and some special characters are not escaped from report comment text (which goes in the email)

What is the root cause of that problem?

reportComment is escaped and fetched in function. However this escaping used ExpensiMark from expensify-common lib.

function completeEngagementModal(text: string, choice: ValueOf<typeof CONST.ONBOARDING_CHOICES>) {
    . . . 
    const reportComment = ReportUtils.buildOptimisticAddCommentReportAction(text, undefined, conciergeAccountID);
    const reportCommentAction: OptimisticAddCommentReportAction = reportComment.reportAction;
    const lastComment = reportCommentAction?.message?.[0];
    const lastCommentText = ReportUtils.formatReportLastMessageText(lastComment?.text ?? '');

Object optimisticReport is filled with lastCommentText for both text as well as formatted html.

        lastMessageText: lastCommentText,
        lastMessageHtml: lastCommentText,

ReportUtils.formatReportLastMessageText() uses ExpensiMark parser from expensify-common lib, which uses underscore package to escape html content and does not take into consideration escaping of single quote. This results in single quote in JSON body being replaced with its equivalent escape string while sending email.

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

ReportUtils.getParsedComment() function to escape single quote using lodash irrespective of length of the string.

return parser.replace(lodashEscape(textWithMention), {shouldEscapeText: shouldAllowRawHTMLMessages()});

Used ExpensiMark parser here for formatting along with lodash for escaping.
Un-escaping the escaped string will result in emails with the right subject and body.

What alternative solutions did you explore? (Optional)

Another alternative is to update ExpensiMark lib in expensify-common to use a package like lodash to escape special characters.

[melvin-bot]

📣 @brkdavis! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[melvin-bot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[melvin-bot]

⚠️ Unable to store contributor details.

[jliexpensify]

Hi @brkdavis - I believe that this issue is now assigned to @filip-solecki to work on!

[brkdavis]

@jliexpensify : Sorry. I didnt see a proposal on this. Out of curiosity investigated and submitted my findings in the proposal.
@filip-solecki : If you don't mind, I can pick this work, if my proposal is accepted.

[brkdavis]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01cef163ba3c7ed327

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[mountiny]

No update yet

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01697f68b79fde14d3

[melvin-bot]

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

[mountiny]

Looking for a quicker movement on this one as the onboarding flow is nearing completion

[melvin-bot]

📣 @rayane-djouah 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[brkdavis]

Proposal

Updated

@rayane-djouah : can you please confirm the changes?

[rayane-djouah]

@brkdavis, please ensure that your proposal results in a correctly formatted email and does not break the message received in Expensify Concierge.

Here is a screenshot showing the Concierge message with your previous proposal:

Here is a screenshot showing the Concierge message with your updated proposal (1 is result of current code, and 2 is result of your updated proposal):

(The title should be bold)

Note that we need to use the ExpensiMark parser to replace markdown with HTML elements for the "custom subject line" format to specify the subject of the notification email. Custom subject lines begin with #. These should be converted to h1 and then they are parsed out on the backend and used as the notification subject.

I tested your proposal but did not receive the email with your changes. Please test using dev.new.expensify.com/onboarding url and review the Concierge chat message in addition to email.

I'm now unsure whether this bug can be fixed from the frontend or need to be fixed from the backend.

[filip-solecki]

In my opinion it should be handled on BE

[brkdavis]

Proposal

Updated

[brkdavis]

Proposal

Updated

@rayane-djouah : thank you for detailed look into this. I have updated the proposal.
I believe frontend definitely needs this fix.
Attached screenshot from Concierge chat with bolded subject and well escaped text content.

In case backend is not handling these correctly, might need a fix there as well.

[rayane-djouah]

@brkdavis, I tested and the email subject and body are broken with the proposal changes.

[sumitskj]

Proposal

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

Subject lines of emails after onboarding modal completion show HTML escaped characters like '

What is the root cause of that problem?

The root cause of the problem is that we are using ExpensiMark() to parse our content to HTML that we want to send in emails. In our current scenario, our message length is less than 10000 characters so it uses ExpensiMark() to parse to HTML with the property shouldEscapeText as true. Due to this, we are converting our message string to HTML something like this.

<h1>Welcome to Expensify, let&#x27;s start tracking your expenses!</h1>Hi there, I&#x27;m Concierge. Chat with me here for anything you need.<br /><br />To track your expenses, create a workspace to keep everything in one place. Here&#x27;s how:<br />1. From the home screen, click the green + button &gt; New Workspace<br />2. Give your workspace a name (e.g. &quot;My business expenses”).<br /><br />Then, add expenses to your workspace:<br />1. Find your workspace using the search field.<br />2. Click the gray + button next to the message field.<br />3. Click Request money, then add your expense type.<br /><br />We&#x27;ll store all expenses in your new workspace for easy access. Let me know if you have any questions!

Now this message is passed to the API to send mail and I guess there you are just copying the text inside HTML tags as it is without trying to unescape the HTML converted h1 string.

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

Inside the method getParsedComment(string) of ReportUtils.ts at the last line when we are returning the our HTML escaped message string, we can substring our message in 2 parts one for the h1 subject content and other for the rest of the message. And for the header we will escape the string with ExpensiMark() parser with property shouldEscapeText set to false and for the rest of the message we will escape the string with ExpensiMark() parser with property shouldEscapeText set to true.

Code Changes:

Before :

return text.length <= CONST.MAX_MARKUP_LENGTH ? parser.replace(textWithMention, {shouldEscapeText: !shouldAllowRawHTMLMessages()}) : lodashEscape(text);

After :

const contentSubject  = textWithMention.substring(text.indexOf('#'), textWithMention.indexOf('\n'));
    const messageData = textWithMention.substring(textWithMention.indexOf('\n'));
    return text.length <= CONST.MAX_MARKUP_LENGTH ? parser.replace(contentSubject, {shouldEscapeText: shouldAllowRawHTMLMessages()}) + parser.replace(messageData, {shouldEscapeText: !shouldAllowRawHTMLMessages()}) : lodashEscape(text);

Doing these changes will fix the issue.

Screenshots:
Mail:

Concierge message:

What alternative solutions did you explore? (Optional)

We can also fix this where the code for sending the mail is present i.e. backend. There as well we can have the same logic of fetching the subject and than unescaping the characters that we don't want.

[melvin-bot]

📣 @sumitskj! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[sumitskj]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01de4e6fe70d63b1fb

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[rayane-djouah]

Thanks for the proposal, @sumitskj. Can you ensure that your proposed changes will not affect other app functionalities? buildOptimisticAddCommentReportAction is used in many places, and we need to make the changes only in the onboarding flow. Also, how can we make the solution for email subjects reusable? We are starting to send the email subject text more and more from the front end.

I'm asking in Slack if we should fix this bug in front-end or back-end.

[sumitskj]

@rayane-djouah You are correct that it might affect other app functionalities as buildOptimisticAddCommentReportAction is used in many places.

To handle this we can add an extra parameter onboardingFlow to buildOptimisticAddCommentReportAction(text?: string, file?: FileObject, actorAccountID?: number, onboardingFlow: boolean) and also to getParsedComment(text: string), based on this new parameter we can decide whether we want to use our new logic or not.

Another option can be to create a new method similar to buildOptimisticAddCommentReportAction just for handling the onboarding flow, and call it from completeEngagementModal, since completeEngagementModal handles all the onboarding modals.

To make email subjects reusable, we can create a common method which will take the message text as input and can return the messageSubject and messageBody. Something like this

function getCommentHeaderAndBody(comment: string) {
    let header = '';
    let body = '';
    if(comment.startsWith('#')){
        header = comment.substring(comment.indexOf('#'), comment.indexOf('\n'));
        body = comment.substring(comment.indexOf('\n'));
        return {header : header, body: body}
    }
    return {header : header, body: comment};
}

[melvin-bot]

Current assignee @rayane-djouah is eligible for the Internal assigner, not assigning anyone new.

[Gonals]

Looking at this, this is an existing issue for all flows that may have ' on the subject. I think the way to go is to add a fix on the backend.

[mountiny]

@Gonals can this be closed now?

[Gonals]

The PR hasn't deployed yet, so let's wait for a bit

[danielrvidal]

It looks like the PR was deployed so can we close this one now?

[Gonals]

Yep!