[HOLD for payment 2023-05-29] [$1000] Editing link removes the link

[kavimuru]

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. Open the app
2. Open any report
3. Send any link eg: Google.com
4. Edit the link part, keep the text same just make any letter capital or small eg: [Google.com](google.com)

Expected Result:

App should keep the edited message as link

Actual Result:

App removes the edited message as link

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.3.11-4
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

app.removes.edited.message.as.link.on.new.staging.mp4

Recording.503.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0145804dc44340d929
• Upwork Job ID: 1656788307683254272
• Last Price Increase: 2023-05-11

[melvin-bot]

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

[melvin-bot]

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

[arielgreen]

Cannot reproduce today.

[dhanashree-sawant]

Hi @arielgreen, there are 2 mistakes in your reproduction steps:

1. Keep link and alias same i.e. google.com
2. Edit link part not the alias part i.e. google.com

This video should be helpful for better understanding

2023-05-09.07-49-12.mp4

[strepanier03]

I was doing some work to create a non-reproducible bug triaging process and came across this GH while searching by the Needs Reproduction label.

@arielgreen I'm going to reopen this because the issue is reproducible following the steps. You just have to be sure you change the link part, not the name part.

2023-05-10_10-38-45.mp4

[eh2077]

I can also reproduce this issue though the original reproduction steps are a little bit unclear. I'd like to post a proposal for it.

cc @arielgreen @strepanier03

[eh2077]

Proposal

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

Edit website to capital letters removes the link.

What is the root cause of that problem?

An autolink comment, like Google.com, is parsed into anchor tag

<a href="https://Google.com" target="_blank" rel="noreferrer noopener">Google.com</a>

Click to edit the link and it's parsed to markdown

[Google.com](https://Google.com)

If edit the above markdown(changing the capital letter 'G' to lower case) into

[Google.com](https://google.com)

Then we'll indentify the link is removed in method getRemovedMarkdownLinks because https://Google.com is different from https://google.com in case sensitive comparing.

Note that domain names are case insensitive according to RFC 4343 , see this SO discussion. So the root cause of the issue is that we don't aware the link is not changed if only changing uppper or lower case letters of domain.

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

To fix this issue, I think we can

1. Format the domain of link to lower case just like what we do for the schema https, see here and here.
2. Skip removing link in method getRemovedMarkdownLinks if only changing uppper or lower case letters of domain.

To achieve this, we can use the website regex to capture the website(including protocol and domain) which is case insensitive.

So we can

1. Format the website to lower case when translating markdown to html in link-to-html-rule and autolink-to-html-rule. For example, we can add a group to capture website in URL_REGEX by changing this line

   const URL_REGEX = `(${URL_WEBSITE_REGEX}${URL_PATH_REGEX}(?:${URL_PARAM_REGEX}|${URL_FRAGMENT_REGEX})*)`;

   to

   const URL_REGEX = `((${URL_WEBSITE_REGEX})${URL_PATH_REGEX}(?:${URL_PARAM_REGEX}|${URL_FRAGMENT_REGEX})*)`;

   See below picture, group 3 captures the website which needs to be converted to lower case.

   

2. Use the website regex to capture and replace website of edited link to lower case and then compare with the origial link in method getRemovedMarkdownLinks.

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0145804dc44340d929

[melvin-bot]

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

[arielgreen]

Great, thanks for the clarification, all.

[melvin-bot]

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

[melvin-bot]

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

[fedirjh]

Thanks for the proposal @eh2077 , your analysis are correct and your solution looks perfect.

cc @MariaHCD, This proposal looks good to me.

🎀 👀 🎀 C+ reviewed

[MariaHCD]

Looks good to me too

[arielgreen]

@fedirjh @dhanashree-sawant @eh2077 offers sent in Upwork

[eh2077]

@arielgreen Accepted. Thank you!

[dhanashree-sawant]

Hi @arielgreen, thanks, I have also accepted the offer.

[arielgreen]

Thanks folks! @fedirjh please also be sure to fill out the checklist above.

[arielgreen]

Reassigning, OOO tomorrow/Monday.

[melvin-bot]

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

[trjExpensify]

Cool, happy to help! @fedirjh, let us know about that checklist. :)

[fedirjh]

• Link to the PR: Autolink when edititng comments except wehn explicit link removal #13551
• Link to comment: Autolink when edititng comments except wehn explicit link removal #13551 (comment)
• Link to discussion: https://expensify.slack.com/archives/C049HHMV9SM/p1685317510288449
• Determine if we should create a regression test for this bug ✅

---

1. Open the App.
2. Navigate to any chat.
3. Send a message containing a link: [google.com](http://google.com/).
4. Long-press the message and click on "Edit message."
5. Edit the domain part of the link, while keeping the text the same but changing the capitalization of the domain letters. For example, change [google.com](http://google.com/) to [google.com](http://Google.COM/).
6. Verify that the message is still displayed as a link, and the domain part maintains its original capitalization.

Do we agree 👍 or 👎

[MariaHCD]

Those regression steps look good to me!

@eh2077 @trjExpensify what do you think?

[trjExpensify]

@fedirjh - you have access to TestRail as a C+, right?

Wouldn't we just modify this existing test case instead of adding a new one entirely?

So from #7 onwards..

7. Send a message that includes a hyperlink i.e [google.com](http://google.com/)
8. Edit the message with the hyperlink (do not modify the link) and save the changes
9. Verify the link is still hyperlinked in the modified message
10. Now edit the domain part of the link, while keeping the text the same but changing the capitalization of the domain letters i.e [google.com](http://Google.COM/)
11. Verify the link is still hyperlinked in the modified message and the domain part maintains its original capitalization
12. As the other user in the conversation, Verify the edited message is displaying the new text and has a "(edited)" status in the conversation history

[trjExpensify]

We're ready for payment here, confirming these from the offers sent:

$1,000 - @fedirjh for C+ review
$1,000 - @eh2077 for the fix
$250 - @dhanashree-sawant for the bug report

[eh2077]

@trjExpensify Thanks for checking this! I think it's eligible for speed bonus. See below timeline FYI
Assigned: datetime="2023-05-15T14:01:23Z"
Merged: datetime="2023-05-18T06:54:24Z"

[trjExpensify]

Awesome, amended offers and settled up with each of you!

[trjExpensify]

@fedirjh @MariaHCD - let me know what you think about this and I'll create an issue for Applause.

[fedirjh]

you have access to TestRail as a C+, right?

@trjExpensify Actually NO , I don’t have access to TestRail. Should I request access on the C+ channel ?

[trjExpensify]

I can check on that, maybe you don't get access anymore.

[trjExpensify]

Ah okay, so confirmed we didn't provide C+ access. Created an issue for Applause to look at. We're done here, closing!

[MariaHCD]

Thanks @trjExpensify! Editing the existing test case as you outlined here makes sense to me!