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

[$250] Mention - @expensify.sms is copied along with phone number when copied to clipboard #38187

Closed
6 tasks done
lanitochka17 opened this issue Mar 12, 2024 · 44 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Mar 12, 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: 1.4.51-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #37559

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to any chat
  3. Send a text with phone number mention
  4. Right click on the message > Copy to clipboard
  5. Paste the content in the composer

Expected Result:

@expensify.sms will not be copied along with phone number

Actual Result:

@expensify.sms is copied along with phone number

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

Bug6411578_1710278333139.20240313_051600.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e5f15c665b86efd4
  • Upwork Job ID: 1772750767801118720
  • Last Price Increase: 2024-04-02
  • Automatic offers:
    • alitoshmatov | Reviewer | 0
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 12, 2024
Copy link

melvin-bot bot commented Mar 12, 2024

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

@lanitochka17
Copy link
Author

@greg-schroeder 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

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp
CC @quinthar

@nkdengineer
Copy link
Contributor

nkdengineer commented Mar 13, 2024

Proposal

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

  • Mention - @expensify.sms is copied along with phone number when copied to clipboard

What is the root cause of that problem?

  • In here we are using the messageHtml to get the copy text in case of this bug

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

  • We can use:
    setClipboardMessage(content.replace(/(<mention-user>)(.*?)(<\/mention-user>)/gi, function(match, openTag, content, closeTag) {
                        // Replace email addresses inside content with *****
                        content = content.replace(new RegExp(EMAIL_PART, "gim"), (match)=>Str.removeSMSDomain(match))
                        // Return the modified content inside the tags
                        return openTag + content + closeTag;
                    }));

in here and here

  • In the above,
    (<mention-user>)(.*?)(<\/mention-user> is the regex matching all the text inside mention-user tag, which will handle the case user types "[email protected]" directly, in this case, we should not remove the "@expensify.com", EMAIL_PART = ([\\w\\-\\+\\'#]+(?:\\.[\\w\\-\\'\\+]+)*@(?:[\\w\\-]+\\.)+[a-z]{2,})" is regex matching an text containing an email (we already used it in here)

  • Or we can just use:

  setClipboardMessage(content.replace(/(<mention-user>)(.*?)(<\/mention-user>)/gi, function(match, openTag, content, closeTag) {
                        content = Str.removeSMSDomain(content);
                        return openTag + content + closeTag;
                    }));

What alternative solutions did you explore? (Optional)

  • We also can update this to:
            const messageHtml = Str.removeSMSDomain(getActionHtml(reportAction));

@melvin-bot melvin-bot bot added the Overdue label Mar 15, 2024
Copy link

melvin-bot bot commented Mar 18, 2024

@greg-schroeder Huh... This is 4 days overdue. Who can take care of this?

Copy link

melvin-bot bot commented Mar 20, 2024

@greg-schroeder Still overdue 6 days?! Let's take care of this!

@greg-schroeder
Copy link
Contributor

Apologies, I was OOO last week. I'll take a look at this shortly

@greg-schroeder
Copy link
Contributor

Adding to #vip-vsb as this is chat-related, and I'm pretty sure this is very simple.

@greg-schroeder greg-schroeder changed the title Mention - @expensify.sms is copied along with phone number when copied to clipboard [$250] Mention - @expensify.sms is copied along with phone number when copied to clipboard Mar 26, 2024
Copy link

melvin-bot bot commented Mar 26, 2024

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

@greg-schroeder greg-schroeder added the External Added to denote the issue can be worked on by a contributor label Mar 26, 2024
Copy link

melvin-bot bot commented Mar 26, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 26, 2024
Copy link

melvin-bot bot commented Mar 26, 2024

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

@allgandalf
Copy link
Contributor

allgandalf commented Mar 27, 2024

Proposal

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

@expensify.sms is copied along with phone number when copied to clipboard

What is the root cause of that problem?

We set the html message received without checking if it is the @expensfiy.sms domain

const messageHtml = getActionHtml(reportAction);

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

We need to use Str.removeSMSDomain from expensify-common to remove the sms domain

It would be probably good to apply this filtering formessageText as well:

const messageText = ReportActionsUtils.getReportActionMessageText(reportAction);

Result

simplescreenrecorder-2024-03-27_07.12.14.mp4

@allgandalf
Copy link
Contributor

allgandalf commented Mar 27, 2024

Updated proposal

Added result video

Sidenote, currently on dev we are facing infinte loop problems #39028

Copy link

melvin-bot bot commented Mar 27, 2024

📣 @Ozeias01Nunes! 📣
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.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@Ozeias01Nunes
Copy link

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

Copy link

melvin-bot bot commented Mar 27, 2024

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

@Ozeias01Nunes
Copy link

Ozeias01Nunes commented Mar 28, 2024

Proposal

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

@expensify.sms is copied along with phone number when copied to clipboard

What is the root cause of that problem?

We set the html message received without checking if it is the @expensfiy.sms domain

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

Whether the text of the selected message ends with the SMS domain. If this happens, I replace the domain with an empty string.

Screen recording demonstrating expected operation.

Screencast-from-2024-04-02-13-14-55.mp4

Screenshot of solution code.

2024-04-02_13-15

Code
} else if (messageText && messageText.endsWith(CONST.SMS.DOMAIN)) { setClipboardMessage(messageText.replace(CONST.SMS.DOMAIN, '')); }

@melvin-bot melvin-bot bot added the Overdue label Mar 30, 2024
@alitoshmatov
Copy link
Contributor

@nkdengineer Thank you for your proposal, It seems like you overcomplicated your approach, can't we just get text inside mention-user tag and remove sms domain addition with Str.removeSmsDomain?

@melvin-bot melvin-bot bot removed the Overdue label Apr 1, 2024
@alitoshmatov
Copy link
Contributor

@GandalfGwaihir the issue occurs when @expensify.sms is within other text

Screen.Recording.2024-04-02.at.1.40.54.PM.mov

Copy link

melvin-bot bot commented Apr 2, 2024

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

@Ozeias01Nunes
Copy link

@alitoshmatov I updated my proposal based on comment.

@alitoshmatov
Copy link
Contributor

@nkdengineer 's proposal looks good which suggests to remove sms domain from texts inside user mention tags

C+ reviewed 🎀 👀 🎀

Copy link

melvin-bot bot commented Apr 2, 2024

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

@greg-schroeder
Copy link
Contributor

Bump @youssef-lr to confirm contributor selection: #38187 (comment)

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Apr 5, 2024
@greg-schroeder
Copy link
Contributor

Friendly bump @youssef-lr

@melvin-bot melvin-bot bot removed the Overdue label Apr 8, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 8, 2024
Copy link

melvin-bot bot commented Apr 8, 2024

📣 @alitoshmatov 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Apr 8, 2024

📣 @nkdengineer You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Apr 9, 2024
@greg-schroeder
Copy link
Contributor

Linked PR in review, just awaiting @youssef-lr's review next!

@youssef-lr
Copy link
Contributor

Merged!

@alitoshmatov
Copy link
Contributor

The pr went into production on 22nd of April, so payment is long overdue

@greg-schroeder
Copy link
Contributor

Hmm. I'm not sure why the automation keeps failing, but I'll process this now.

@greg-schroeder
Copy link
Contributor

greg-schroeder commented May 9, 2024

@nkdengineer you need to follow the steps here in order for me to pay you.

@alitoshmatov I paid you!

@nkdengineer
Copy link
Contributor

@greg-schroeder Thank you 🙇 I've applied to the job

@greg-schroeder
Copy link
Contributor

greg-schroeder commented May 9, 2024

Thanks! Will pay you now as soon as you accept the offer!

@nkdengineer
Copy link
Contributor

@greg-schroeder I accepted the offer!

@greg-schroeder
Copy link
Contributor

Okay, paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

7 participants