fixed new room page form validation

src/CONST/index.js

@@ -526,6 +526,7 @@ const CONST = {
         ROLE: {
             ADMIN: 'admin',
         },
+        ROOM_PREFIX: '#',
     },
 
     TERMS: {

src/components/RoomNameInput.js

@@ -1,6 +1,5 @@
 import React, {Component} from 'react';
 import PropTypes from 'prop-types';
-import _ from 'underscore';
 import {withOnyx} from 'react-native-onyx';
 import CONST from '../CONST';
 import ONYXKEYS from '../ONYXKEYS';
@@ -14,17 +13,14 @@ const propTypes = {
     /** Callback to execute when the text input is modified correctly */
     onChangeText: PropTypes.func,
 
-    /** Callback to execute when an error gets found/cleared/modified */
-    onChangeError: PropTypes.func,
-
     /** Initial room name to show in input field. This should include the '#' already prefixed to the name */
     initialValue: PropTypes.string,
 
     /** Whether we should show the input as disabled */
     disabled: PropTypes.bool,
 
-    /** ID of policy whose room names we should be checking for duplicates */
-    policyID: PropTypes.string,
+    /** Error text to show */
+    errorText: PropTypes.string,
 
     ...withLocalizePropTypes,
     ...fullPolicyPropTypes,
@@ -52,10 +48,9 @@ const propTypes = {
 
 const defaultProps = {
     onChangeText: () => {},
-    onChangeError: () => {},
     initialValue: '',
     disabled: false,
-    policyID: '',
+    errorText: '',
     ...fullPolicyDefaultProps,
 };
 
@@ -64,94 +59,27 @@ class RoomNameInput extends Component {
         super(props);
         this.state = {
             roomName: props.initialValue,
-            error: '',
         };
-
-        this.originalRoomName = props.initialValue;
-
-        this.checkAndModifyRoomName = this.checkAndModifyRoomName.bind(this);
-        this.checkExistingRoomName = this.checkExistingRoomName.bind(this);
-    }
-
-    componentDidUpdate(prevProps, prevState) {
-        // As we are modifying the text input, we'll bubble up any changes/errors so the parent component can see it
-        if (prevState.roomName !== this.state.roomName) {
-            this.props.onChangeText(this.state.roomName);
-        }
-        if (prevState.error !== this.state.error) {
-            this.props.onChangeError(this.state.error);
-        }
-
-        // If the selected policyID has changed we need to check if the room name already exists on this new policy.
-        if (prevProps.policyID !== this.props.policyID) {
-            this.checkExistingRoomName(this.state.roomName);
-        }
     }
 
     /**
      * Modifies the room name to follow our conventions:
      * - Max length 80 characters
      * - Cannot not include space or special characters, and we automatically apply an underscore for spaces
      * - Must be lowercase
-     * Also checks to see if this room name already exists, and displays an error message if so.
-     * @param {Event} event
-     *
+     * @param {String} roomName
+     * @returns {String}
      */
-    checkAndModifyRoomName(event) {
-        const nativeEvent = event.nativeEvent;
-        const roomName = nativeEvent.text;
-        const target = nativeEvent.target;
-        const selection = target.selectionStart;
-
+    modifyRoomName(roomName) {
         const modifiedRoomNameWithoutHash = roomName
             .replace(/ /g, '_')
             .replace(/[^a-zA-Z\d_]/g, '')
-            .substring(0, CONST.REPORT.MAX_ROOM_NAME_LENGTH)
+            .substr(0, CONST.REPORT.MAX_ROOM_NAME_LENGTH)
             .toLowerCase();
-        const finalRoomName = `#${modifiedRoomNameWithoutHash}`;
-
-        this.checkExistingRoomName(finalRoomName);
-
-        this.setState({
-            roomName: finalRoomName,
-        }, () => {
-            if (!selection) {
-                return;
-            }
-            target.selectionEnd = selection;
-        });
-    }
 
-    /**
-     * Checks to see if this room name already exists, and displays an error message if so.
-     * @param {String} roomName
-     *
-     */
-    checkExistingRoomName(roomName) {
-        const isExistingRoomName = _.some(
-            _.values(this.props.reports),
-            report => report && report.policyID === this.props.policyID && report.reportName === roomName,
-        );
-
-        let error = '';
-
-        // We error if the room name already exists. We don't care if it matches the original name provided in this
-        // component because then we are not changing the room's name.
-        if (isExistingRoomName && roomName !== this.originalRoomName) {
-            error = this.props.translate('newRoomPage.roomAlreadyExistsError');
-        }
-
-        // Certain names are reserved for default rooms and should not be used for policy rooms.
-        if (_.contains(CONST.REPORT.RESERVED_ROOM_NAMES, roomName)) {
-            error = this.props.translate('newRoomPage.roomNameReservedError');
-        }
-
-        this.setState({
-            error,
-        });
+        return `${CONST.POLICY.ROOM_PREFIX}${modifiedRoomNameWithoutHash}`;
     }
 
-
     render() {
         return (
             <TextInputWithPrefix
@@ -160,9 +88,13 @@ class RoomNameInput extends Component {
                 prefixCharacter="#"
                 placeholder={this.props.translate('newRoomPage.social')}
                 containerStyles={[styles.mb5]}
-                onChange={this.checkAndModifyRoomName}
+                onChangeText={(roomName) => {
+                    const modifiedRoomName = this.modifyRoomName(roomName);
+                    this.setState({roomName: modifiedRoomName});
+                    this.props.onChangeText(modifiedRoomName);
+                }}
                 value={this.state.roomName.substring(1)}
-                errorText={this.state.error}
+                errorText={this.props.errorText}
                 autoCapitalize="none"
             />
         );

src/languages/en.js

@@ -896,6 +896,8 @@ export default {
         policyRoomRenamed: 'Policy room renamed!',
         roomAlreadyExistsError: 'A room with this name already exists',
         roomNameReservedError: 'This name is reserved and cannot be used',
+        pleaseEnterRoomName: 'Please enter a room name',
+        pleaseSelectWorkspace: 'Please select a workspace',
         renamedRoomAction: ({oldName, newName}) => ` renamed this room from ${oldName} to ${newName}`,
         social: 'social',
         selectAWorkspace: 'Select a workspace',

src/languages/es.js

@@ -898,6 +898,8 @@ export default {
         policyRoomRenamed: '¡Espacio de trabajo renombrado!',
         roomAlreadyExistsError: 'Ya existe una sala con este nombre',
         roomNameReservedError: 'Este nombre está reservado y no puede usarse',
+        pleaseEnterRoomName: 'Por favor escribe el nombre de una sala',
+        pleaseSelectWorkspace: 'Por favor, selecciona un espacio de trabajo',
         renamedRoomAction: ({oldName, newName}) => ` cambió el nombre de la sala de ${oldName} a ${newName}`,
         social: 'social',
         selectAWorkspace: 'Seleccionar un espacio de trabajo',

src/libs/ValidationUtils.js

@@ -321,6 +321,34 @@ function doesFailCharacterLimit(maxLength, valuesToBeValidated) {
     return _.map(valuesToBeValidated, value => value.length > maxLength);
 }
 
+/**
+ * Checks if is one of the certain names which are reserved for default rooms
+ * and should not be used for policy rooms.
+ *
+ * @param {String} roomName
+ * @returns {Boolean}
+ */
+function isReservedRoomName(roomName) {
+    return _.contains(CONST.REPORT.RESERVED_ROOM_NAMES, roomName);
+}
+
+/**
+ * Checks if the room name already exists. We don't care if it matches the original name provided in this
+ * component because then we are not changing the room's name
+ *
+ * @param {String} roomName
+ * @param {Object[]} reports
+ * @param {String} policyID
+ * @returns {Boolean}
+ */
+function isExistingRoomName(roomName, reports, policyID) {
+    return _.some(
+        _.values(reports),
+        report => report && report.policyID === policyID
+        && report.reportName === roomName,
+    );
+}
+
 export {
     meetsAgeRequirements,
     isValidAddress,
@@ -344,4 +372,6 @@ export {
     isValidRoutingNumber,
     isValidSSNLastFour,
     doesFailCharacterLimit,
+    isReservedRoomName,
+    isExistingRoomName,
 };

src/pages/ReportSettingsPage.js

@@ -18,6 +18,7 @@ import Button from '../components/Button';
 import RoomNameInput from '../components/RoomNameInput';
 import Picker from '../components/Picker';
 import withFullPolicy, {fullPolicyDefaultProps, fullPolicyPropTypes} from './workspace/withFullPolicy';
+import * as ValidationUtils from '../libs/ValidationUtils';
 
 
 const propTypes = {
@@ -49,6 +50,18 @@ const propTypes = {
         notificationPreference: PropTypes.string,
     }).isRequired,
 
+    /** All reports shared with the user */
+    reports: PropTypes.shape({
+        /** The report name */
+        reportName: PropTypes.string,
+
+        /** The report type */
+        type: PropTypes.string,
+
+        /** ID of the policy */
+        policyID: PropTypes.string,
+    }).isRequired,
+
     /** The policies which the user has access to and which the report could be tied to */
     policies: PropTypes.shape({
         /** The policy name */
@@ -84,12 +97,58 @@ class ReportSettingsPage extends Component {
 
         this.state = {
             newRoomName: this.props.report.reportName,
-            error: '',
+            errors: {},
         };
+
+        this.validateAndRenameReport = this.validateAndRenameReport.bind(this);
+    }
+
+    validateAndRenameReport() {
+        if (!this.validate() || this.props.report.reportName === this.state.newRoomName) {
+            return;
+        }
+        Report.renameReport(this.props.report.reportID, this.state.newRoomName);
+    }
+
+    validate() {
+        const errors = {};
+
+        // We error if the user doesn't enter a room name or left blank
+        if (!this.state.newRoomName || this.state.newRoomName === CONST.POLICY.ROOM_PREFIX) {
+            errors.newRoomName = this.props.translate('newRoomPage.pleaseEnterRoomName');
+        }
+
+        // We error if the room name already exists. We don't care if it matches the original name provided in this
+        // component because then we are not changing the room's name.
+        if (ValidationUtils.isExistingRoomName(this.state.newRoomName, this.props.reports, this.props.report.policyID) && this.state.newRoomName !== this.props.report.reportName) {
+            errors.newRoomName = this.props.translate('newRoomPage.roomAlreadyExistsError');
+        }
+
+        // Certain names are reserved for default rooms and should not be used for policy rooms.
+        if (ValidationUtils.isReservedRoomName(this.state.newRoomName)) {
+            errors.newRoomName = this.props.translate('newRoomPage.roomNameReservedError');
+        }
+
+        this.setState({errors});
+        return _.isEmpty(errors);
+    }
+
+    /**
+     * @param {String} inputKey
+     * @param {String} value
+     */
+    clearErrorAndSetValue(inputKey, value) {
+        this.setState(prevState => ({
+            [inputKey]: value,
+            errors: {
+                ...prevState.errors,
+                [inputKey]: '',
+            },
+        }));
     }
 
     render() {
-        const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report) || this.props.isLoadingRenamePolicyRoom;
+        const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report);
         const linkedWorkspace = _.find(this.props.policies, policy => policy.id === this.props.report.policyID);
 
         return (
@@ -128,26 +187,22 @@ class ReportSettingsPage extends Component {
                         <View style={[styles.flexRow]}>
                             <View style={[styles.flex3]}>
                                 <RoomNameInput
-                                    onChangeText={newRoomName => this.setState({newRoomName})}
-                                    onChangeError={error => this.setState({error})}
                                     initialValue={this.state.newRoomName}
-                                    disabled={shouldDisableRename}
                                     policyID={linkedWorkspace && linkedWorkspace.id}
+                                    errorText={this.state.errors.newRoomName}
+                                    onChangeText={newRoomName => this.clearErrorAndSetValue('newRoomName', newRoomName)}
+                                    disabled={shouldDisableRename}
                                 />
                             </View>
                             <Button
                                 success={!shouldDisableRename}
                                 text={this.props.translate('common.save')}
-                                onPress={() => Report.renameReport(this.props.report.reportID, this.state.newRoomName)}
+                                onPress={this.validateAndRenameReport}
                                 style={[styles.ml2, styles.flex1]}
                                 textStyles={[styles.label]}
                                 innerStyles={[styles.reportSettingsChangeNameButton]}
-                                isDisabled={Boolean(
-                                    shouldDisableRename
-                                    || this.state.newRoomName === this.props.report.reportName
-                                    || this.state.error,
-                                )}
                                 isLoading={this.props.isLoadingRenamePolicyRoom}
+                                isDisabled={shouldDisableRename}
                             />
                         </View>
                     </View>
@@ -199,5 +254,8 @@ export default compose(
         isLoadingRenamePolicyRoom: {
             key: ONYXKEYS.IS_LOADING_RENAME_POLICY_ROOM,
         },
+        reports: {
+            key: ONYXKEYS.COLLECTION.REPORT,
+        },
     }),
 )(ReportSettingsPage);