[HOLD for payment 2023-09-20] [$1000] Web -Workspace - Inconsistency in Workspace Notification for HTML Character-Based Workspace Names

[izarutskaya]

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

---

Action Performed:

1. Access the workspace settings by clicking on settings > workspace. If there is no existing workspace, create a new one.
2. Enter the HTML character code-based name "Hello" for the workspace in the settings.
3. Navigate back to the workspace and click on the three dots, then enter the admin room. Observe that the workspace name is correctly displayed.
4. Return to the workspace, click the three dots in the right corner again, and delete the workspace. Go back to the admin room and observe the canceled workspace notification. Notice that "Hello" is changed to "Hello".

Expected Result:

The canceled workspace notification should display the original HTML character code-based name ""Hello"". It should show the same name as observed in the admin room, maintaining consistency and avoiding confusion for users.

Actual Result:

When a workspace name is set using HTML character codes "Hello", it changes to "Hello" in the canceled workspace notification. This inconsistency in the display of the workspace name can create confusion and incongruity for users.

Workaround:

unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.3.53-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

Notes/Photos/Videos: Any additional supporting documentation

screen-capture.-.2023-08-03T073415.177.mp4

Recording.1173.mp4

Expensify/Expensify Issue URL:

Issue reported by: @tewodrosGirmaA

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691073090566039

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ce586e8be7aadd12
• Upwork Job ID: 1691967857451331584
• Last Price Increase: 2023-08-31
• Automatic offers:
• TewodrosGirmaA | Reporter | 26535067

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[kursat]

Proposal

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

When a workspace name is set using HTML character codes "Hello", it changes to "Hello" in the canceled workspace notification. This inconsistency in the display of the workspace name can create confusion and incongruity for users.

What is the root cause of that problem?

HTML characters are not escaped in ArchivedReportFooter

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

We need to escape policyName in ArchivedReportFooter.js:

policyName: `<strong>${_.escape(ReportUtils.getPolicyName(props.report))}</strong>`,

What alternative solutions did you explore? (Optional)

[melvin-bot]

📣 @kursat! 📣
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. 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.
2. 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.
3. 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>

[kursat]

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

[melvin-bot]

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

[bernhardoj]

Proposal

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

In the workspace archived report footer, the workspace name is decoded.

What is the root cause of that problem?

When we close a workspace, the archive reason will be CONST.REPORT.ARCHIVE_REASON.POLICY_DELETED

App/src/components/ArchivedReportFooter.js

Lines 64 to 73 in 29e6b61

	<Banner
	containerStyles={[styles.archivedReportFooter]}
	text={props.translate(`reportArchiveReasons.${archiveReason}`, {
	displayName: `<strong>${displayName}</strong>`,
	oldDisplayName: `<strong>${oldDisplayName}</strong>`,
	policyName: `<strong>${ReportUtils.getPolicyName(props.report)}</strong>`,
	})}
	shouldRenderHTML={archiveReason !== CONST.REPORT.ARCHIVE_REASON.DEFAULT}
	shouldShowIcon
	/>

If it's not CONST.REPORT.ARCHIVE_REASON.DEFAULT, we will render the text as HTML (using RenderHTML). Because the workspace name contains HTML entity characters, it will be decoded when rendered as HTML (for example & to &).

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

Encode (Str.htmlEncode/_.escape) the policy name only if shouldRenderHTML is true.

[alexpensify]

I ran out of time to test this one yesterday but will review it on Monday.

[alexpensify]

Tomorrow, I will test more.

[melvin-bot]

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

[melvin-bot]

Current assignee @alexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[alexpensify]

@situchan - when you get a chance, can you review the proposals and identify if one will help address this error? Thanks!

[tienifr]

Proposal

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

When a workspace name is set using HTML character codes "Hello", it changes to "Hello" in the canceled workspace notification.

This also happens if the displayName of the ownerAccountID contains HTML character code, or displayName of oldAccountID and newAccountID in case of an account merge. Those names will also display in the decoded format, different from what's shown elsewhere in the app.
(If we think this is out of scope of this bug, I'd be happy to report it separately)

What is the root cause of that problem?

If the workspace archive reason is not DEFAULT, we're showing them in RenderHTML as can be seen here. This will decode the HTML character in the policyName and user's displayName, causing the inconsistency with the rest of the app.

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

We should _.escape the displayName, oldDisplayName and policyName inside the <strong> tag in these lines
if we're going to render them as HTML (this condition is optional since if we're not rendering them as HTML, the text also will not use those fields so it will still work well).

What alternative solutions did you explore? (Optional)

Instead of _.escape, we can use Str.htmlEncode.

[alexpensify]

No update yet, these proposals need to go through a review.

[alexpensify]

@situchan - can we get an update here regarding the proposal and if one will work for this GH? Thanks!

[alexpensify]

@situchan any update here?

[melvin-bot]

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

[situchan]

Testing proposals with various cases. Will update soon

[tewodrosGirmaA]

@alexpensify Could you please adjust the offer from $50 to $250, as the issue was created before August 31st when the offer letters were set at $250? Thank you.

[alexpensify]

@tewodrosGirmaA - I've not completed the process since the payment date is tomorrow. Sorry for the confusion, but it looks like the automation got the amount wrong. I've already flagged that I will add the remaining amount as a bonus.

#24427 (comment)

[tienifr]

@alexpensify The Contributor compensation should be split between me and @kursat as confirmed in here and here. Could you send me the offer as well?

[tewodrosGirmaA]

@alexpensify I have not yet accepted the offer and I was wondering if it would be possible to accept the offer along with an additional bonus, or if an alternate offer could be presented. I would appreciate your thoughts on this matter.

[alexpensify]

@tienifr thanks for flagging, I've updated the compensation amounts here:

#24427 (comment)

Please apply for the job here:

https://www.upwork.com/jobs/~01ce586e8be7aadd12

[alexpensify]

@tewodrosGirmaA - please accept as is, I think I can apply the bonus amount when I close the job. If I can't, then I've already noted there is a remaining $200, I can create a new job to cover the missing amount. I will take care of this tomorrow.

[tewodrosGirmaA]

I accept it, Thank you

[tienifr]

Please apply for the job here:

@alexpensify I applied, thanks!

[kursat]

@alexpensify I've already applied, thanks

[alexpensify]

Alright, @tewodrosGirmaA as promised you were paid $250 via Upwork -- Done!

@tienifr and @kursat - please accept and we should be all set

@allroundexperts -- please complete the checklist and we can close this GH. Thanks!

[tewodrosGirmaA]

@alexpensify Thank you so much

[alexpensify]

Alright, @tienifr and @kursat - you have been paid via Upwork.

@allroundexperts please submit a request in Chat and complete the Checklist. After, we can close this GH. Thanks!

[marcaaron]

I think we are just waiting for payment here.

[alexpensify]

@marcaaron no, this is open because the checklist is not completed yet.

@allroundexperts -- please complete the checklist and we can close this GH. Thanks!

[alexpensify]

@allroundexperts bumping again, we need to complete the checklist before I can close -- thanks!

[allroundexperts]

Apologies for the delay. I'll complete this today!

[alexpensify]

Thank you @allroundexperts!

[allroundexperts]

Reviewer Checklist

1. I don't think that this was ever working and that any PR caused this. In my opinion, this was a missed edge case.
2. N/A
3. N/A
4. A regression test would be helpful here. The following are the test steps.

Regression test

1. Create a new workspace and set its name to something with HTML characters eg. test
2. Delete the created workspace.
3. Go to the admin room of the workspace (which should be archived by now) and verify that the workspace name appears correctly in the chat eg. test and not Test.

Do we 👍 or 👎?

[alexpensify]

@allroundexperts - thanks for completing this task. @marcaaron - can you review the regression test and confirm if you agree? Thanks!

[marcaaron]

LGTM

[alexpensify]

Closing - the regression test has been created for this case.

[marcaaron]

Thanks @alexpensify 🙇

[JmillsExpensify]

$1,500 payment approved for @allroundexperts based on BZ summary.