[HOLD for payment 2023-08-10] [$1000] Web - Select and copy does not copy New Expensify link in link format #23251

kbecciv · 2023-07-20T10:23:29Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Open the app
Open any report
Send normal link eg: [test](google.com)
Select the text and copy
Paste and see it has proper link format
Send newdot link eg: [test](staging.new.expensify.com/r/64705864) (note, the home link new.expensify.com does not reproduce this issue)
Select the text and copy, paste and observe that now text is only displayed without link

Expected Result:

App should copy all links equally on select and copy

Actual Result:

App does not copy expensify links as links and display their text normally

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.42-18
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

copy.does.not.copy.expensify.link.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689704143206879

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~011c078f18bc8d09c6
Upwork Job ID: 1683719949577637888
Automatic offers:

dhanashree-sawant | Reporter | 25815614

The text was updated successfully, but these errors were encountered:

melvin-bot · 2023-07-20T10:23:34Z

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

melvin-bot · 2023-07-20T10:23:36Z

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

samh-nl · 2023-07-20T11:05:44Z

Proposal

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

Copying an expensify link does not translate to markdown when pasted into the composer.

What is the root cause of that problem?

Expensify links are handled via onPress rather than href. As no href prop is supplied, it causes the link to not be recognized as a valid link for conversion to markdown when pasting.

App/src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.js

Lines 101 to 102 in 82fd2f9

    
           // Only pass the press handler for internal links. For public links or whitelisted internal links fallback to default link handling 
        
           onPress={internalNewExpensifyPath || internalExpensifyPath ? navigateToLink : undefined}

App/src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js

Lines 47 to 51 in e87f2eb

    
           if (_.isFunction(props.onPress)) { 
        
               linkProps.onPress = props.onPress; 
        
           } else { 
        
               linkProps.href = props.href; 
        
           }

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

We should provide href also for Expensify links, however we should maintain navigation via onPress by stopping the event default behavior inside the handler function.

What alternative solutions did you explore? (Optional)

N/A

kadiealexander · 2023-07-21T05:08:41Z

I'm not able to reproduce this:

2023-07-21_17-06-51.mp4

@dhanashree-sawant could you please take a look and see if we need to update the reproduction steps?

dhanashree-sawant · 2023-07-21T05:18:13Z

Hi @kadiealexander, can you try once with newdot expensify link? I am able to reproduce with that. On staging, with staging link and on live, with live link

melvin-bot · 2023-07-24T20:10:24Z

@kadiealexander Whoops! This issue is 2 days overdue. Let's get this updated quick!

kadiealexander · 2023-07-25T04:13:04Z

@dhanashree-sawant I tried both and was still unable to reproduce:

Tested with new.expensify.com link from new.expensify.com on Chrome:

2023-07-25_16-10-30.mp4

Tested with staging.new.expensify.com link from staging.new.expensify.com on Chrome:

2023-07-25_16-12-27.mp4

dhanashree-sawant · 2023-07-25T05:40:09Z

Thanks yes, sorry now I got it, I have tested with plain main page links and that seems to be working fine, if I write anything after the first '/', it causes the issue

eg: Try to copy any other report URL and send it as link with different alias test and that will trigger the error.

Sorry for the inconvenience till now.

2023-07-25.11-06-31.mp4

kadiealexander · 2023-07-25T06:03:08Z

All good, glad we got to the root of it! :)

2023-07-25_18-02-45.mp4

kadiealexander · 2023-07-25T06:04:42Z

Updated the reproduction steps with our findings.

melvin-bot · 2023-07-25T06:04:54Z

Job added to Upwork: https://www.upwork.com/jobs/~011c078f18bc8d09c6

melvin-bot · 2023-07-25T06:04:57Z

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

melvin-bot · 2023-07-25T06:05:03Z

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

Parshvi16 · 2023-07-25T09:26:05Z

Proposal

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

new dot expensify URL can't be copied as URL if it is having some nesting in route

What is the root cause of that problem?

in BaseAnchorForCommentsOnly.js file

in our case, we are not getting href in linkProps, we are getting only onPress in linkProps.

when no href is present, Text component from react-native is not treating this as a Link, it is treating the data as text only, therefore we are not able to copy the link.

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

App/src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js

Lines 78 to 85 in e87f2eb

    
           hrefAttrs={{ 
        
               rel: props.rel, 
        
               target: isEmail ? '_self' : props.target, 
        
           }} 
        
           href={linkProps.href} 
        
           // Add testID so it gets selected as an anchor tag by SelectionScraper 
        
           testID="a" 
        
           // eslint-disable-next-line react/jsx-props-no-spreading

here no href is being passed because linkProps.href is null/undefined in our case.

so we can pass href={linkProps.href || props.href} in line no. 82.
This solves the issue and we are able to copy it as a link.

but, it leads to a problem, this internal link starts opening in new window.

to fix this, we modify line no. 80 as:

    target: isEmail || !linkProps.href ? '_self' : props.target,

if following event.preventDefault, to avoid refreshing the page, we need the following code:

onPress={(event) => {
                        if (!linkProps.onPress) {
                          return;
                        }
                      
                        event.preventDefault();
                        linkProps.onPress();
                      }}

What alternative solutions did you explore? (Optional)

N/A

mananjadhav · 2023-07-26T18:59:56Z

Both the proposals have the correct RCA, but @Parshvi16's proposal is more complete as it mentions how to deal with links opened in new window.

🎀 👀 🎀 C+ reviewed.

mananjadhav · 2023-07-31T14:43:38Z

Apologies for the delay in the response here. I see some of the comments edited, hence wanted to verify before I respond.

@samh-nl to address your comments, your proposal originally lacked any technical details. I don't expect the code, but at-least where/how do we pass the hrefs, etc. should've been a part of the proposal. And we do work out changes in the PR, bugs that come across testing when the PR is raised.

About you mentioning the proposal being completely incorrect, if we come across this in the PR/issue, we would open up again for the proposals, where the new/existing proposals would be reviewed for the expected behavior. This generally doesn't happen, but I won't say it has never happened.

Lastly, @Parshvi16 I would recommend sticking to the original proposals while implementing the solution, so that we're fair in any process without any conflicts.

samh-nl · 2023-07-31T14:47:57Z

@mananjadhav Ok appreciate the feedback, I will take that into account going forward.

melvin-bot · 2023-08-01T19:00:52Z

🎯 ⚡️ Woah @mananjadhav / @Parshvi16, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

when @Parshvi16 got assigned: 2023-07-28 19:23:58 Z
when the PR got merged: 2023-08-01 19:00:50 UTC

On to the next one 🚀

Parshvi16 · 2023-08-02T11:21:43Z

@roryabraham I'm not yet hired on upwork for this issue. Please check.

mananjadhav · 2023-08-03T03:56:18Z

@roryabraham I'm not yet hired on upwork for this issue. Please check.

@kadiealexander Can you please help here?

kadiealexander · 2023-08-03T04:19:54Z

@Parshvi16 please apply to the Upwork job, as I can't find your application. Thanks!

Parshvi16 · 2023-08-03T07:55:07Z

Sorry, I just checked, my account is on hold due to surname spelling issue. can I use my friend's Upwork account? or should I wait?
I'm waiting to be paid for 2 issue:
https://www.upwork.com/jobs/~011c078f18bc8d09c6
https://www.upwork.com/jobs/~01a5685c1928bbd220 (hired but not payed yet)
@kadiealexander please guide here

melvin-bot · 2023-08-03T17:07:23Z

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

melvin-bot · 2023-08-03T17:07:27Z

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

fix: Select and copy does not copy New Expensify link in link format #23704

If no regressions arise, payment will be issued on 2023-08-10. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

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

@mananjadhav does not require payment (Eligable for Manual Requests)
@Parshvi16 requires payment

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 · 2023-08-03T17:07:29Z

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:

[@mananjadhav] The PR that introduced the bug has been identified. Link to the PR:
[@mananjadhav] 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:
[@mananjadhav] 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:
[@mananjadhav] Determine if we should create a regression test for this bug.
[@mananjadhav] 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.
[@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Parshvi16 · 2023-08-04T00:41:25Z

@kadiealexander please check #23251 (comment)
I'd like to update my upwork account link.
also, for https://www.upwork.com/jobs/~01a5685c1928bbd220, please end the contract and re-hire me through this new upwork account. should i provide new account link here?
also, please update it in automation(melvin).

kadiealexander · 2023-08-04T00:44:21Z

@Parshvi16 Please post on the associated Github issues for those jobs and the relevant team member will assist you.

For this job, I need you to apply here: https://www.upwork.com/jobs/~011c078f18bc8d09c6

Parshvi16 · 2023-08-04T00:55:33Z

@kadiealexander submitted the proposal, please accept.

kadiealexander · 2023-08-04T00:56:39Z

Could you please let me know the name of the person you used to apply?

Parshvi16 · 2023-08-04T00:57:50Z

https://www.upwork.com/freelancers/~01c5aafb1ea741db18
Sunita R

kadiealexander · 2023-08-04T01:02:14Z

Offer sent :)

kadiealexander · 2023-08-04T01:05:16Z

Payouts due on August 10th:

Issue Reporter: $250 @dhanashree-sawant
Contributor: $1000 + $500 bonus @Parshvi16
Contributor+: $1000 + $500 bonus @mananjadhav

Eligible for 50% #urgency bonus? Yes

Upwork job is here.

mananjadhav · 2023-08-08T16:55:36Z

I can't pinpoint to a specific PR, as we've got the links working since a long time. Hence, there is no offending PR.

I also don't think we need a regression test for this one.

mananjadhav · 2023-08-10T03:44:45Z

@kadiealexander I've raised my request on NewDot.

JmillsExpensify · 2023-08-10T03:48:38Z

Reviewed the details for @mananjadhav. This is approved for payment in NewDot.

kbecciv added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Jul 20, 2023

melvin-bot bot assigned kadiealexander Jul 20, 2023

melvin-bot bot added the Overdue label Jul 24, 2023

melvin-bot bot removed the Overdue label Jul 25, 2023

kadiealexander added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Jul 25, 2023

melvin-bot bot changed the title ~~Web - Select and copy does not copy expensify link in link format~~ [$1000] Web - Select and copy does not copy expensify link in link format Jul 25, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 25, 2023

melvin-bot bot assigned mananjadhav Jul 25, 2023

kadiealexander changed the title ~~[$1000] Web - Select and copy does not copy expensify link in link format~~ [$1000] Web - Select and copy does not copy New Expensify link in link format Jul 25, 2023

This comment was marked as resolved.

Sign in to view

jayeshmangwani mentioned this issue Aug 1, 2023

[HOLD for payment 2023-08-10] [$250] Migrate ScrollViewWithContext.js to function component #16200

Closed

1 task

mananjadhav mentioned this issue Aug 3, 2023

Payment issue for E/App PR#23704 #23926

Closed

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Aug 3, 2023

melvin-bot bot changed the title ~~[$1000] Web - Select and copy does not copy New Expensify link in link format~~ [HOLD for payment 2023-08-10] [$1000] Web - Select and copy does not copy New Expensify link in link format Aug 3, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Aug 3, 2023

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 9, 2023

kadiealexander closed this as completed Aug 10, 2023

[HOLD for payment 2023-08-10] [$1000] Web - Select and copy does not copy New Expensify link in link format #23251

[HOLD for payment 2023-08-10] [$1000] Web - Select and copy does not copy New Expensify link in link format #23251

Comments

kbecciv commented Jul 20, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jul 20, 2023

melvin-bot bot commented Jul 20, 2023

Bug0 Triage Checklist (Main S/O)

samh-nl commented Jul 20, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

kadiealexander commented Jul 21, 2023

dhanashree-sawant commented Jul 21, 2023

melvin-bot bot commented Jul 24, 2023

kadiealexander commented Jul 25, 2023

dhanashree-sawant commented Jul 25, 2023

kadiealexander commented Jul 25, 2023 • edited Loading

kadiealexander commented Jul 25, 2023

melvin-bot bot commented Jul 25, 2023

melvin-bot bot commented Jul 25, 2023

melvin-bot bot commented Jul 25, 2023

Parshvi16 commented Jul 25, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

mananjadhav commented Jul 26, 2023 • edited Loading

mananjadhav commented Jul 31, 2023

samh-nl commented Jul 31, 2023

melvin-bot bot commented Aug 1, 2023

Parshvi16 commented Aug 2, 2023

mananjadhav commented Aug 3, 2023

kadiealexander commented Aug 3, 2023

Parshvi16 commented Aug 3, 2023

melvin-bot bot commented Aug 3, 2023

melvin-bot bot commented Aug 3, 2023 • edited by kadiealexander Loading

melvin-bot bot commented Aug 3, 2023

Parshvi16 commented Aug 4, 2023

kadiealexander commented Aug 4, 2023

Parshvi16 commented Aug 4, 2023

kadiealexander commented Aug 4, 2023

Parshvi16 commented Aug 4, 2023

kadiealexander commented Aug 4, 2023

kadiealexander commented Aug 4, 2023 • edited Loading

mananjadhav commented Aug 8, 2023 • edited Loading

mananjadhav commented Aug 10, 2023

JmillsExpensify commented Aug 10, 2023

kbecciv commented Jul 20, 2023 •

edited by melvin-bot bot

Loading

samh-nl commented Jul 20, 2023 •

edited

Loading

kadiealexander commented Jul 25, 2023 •

edited

Loading

Parshvi16 commented Jul 25, 2023 •

edited

Loading

mananjadhav commented Jul 26, 2023 •

edited

Loading

melvin-bot bot commented Aug 3, 2023 •

edited by kadiealexander

Loading

kadiealexander commented Aug 4, 2023 •

edited

Loading

mananjadhav commented Aug 8, 2023 •

edited

Loading