Fixing inconsistencies between emails and autoEmail

[abdulrahuman5196]

@cristipaval @Santhosh-Sellavel

Fixed Issues

$ Expensify/App#15621 (comment)

Tests

What unit/integration tests cover your change? What autoQA tests cover your change?
Added linked email cases to existing email test cases for below
Test markdown replacement for emails wrapped in bold/strikethrough/italic in multi-line
Test markdown replacement for emails wrapped in bold/strikethrough/italic in a single line
Test markdown replacement for emails containing bold/strikethrough/italic

What tests did you perform that validates your changed worked?

Web

Untitled.14.mp4

Mobile Web - Chrome

Untitled.15.mp4

Mobile Web - Safari

Untitled.16.mp4

Desktop

Untitled.17.mp4

iOS

Untitled.18.mp4

Android

Untitled.19.mp4

QA

1. What does QA need to do to validate your changes?
   Input the messages
   Input:

[test](~abc@gmail.com)

Result:
Only 'abc@gmail.com' should be recognised as emailId

Input:

[test](*abc@gmail.com)

Result:
Only 'abc@gmail.com' should be recognised as emailId

Input:

[test](abc@gmail.com)

Result:
The message should be recognised as 'test' with emailId as link

1. What areas to they need to test for regressions?
   Areas involving emailIds

[abdulrahuman5196]

Suppressing the prefer-default-import rule as this file is expected grow and have more functions similar to url.js

[danieldoglas]

@Santhosh-Sellavel please add a comment here so you can be added as a reviewer.

[Santhosh-Sellavel]

Adding myself as reviewer

[abdulrahuman5196]

This can be an additional code improvement but not mandatory requirement for this bug fix
FYI: It seems there is an another old out-dated email regex(blame shows 4 years) in the common repository here - https://github.com/Expensify/expensify-common/blob/main/lib/CONST.jsx#L3.
It doesn't seem to have any usage other than right click action of email message which should be fixed by this issue - Expensify/App#15620
Can we mark that regex as deprecated? And to only use our new common email regex for future.

[Santhosh-Sellavel]

@abdulrahuman5196 This could still be used internally on other apps (Private Repos).

[abdulrahuman5196]

@abdulrahuman5196 This could still be used internally on other apps (Private Repos).

Yes. Agree. That's why I am opposed to changing that, part of this bug. Maybe we can just add a comment that its deprecated, so that if in future anyone comes across this issue. They can start using the new regex we added. Better to be safe than sorry because we are not sure of those usages.
Or we can also choose to do nothing there and just focus on this bug alone. Just wanted to think loud.

[Santhosh-Sellavel]

Not required I guess, because the change we are doing is to be part of markdown i.e for messages formatting in New dot. This would not be used for Email Validation.

[abdulrahuman5196]

I agree. Thank you for confirmation @Santhosh-Sellavel.

Then only action item is "Do we move the new regex to CONST or follow markdown URLs pattern"?
Now, I feel its better to follow the markdown URLs pattern as in this PR instead of moving the same to CONST to avoid regex confusions. But let me know if we are aligned to move the regex to different place, I can make the required changes.

[Santhosh-Sellavel]

@cristipaval LGTM, just one comment. Can you run PR Automation checks, if all good let's merge and continue testing on PR Against E/App

[abdulrahuman5196]

Thank you @Santhosh-Sellavel. Updated the minor comment.

@cristipaval Could you kindly start the Automation Workflows? It seems I can't start it myself.

[jjcoffee]

@abdulrahuman5196 Could you confirm why you added a new regex here for markdown emails specifically, i.e. does it do much that EMAIL_BASE_REGEX does not? You mention here that EMAIL_BASE_REGEX is old, but it is used by Str.isValidEmail (which is used to validate emails when searching on starting a New Chat), which seems to be in sync with the backend. Would be nice if all these behaviours were consistent!