use inflection to suggest relationship names (close #1665) (#1801)

This breaks existing behaviour on how relationship names are generated.

console/package.json

@@ -62,6 +62,7 @@
     "hasura-console-graphiql": "0.0.10",
     "history": "^3.0.0",
     "hoist-non-react-statics": "^1.0.3",
+    "inflection": "^1.12.0",
     "invariant": "^2.2.0",
     "isomorphic-fetch": "^2.2.1",
     "less": "^3.7.1",
@@ -69,7 +70,7 @@
     "map-props": "^1.0.0",
     "match-sorter": "^2.3.0",
     "multireducer": "^1.0.2",
-    "piping": "^0.3.0",
+    "piping": "^0.3.2",
     "prettier": "^1.16.4",
     "pretty-error": "^1.2.0",
     "prop-types": "^15.6.0",
@@ -122,7 +123,7 @@
     "@babel/runtime": "^7.0.0",
     "babel-eslint": "^9.0.0",
     "babel-loader": "^8.0.0",
-    "babel-plugin-istanbul": "^4.1.6",
+    "babel-plugin-istanbul": "^5.1.1",
     "babel-plugin-transform-react-remove-prop-types": "^0.4.10",
     "babel-plugin-typecheck": "^2.0.0",
     "better-npm-run": "^0.1.0",
@@ -131,7 +132,7 @@
     "clean-webpack-plugin": "^0.1.17",
     "concurrently": "^3.5.0",
     "css-loader": "^0.28.11",
-    "cypress": "^3.1.0",
+    "cypress": "^3.2.0",
     "dotenv": "^5.0.1",
     "eslint": "^4.19.1",
     "eslint-config-airbnb": "16.1.0",
@@ -156,7 +157,7 @@
     "lint-staged": "^6.1.1",
     "mini-css-extract-plugin": "^0.4.0",
     "node-sass": "^4.9.2",
-    "nyc": "^12.0.2",
+    "nyc": "^13.3.0",
     "optimize-css-assets-webpack-plugin": "^4.0.2",
     "react-a11y": "^0.2.6",
     "react-addons-test-utils": "^15.0.3",

console/src/components/Services/Data/TableRelationships/Actions.js

@@ -1,3 +1,5 @@
+import inflection from 'inflection';
+
 import {
   makeMigrationCall,
   loadUntrackedRelations,
@@ -377,84 +379,135 @@ const addRelViewMigrate = tableName => (dispatch, getState) => {
   }
 };
 
-const sanitizeRelName = arg =>
-  arg
-    .trim()
-    .toLowerCase()
-    .replace(/([^A-Z0-9]+)(.)/gi, function modifyRel() {
-      return arguments[2].toUpperCase();
-    });
+const sanitizeRelName = arg => arg.trim();
 
-const formRelName = relMeta => {
+const fallBackRelName = (relMeta, existingFields, iterNumber = 0) => {
+  let relName;
+  const targetTable = sanitizeRelName(relMeta.rTable);
+  if (relMeta.isObjRel) {
+    const objLCol = sanitizeRelName(relMeta.lcol.join('_'));
+    relName = `${inflection.singularize(targetTable)}_by_${objLCol}${
+      iterNumber ? '_' + iterNumber : ''
+    }`;
+  } else {
+    const arrRCol = sanitizeRelName(relMeta.rcol.join('_'));
+    relName = `${inflection.pluralize(targetTable)}_by_${arrRCol}${
+      iterNumber ? '_' + iterNumber : ''
+    }`;
+  }
+  relName = inflection.camelize(relName, true);
+  /*
+   * Recurse until a unique relationship name is found and keep prefixing an integer at the end to fix collision
+   * */
+  return relName in existingFields
+    ? fallBackRelName(relMeta, existingFields, ++iterNumber)
+    : relName;
+};
+
+const formRelName = (relMeta, existingFields) => {
   try {
     let finalRelName;
-    // remove special chars and change first letter after underscore to uppercase
     const targetTable = sanitizeRelName(relMeta.rTable);
     if (relMeta.isObjRel) {
-      const objLCol = sanitizeRelName(relMeta.lcol.join(','));
-      finalRelName = `${targetTable}By${objLCol}`;
+      finalRelName = inflection.singularize(targetTable);
     } else {
-      const arrRCol = sanitizeRelName(relMeta.rcol.join(','));
-      finalRelName =
-        `${
-          targetTable
-          // (targetTable[targetTable.length - 1] !== 's' ? 's' : '') + // add s only if the last char is not s
-        }s` + `By${arrRCol}`;
+      finalRelName = inflection.pluralize(targetTable);
     }
+
+    /* Check if it is existing, fallback to guaranteed unique name */
+    if (existingFields && finalRelName in existingFields) {
+      finalRelName = fallBackRelName(relMeta, existingFields);
+    }
+
     return finalRelName;
   } catch (e) {
     return '';
   }
 };
 
+const getExistingFieldsMap = tableSchema => {
+  const fieldMap = {};
+
+  tableSchema.relationships.forEach(tr => {
+    fieldMap[tr.rel_name] = true;
+  });
+
+  tableSchema.columns.forEach(tc => {
+    fieldMap[tc.column_name] = true;
+  });
+
+  return fieldMap;
+};
+
 const getAllUnTrackedRelations = (allSchemas, currentSchema) => {
   const tableRelMapping = allSchemas.map(table => ({
     table_name: table.table_name,
+    existingFields: getExistingFieldsMap(table),
     relations: suggestedRelationshipsRaw(table.table_name, allSchemas),
   }));
+
   const bulkRelTrack = [];
   const bulkRelTrackDown = [];
-  tableRelMapping.map(table => {
+
+  tableRelMapping.forEach(table => {
     // check relations.obj and relations.arr length and form queries
     if (table.relations.objectRel.length) {
-      table.relations.objectRel.map(indivObjectRel => {
+      table.relations.objectRel.forEach(indivObjectRel => {
+        const suggestedRelName = formRelName(
+          indivObjectRel,
+          table.existingFields
+        );
+        /* Added to ensure that fallback relationship name is created in case of tracking all relationship at once */
+        table.existingFields[suggestedRelName] = true;
         const { upQuery, downQuery } = generateRelationshipsQuery(
           indivObjectRel.tableName,
-          formRelName(indivObjectRel),
+          suggestedRelName,
           indivObjectRel.lcol,
           indivObjectRel.rTable,
           indivObjectRel.rcol,
           true,
           currentSchema
         );
+
         const objTrack = {
           upQuery,
           downQuery,
           data: indivObjectRel,
         };
+
         bulkRelTrack.push(objTrack);
       });
     }
+
     if (table.relations.arrayRel.length) {
-      table.relations.arrayRel.map(indivArrayRel => {
+      table.relations.arrayRel.forEach(indivArrayRel => {
+        const suggestedRelName = formRelName(
+          indivArrayRel,
+          table.existingFields
+        );
+        /* Added to ensure that fallback relationship name is created in case of tracking all relationship at once */
+        table.existingFields[suggestedRelName] = true;
         const { upQuery, downQuery } = generateRelationshipsQuery(
           indivArrayRel.tableName,
-          formRelName(indivArrayRel),
+          suggestedRelName,
           indivArrayRel.lcol,
           indivArrayRel.rTable,
           indivArrayRel.rcol,
           false,
           currentSchema
         );
+
         const arrTrack = {
           upQuery,
           downQuery,
           data: indivArrayRel,
         };
+
         bulkRelTrack.push(arrTrack);
       });
     }
   });
+
   return { bulkRelTrack: bulkRelTrack, bulkRelTrackDown: bulkRelTrackDown };
 };
 
@@ -538,4 +591,5 @@ export {
   formRelName,
   getAllUnTrackedRelations,
   saveRenameRelationship,
+  getExistingFieldsMap,
 };

console/src/components/Services/Data/TableRelationships/Relationships.js

@@ -10,6 +10,7 @@ import {
   resetRelationshipForm,
   relManualAddClicked,
   formRelName,
+  getExistingFieldsMap,
 } from './Actions';
 import { findAllFromRel } from '../utils';
 import { showErrorNotification } from '../Notification';
@@ -51,12 +52,15 @@ const addRelationshipCellView = (
   selectedRelationship,
   selectedRelationshipName,
   tableStyles,
-  relMetaData
+  relMetaData,
+  tableSchema
 ) => {
   const onAdd = e => {
     e.preventDefault();
     dispatch(relSelectionChanged(rel));
-    dispatch(relNameChanged(formRelName(rel)));
+    dispatch(
+      relNameChanged(formRelName(rel, getExistingFieldsMap(tableSchema)))
+    );
   };
 
   const onRelationshipNameChanged = e => {
@@ -146,7 +150,8 @@ const AddRelationship = ({
   dispatch,
   tableStyles,
 }) => {
-  // eslint-disable-line no-unused-vars
+  const cTable = allSchemas.find(t => t.table_name === tableName);
+
   const suggestedRelationshipsData = suggestedRelationshipsRaw(
     tableName,
     allSchemas
@@ -214,8 +219,10 @@ const AddRelationship = ({
   const relName = cachedRelationshipData.name
     ? cachedRelationshipData.name
     : '';
+
   const column1 = [];
   const column2 = [];
+
   suggestedRelationshipsData.objectRel.map((rel, i) => {
     column1.push(
       rel.isObjRel ? (
@@ -225,13 +232,15 @@ const AddRelationship = ({
           selectedRelationship,
           relName,
           tableStyles,
-          ['object', i]
+          ['object', i],
+          cTable
         )
       ) : (
         <td />
       )
     );
   });
+
   suggestedRelationshipsData.arrayRel.map((rel, i) => {
     column2.push(
       rel.isObjRel ? (
@@ -243,13 +252,16 @@ const AddRelationship = ({
           selectedRelationship,
           relName,
           tableStyles,
-          ['array', i]
+          ['array', i],
+          cTable
         )
       )
     );
   });
+
   const length =
     column1.length > column2.length ? column1.length : column2.length;
+
   const combinedRels = [];
   for (let i = 0; i < length; i++) {
     const objRel = column1[i] ? column1[i] : <td />;
@@ -259,6 +271,7 @@ const AddRelationship = ({
       arrRel,
     });
   }
+
   return (
     <div>
       <div>
@@ -403,7 +416,7 @@ class Relationships extends Component {
             </thead>
             <tbody>
               {getObjArrayRelationshipList(tableSchema.relationships).map(
-                (rel, i) => {
+                rel => {
                   const column1 = rel.objRel ? (
                     <RelationshipEditor
                       dispatch={dispatch}