Merge pull request #791 from Its-treason/feature/variable-name-validation

Enforce variable strictness & validation

packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/StyledWrapper.js

@@ -13,10 +13,12 @@ const Wrapper = styled.div`
       padding: 4px 10px;
 
       &:nth-child(1),
-      &:nth-child(4),
+      &:nth-child(4) {
-      &:nth-child(5) {
         width: 70px;
       }
+      &:nth-child(5) {
+        width: 40px;
+      }
 
       &:nth-child(2) {
         width: 25%;

packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js

@@ -1,68 +1,79 @@
-import React, { useReducer } from 'react';
+import React from 'react';
 import toast from 'react-hot-toast';
 import cloneDeep from 'lodash/cloneDeep';
 import { IconTrash } from '@tabler/icons';
 import { useTheme } from 'providers/Theme';
 import { useDispatch } from 'react-redux';
 import { saveEnvironment } from 'providers/ReduxStore/slices/collections/actions';
-import reducer from './reducer';
 import SingleLineEditor from 'components/SingleLineEditor';
 import StyledWrapper from './StyledWrapper';
+import { useFormik } from 'formik';
+import * as Yup from 'yup';
+import { uuid } from 'utils/common';
 
 const EnvironmentVariables = ({ environment, collection }) => {
   const dispatch = useDispatch();
   const { storedTheme } = useTheme();
-  const [state, reducerDispatch] = useReducer(reducer, { hasChanges: false, variables: environment.variables || [] });
-  const { variables, hasChanges } = state;
 
-  const saveChanges = () => {
+  const formik = useFormik({
-    dispatch(saveEnvironment(cloneDeep(variables), environment.uid, collection.uid))
+    enableReinitialize: true,
-      .then(() => {
+    initialValues: environment.variables || [],
-        toast.success('Changes saved successfully');
+    validationSchema: Yup.array().of(
-        reducerDispatch({
+      Yup.object({
-          type: 'CHANGES_SAVED'
+        enabled: Yup.boolean(),
-        });
+        name: Yup.string()
+          .required('Name cannot be empty')
+          .matches(/^(?!\d)\w*$/, 'Name contains invalid characters')
+          .trim(),
+        secret: Yup.boolean(),
+        type: Yup.string(),
+        uid: Yup.string(),
+        value: Yup.string().trim()
       })
-      .catch(() => toast.error('An error occurred while saving the changes'));
+    ),
+    onSubmit: (values) => {
+      if (!formik.dirty) {
+        toast.error('Nothing to save');
+        return;
+      }
+
+      dispatch(saveEnvironment(cloneDeep(values), environment.uid, collection.uid))
+        .then(() => {
+          toast.success('Changes saved successfully');
+          formik.resetForm({ values });
+        })
+        .catch(() => toast.error('An error occurred while saving the changes'));
+    }
+  });
+
+  const ErrorMessage = ({ name }) => {
+    const meta = formik.getFieldMeta(name);
+    console.log(name, meta);
+    if (!meta.error) {
+      return null;
+    }
+
+    return (
+      <label htmlFor={name} className="text-red-500">
+        {meta.error}
+      </label>
+    );
   };
 
   const addVariable = () => {
-    reducerDispatch({
+    const newVariable = {
-      type: 'ADD_VAR'
+      uid: uuid(),
-    });
+      name: '',
+      value: '',
+      type: 'text',
+      secret: false,
+      enabled: true
+    };
+    formik.setFieldValue(formik.values.length, newVariable, false);
   };
 
-  const handleVarChange = (e, _variable, type) => {
+  const handleRemoveVar = (id) => {
-    const variable = cloneDeep(_variable);
+    formik.setValues(formik.values.filter((variable) => variable.uid !== id));
-    switch (type) {
-      case 'name': {
-        variable.name = e.target.value;
-        break;
-      }
-      case 'value': {
-        variable.value = e.target.value;
-        break;
-      }
-      case 'enabled': {
-        variable.enabled = e.target.checked;
-        break;
-      }
-      case 'secret': {
-        variable.secret = e.target.checked;
-        break;
-      }
-    }
-    reducerDispatch({
-      type: 'UPDATE_VAR',
-      variable
-    });
-  };
-
-  const handleRemoveVars = (variable) => {
-    reducerDispatch({
-      type: 'DELETE_VAR',
-      variable
-    });
   };
 
   return (
@@ -78,55 +89,57 @@ const EnvironmentVariables = ({ environment, collection }) => {
           </tr>
         </thead>
         <tbody>
-          {variables && variables.length
+          {formik.values.map((variable, index) => (
-            ? variables.map((variable, index) => {
+            <tr key={variable.uid}>
-                return (
+              <td className="text-center">
-                  <tr key={variable.uid}>
+                <input
-                    <td className="text-center">
+                  type="checkbox"
-                      <input
+                  className="mr-3 mousetrap"
-                        type="checkbox"
+                  name={`${index}.enabled`}
-                        checked={variable.enabled}
+                  checked={variable.enabled}
-                        className="mr-3 mousetrap"
+                  onChange={formik.handleChange}
-                        onChange={(e) => handleVarChange(e, variable, 'enabled')}
+                />
-                      />
+              </td>
-                    </td>
+              <td>
-                    <td>
+                <input
-                      <input
+                  type="text"
-                        type="text"
+                  autoComplete="off"
-                        autoComplete="off"
+                  autoCorrect="off"
-                        autoCorrect="off"
+                  autoCapitalize="off"
-                        autoCapitalize="off"
+                  spellCheck="false"
-                        spellCheck="false"
+                  className="mousetrap"
-                        value={variable.name}
+                  id={`${index}.name`}
-                        className="mousetrap"
+                  name={`${index}.name`}
-                        onChange={(e) => handleVarChange(e, variable, 'name')}
+                  value={formik.values[index].name}
-                      />
+                  onChange={formik.handleChange}
-                    </td>
+                />
-                    <td>
+                <ErrorMessage name={`${index}.name`} />
-                      <SingleLineEditor
+              </td>
-                        value={variable.value}
+              <td>
-                        theme={storedTheme}
+                <SingleLineEditor
-                        onChange={(newValue) => handleVarChange({ target: { value: newValue } }, variable, 'value')}
+                  theme={storedTheme}
-                        collection={collection}
+                  collection={collection}
-                      />
+                  name={`${index}.value`}
-                    </td>
+                  value={variable.value}
-                    <td>
+                  onChange={(newValue) => formik.setFieldValue(`${index}.value`, newValue, true)}
-                      <input
+                />
-                        type="checkbox"
+              </td>
-                        checked={variable.secret}
+              <td>
-                        className="mr-3 mousetrap"
+                <input
-                        onChange={(e) => handleVarChange(e, variable, 'secret')}
+                  type="checkbox"
-                      />
+                  className="mr-3 mousetrap"
-                    </td>
+                  name={`${index}.secret`}
-                    <td>
+                  checked={variable.secret}
-                      <button onClick={() => handleRemoveVars(variable)}>
+                  onChange={formik.handleChange}
-                        <IconTrash strokeWidth={1.5} size={20} />
+                />
-                      </button>
+              </td>
-                    </td>
+              <td>
-                  </tr>
+                <button onClick={() => handleRemoveVar(variable.uid)}>
-                );
+                  <IconTrash strokeWidth={1.5} size={20} />
-              })
+                </button>
-            : null}
+              </td>
+            </tr>
+          ))}
         </tbody>
       </table>
 
@@ -137,12 +150,7 @@ const EnvironmentVariables = ({ environment, collection }) => {
       </div>
 
       <div>
-        <button
+        <button type="submit" className="submit btn btn-md btn-secondary mt-2" onClick={formik.handleSubmit}>
-          type="submit"
-          className="submit btn btn-md btn-secondary mt-2"
-          disabled={!hasChanges}
-          onClick={saveChanges}
-        >
           Save
         </button>
       </div>

packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/reducer.js

@@ -1,52 +0,0 @@
-import produce from 'immer';
-import find from 'lodash/find';
-import filter from 'lodash/filter';
-import { uuid } from 'utils/common';
-
-const reducer = (state, action) => {
-  switch (action.type) {
-    case 'ADD_VAR': {
-      return produce(state, (draft) => {
-        draft.variables.push({
-          uid: uuid(),
-          name: '',
-          value: '',
-          type: 'text',
-          secret: false,
-          enabled: true
-        });
-        draft.hasChanges = true;
-      });
-    }
-
-    case 'UPDATE_VAR': {
-      return produce(state, (draft) => {
-        const variable = find(draft.variables, (v) => v.uid === action.variable.uid);
-        variable.name = action.variable.name;
-        variable.value = action.variable.value;
-        variable.enabled = action.variable.enabled;
-        variable.secret = action.variable.secret;
-        draft.hasChanges = true;
-      });
-    }
-
-    case 'DELETE_VAR': {
-      return produce(state, (draft) => {
-        draft.variables = filter(draft.variables, (v) => v.uid !== action.variable.uid);
-        draft.hasChanges = true;
-      });
-    }
-
-    case 'CHANGES_SAVED': {
-      return produce(state, (draft) => {
-        draft.hasChanges = false;
-      });
-    }
-
-    default: {
-      return state;
-    }
-  }
-};
-
-export default reducer;

packages/bruno-app/src/components/RequestPane/Vars/VarsTable/index.js

@@ -8,6 +8,7 @@ import { sendRequest, saveRequest } from 'providers/ReduxStore/slices/collection
 import SingleLineEditor from 'components/SingleLineEditor';
 import Tooltip from 'components/Tooltip';
 import StyledWrapper from './StyledWrapper';
+import toast from 'react-hot-toast';
 
 const VarsTable = ({ item, collection, vars, varType }) => {
   const dispatch = useDispatch();
@@ -29,7 +30,19 @@ const VarsTable = ({ item, collection, vars, varType }) => {
     const _var = cloneDeep(v);
     switch (type) {
       case 'name': {
-        _var.name = e.target.value;
+        const value = e.target.value;
+
+        if (/^(?!\d).*$/.test(value) === false) {
+          toast.error('Variable names must not start with a number!');
+          return;
+        }
+
+        if (/^\w*$/.test(value) === false) {
+          toast.error('Variable contains invalid character! Variables must only contain alpha-numeric characters.');
+          return;
+        }
+
+        _var.name = value;
         break;
       }
       case 'value': {
@@ -88,7 +101,7 @@ const VarsTable = ({ item, collection, vars, varType }) => {
         </thead>
         <tbody>
           {vars && vars.length
-            ? vars.map((_var, index) => {
+            ? vars.map((_var) => {
                 return (
                   <tr key={_var.uid}>
                     <td>

packages/bruno-app/src/components/RequestPane/Vars/index.js

@@ -1,42 +1,12 @@
 import React from 'react';
 import get from 'lodash/get';
-import { useDispatch } from 'react-redux';
-import { updateRequestScript, updateResponseScript } from 'providers/ReduxStore/slices/collections';
-import { sendRequest, saveRequest } from 'providers/ReduxStore/slices/collections/actions';
-import { useTheme } from 'providers/Theme';
 import VarsTable from './VarsTable';
 import StyledWrapper from './StyledWrapper';
 
 const Vars = ({ item, collection }) => {
-  const dispatch = useDispatch();
   const requestVars = item.draft ? get(item, 'draft.request.vars.req') : get(item, 'request.vars.req');
   const responseVars = item.draft ? get(item, 'draft.request.vars.res') : get(item, 'request.vars.res');
 
-  const { storedTheme } = useTheme();
-
-  const onRequestScriptEdit = (value) => {
-    dispatch(
-      updateRequestScript({
-        script: value,
-        itemUid: item.uid,
-        collectionUid: collection.uid
-      })
-    );
-  };
-
-  const onResponseScriptEdit = (value) => {
-    dispatch(
-      updateResponseScript({
-        script: value,
-        itemUid: item.uid,
-        collectionUid: collection.uid
-      })
-    );
-  };
-
-  const onRun = () => dispatch(sendRequest(item, collection.uid));
-  const onSave = () => dispatch(saveRequest(item.uid, collection.uid));
-
   return (
     <StyledWrapper className="w-full flex flex-col">
       <div className="flex-1 mt-2">

packages/bruno-js/src/bru.js

@@ -59,10 +59,24 @@ class Bru {
       throw new Error('Key is required');
     }
 
+    if (/^(?!\d)\w*$/.test(key) === false) {
+      throw new Error(
+        `Variable name: "${key}" contains invalid characters!` +
+          ' Names must only contain alpha-numeric characters and cannot start with a digit.'
+      );
+    }
+
     this.collectionVariables[key] = value;
   }
 
   getVar(key) {
+    if (/^(?!\d)\w*$/.test(key) === false) {
+      throw new Error(
+        `Variable name: "${key}" contains invalid characters!` +
+          ' Names must only contain alpha-numeric characters and cannot start with a digit.'
+      );
+    }
+
     return this.collectionVariables[key];
   }
 }

packages/bruno-lang/v2/tests/fixtures/request.bru

@@ -66,7 +66,7 @@ body:sparql {
 
 body:form-urlencoded {
   apikey: secret
-  numbers: +91998877665
+  numbers: %2B91998877665
   ~message: hello
 }