Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.29k stars 2.73k forks source link

[HOLD for payment 2024-08-07] [$250] Group - Invalid group chat name entered after group creation shows error. #45586

Closed izarutskaya closed 4 weeks ago

izarutskaya commented 1 month ago

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


Version Number: 9.0.8 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4730508 Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Go to https://staging.new.expensify.com/home
  2. Tap fab - start chat
  3. Select " Add to group" next to a user
  4. Tap next
  5. Tap group name
  6. Remove the text and paste invalid group name - adoadoadoajdoajdoadoajdoajadoadoadoajdoajdoadoajdoajadoadoadoajdoajdoadoajdoajadoadoadoajdoajdoadoaỏ
  7. Tap start group
  8. Tap header
  9. Tap group name
  10. Remove the group name and replace with any text and save it
  11. Again tap group name
  12. Paste - invalid group name - adoadoadoajdoajdoadoajdoajadoadoadoajdoajdoadoajdoajadoadoadoajdoajdoadoajdoajadoadoadoajdoajdoadoaỏ
  13. Tap save

Expected Result:

Invalid group chat name entered while creating group or after group creation must behave in the same way.

Actual Result:

Invalid group chat name entered while creating group not showing error but entered after group creation shows error.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/b1129b91-b0a2-49fe-a8e4-8c1817d30b3a

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0114cd4bde59962aa8
  • Upwork Job ID: 1815530149727742960
  • Last Price Increase: 2024-07-22
  • Automatic offers:
    • dominictb | Contributor | 103256417
Issue OwnerCurrent Issue Owner: @alexpensify
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

izarutskaya commented 1 month ago

We think this issue might be related to the #vip-vsb.

Krishna2323 commented 1 month ago

Proposal

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

Group - Invalid group chat name entered after group creation shows error.

What is the root cause of that problem?

No validation for invalid group name is added on the frontend and the backend only returns error when group name is updated. https://github.com/Expensify/App/blob/e3b508b89933b44f5ccd59e65e8152ad2f47a6d6/src/pages/GroupChatNameEditPage.tsx#L56-L66

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

We should add validation check on the frontend also, for validation we should add the same regex/check used in the backend.

What alternative solutions did you explore? (Optional)

gijoe0295 commented 1 month ago

Proposal

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

Invalid group chat name entered while creating group from the BE.

What is the root cause of that problem?

We simply validated the name length by name.length:

https://github.com/Expensify/App/blob/81bc5b0bc03d1cbf731a4ab2de1e1968e0aef781/src/libs/ValidationUtils.ts#L338

However, in the test string, there was an Unicode character or which would be counted differently instead of just 1 (reference here). That's why the validation passed in the FE but failed in the BE.

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

We need to match the length validation with the BE by using getCommentLength:

https://github.com/Expensify/App/blob/88f9419dacb076cb2eba3342b137756fc599eed2/src/libs/ReportUtils.ts#L5704-L5709

alexpensify commented 1 month ago

Other GHs have been a priority, I'll review soon

melvin-bot[bot] commented 1 month ago

Job added to Upwork: https://www.upwork.com/jobs/~0114cd4bde59962aa8

melvin-bot[bot] commented 1 month ago

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

alexpensify commented 1 month ago

@sobitneupane - can you review if one of these proposals will fix this problem? Thanks!

dominictb commented 1 month ago

Proposal

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

What is the root cause of that problem?

sobitneupane commented 1 month ago

Thanks for the proposal everyone.

Proposal from @dominictb looks good to me. The change proposed will ensure the FE validation. We might want to validate the groupChatName in BE as well during group creation. Looks like the validation is only done when groupChatName is updated.

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 1 month ago

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

NikkiWines commented 1 month ago

agreed that @dominictb's proposal looks good 👍

melvin-bot[bot] commented 1 month ago

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

alexpensify commented 1 month ago

Awesome, this PR is moving along, and we are waiting for it to go to production.

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

If no regressions arise, payment will be issued on 2024-08-07. :confetti_ball:

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

melvin-bot[bot] commented 1 month ago

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:

alexpensify commented 1 month ago

Noted that the payment date is on August 7

alexpensify commented 4 weeks ago

Payouts due: August 7, 2024

Upwork job is here.

alexpensify commented 4 weeks ago

Closing - the upwork process is complete

garrettmknight commented 4 days ago

$250 approved for @sobitneupane