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 2023-01-23] [$4000] mWeb - Copy to email/URL is not working correctly - reported by @mateusbra #8311

Closed
kavimuru opened this issue Mar 24, 2022 · 191 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 External Added to denote the issue can be worked on by a contributor

Comments

@kavimuru
Copy link

kavimuru commented Mar 24, 2022

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


Issue was found when executing #8195

Action Performed:

  1. Log in on New Expensify.
  2. Open a report.
  3. Type an URL and an e-mail on chat.

Expected Result:

The message "Copy e-mail to clipboard" displayed when you try to copy an email and "copy URL to clipboard" tapping URL

Actual Result:

The message "Copy to clipboard" displayed when you try to copy an email. Sometimes it shows as regular context menu

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Mobile Web

Version Number: 1.1.46-0

Reproducible in staging?: y

Reproducible in production?: y (New feature)

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

Bug5505102_Record_2022-03-25-00-35-55__1_.mp4
screen-20220324-194533.mp4

Expensify/Expensify Issue URL:

Issue reported by: Reported by @mateusbra https://expensify.slack.com/archives/C01GTK53T8Q/p1647474115298459

Slack conversation:

View all open jobs on GitHub
Issue was found when executing #8195

Upwork Automation - Do Not Edit

@melvin-bot
Copy link

melvin-bot bot commented Mar 24, 2022

Triggered auto assignment to @johnmlee101 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@mateusbra
Copy link
Contributor

@kavimuru I found this issue while working on #8195 and reported on slack here: https://app.slack.com/client/T03SC9DTT/C01GTK53T8Q does that count for reporting bounty?

@kavimuru
Copy link
Author

@mateusbra I don't have access to the slack. But as per this comment, #8195 (comment) yes. Also tester can repro this bug too #8006 (comment)

@johnmlee101
Copy link
Contributor

Opening this up!

@johnmlee101 johnmlee101 added the External Added to denote the issue can be worked on by a contributor label Mar 25, 2022
@johnmlee101 johnmlee101 removed their assignment Mar 25, 2022
@melvin-bot
Copy link

melvin-bot bot commented Mar 25, 2022

Triggered auto assignment to @Christinadobrzyn (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@johnmlee101
Copy link
Contributor

This should stay daily priority

@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Mar 28, 2022

@mateusbra do you have interest in working on this one since you discovered it? Feel free to leave a proposal if so.

@kavimuru can you please make sure we at the "@ reported by" to a GH title if they are due the bug reporting bounty? Seems applicable in this case for @mateusbra but maybe I'm not understanding fully?

@MelvinBot
Copy link

@Christinadobrzyn Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Mar 29, 2022

Sorry for the delay, i was ooo. Posted job to Upwork;

Internal job - https://www.upwork.com/ab/applicants/1508625903649284096/job-details
External job - https://www.upwork.com/jobs/~01bd32eda238a19082

It does look like @mateusbra found/reported this issue so added them to the OP as the reporter.

@melvin-bot
Copy link

melvin-bot bot commented Mar 29, 2022

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

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

melvin-bot bot commented Mar 29, 2022

Triggered auto assignment to @marcochavezf (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@zamarlancer
Copy link

Hi,
This is Zamar from Upwork.com
I will fix the issue by implementing URL parsing feature in ContextMenuAction.
Now there is no checking parts for url/email when the context menu is shown.
So there will be a validation for the clipboard text, which will check if it is an email address or an url; and switch the caption of the menu item according to the result.
Thank you.

@parasharrajat
Copy link
Member

parasharrajat commented Mar 30, 2022

I don't see how this is different from #8195. This should have been fixed on #8311. cc: @Santhosh-Sellavel @mateusbra

why do we need a different issue?

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Jan 16, 2023
@melvin-bot melvin-bot bot changed the title [$4000] mWeb - Copy to email/URL is not working correctly - reported by @mateusbra [HOLD for payment 2023-01-23] [$4000] mWeb - Copy to email/URL is not working correctly - reported by @mateusbra Jan 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 16, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.54-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 2023-01-23. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot
Copy link

melvin-bot bot commented Jan 16, 2023

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:

  • [@marcochavezf] The PR that introduced the bug has been identified. Link to the PR: According to this message, this issue was introduced here.
  • [@marcochavezf] 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: The previous message contains the details about the regression.
  • [@marcochavezf] 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: This issue already had a long discussion and the consensus about what solution to take wasn't clear several times, so I think the best way to prevent this issue in the future is to add regression tests to TR.
  • [@arielgreen] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

@mallenexpensify mallenexpensify added Daily KSv2 and removed Reviewing Has a PR in review Weekly KSv2 labels Jan 20, 2023
@arielgreen
Copy link
Contributor

@marcochavezf @parasharrajat @b1tjoy please see and complete checklist

@melvin-bot melvin-bot bot removed the Overdue label Jan 20, 2023
@parasharrajat
Copy link
Member

Note: I haven't reviewed the PR for this issue due to unavailability.

@arielgreen
Copy link
Contributor

Touching base with @marcochavezf here.

@marcochavezf
Copy link
Contributor

Completed the BugZero checklist 👍🏽

@melvin-bot melvin-bot bot added the Overdue label Jan 30, 2023
@arielgreen
Copy link
Contributor

still discussing

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jan 31, 2023
@arielgreen
Copy link
Contributor

Payments issued.

No regression test needed, it's already covered here.

@melvin-bot melvin-bot bot removed the Overdue label Feb 6, 2023
@parasharrajat
Copy link
Member

parasharrajat commented Feb 6, 2023

Am I eligible for partial C+ payment(maybe 50% or 25%) on this issue?

Why?

  1. Complexity for this issue was higher.
  2. Proposals were reviewed.
  3. It took extensive analysis and discussions to come up with the final proposal.

Why not?

  1. PR was not reviewed by C+ [HOLD for payment 2023-01-23] [$4000] mWeb - Copy to email/URL is not working correctly - reported by @mateusbra #8311 (comment). I was on a family vacation and couldn't spare time for it.

cc: @marcochavezf @arielgreen

@parasharrajat
Copy link
Member

Sorry for bumping this very old issue. Any comments on #8311 (comment)

cc: @marcochavezf @arielgreen

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 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests