[Awaiting payment 21-12-23] [$1000] Duplicate room name error doesn't appear if you aren't a member of the original room

[kbecciv]

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

---

Action Performed:

1. Go to staging.new.expensify.com
2. Create a workspace
3. Now create a New room and choose the workspace you created
4. Click the room header and leave the room
5. Now create a New room again using the same name of the room you just left and the same workspace you selected for the first room

Expected Result:

The room name field should have an in-line error message upon losing focus of that field

Actual Result:

You're allowed to proceed to create the room and run into an error after the creation attempt. We suspect this has something to do with the check for a duplicate room existing being limited to rooms you are a member of, not the workspace you are trying to create the room on.

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.32-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

green-2023-06-25_13.46.19.1.mp4

Recording.919.mp4

Expensify/Expensify Issue URL:
Issue reported by: @priya-zha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687680050819699

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cbde78f15d909ff4
• Upwork Job ID: 1675822342302105600
• Last Price Increase: 2023-07-10
• Automatic offers:
• esh-g | Contributor | 27774680
• priya-zha | Reporter | 27774681

Issue Owner

Current Issue Owner: @jasperhuangg

[dukenv0307]

Proposal

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

Settings doesn't work on a room that has an error of 'previously created'

What is the root cause of that problem?

In ReportDetailPage, we don't have any check to hide setting item if report has an error

App/src/pages/ReportDetailsPage.js

Lines 101 to 105 in 64b720c

	if (isPolicyExpenseChat || isChatRoom || isThread) {
	items.push({
	key: CONST.REPORT_DETAILS_MENU_ITEM.SETTINGS,
	translationKey: 'common.settings',
	icon: Expensicons.Gear,

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

We can add a check here not to show setting item if report has an error

const addWorkspaceRoomOrChatErrors = lodashGet(props.report, 'errorFields.addWorkspaceRoom') || lodashGet(props.report, 'errorFields.createChat');
if ((isPolicyExpenseChat || isChatRoom || isThread) && !addWorkspaceRoomOrChatErrors) {

App/src/pages/home/HeaderView.js

Line 161 in 64b720c

onPress={() => ReportUtils.navigateToDetailsPage(props.report)}

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[trjExpensify]

Okay, so I actually don't think the user should be getting this far in the flow if the room name is a dupe. When you're still a member of a room, we show an in-line error:

But here, when you leave the room AND then try and create a new room with the same name, we don't do that. We let you continue on and error out on creation. So I think we should be showing a error message in-line the room name field if the room has a dupe name on the workspace you've selected. That would avoid this scenario. It seems like our check for if a room already exists takes into account rooms you are a member of, which in this case, you aren't a member of the room.

[trjExpensify]

Okay, updated the OP following the discussion in thread.

CC: @MitchExpensify

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01cbde78f15d909ff4

[melvin-bot]

Current assignee @trjExpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[trjExpensify]

Awaiting proposals. @dukenv0307 not sure if you want to modify yours given the expected behaviour.

[Nodebrute]

@trjExpensify we use isExistingRoomto throw an error, we are passing report data to check if that report exists or not. However, in the report data, we don't have any details about the previous room

[allroundexperts]

@trjExpensify Let's bump this on slack to get more eyes on this?

[Nodebrute]

Proposal

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

Duplicate room name error doesn't appear if you aren't a member of the original room.

What is the root cause of that problem?

App/src/libs/actions/Report.js

Lines 1709 to 1718 in 60f4817

	optimisticData: [
	{
	onyxMethod: Onyx.METHOD.SET,
	key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
	value: {
	stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
	statusNum: CONST.REPORT.STATUS.CLOSED,
	},
	},
	],

When we leave the room, we use the SET method. The SET method is used to delete the key from storage.

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

We can use the MERGE method here so that the key is not completely removed from Onyx. This also requires a backend fix.

When we delete a workspace, we utilize Onyx's merge method to update the stateNum and statusNum. By using the merge method, this data remains in Onyx. As for these reports, we can utilize the 'isArchivedRoom' utility function. However, when we leave a room, we simply delete the Onyx keys associated with that report.

What alternative solutions did you explore? (Optional)

[trjExpensify]

@trjExpensify Let's bump this on slack to get more eyes on this?

Bump where? We can chat more in the thread if you want though to discuss the best approach or clarify the intention. Thanks!

[allroundexperts]

@trjExpensify Let's bump this on slack to get more eyes on this?

Bump where? We can chat more in the thread if you want though to discuss the best approach or clarify the intention. Thanks!

I meant to ask for more proposals. But I see that we have a new proposal since then. Will review it and get back!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@trjExpensify, @allroundexperts, @jasperhuangg Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@trjExpensify, @allroundexperts, @jasperhuangg Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@trjExpensify, @allroundexperts, @jasperhuangg Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[jasperhuangg]

@pecanoro recently put up a PR that should set those errors in the back-end, thanks!

It seems like that PR was recently deployed to staging @esh-g

[melvin-bot]

@trjExpensify, @allroundexperts, @jasperhuangg Eep! 4 days overdue now. Issues have feelings too...

[esh-g]

@jasperhuangg I have tested my proposal with the backend change that was done and it works just as expected.

Screen.Recording.2023-11-22.at.12.12.38.AM.mov

Feel free to assign me and I can raise a PR!

[melvin-bot]

📣 @allroundexperts Please request via NewDot manual requests for the Reviewer role ($1000)

[melvin-bot]

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

[melvin-bot]

📣 @priya-zha 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @allroundexperts Please request via NewDot manual requests for the Reviewer role ($1000)

[jasperhuangg]

@esh-g awesome, let's move forward with your proposal! thanks :)

[esh-g]

PR is here: #31655

cc @jasperhuangg @allroundexperts

[jasperhuangg]

@allroundexperts bump on the review, thanks!

[esh-g]

@trjExpensify @jasperhuangg Could we get the payment for this processed before the holidays? Thanks!
Seems like since this is still marked as hold so the payment automation date didn't happen...

[trjExpensify]

Confirming payments:

• $1000 to @esh-g for the fix
• $1000 to @allroundexperts for the C+ review
• $50 to @priya-zha for the bug report (predates the programme changes).

Settled up with @esh-g and @priya-zha. @allroundexperts you can request. Closing!

[JmillsExpensify]

$1,000 payment to @allroundexperts based on comment above.