[HOLD for payment 2024-02-20] [$500] 0 member is formatted weirdly when leaving a room and revisiting room details

[m-natarajan]

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.33-0
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
Expensify/Expensify Issue URL:
Issue reported by: @blimpich
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1706559325923909

Action Performed:

1. create a workspace
2. create a room with workspace visibility
3. open the details pane and copy the link in the url to your clipboard
4. leave the room
5. paste the link you copied into the URL

Expected Result:

We should see the members menu item with a correctly formatted zero representing zero members

Actual Result:

The zero is there but its formatted really strangely

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

Add any screenshot/video evidence

Recording.2666.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018691b573ff8ea80e
• Upwork Job ID: 1752221236965654528
• Last Price Increase: 2024-01-30
• Automatic offers:
• dukenv0307 | Contributor | 28137708
• shubham1206agra | Contributor | 28137709

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~018691b573ff8ea80e

[melvin-bot]

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

[melvin-bot]

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

[jyash97]

Hey 👋🏻

new to contributing to Expensify, is there any way I can take this issue?

[melvin-bot]

📣 @jyash97! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[dukenv0307]

Proposal

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

The zero is there but its formatted really strangely

What is the root cause of that problem?

This is regression from f74f3c3#diff-9db0c2baf85f38b9a238d03f1089a686039ab6c6b94ef412bda920293909b7a4

Let's see this line that shows the Members count

{(subtitle ?? subtitle === 0) && (

will mean 0 will display as is without any wrapper component.

Because subtitle ?? subtitle === 0 means 0 (because ?? won't care about the second part of the operator if the first part is already not nullish)
Then (0 && <Component means pure 0 as well.

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

Should use subtitle || subtitle === 0 like before

We can structure it as

(!!subtitle || (subtitle === 0))

or

subtitle === 0 || subtitle

So that it won't trigger lint error. In here when we check subtitle, that means we're checking if subtitle exists/is truthy, so we should use !!subtitle ||, if we use subtitle || it's similar to coalesing using || and is discouraged by our lint settings.

Even better is to extract it to a shouldShowSubtitle boolean and use it, will be cleaner.

What alternative solutions did you explore? (Optional)

Unrelated but there's another issue where when opening that Member list with 0 member, we'll see an empty Member list, to fix it we might want to:

Option 1:

• Do not show right arrow if member count is 0, and make the MenuItem disabled
• When the user navigates to the Member list directly via deeplink, automatically go back to previous screen/or show not found page.

Option 2:

• Just show a text like There's no member in this room if there's no member

Those are my suggestions, the implementation for it is straight-forward, but I think it should be addressed separately and we need to decide the UX, since it's separate from the issue in OP and has a different root cause.

[jyash97]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~015f3056aaa4d9252e

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[gotcadumitru]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~0182480069006bff70

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[CortneyOfstad]

new to contributing to Expensify, is there any way I can take this issue?

@jyash97 welcome! We ask that you provide proposals for the issue, in a similar format to the one here. Then the C+ will review to see which proposals will best fit the issue 👍

[CortneyOfstad]

@shubham1206agra any thoughts on the proposal here?

[shubham1206agra]

@dukenv0307's proposal looks good.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[hayata-suenaga]

I messed up assignment 😭

hope it didn't break anything...

[CortneyOfstad]

@hayata-suenaga no worries! Everything looks to be okay, and regardless — doesn't take much to make a manual change on the Upwork side! All good!

So to confirm, @dukenv0307 has been hired as the contributor and @shubham1206agra has been hired as the C+? Just want to make sure I have that correct. Thanks!

[hayata-suenaga]

yes that's right! thank you!

[dukenv0307]

@shubham1206agra The PR is ready for review.

[melvin-bot]

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

[melvin-bot]

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

• Fix 0 member format when leaving a room and revisit #35868

If no regressions arise, payment will be issued on 2024-02-20. 🎊

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

• @shubham1206agra requires payment automatic offer (Contributor) — paid $500 via Upwork
• @dukenv0307 requires payment automatic offer (Contributor) — paid $500 via Upwork

[melvin-bot]

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:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR:
• [@shubham1206agra] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@shubham1206agra] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@shubham1206agra] Determine if we should create a regression test for this bug.
• [@shubham1206agra] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@CortneyOfstad] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[CortneyOfstad]

Payment is scheduled for next week! 🎉

[CortneyOfstad]

@shubham1206agra it looks like we're still waiting for you to accept the proposal in Upwork here — can you let me know once you accept!

@dukenv0307 you have been paid via Upwork, so you are all set!

[shubham1206agra]

@CortneyOfstad Accepted.

[CortneyOfstad]

Perfect! Payment has been made! Once you have completed the checklist @shubham1206agra — we can get this closed. Thank you!

[CortneyOfstad]

@shubham1206agra any chance you can have the checklist completed by EOD today?

[shubham1206agra]

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:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR: [TS migration] Migrate MenuItem to Typescript #31141
• [@shubham1206agra] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/31141/files#r1497907022
• [@shubham1206agra] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: Not Required
• [@shubham1206agra] Determine if we should create a regression test for this bug. No
• [@shubham1206agra] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@CortneyOfstad] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[CortneyOfstad]

Perfect! Thanks so much!