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

[HOLD for payment 2024-10-22] Expense - "auto paid with Expensify" system message has <a href="link"> and </a> when pasted #50090

Closed
6 tasks done
IuliiaHerets opened this issue Oct 2, 2024 · 44 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Oct 2, 2024

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: 9.0.43-0
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  • Workspace has set up bank account.
  • Employee does not have personal bank account.
  1. Go to staging.new.expensify.com
  2. [Employee] Submit an expense in the workspace chat.
  3. [Admin] Pay the expense via Expensify.
  4. [Employee] Click on the expense preview.
  5. [Employee] Click Add bank account button.
  6. [Employee] Add a bank account.
  7. [Employee] Right click on the system message "automatically paid $123.00 with Expensify via workspace rules" > Copy to clipboard.
  8. [Employee] Paste the content in the composer.

Expected Result:

The content will be pasted correctly.

Actual Result:

The pasted content has <a href="link"> and </a>.
The system message also shows <a href="link"> and </a> when the report is opened for the first time (after relogin).

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6622263_1727884910251.first_time_open_report.mp4
Bug6622263_1727883711991.20241002_233604.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @sakluger
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels Oct 2, 2024
Copy link

melvin-bot bot commented Oct 2, 2024

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

Copy link

melvin-bot bot commented Oct 2, 2024

Triggered auto assignment to @sakluger (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 melvin-bot bot added the Daily KSv2 label Oct 2, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Oct 2, 2024
Copy link
Contributor

github-actions bot commented Oct 2, 2024

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

@IuliiaHerets
Copy link
Author

We think that this bug might be related to #wave-collect - Release 1

@gijoe0295
Copy link
Contributor

gijoe0295 commented Oct 2, 2024

Edited by proposal-police: This proposal was edited at 2024-10-02 18:27:11 UTC.

Proposal

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

  1. The pasted content has html anchor tag
  2. The system message also shows html anchor tag when the report is opened for the first time (after relogin).

What is the root cause of that problem?

  1. We're directly using anchor tags in copy:

App/src/languages/en.ts

Lines 928 to 929 in 66d1025

automaticallyPaidWithExpensify: ({payer, amount}: PaidWithExpensifyWithAmountParams) =>
`${payer ? `${payer} ` : ''}automatically paid ${amount} with Expensify via <a href="${CONST.CONFIGURE_REIMBURSEMENT_SETTINGS_HELP_URL}">workspace rules</a>`,

  1. This is a BE bug where OpenApp returns the message's html as plain text:
Screenshot 2024-10-03 at 01 22 50

causing the shouldRenderAsText here to be false (because html and text are the same in this case) and thus the iouMessage renders as plain text:

if (!shouldRenderAsText(html, text ?? '') && !(containsOnlyEmojis && styleAsDeleted)) {

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

  1. We should use markdown link instead of html anchor tag like we did here:

...with Expensify via [workspace rules](${CONST.CONFIGURE_REIMBURSEMENT_SETTINGS_HELP_URL})

  1. Fix the BE

@sakluger
Copy link
Contributor

sakluger commented Oct 2, 2024

I don't know if this needs to be a deploy blocker, it's not really blocking users from completing actions in Expensify. @luacmartins what do you think?

@Gonals Gonals removed the DeployBlocker Indicates it should block deploying the API label Oct 2, 2024
@jasperhuangg
Copy link
Contributor

@sakluger I think we should still try to fix the issue rather than letting the problem go out to production. @luacmartins were you able to look into this yet?

@luacmartins
Copy link
Contributor

Not yet, but as pointed out by @gijoe0295 this seems to be a backend issue, so maybe a DeployBlocker for web-e, not App.

@luacmartins luacmartins added DeployBlocker Indicates it should block deploying the API and removed DeployBlockerCash This issue or pull request should block deployment labels Oct 2, 2024
@Gonals
Copy link
Contributor

Gonals commented Oct 2, 2024

Do we know which PR caused this? At this point, we should revert it or we won't deploy today

@luacmartins luacmartins added DeployBlockerCash This issue or pull request should block deployment and removed DeployBlocker Indicates it should block deploying the API labels Oct 2, 2024
Copy link

melvin-bot bot commented Oct 2, 2024

Current assignee @luacmartins is eligible for the DeployBlockerCash assigner, not assigning anyone new.

Copy link
Contributor

github-actions bot commented Oct 2, 2024

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

@luacmartins
Copy link
Contributor

luacmartins commented Oct 2, 2024

Actually, I think the main cause is #49187. I pinged Alex in Slack

@Beamanator
Copy link
Contributor

yep yep, lookingggg

@Beamanator
Copy link
Contributor

Hmm honestly i wouldn't call this a blocker - the copy / paste including the href part - i did that on purpose... If we want to change that so it doesn't have the link, i'm fine with that but doesn't need to be blocker-priority

I don't understand the other point - The system message also shows and when the report is opened for the first time (after relogin). - I don't see that in the videos

@luacmartins
Copy link
Contributor

Demoting to NAB given Alex's comment

@sakluger
Copy link
Contributor

sakluger commented Oct 7, 2024

Got it. So where is the other PR? I don't see any others linked to this GH issue.

@Beamanator
Copy link
Contributor

Aah the other one is https://github.com/Expensify/Auth/pull/12691! Which is on prod as of last night so hopefully we can see it's fixed now! 🙏

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 Weekly KSv2 labels Oct 8, 2024
@melvin-bot melvin-bot bot changed the title Expense - "auto paid with Expensify" system message has <a href="link"> and </a> when pasted [HOLD for payment 2024-10-22] Expense - "auto paid with Expensify" system message has <a href="link"> and </a> when pasted Oct 15, 2024
Copy link

melvin-bot bot commented Oct 15, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Oct 15, 2024
Copy link

melvin-bot bot commented Oct 15, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.48-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-10-22. 🎊

For reference, here are some details about the assignees on this issue:

  • @ishpaul777 requires payment (Needs manual offer from BZ)

Copy link

melvin-bot bot commented Oct 15, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@ishpaul777] The PR that introduced the bug has been identified. Link to the PR:
  • [@ishpaul777] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@ishpaul777] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@ishpaul777] Determine if we should create a regression test for this bug.
  • [@ishpaul777] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@luacmartins luacmartins added Daily KSv2 and removed Weekly KSv2 labels Oct 16, 2024
@sakluger
Copy link
Contributor

@sakluger sakluger added Weekly KSv2 and removed Daily KSv2 labels Oct 17, 2024
@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Oct 21, 2024
@sakluger
Copy link
Contributor

@ishpaul777 do we need any regression new tests for this one?

Copy link

melvin-bot bot commented Oct 22, 2024

Payment Summary

Upwork Job

BugZero Checklist (@sakluger)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@sakluger
Copy link
Contributor

This has already been paid in #50742.

@ishpaul777 let us know if we need any regression tests before we close the issue.

@ishpaul777
Copy link
Contributor

Sorry i missed ping, no i dont think we need regression becuase its found in normal QA process we might have tests covering this already

@ishpaul777
Copy link
Contributor

@sakluger Isn't #50742 a payment issue for different PR ?

@sakluger
Copy link
Contributor

Oh wow, you're right - I misread the issue number on that one. My mistake!

Here's an offer for this one: https://www.upwork.com/nx/wm/offer/104577466

@sakluger
Copy link
Contributor

Paid! Thanks again for staying on top of this @ishpaul777.

@github-project-automation github-project-automation bot moved this from Polish to Done in [#whatsnext] #expense Oct 24, 2024
Copy link

melvin-bot bot commented Oct 24, 2024

@Beamanator @sakluger @luacmartins 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
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering
Projects
Status: Done
Development

No branches or pull requests

9 participants