Closed IuliiaHerets closed 6 hours ago
Triggered auto assignment to @kevinksullivan (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.
We think that this bug might be related to #vip-vsb
@kevinksullivan FYI 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
Edited by proposal-police: This proposal was edited at 2024-08-15 11:38:39 UTC.
Copy to clipboard & pasting phone number, shows @expensify.sms after number
We forgot to remove sms domain for the text here https://github.com/Expensify/App/blob/d4d5a2586910ff46147219ee3e98bb3e936f8037/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L52-L53 and htmlToText by default doesn't remove sms domains
We need to update our expensify-common lib, that is, we need to remove sms domain for htmlToText
case same as we do for htmlToMarkdown here
So we need to change code here
to
return `@${Str.removeSMSDomain(extras.accountIDToName?.[g1]?? '') }`;
We need to remove the sms domain
function setClipboardMessage(content: string) {
if (!Clipboard.canSetHtml()) {
Clipboard.setString(Str.removeSMSDomain(Parser.htmlToMarkdown(content)));
} else {
const anchorRegex = CONST.REGEX_LINK_IN_ANCHOR;
const isAnchorTag = anchorRegex.test(content);
const plainText = isAnchorTag ? Parser.htmlToMarkdown(content) : Parser.htmlToText(content);
Clipboard.setHtml(content, Str.removeSMSDomain(plainText));
}
}
or only apply the removing to htmlToText
cases as htmlToMarkdown
properly removes the sms domain
alternatively We can also take the cleaning code to Clipboard.setHtml and setString
@kevinksullivan Huh... This is 4 days overdue. Who can take care of this?
@kevinksullivan Still overdue 6 days?! Let's take care of this!
Job added to Upwork: https://www.upwork.com/jobs/~01e408113871c83775
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ZhenjaHorbach (External
)
@FitseTLT Thank you for your proposal ! I like your main solution and I agree that we need to update userMention rule So I'm happy to go with this proposal
🎀👀🎀 C+ reviewed
Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
📣 @ZhenjaHorbach 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!
📣 @FitseTLT 🎉 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 📖
@kevinksullivan We are ready for payment !
https://github.com/Expensify/App/pull/48280#issuecomment-2327618578
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:
- [x] [@ZhenjaHorbach] The PR that introduced the bug has been identified. Link to the PR:
It's more like a new feature Since now htmlToText removes sms domains by default as we do for htmlToMarkdown
- [x] [ @ZhenjaHorbach] 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:
NA
- [x] [@ZhenjaHorbach] 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:
NA
- [x] [@ZhenjaHorbach] Determine if we should create a regression test for this bug.
- [x] [@ZhenjaHorbach] 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.
Regression Test Proposal
Do we agree 👍 or 👎
All set!
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: 9.0.20 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
Copy to clipboard and pasting phone number, must not show @expensify.sms after number.
Actual Result:
Copy to clipboard and pasting phone number, shows @expensify.sms after number.
Workaround:
Unknown
Platforms:
Screenshots/Videos
https://github.com/user-attachments/assets/38faca5c-1151-446e-a781-fb6785ea218d
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @ZhenjaHorbach