"A room with this name already exists" message appears on Create New Room page #14010

kavimuru · 2023-01-05T02:34:56Z

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

Action Performed:

Login with account that has policyRooms beta enabled
Click FAB -> New Room
Don't input anything and just click "Create Room" button
"A room with this name already exists" message shows below Room Name input field
Type any letter on Room Name field and erase all
This time, "Please enter a room name" message shows below Room Name input field

Expected Result:

Error message should be consistent as "Please enter a room name"

Actual Result:

"A room with this name already exists" error shows first time

Workaround:

unknown

Platforms:

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

**Version Number:**v1.2.48-2
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

209856169-65c49370-e10c-4b86-9500-af45593cd55d.mov

Recording.1214.mp4

Expensify/Expensify Issue URL:
Issue reported by: @0xmiroslav
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1672852094448169

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01226a954977211217
Upwork Job ID: 1610968589634461696
Last Price Increase: 2023-01-05

melvin-bot · 2023-01-05T02:35:01Z

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

melvin-bot · 2023-01-05T11:56:59Z

Job added to Upwork: https://www.upwork.com/jobs/~01226a954977211217

melvin-bot · 2023-01-05T11:57:04Z

Triggered auto assignment to Contributor Plus for review of internal employee PR - @thesahindia (Internal)

yuwenmemon · 2023-01-05T11:57:07Z

I can take a look at this one.

jatinsonijs · 2023-01-05T19:44:01Z

@yuwenmemon I have found root case of this issue during testing. If you want I can post here. I'm asking bcz this issue is Internal and not allowed for external contributors.

0xmiros · 2023-01-05T20:00:22Z

App/src/libs/ValidationUtils.js

Lines 386 to 391 in dee9ac5

    
           function isExistingRoomName(roomName, reports, policyID) { 
        
               return _.some( 
        
                   reports, 
        
                   report => report && report.policyID === policyID 
        
                   && report.reportName === roomName, 
        
               );

undefined === undefined is the root cause.
I think we should add some more filters like this:

     return _.some( 
         reports, 
-        report => report && report.policyID === policyID 
-        && report.reportName === roomName,
+        report => report && !_.isEmpty(report.policyID) && report.policyID === policyID 
+        && !_.isEmpty(report.reportName) && report.reportName === roomName, 
     );

jatinsonijs · 2023-01-05T20:08:11Z

What I have found is

we have added ReportFooter with loading state view.
In this PR #13523

/src/pages/home/ReportScreen.js

 <>
      <ReportActionsSkeletonView containerHeight={this.state.skeletonViewContainerHeight} />
      <ReportFooter shouldDisableCompose isOffline={this.props.network.isOffline} />
</>

We are not passing report in this case. Due to report not exist ReportFooter will use default prop
/src/pages/home/report/ReportFooter.js
LN: 52
report: {reportID: '0'},

and it will pass down to ReportActionCompose

<ReportActionCompose
    onSubmit={this.props.onSubmitComment}
    reportID={this.props.report.reportID.toString()}
    reportActions={this.props.reportActions}
    report={this.props.report}
    isComposerFullSize={this.props.isComposerFullSize}
    disabled={this.props.shouldDisableCompose}
/>

On component mount inside ReportActionCompose updateComponent function will trigger and it will call
// The draft has been deleted. if (newComment.length === 0) { Report.setReportWithDraft(this.props.reportID, false); }

Now in this case new incorrect report record will created with report_0: { hasDraft: false }

It will impact condition of check room name already exist or not

function isExistingRoomName(roomName, reports, policyID) {
  return _.some(
      reports,
      report => report && report.policyID === policyID
      && report.reportName === roomName,
  );
}

app has incorrect report record report_0: { hasDraft: false }

so it will check report exist true,
report.policyID === PolicyID will be true bcz both are undefined bcz we don't have default value of room name.
report.reportName === roomName will be true bcz both are undefined.

fedirjh · 2023-01-05T22:07:58Z

I think we should add validation as @0xmiroslav stated , we also have change this block to if else statement , the second validation is overwriting the first one

App/src/pages/workspace/WorkspaceNewRoomPage.js

Lines 84 to 92 in dee9ac5

    
           // We error if the user doesn't enter a room name or left blank 
        
           if (!values.roomName || values.roomName === CONST.POLICY.ROOM_PREFIX) { 
        
               errors.roomName = this.props.translate('newRoomPage.pleaseEnterRoomName'); 
        
           } 
        
           // We error if the room name already exists. 
        
           if (ValidationUtils.isExistingRoomName(values.roomName, this.props.reports, values.policyID)) { 
        
               errors.roomName = this.props.translate('newRoomPage.roomAlreadyExistsError'); 
        
           }

         // We error if the user doesn't enter a room name or left blank
+        // We error if the room name already exists.
         if (!values.roomName || values.roomName === CONST.POLICY.ROOM_PREFIX) {
             errors.roomName = this.props.translate('newRoomPage.pleaseEnterRoomName');
-        }
-
-        // We error if the room name already exists.
-        if (ValidationUtils.isExistingRoomName(values.roomName, this.props.reports, values.policyID)) {
+        } else if (ValidationUtils.isExistingRoomName(values.roomName, this.props.reports, values.policyID)) {
             errors.roomName = this.props.translate('newRoomPage.roomAlreadyExistsError');
         }

and for validation here I think this is enough

 function isExistingRoomName(roomName, reports, policyID) {
-    return _.some(
+    return roomName && _.some(
         reports,
         report => report && report.policyID === policyID
         && report.reportName === roomName,

dhairyasenjaliya · 2023-01-06T01:58:15Z

Is this issue open for external? @yuwenmemon

dhairyasenjaliya · 2023-01-06T02:51:54Z

not sure if it's available to work but I'm adding my analysis below
Here on the very first if we press save without adding any data we are validating the room name with the selected workspace which will fail because we never select any the workspace name in the first place (which will be always undefined)
In order to solve this issue we need one more conditional check for values.policyID and make sure if we select any workspace then only need to validate for existing workspace name into WorkspaceNewRoomPage.js

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 90 in 69fd192

if (ValidationUtils.isExistingRoomName(values.roomName, this.props.reports, values.policyID)) {
We will validate the Room name only if we select the workspace that will make sure to show the correct error validation

Changes

+        if (values.policyID && ValidationUtils.isExistingRoomName(values.roomName, this.props.reports, values.policyID)) {
            errors.roomName = this.props.translate('newRoomPage.roomAlreadyExistsError');
        }

Result

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-01-05.at.17.08.44.mp4

@yuwenmemon

yuwenmemon · 2023-01-06T14:59:51Z

This was fixed with: #13994

Also yes FYI @dhairyasenjaliya @fedirjh @0xmiroslav @jatinsonijs I appreciate the input but do please understand that this issue was tagged Internal and not marked either External or Help Wanted.

0xmiros · 2023-01-06T15:13:53Z

@yuwenmemon though this is internal, I think I am eligible for reporting compensation

yuwenmemon · 2023-01-06T15:41:23Z

I believe that's correct @0xmiroslav but I will defer to @NicMendonca on that point.

fedirjh · 2023-01-06T17:38:52Z

@yuwenmemon I think this has not been fixed yet , now with this #13994 It will always skip the first validation , I still think that validation to same input field should go through an if - else :

if (empty) shows empty error
else if (invalid name) shows invalid error
else if (used name) shows already used error
else if (reserved name) shows reserved error

I think we can also add this rule to FORMS docs.

yuwenmemon · 2023-01-06T19:13:53Z

@fedirjh that's a good observation. I'll make that change, and we can work with @NicMendonca to ensure you get a bug reporting bonus.

NicMendonca · 2023-01-06T21:06:45Z

@fedirjh can you please accept my proposal to the job here?

fedirjh · 2023-01-06T21:13:01Z

@NicMendonca Thank you, accepted!

NicMendonca · 2023-01-09T23:07:16Z

@fedirjh paid the reporting bonus - thanks!

0xmiros · 2023-01-10T03:18:38Z

@NicMendonca I am also eligible for reporting bonus here

yuwenmemon · 2023-01-10T03:42:09Z

@NicMendonca @0xmiroslav is correct 👍

NicMendonca · 2023-01-11T15:55:57Z

@0xmiroslav can you please accept the job offer?

0xmiros · 2023-01-11T15:58:25Z

@0xmiroslav can you please accept the job offer?

accepted. thank you

0xmiros · 2023-01-11T15:59:39Z

@NicMendonca FYI: you hired me for $1000 but I am eligible for only $250

NicMendonca · 2023-01-13T00:04:39Z

Ah, Thank you, I've created a new milestone 😄 Paid for $250

NicMendonca · 2023-01-13T00:04:58Z

bonuses have both been paid ✅

thesahindia · 2023-01-18T12:49:46Z

Hey @NicMendonca, C+ review compensation is due (sorry for the late comment).

thesahindia · 2023-01-23T18:01:51Z

Bump @NicMendonca

NicMendonca · 2023-01-23T22:52:23Z

@thesahindia can you accept the job offer, please?

NicMendonca · 2023-01-24T14:12:03Z

@thesahindia paid!

thesahindia · 2023-01-24T19:08:23Z

I think this can be closed now?

kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 5, 2023

melvin-bot bot assigned NicMendonca Jan 5, 2023

melvin-bot bot locked and limited conversation to collaborators Jan 5, 2023

yuwenmemon self-assigned this Jan 5, 2023

yuwenmemon added the Internal Requires API changes or must be handled by Expensify staff label Jan 5, 2023

melvin-bot bot unlocked this conversation Jan 5, 2023

yuwenmemon added the Engineering label Jan 5, 2023

melvin-bot bot assigned thesahindia Jan 5, 2023

yuwenmemon closed this as completed Jan 6, 2023

yuwenmemon reopened this Jan 6, 2023

yuwenmemon mentioned this issue Jan 6, 2023

Use else-ifs to establish precedence for room name validations #14068

Merged

53 tasks

melvin-bot bot added the Reviewing Has a PR in review label Jan 6, 2023

NicMendonca changed the title ~~"A room with this name already exists" message appears on Create New Room page~~ [Reporting bonus] "A room with this name already exists" message appears on Create New Room page Jan 6, 2023

NicMendonca changed the title ~~[Reporting bonus] "A room with this name already exists" message appears on Create New Room page~~ "A room with this name already exists" message appears on Create New Room page Jan 9, 2023

NicMendonca changed the title ~~"A room with this name already exists" message appears on Create New Room page~~ [Hold for C+ Payment] "A room with this name already exists" message appears on Create New Room page Jan 23, 2023

NicMendonca changed the title ~~[Hold for C+ Payment] "A room with this name already exists" message appears on Create New Room page~~ "A room with this name already exists" message appears on Create New Room page Jan 24, 2023

NicMendonca closed this as completed Jan 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

"A room with this name already exists" message appears on Create New Room page #14010

"A room with this name already exists" message appears on Create New Room page #14010

kavimuru commented Jan 5, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jan 5, 2023

melvin-bot bot commented Jan 5, 2023

melvin-bot bot commented Jan 5, 2023

yuwenmemon commented Jan 5, 2023

jatinsonijs commented Jan 5, 2023

0xmiros commented Jan 5, 2023

jatinsonijs commented Jan 5, 2023

fedirjh commented Jan 5, 2023

dhairyasenjaliya commented Jan 6, 2023

dhairyasenjaliya commented Jan 6, 2023

yuwenmemon commented Jan 6, 2023

0xmiros commented Jan 6, 2023

yuwenmemon commented Jan 6, 2023

fedirjh commented Jan 6, 2023

yuwenmemon commented Jan 6, 2023

NicMendonca commented Jan 6, 2023 •

edited

Loading

fedirjh commented Jan 6, 2023

NicMendonca commented Jan 9, 2023

0xmiros commented Jan 10, 2023

yuwenmemon commented Jan 10, 2023

NicMendonca commented Jan 11, 2023

0xmiros commented Jan 11, 2023

0xmiros commented Jan 11, 2023

NicMendonca commented Jan 13, 2023

NicMendonca commented Jan 13, 2023

thesahindia commented Jan 18, 2023

thesahindia commented Jan 23, 2023

NicMendonca commented Jan 23, 2023

NicMendonca commented Jan 24, 2023

thesahindia commented Jan 24, 2023

"A room with this name already exists" message appears on Create New Room page #14010

"A room with this name already exists" message appears on Create New Room page #14010

Comments

kavimuru commented Jan 5, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jan 5, 2023

melvin-bot bot commented Jan 5, 2023

melvin-bot bot commented Jan 5, 2023

yuwenmemon commented Jan 5, 2023

jatinsonijs commented Jan 5, 2023

0xmiros commented Jan 5, 2023

jatinsonijs commented Jan 5, 2023

fedirjh commented Jan 5, 2023

dhairyasenjaliya commented Jan 6, 2023

dhairyasenjaliya commented Jan 6, 2023

yuwenmemon commented Jan 6, 2023

0xmiros commented Jan 6, 2023

yuwenmemon commented Jan 6, 2023

fedirjh commented Jan 6, 2023

yuwenmemon commented Jan 6, 2023

NicMendonca commented Jan 6, 2023 • edited Loading

fedirjh commented Jan 6, 2023

NicMendonca commented Jan 9, 2023

0xmiros commented Jan 10, 2023

yuwenmemon commented Jan 10, 2023

NicMendonca commented Jan 11, 2023

0xmiros commented Jan 11, 2023

0xmiros commented Jan 11, 2023

NicMendonca commented Jan 13, 2023

NicMendonca commented Jan 13, 2023

thesahindia commented Jan 18, 2023

thesahindia commented Jan 23, 2023

NicMendonca commented Jan 23, 2023

NicMendonca commented Jan 24, 2023

thesahindia commented Jan 24, 2023

kavimuru commented Jan 5, 2023 •

edited by melvin-bot bot

Loading

NicMendonca commented Jan 6, 2023 •

edited

Loading