Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HOLD for Payment [2024-05-16] [$250] Room - Room name character limit is 79 characters instead of 100 #40968

Closed
6 tasks done
izarutskaya opened this issue Apr 25, 2024 · 19 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Apr 25, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.4.65-5
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/4506180
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to FAB > Start chat.
  3. Go to Room tab.
  4. Try to enter more than 79 characters.

Expected Result:

The room name character limit is 100 characters.

Actual Result:

The room name character limit is 79 characters.

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6461316_1714034965849.bandicam_2024-04-25_16-46-52-225.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~014d41cf935d9bcfd5
  • Upwork Job ID: 1783512147519520768
  • Last Price Increase: 2024-04-25
  • Automatic offers:
    • eh2077 | Reviewer | 0
    • GandalfGwaihir | Contributor | 0
Issue OwnerCurrent Issue Owner: @isabelastisser
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

Triggered auto assignment to @isabelastisser (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
Copy link
Author

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

@allgandalf
Copy link
Contributor

Proposal

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

Room title (name) limit is not 100

What is the root cause of that problem?

Currently we have set the limit of title to 79:

maxLength={CONST.REPORT.MAX_ROOM_NAME_LENGTH}

MAX_ROOM_NAME_LENGTH: 79,

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

First solution is to update MAX_ROOM_NAME_LENGTH to 99.
(99 because we prefix a # to a room name, so we need to consider one less character, we currently have the limit set to 79 for the same reason)

OR
Currently we do not have any value defined for room title set to 99 in CONST.ts so first we will need to add a new const:

REPORT_TITLE: {
        MAX_LENGTH: 99,
    },

Note:

We also need to update the validation regex below:

App/src/CONST.ts

Line 1759 in 0b96654

ROOM_NAME: /^#[\p{Ll}0-9-]{1,80}$/u,

We need to now allow upto 100 characters:

ROOM_NAME: /^#[\p{Ll}0-9-]{1,100}$/u,

So this update will now not throw a character invalid result.

What alternative solutions did you explore? (Optional)

@isabelastisser isabelastisser added External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Apr 25, 2024
@melvin-bot melvin-bot bot changed the title Room - Room name character limit is 79 characters instead of 100 [$250] Room - Room name character limit is 79 characters instead of 100 Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

Job added to Upwork: https://www.upwork.com/jobs/~014d41cf935d9bcfd5

Copy link

melvin-bot bot commented Apr 25, 2024

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

@eh2077
Copy link
Contributor

eh2077 commented Apr 26, 2024

@GandalfGwaihir 's proposal looks good to me. The root cause is clear. We can use this constant for the length limit

App/src/CONST.ts

Line 1913 in 0b96654

TITLE_CHARACTER_LIMIT: 100,

We can discuss the detail implementation in PR.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Apr 26, 2024

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

@dangrous
Copy link
Contributor

That proposal works for me as well! We might want to keep a separate constant for the 99 / 100 issue, but yes can discuss.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 26, 2024
Copy link

melvin-bot bot commented Apr 26, 2024

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

Offer link
Upwork job

Copy link

melvin-bot bot commented Apr 26, 2024

📣 @GandalfGwaihir 🎉 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 📖

@allgandalf
Copy link
Contributor

PR would be up by EOD

@allgandalf
Copy link
Contributor

Pr ready for review c.c. @eh2077 , left some comments too :)

@isabelastisser
Copy link
Contributor

@GandalfGwaihir is this ready for payment? Please provide an update. Thanks!

@eh2077
Copy link
Contributor

eh2077 commented May 9, 2024

@GandalfGwaihir is this ready for payment? Please provide an update. Thanks!

@isabelastisser It's not ready yet.

✋ This PR was not deployed to staging yet because QA is ongoing.

@allgandalf
Copy link
Contributor

⚠️ Automation failed -> this should be on [HOLD for Payment [2024-05-16]] according to 2 days past production deploy from #41123 (comment)

c.c. @isabelastisser

@isabelastisser isabelastisser changed the title [$250] Room - Room name character limit is 79 characters instead of 100 HOLD for Payment [2024-05-16] [$250] Room - Room name character limit is 79 characters instead of 100 May 13, 2024
@dangrous
Copy link
Contributor

ready for payment!

@allgandalf
Copy link
Contributor

Hello @isabelastisser , this should be ready for payment :)

@isabelastisser
Copy link
Contributor

Reviewing!

@isabelastisser
Copy link
Contributor

The payments were processed in Upwork. This should be all set!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

5 participants