App: [HOLD for Payment 2023-09-20][$2000] Can't parse deep link with email param.

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:

  1. Try to open user detail from deep link on browser (for example: https://staging.new.expensify.com/details?login=testing@gmail.com).
  2. Now go to any report, paste that link and click send.
  3. Notice that it can’t parse the whole link, and when user try to click on it, it opens the email app with the wrong email filled.

Expected Result:

App should parse the deep link detail with email param.

Actual Result:

App can’t parse the deep link detail with email param. The URL prefix (https://) is not part of the link, and we instead link incorrectly.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.2.92-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: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/43996225/228922658-ef1627e6-9f3e-4f91-a52a-8cb26a5ea47f.mov

Untitled

Expensify/Expensify Issue URL: Issue reported by: @hungvu193 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1680191331185739

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~013659e025d6ce0e4f
  • Upwork Job ID: 1644482548992663552
  • Last Price Increase: 2023-06-26
  • Automatic offers:
    • 0xmiroslav | Reviewer | 26524537
    • Antasel | Contributor | 26524539
    • hungvu193 | Reporter | 26524541

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Comments: 207 (146 by maintainers)

Most upvoted comments

Ok. I’ve tested on staging web. working as expected.

Screencast from 12-9-23 05:25:49+06.webm

The PR isn’t showing that it was deployed to staging or prod, @0xmiroslav is that specific to Expensfy/expensify-common? I want to make sure the payment date is reflected correctly.

Correct, in general there should be app PR linked to this issue which just bumps expensify-common version. But this is edge case. Version was bumped in other PR which fixes another issue.

In our case, we can update issue title manually. Deployed to production on Sep 13 - https://github.com/Expensify/App/pull/27169#issuecomment-1717199809

@0xmiroslav The following test cases are covering all of what you mentioned

Test Cases
test('Test a url with email autolinks correctly', () => {
    let testString = 'https://staging.new.expensify.com/details?login=testing@gmail.com';
    let resultString = '<a href="https://staging.new.expensify.com/details?login=testing@gmail.com" target="_blank" rel="noreferrer noopener">https://staging.new.expensify.com/details?login=testing@gmail.com</a>';
    testString = 'test@expensify.com/https://www.expensify.com';
    resultString = '<a href="mailto:test@expensify.com">test@expensify.com</a>/<a href="https://www.expensify.com" target="_blank" rel="noreferrer noopener">https://www.expensify.com</a>';
    expect(parser.replace(testString)).toBe(resultString);
});
test('Test a url autolinks correctly', () => {
    let testString = 'test@expensify.com https://www.expensify.com\n' +
    'test@expensify.com-https://www.expensify.com\n' +
    'test@expensify.com/https://www.expensify.com\n' +
    'test@expensify.com?https://www.expensify.com\n' +
    'test@expensify.com>https://www.expensify.com\n' +
    'https://staging.new.expensify.com/details/test@expensify.com\n' +
    'staging.new.expensify.com/details\n\n' +
    'https://www.expensify.com?name=test&email=test@expensify.com\n' +
    'https://staging.new.expensify.com/details?login=testing@gmail.com\n' +
    'staging.new.expensify.com/details?login=testing@gmail.com\n' +
    'http://necolas.github.io/react-native-web/docs/?path=/docs/components-pressable--disabled\n' +
    '-https://www.expensify.com /https://www.expensify.com @https://www.expensify.com\n' +
    'expensify.com -expensify.com @expensify.com\n' +
    'https//www.expensify.com\n' +
    '//www.expensify.com?name=test&email=test@expensify.com\n' +
    '//staging.new.expensify.com/details?login=testing@gmail.com\n' +
    '/details?login=testing@gmail.com\n' +
    '?name=test&email=test@expensify.com\n\n' +
    'example.com/https://www.expensify.com\n' +
    'test@gmail.com staging.new.expensify.com/details?login=testing@gmail.com&redirectUrl=https://google.com\n' +
    'test@gmail.com //staging.new.expensify.com/details?login=testing@gmail.com&redirectUrl=https://google.com\n' +
    'test@gmail.com-https://staging.new.expensify.com/details?login=testing@gmail.com&redirectUrl=https://google.com';
    let resultString = '<a href="mailto:test@expensify.com">test@expensify.com</a> <a href="https://www.expensify.com" target="_blank" rel="noreferrer noopener">https://www.expensify.com</a><br />' + 
    '<a href="mailto:test@expensify.com">test@expensify.com</a>-<a href="https://www.expensify.com" target="_blank" rel="noreferrer noopener">https://www.expensify.com</a><br />' +
    '<a href="mailto:test@expensify.com">test@expensify.com</a>/<a href="https://www.expensify.com" target="_blank" rel="noreferrer noopener">https://www.expensify.com</a><br />' +
    '<a href="mailto:test@expensify.com">test@expensify.com</a>?<a href="https://www.expensify.com" target="_blank" rel="noreferrer noopener">https://www.expensify.com</a><br />' + 
    '<a href="mailto:test@expensify.com">test@expensify.com</a>&gt;<a href="https://www.expensify.com" target="_blank" rel="noreferrer noopener">https://www.expensify.com</a><br />' +
    '<a href="https://staging.new.expensify.com/details/test@expensify.com" target="_blank" rel="noreferrer noopener">https://staging.new.expensify.com/details/test@expensify.com</a><br />' +
    '<a href="https://staging.new.expensify.com/details" target="_blank" rel="noreferrer noopener">staging.new.expensify.com/details</a><br /><br />'+ 
    '<a href="https://www.expensify.com?name=test&amp;email=test@expensify.com" target="_blank" rel="noreferrer noopener">https://www.expensify.com?name=test&amp;email=test@expensify.com</a><br />' +
    '<a href="https://staging.new.expensify.com/details?login=testing@gmail.com" target="_blank" rel="noreferrer noopener">https://staging.new.expensify.com/details?login=testing@gmail.com</a><br />' +
    '<a href="https://staging.new.expensify.com/details?login=testing@gmail.com" target="_blank" rel="noreferrer noopener">staging.new.expensify.com/details?login=testing@gmail.com</a><br />' +
    '<a href="http://necolas.github.io/react-native-web/docs/?path=/docs/components-pressable--disabled" target="_blank" rel="noreferrer noopener">http://necolas.github.io/react-native-web/docs/?path=/docs/components-pressable--disabled</a><br />' +
    '-<a href="https://www.expensify.com" target="_blank" rel="noreferrer noopener">https://www.expensify.com</a> /<a href="https://www.expensify.com" target="_blank" rel="noreferrer noopener">https://www.expensify.com</a> @https://www.expensify.com<br />' +
    '<a href="https://expensify.com" target="_blank" rel="noreferrer noopener">expensify.com</a> -<a href="https://expensify.com" target="_blank" rel="noreferrer noopener">expensify.com</a> @expensify.com<br />' +
    'https//<a href="https://www.expensify.com" target="_blank" rel="noreferrer noopener">www.expensify.com</a><br />' +
    '//<a href="https://www.expensify.com?name=test&amp;email=test@expensify.com" target="_blank" rel="noreferrer noopener">www.expensify.com?name=test&amp;email=test@expensify.com</a><br />' +
    '//<a href="https://staging.new.expensify.com/details?login=testing@gmail.com" target="_blank" rel="noreferrer noopener">staging.new.expensify.com/details?login=testing@gmail.com</a><br />' +
    '/details?login=<a href="mailto:testing@gmail.com">testing@gmail.com</a><br />' +
    '?name=test&amp;email=<a href="mailto:test@expensify.com">test@expensify.com</a><br /><br />' +
    '<a href="https://example.com/https://www.expensify.com" target="_blank" rel="noreferrer noopener">example.com/https://www.expensify.com</a><br />' +
    '<a href="mailto:test@gmail.com">test@gmail.com</a> <a href="https://staging.new.expensify.com/details?login=testing@gmail.com&amp;redirectUrl=https://google.com" target="_blank" rel="noreferrer noopener">staging.new.expensify.com/details?login=testing@gmail.com&amp;redirectUrl=https://google.com</a><br />' +
    '<a href="mailto:test@gmail.com">test@gmail.com</a> //<a href="https://staging.new.expensify.com/details?login=testing@gmail.com&amp;redirectUrl=https://google.com" target="_blank" rel="noreferrer noopener">staging.new.expensify.com/details?login=testing@gmail.com&amp;redirectUrl=https://google.com</a><br />' +
    '<a href="mailto:test@gmail.com">test@gmail.com</a>-<a href="https://staging.new.expensify.com/details?login=testing@gmail.com&amp;redirectUrl=https://google.com" target="_blank" rel="noreferrer noopener">https://staging.new.expensify.com/details?login=testing@gmail.com&amp;redirectUrl=https://google.com</a>';
    expect(parser.replace(testString)).toBe(resultString);

    testString = 'test@gmail.com/https://example.com/google@email.com?email=asd@email.com';
    resultString = '<a href="mailto:test@gmail.com\">test@gmail.com</a>/<a href="https://example.com/google@email.com?email=asd@email.com" target="_blank" rel="noreferrer noopener">https://example.com/google@email.com?email=asd@email.com</a>';
    expect(parser.replace(testString)).toBe(resultString);

    testString = 'test@gmail.com/test@gmail.com/https://example.com/google@email.com?email=asd@email.com';
    resultString = '<a href="mailto:test@gmail.com">test@gmail.com</a>/<a href="mailto:test@gmail.com">test@gmail.com</a>/<a href="https://example.com/google@email.com?email=asd@email.com" target="_blank" rel="noreferrer noopener">https://example.com/google@email.com?email=asd@email.com</a>';
    expect(parser.replace(testString)).toBe(resultString);
});

📣 @ronibd0! 📣 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. 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.
  2. 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.
  3. 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>

Leaving there what I wrote in Slack: I was working on that issue not to submit a proposal but instead test all existing proposals and make a summary of them, in order to help select the best one. Since it was put on hold, that work was stopped, since all proposals were about to become “obsolete”. Antasel’s proposal looks good, I think it’s a good one to move forward with.

Thanks @BeeMargarida. I think we should wait until the hold issue is complete. If the proposal by @Antasel works well after the hold issue is deployed, then we can consider it.

I’ve not submitted a proposal, I just reviewed all existing proposals to find the best one and unblock the issue. At the time, it was not clear what the expected behaviour was, and some proposals were more complete than others. This issue is dependent on https://github.com/Expensify/App/issues/17387, which has not yet been merged, so I don’t think it’s still worth it to move on with this one, since the email regex will be changed.

I’m not sure what the process is when an issue is on hold, if proposals made while it’s on hold are to be considered, but I’ll leave that judgement to @0xmiroslav and @sakluger. Since this proposal already takes into account the changes made for https://github.com/Expensify/App/issues/17387, it might be worth considering.

@0xmiroslav Are you planning on leaving this on hold until https://github.com/Expensify/App/issues/17387 is resolved? It’s related and might impact this if the regex is changed.

yes, I was about to suggest that as email regex changes directly affect this issue. cc: @johnmlee101 @sakluger

@BeeMargarida thanks for the detailed review and testing. I think RFC part can be out of scope here. Email regex issues will be handled separately in #17387 so I think in this GH, we can skip extreme edge cases where we can’t 100% confirm that it’s valid email or not.

//staging.new.expensify.com/details?login=testing@gmail.com I tested this in various platforms. Slack, Telegram treat this as valid url. Skype, Discord treat this as invalid url. It seems only the links that include “https://” are valid urls. But none of platforms treat this full text as email. WhatsApp treats login=testing@gmail.com as email.

Update: Still investigating and testing all proposals, will post a summary soon

Hey 👋 I’m Ana from Callstack, an expert agency, and I would like to help close this issue out. I see that there are a lot of proposals and an additional test suite. I’ll go through the proposals and test with the existing and @0xmiroslav test suites and try make a summary of the pros/cons of each proposal. Does it sound good?

Sounds good. Thanks

@0xmiroslav I think you missed whitespace in test cases Bad urls > urlTestReplacedString

+ '/details?login=<a href="mailto:testing@gmail.com">testing@gmail.com</a>'
+ '/details?login=<a href="mailto:testing@gmail.com">testing@gmail.com</a> ' //<<<<< here

@Nodebrute yes open. Help Wanted label still added as you see

Oh, sorry. Wrong link. Here it is. https://github.com/Expensify/App/issues/16762#issuecomment-1598476985 And if you want, I can submit a new proposal with the template for my updated solution.

Apologize for extreme delays on this issue. was focusing on more priority issues meanwhile. Update: added most automated cases myself locally and still fixing expected strings. I will give final update on Monday.

@0xmiroslav it’s been more than 10 days here without hearing from you. What’s going on? Also @sakluger as you’re back I’m returning this to you.

@tienifr He’s adding more test cases in his end and validate all our proposals. So just wait @0xmiroslav

@akinwale your latest proposal is still failing my test cases.

I am currently adding more edge cases in automated tests myself locally to validate all proposals in an automatic way.

Still evaluating proposals. I will provide update tomorrow.

Sorry for delay here, been busy on urgent PRs for EC3. I will provide feedback asap on latest proposals after last review

@sakluger as this is really not an easy issue, we can bump the price so that contributors investigate more time to avoid any regressions. or @johnmlee101 will work on this internally?

@ahmedGaber93 did you try to send message with your solution?

Screenshot 2023-04-26 at 12 09 35 PM

Please thoroughly test all possible cases before posting updated proposal.

@akinwale I still feel it’s workaround, case-by-case fix. Your updated solution will fail this case:

[test ]](test@expensify.com)

Email should be parsed.

I forgot to add word boundaries in my update, which I actually had in my implementation. I have updated my proposal which should handle this case properly. The regex should now cover all scenarios properly.

Is there a list of standard test cases that we can test against?

Proposal

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

App can’t parse the deep link detail with email param. The URL prefix (https://) is not part of the link, and we instead link incorrectly.

What is the root cause of that problem?

1 - When the user tries to send a deeplink with email as param, the parser rule that we have defined in the library rule autoEmail should automatically links the email. However our current rule can only work with HTML tags not the raw URL string.

2 - The placement of autoemail rule is also contributing to the problem, as it’s been placed before autolink.

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

Step 1 - Update the regex rule to prevent match emails if string contains http or https

Step 2 - Reorder autoEmail rule to check if the matching string is a URL, we will use URL_REGEX and don’t need to format to an email link

What alternative solutions did you explore? (Optional)

none