fix: frappe.registerModels

- Directly register from models object, not models.models
- frappe.getModels
- Validate model before registering
- Fix model.extend bug for duplicate fields
- Remove usage of deepmerge and use lodash/cloneDeep

index.js

@@ -1,4 +1,5 @@
 const Observable = require('./utils/observable');
+const utils = require('./utils');
 
 module.exports = {
   async init() {
@@ -34,40 +35,36 @@ module.exports = {
     common.initLibs(this);
   },
 
-  registerModels(models, type) {
+  registerModels(models) {
     // register models from app/models/index.js
-    const toAdd = Object.assign({}, models.models);
+    for (let doctype in models) {
+      let metaDefinition = models[doctype];
+      if (!metaDefinition.name) {
+        throw new Error(`Name is mandatory for ${doctype}`);
+      }
+      if (metaDefinition.name !== doctype) {
+        throw new Error(
+          `Model name mismatch for ${doctype}: ${metaDefinition.name}`
+        );
+      }
+      let fieldnames = (metaDefinition.fields || []).map(df => df.fieldname).sort();
+      let duplicateFieldnames = utils.getDuplicates(fieldnames);
+      if (duplicateFieldnames.length > 0) {
+        throw new Error(
+          `Duplicate fields in ${doctype}: ${duplicateFieldnames.join(', ')}`
+        );
+      }
 
-    // post process based on type
-    if (models[type]) {
-      models[type](toAdd);
+      this.models[doctype] = metaDefinition;
     }
-
-    Object.assign(this.models, toAdd);
   },
 
-  getDoctypeList(filters) {
-    let doctypeList = [];
-    if (filters && Object.keys(filters).length) {
-      for (let model in this.models) {
-        let doctypeName = model;
-        let doctype = this.models[doctypeName];
-        let matchedFields = 0;
-        for (let key in filters) {
-          let field = key;
-          let value = filters[field];
-
-          if (Boolean(doctype[field]) === Boolean(value)) {
-            matchedFields++;
-          }
-        }
-
-        if (matchedFields === Object.keys(filters).length)
-          doctypeList.push(doctypeName);
-      }
+  getModels(filterFunction) {
+    let models = [];
+    for (let doctype in this.models) {
+      models.push(this.models[doctype]);
     }
-
-    return doctypeList;
+    return filterFunction ? models.filter(filterFunction) : models;
   },
 
   registerView(view, name, module) {
@@ -138,7 +135,7 @@ module.exports = {
   removeFromCache(doctype, name) {
     try {
       delete this.docs[doctype][name];
-    } catch(e) {
+    } catch (e) {
       console.warn(`Document ${doctype} ${name} does not exist`);
     }
   },

model/index.js

@@ -1,71 +1,114 @@
-const deepmerge = require('deepmerge');
+const cloneDeep = require('lodash/cloneDeep');
 
 module.exports = {
-    extend: (base, target, options = {}) => {
-        base = deepmerge({}, base);
-        const fieldsToMerge = (target.fields || []).map(df => df.fieldname);
-        const fieldsToRemove = options.skipFields || [];
+  extend: (base, target, options = {}) => {
+    base = cloneDeep(base);
+    const fieldsToMerge = (target.fields || []).map(df => df.fieldname);
+    const fieldsToRemove = options.skipFields || [];
+    const overrideProps = options.overrideProps || [];
+    for (let prop of overrideProps) {
+      if (base.hasOwnProperty(prop)) {
+        delete base[prop];
+      }
+    }
 
-        base.fields = base.fields
-            .filter(df => !fieldsToRemove.includes(df.fieldname))
-            .map(df => {
-                if (fieldsToMerge.includes(df.fieldname)) {
-                    return deepmerge(df, target.fields.find(tdf => tdf.fieldname === df.fieldname));
-                }
-                return df;
-            });
+    let mergeFields = (baseFields, targetFields) => {
+      let fields = cloneDeep(baseFields);
+      fields = fields
+        .filter(df => !fieldsToRemove.includes(df.fieldname))
+        .map(df => {
+          if (fieldsToMerge.includes(df.fieldname)) {
+            let copy = cloneDeep(df);
+            return Object.assign(
+              copy,
+              targetFields.find(tdf => tdf.fieldname === df.fieldname)
+            );
+          }
+          return df;
+        });
+      let fieldsAdded = fields.map(df => df.fieldname);
+      let fieldsToAdd = targetFields.filter(
+        df => !fieldsAdded.includes(df.fieldname)
+      );
+      return fields.concat(fieldsToAdd);
+    };
 
-        const overrideProps = options.overrideProps || [];
-        for (let prop of overrideProps) {
-            if (base.hasOwnProperty(prop)) {
-                delete base[prop];
-            }
-        }
+    let fields = mergeFields(base.fields, target.fields || []);
+    let out = Object.assign(base, target);
+    out.fields = fields;
 
-        return deepmerge(base, target);
+    return out;
+  },
+  commonFields: [
+    {
+      fieldname: 'name',
+      fieldtype: 'Data',
+      required: 1
+    }
+  ],
+  submittableFields: [
+    {
+      fieldname: 'submitted',
+      fieldtype: 'Check',
+      required: 1
+    }
+  ],
+  parentFields: [
+    {
+      fieldname: 'owner',
+      fieldtype: 'Data',
+      required: 1
     },
-    commonFields: [
-        {
-            fieldname: 'name', fieldtype: 'Data', required: 1
-        }
-    ],
-    parentFields: [
-        {
-            fieldname: 'owner', fieldtype: 'Data', required: 1
-        },
-        {
-            fieldname: 'modifiedBy', fieldtype: 'Data', required: 1
-        },
-        {
-            fieldname: 'creation', fieldtype: 'Datetime', required: 1
-        },
-        {
-            fieldname: 'modified', fieldtype: 'Datetime', required: 1
-        },
-        {
-            fieldname: 'keywords', fieldtype: 'Text'
-        }
-    ],
-    childFields: [
-        {
-            fieldname: 'idx', fieldtype: 'Int', required: 1
-        },
-        {
-            fieldname: 'parent', fieldtype: 'Data', required: 1
-        },
-        {
-            fieldname: 'parenttype', fieldtype: 'Data', required: 1
-        },
-        {
-            fieldname: 'parentfield', fieldtype: 'Data', required: 1
-        }
-    ],
-    treeFields: [
-        {
-            fieldname: 'lft', fieldtype: 'Int'
-        },
-        {
-            fieldname: 'rgt', fieldtype: 'Int'
-        }
-    ]
+    {
+      fieldname: 'modifiedBy',
+      fieldtype: 'Data',
+      required: 1
+    },
+    {
+      fieldname: 'creation',
+      fieldtype: 'Datetime',
+      required: 1
+    },
+    {
+      fieldname: 'modified',
+      fieldtype: 'Datetime',
+      required: 1
+    },
+    {
+      fieldname: 'keywords',
+      fieldtype: 'Text'
+    }
+  ],
+  childFields: [
+    {
+      fieldname: 'idx',
+      fieldtype: 'Int',
+      required: 1
+    },
+    {
+      fieldname: 'parent',
+      fieldtype: 'Data',
+      required: 1
+    },
+    {
+      fieldname: 'parenttype',
+      fieldtype: 'Data',
+      required: 1
+    },
+    {
+      fieldname: 'parentfield',
+      fieldtype: 'Data',
+      required: 1
+    }
+  ],
+  treeFields: [
+    {
+      fieldname: 'lft',
+      fieldtype: 'Int'
+    },
+    {
+      fieldname: 'rgt',
+      fieldtype: 'Int'
+    }
+  ]
 };

models/index.js

@@ -1,17 +1,15 @@
 module.exports = {
-    models: {
-        FilterItem: require('./doctype/FilterItem/FilterItem.js'),
-        FilterGroup: require('./doctype/FilterGroup/FilterGroup.js'),
-        FilterSelector: require('./doctype/FilterSelector/FilterSelector.js'),
-        NumberSeries: require('./doctype/NumberSeries/NumberSeries.js'),
-        PrintFormat: require('./doctype/PrintFormat/PrintFormat.js'),
-        Role: require('./doctype/Role/Role.js'),
-        Session: require('./doctype/Session/Session.js'),
-        SingleValue: require('./doctype/SingleValue/SingleValue.js'),
-        SystemSettings: require('./doctype/SystemSettings/SystemSettings.js'),
-        ToDo: require('./doctype/ToDo/ToDo.js'),
-        User: require('./doctype/User/User.js'),
-        UserRole: require('./doctype/UserRole/UserRole.js'),
-        File: require('./doctype/File/File.js'),
-    }
-}
+  FilterItem: require('./doctype/FilterItem/FilterItem.js'),
+  FilterGroup: require('./doctype/FilterGroup/FilterGroup.js'),
+  FilterSelector: require('./doctype/FilterSelector/FilterSelector.js'),
+  NumberSeries: require('./doctype/NumberSeries/NumberSeries.js'),
+  PrintFormat: require('./doctype/PrintFormat/PrintFormat.js'),
+  Role: require('./doctype/Role/Role.js'),
+  Session: require('./doctype/Session/Session.js'),
+  SingleValue: require('./doctype/SingleValue/SingleValue.js'),
+  SystemSettings: require('./doctype/SystemSettings/SystemSettings.js'),
+  ToDo: require('./doctype/ToDo/ToDo.js'),
+  User: require('./doctype/User/User.js'),
+  UserRole: require('./doctype/UserRole/UserRole.js'),
+  File: require('./doctype/File/File.js')
+};

package.json

@@ -23,7 +23,6 @@
     "cross-env": "^5.2.0",
     "css-loader": "^1.0.0",
     "csvjson-csv2json": "5.0.6",
-    "deepmerge": "^2.1.0",
     "electron": "5.0.0",
     "electron-builder": "^21.0.15",
     "electron-debug": "^2.0.0",
@@ -34,6 +33,7 @@
     "friendly-errors-webpack-plugin": "^1.7.0",
     "html-webpack-plugin": "^3.2.0",
     "knex": "^0.20.3",
+    "lodash": "^4.17.15",
     "luxon": "^1.0.0",
     "mkdirp": "^0.5.1",
     "morgan": "^1.9.0",

utils/index.js

@@ -81,6 +81,21 @@ function unique(list, key = it => it) {
     });
 }
 
+function getDuplicates(array) {
+  let duplicates = [];
+  for (let i in array) {
+    let previous = array[i - 1];
+    let current = array[i];
+
+    if (current === previous) {
+      if (!duplicates.includes(current)) {
+        duplicates.push(current);
+      }
+    }
+  }
+  return duplicates;
+}
+
 module.exports = {
     _,
     slug,
@@ -91,4 +106,5 @@ module.exports = {
     asyncHandler,
     range,
     unique,
-}
+    getDuplicates
+}

yarn.lock

@@ -1931,11 +1931,6 @@ deep-extend@^0.6.0:
   resolved "https://registry.yarnpkg.com/deep-extend/-/deep-extend-0.6.0.tgz#c4fa7c95404a17a9c3e8ca7e1537312b736330ac"
   integrity sha512-LOHxIOaPYdHlJRtCQfDIVZtfw/ufM8+rVj649RIHzcm/vGwQRXFt6OPqIFWsm2XEMrNIEtWR64sY1LEKD2vAOA==
 
-deepmerge@^2.1.0:
-  version "2.2.1"
-  resolved "https://registry.yarnpkg.com/deepmerge/-/deepmerge-2.2.1.tgz#5d3ff22a01c00f645405a2fbc17d0778a1801170"
-  integrity sha512-R9hc1Xa/NOBi9WRVUWg19rl1UB7Tt4kuPd+thNJgFZoxXsTz7ncaPaeIm+40oSGuP33DfMb4sZt1QIGiJzC4EA==
-
 default-gateway@^4.2.0:
   version "4.2.0"
   resolved "https://registry.yarnpkg.com/default-gateway/-/default-gateway-4.2.0.tgz#167104c7500c2115f6dd69b0a536bb8ed720552b"