Skip to content

fix: 'No content to add' error if we send empty alias for email#555

Merged
Gonals merged 2 commits into
Expensify:mainfrom
eh2077:21234-fix-no-content-added-error-for-empty-alias-of-email-markdown
Jul 4, 2023
Merged

fix: 'No content to add' error if we send empty alias for email#555
Gonals merged 2 commits into
Expensify:mainfrom
eh2077:21234-fix-no-content-added-error-for-empty-alias-of-email-markdown

Conversation

@eh2077

@eh2077 eh2077 commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

Fixed Issues

$ Expensify/App#21234

Tests

  1. Go to a chat and add following comment
[](concierge@expensify.com)
[   ](concierge@expensify.com)
[   

 ](concierge@expensify.com)
  1. Verify that
    a. Only emails inside () are parsed as email link
    b. [], () and spaces(including line break) inside [] are not parsed
  2. Click to edit the comment
  3. Verify that the initial draft is same as the original input.

See below demo video

web.mov

I'll fill out the full checklist in the PR to update version of App repo.

QA

Same as test

@eh2077 eh2077 requested a review from a team as a code owner July 4, 2023 08:22
@melvin-bot melvin-bot Bot requested review from MariaHCD and removed request for a team July 4, 2023 08:23
@eh2077 eh2077 mentioned this pull request Jul 4, 2023
Closed
6 tasks
@MariaHCD MariaHCD requested a review from Gonals July 4, 2023 08:59
@MariaHCD

MariaHCD commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

Tagging @parasharrajat here! (can't assign you as a reviewer, apparently)

MariaHCD
MariaHCD approved these changes Jul 4, 2023
View reviewed changes

@MariaHCD MariaHCD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lgtm but leaving the final reviews to @parasharrajat and @Gonals!

@Gonals

Gonals commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

LGTM @parasharrajat, anything to add?

@parasharrajat parasharrajat mentioned this pull request Jul 4, 2023
Merged
57 tasks
parasharrajat
parasharrajat approved these changes Jul 4, 2023
View reviewed changes

@parasharrajat parasharrajat left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. cc: @Gonals @MariaHCD

🎀 👀 🎀 C+ reviewed

@parasharrajat

parasharrajat commented Jul 4, 2023

Copy link
Copy Markdown
Member

Ready for merge

@Gonals Gonals merged commit 06a5ff5 into Expensify:main Jul 4, 2023
@techievivek techievivek mentioned this pull request Jul 7, 2023
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MariaHCD MariaHCD MariaHCD approved these changes
@Gonals Gonals Gonals approved these changes
+1 more reviewer
@parasharrajat parasharrajat parasharrajat approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@eh2077 @MariaHCD @Gonals @parasharrajat