Fix relations and graphql nested lookups for both mongo and sql

packages/strapi-hook-bookshelf/lib/buildQuery.js

@@ -1,4 +1,5 @@
 const _ = require('lodash');
+const pluralize = require('pluralize');
 
 const buildQuery = ({ model, filters }) => qb => {
   if (_.has(filters, 'where') && Array.isArray(filters.where)) {
@@ -159,7 +160,7 @@ const buildSingleJoin = (qb, strapiModel, astModel, association) => {
     // Join on both ends
     qb.innerJoin(
       association.tableCollectionName,
-      `${association.tableCollectionName}.${strapiModel.info.name}_${strapiModel.primaryKey}`,
+      `${association.tableCollectionName}.${pluralize.singular(strapiModel.collectionName)}_${strapiModel.primaryKey}`,
       `${strapiModel.collectionName}.${strapiModel.primaryKey}`
     );
 

packages/strapi-hook-bookshelf/lib/relations.js

@@ -30,11 +30,13 @@ const transformToArrayID = (array, association) => {
   return [];
 };
 
-module.exports = {
+const getModel = (model, plugin) => {
-  getModel: function (model, plugin) {
   return _.get(strapi.plugins, [plugin, 'models', model]) || _.get(strapi, ['models', model]) || undefined;
-  },
+};
 
+const removeUndefinedKeys = obj => _.pickBy(obj, _.negate(_.isUndefined));
+
+module.exports = {
   findOne: async function (params, populate) {
     const record = await this
       .forge({
@@ -69,126 +71,90 @@ module.exports = {
   },
 
   update: async function (params) {
-    const virtualFields = [];
+    const relationUpdates = [];
+    const primaryKeyValue = getValuePrimaryKey(params, this.primaryKey);
     const response = await module.exports.findOne.call(this, params);
 
     // Only update fields which are on this document.
-    const values = params.parseRelationships === false ? params.values : Object.keys(JSON.parse(JSON.stringify(params.values))).reduce((acc, current) => {
+    const values = params.parseRelationships === false ? params.values : Object.keys(removeUndefinedKeys(params.values)).reduce((acc, current) => {
+      const property = params.values[current];
       const association = this.associations.filter(x => x.alias === current)[0];
       const details = this._attributes[current];
 
-      if (_.get(this._attributes, `${current}.isVirtual`) !== true && _.isUndefined(association)) {
+      if (!association && _.get(details, 'isVirtual') !== true) {
-        acc[current] = params.values[current];
+        return _.set(acc, current, property);
-      } else {
+      }
+
+      const assocModel = getModel(details.model || details.collection, details.plugin);
       switch (association.nature) {
-          case 'oneWay':
+        case 'oneWay': {
-            acc[current] = _.get(params.values[current], this.primaryKey, params.values[current]) || null;
+          return _.set(acc, current, _.get(property, assocModel.primaryKey, property));
+        }
+        case 'oneToOne': {
+          if (response[current] === property) return acc;
 
-            break;
+          if (_.isNull(property)) {
-          case 'oneToOne':
+            const updatePromise = assocModel.where({
-            if (response[current] !== params.values[current]) {
+              [assocModel.primaryKey]: getValuePrimaryKey(response[current], assocModel.primaryKey)
-              const value = _.isNull(params.values[current]) ? response[current] : params.values;
+            }).save({ [details.via]: null }, {method: 'update', patch: true, require: false});
-              const recordId = _.isNull(params.values[current]) ? getValuePrimaryKey(value, this.primaryKey) : value[current];
 
-              const model = module.exports.getModel(details.collection || details.model, details.plugin);
+            relationUpdates.push(updatePromise);
-
+            return _.set(acc, current, null);
-              // Remove relation in the user side.
-              virtualFields.push(
-                module.exports.findOne
-                  .call(model, { [model.primaryKey]: recordId }, [details.via])
-                  .then(record => {
-                    if (record && _.isObject(record[details.via]) && record.id !== record[details.via][current]) {
-                      return module.exports.update.call(this, {
-                        id: getValuePrimaryKey(record[details.via], model.primaryKey),
-                        values: {
-                          [current]: null
-                        },
-                        parseRelationships: false
-                      });
           }
 
-                    return Promise.resolve();
+
-                  })
+          // set old relations to null
+          const updateLink = this.where({ [current]: property })
+            .save({ [current]: null }, {method: 'update', patch: true, require: false})
             .then(() => {
-                    return module.exports.update.call(model, {
+              return assocModel
-                      id: getValuePrimaryKey(response[current] || {}, this.primaryKey) || value[current],
+                .where({ [this.primaryKey]: property })
-                      values: {
+                .save({ [details.via] : primaryKeyValue}, {method: 'update', patch: true, require: false});
-                        [details.via]: null
-                      },
-                      parseRelationships: false
             });
-                  })
+
+          // set new relation
+          relationUpdates.push(updateLink);
+          return _.set(acc, current, property);
+        }
+        case 'oneToMany': {
+          // receive array of ids or array of objects with ids
+
+          // set relation to null for all the ids not in the list
+          const currentIds = response[current];
+          const diff = _.differenceWith(property, currentIds, (a, b) => {
+            return `${a[assocModel.primaryKey] || a}` === `${b[assocModel.primaryKey] || b}`;
+          });
+
+          const updatePromise = assocModel
+            .where(assocModel.primaryKey, 'in', currentIds.map(val => val[assocModel.primaryKey]||val))
+            .save({ [details.via] : null }, { method: 'update', patch: true, require: false })
             .then(() => {
-                    if (!_.isNull(params.values[current])) {
+              return assocModel
-                      return module.exports.update.call(model, {
+                .where(assocModel.primaryKey, 'in', diff.map(val => val[assocModel.primaryKey]||val))
-                        id: recordId,
+                .save({ [details.via] : primaryKeyValue }, { method: 'update', patch: true, require: false });
-                        values: {
-                          [details.via]: getValuePrimaryKey(params, this.primaryKey) || null
-                        },
-                        parseRelationships: false
             });
+
+          relationUpdates.push(updatePromise);
+          return acc;
         }
-
+        case 'manyToOne': {
-                    return Promise.resolve();
+          return _.set(acc, current, _.get(property, assocModel.primaryKey, property));
-                  })
-              );
-
-              acc[current] = _.isNull(params.values[current]) ? null : value[current];
         }
-
+        case 'manyToMany': {
-            break;
-          case 'oneToMany':
-          case 'manyToOne':
-          case 'manyToMany':
-            if (response[current] && _.isArray(response[current]) && current !== 'id') {
-              // Compare array of ID to find deleted files.
           const currentValue = transformToArrayID(response[current], association).map(id => id.toString());
           const storedValue = transformToArrayID(params.values[current], association).map(id => id.toString());
 
           const toAdd = _.difference(storedValue, currentValue);
           const toRemove = _.difference(currentValue, storedValue);
 
-              const model = module.exports.getModel(details.collection || details.model, details.plugin);
+          const collection = this.forge({ [this.primaryKey]: primaryKeyValue })[association.alias]();
+          const updatePromise = collection
+            .detach(toRemove)
+            .then(() => collection.attach(toAdd));
 
-              // Push the work into the flow process.
+          relationUpdates.push(updatePromise);
-              toAdd.forEach(value => {
+          return acc;
-                value = _.isString(value) || _.isNumber(value) ? { [this.primaryKey]: value } : value;
-
-                value[details.via] = params.values[this.primaryKey] || params[this.primaryKey];
-
-                virtualFields.push(
-                  module.exports.addRelation.call(model, {
-                    id: getValuePrimaryKey(value, this.primaryKey),
-                    values: value,
-                    foreignKey: current
-                  })
-                );
-              });
-
-              toRemove.forEach(value => {
-                value = _.isString(value) || _.isNumber(value) ? { [this.primaryKey]: value } : value;
-
-                value[details.via] = association.nature !== 'manyToMany' ?
-                  null :
-                  params.values[this.primaryKey] || params[this.primaryKey];
-
-                virtualFields.push(
-                  module.exports.removeRelation.call(model, {
-                    id: getValuePrimaryKey(value, this.primaryKey),
-                    values: value,
-                    foreignKey: current
-                  })
-                );
-              });
-            } else if (_.get(this._attributes, `${current}.isVirtual`) !== true) {
-              if (params.values[current] && typeof params.values[current] === 'object') {
-                acc[current] = _.get(params.values[current], this.primaryKey);
-              } else {
-                acc[current] = params.values[current];
         }
-            }
-
-            break;
         case 'manyMorphToMany':
         case 'manyMorphToOne':
           // Update the relational array.
@@ -200,7 +166,7 @@ module.exports = {
             // Remove existing relationship because only one file
             // can be related to this field.
             if (association.nature === 'manyMorphToOne') {
-                virtualFields.push(
+              relationUpdates.push(
                 module.exports.removeRelationMorph.call(this, {
                   alias: association.alias,
                   ref: model.collectionName,
@@ -218,7 +184,7 @@ module.exports = {
                   )
               );
             } else {
-                virtualFields.push(module.exports.addRelationMorph.call(this, {
+              relationUpdates.push(module.exports.addRelationMorph.call(this, {
                 id: response[this.primaryKey],
                 alias: association.alias,
                 ref: model.collectionName,
@@ -237,10 +203,10 @@ module.exports = {
           const toAdd = _.difference(storedValue, currentValue);
           const toRemove = _.difference(currentValue, storedValue);
 
-            const model = module.exports.getModel(details.collection || details.model, details.plugin);
+          const model = getModel(details.collection || details.model, details.plugin);
 
           toAdd.forEach(id => {
-              virtualFields.push(
+            relationUpdates.push(
               module.exports.addRelationMorph.call(model, {
                 id,
                 alias: association.via,
@@ -253,7 +219,7 @@ module.exports = {
 
           // Update the relational array.
           toRemove.forEach(id => {
-              virtualFields.push(
+            relationUpdates.push(
               module.exports.removeRelationMorph.call(model, {
                 id,
                 alias: association.via,
@@ -270,13 +236,12 @@ module.exports = {
           break;
         default:
       }
-      }
 
       return acc;
     }, {});
 
     if (!_.isEmpty(values)) {
-      virtualFields.push(
+      relationUpdates.push(
         this
           .forge({
             [this.primaryKey]: getValuePrimaryKey(params, this.primaryKey)
@@ -286,11 +251,11 @@ module.exports = {
           })
       );
     } else {
-      virtualFields.push(Promise.resolve(_.assign(response, params.values)));
+      relationUpdates.push(Promise.resolve(_.assign(response, params.values)));
     }
 
     // Update virtuals fields.
-    await Promise.all(virtualFields);
+    await Promise.all(relationUpdates);
 
     return await this
       .forge({

packages/strapi-hook-mongoose/lib/relations.js

@@ -5,7 +5,6 @@
 // Public node modules.
 const _ = require('lodash');
 const mongoose = require('mongoose');
-const util = require('util');
 
 // Utils
 const { models: { getValuePrimaryKey } } = require('strapi-utils');
@@ -14,23 +13,21 @@ const getModel = function (model, plugin) {
   return _.get(strapi.plugins, [plugin, 'models', model]) || _.get(strapi, ['models', model]) || undefined;
 };
 
+const removeUndefinedKeys = obj => _.pickBy(obj, _.negate(_.isUndefined));
+
 module.exports = {
-
-
   update: async function (params) {
     const relationUpdates = [];
     const populate = this.associations.map(x => x.alias).join(' ');
     const primaryKeyValue = getValuePrimaryKey(params, this.primaryKey);
 
     const response = await this
-      .findOne({
+      .findOne({ [this.primaryKey]: primaryKeyValue })
-        [this.primaryKey]: primaryKeyValue
-      })
       .populate(populate)
       .lean();
 
     // Only update fields which are on this document.
-    const values = params.parseRelationships === false ? params.values : Object.keys(params.values).reduce((acc, current) => {
+    const values = params.parseRelationships === false ? params.values : Object.keys(removeUndefinedKeys(params.values)).reduce((acc, current) => {
       const property = params.values[current];
       const association = this.associations.find(x => x.alias === current);
       const details = this._attributes[current];
@@ -53,21 +50,17 @@ module.exports = {
           if (_.isNull(property)) {
             const updatePromise = assocModel.updateOne({
               [assocModel.primaryKey]: getValuePrimaryKey(response[current], assocModel.primaryKey)
-            }, { [details.via]: null })
+            }, { [details.via]: null });
 
-            relationUpdates.push(updatePromise)
+            relationUpdates.push(updatePromise);
             return _.set(acc, current, null);
           }
 
           // set old relations to null
-          const updateLink = this.updateOne({
+          const updateLink = this.updateOne({ [current]: new mongoose.Types.ObjectId(property) }, { [current]: null })
-            [current]: new mongoose.Types.ObjectId(property)
-          }, { [current]: null })
             .then(() => {
-            return assocModel.updateOne({
+              return assocModel.updateOne({ [this.primaryKey]: new mongoose.Types.ObjectId(property) }, { [details.via] : primaryKeyValue});
-              [this.primaryKey]: new mongoose.Types.ObjectId(property)
+            });
-            }, { [details.via] : primaryKeyValue})
-          })
 
           // set new relation
           relationUpdates.push(updateLink);
@@ -79,8 +72,8 @@ module.exports = {
           // set relation to null for all the ids not in the list
           const currentIds = response[current];
           const diff = _.differenceWith(property, currentIds, (a, b) => {
-            `${a[assocModel.primaryKey] || a}` === `${b[assocModel.primaryKey] || b}`
+            return `${a[assocModel.primaryKey] || a}` === `${b[assocModel.primaryKey] || b}`;
-          })
+          });
 
           const updatePromise = assocModel.updateMany({
             [assocModel.primaryKey]: {
@@ -92,17 +85,16 @@ module.exports = {
                 [assocModel.primaryKey]: {
                   $in: diff.map(val => new mongoose.Types.ObjectId(val[assocModel.primaryKey]||val))
                 }
-            }, { [details.via] : primaryKeyValue })
+              }, { [details.via] : primaryKeyValue });
-          })
+            });
 
-          relationUpdates.push(updatePromise)
+          relationUpdates.push(updatePromise);
           return acc;
         }
         case 'manyToOne': {
           return _.set(acc, current, _.get(property, assocModel.primaryKey, property));
         }
         case 'manyToMany': {
-
           if (details.dominant) {
             return _.set(acc, current, property.map(val => val[assocModel.primaryKey] || val));
           }
@@ -120,76 +112,12 @@ module.exports = {
                   $in: property.map(val => new mongoose.Types.ObjectId(val[assocModel.primaryKey] || val))
                 }
               }, {
-              $addToSet: { [association.via]: [primaryKeyValue] }
+                $addToSet: { [association.via]: [primaryKeyValue] }
-            })
+              });
-          })
+            });
 
           relationUpdates.push(updatePomise);
           return acc;
-
-
-          // TODO: handle concat or remove from current
-          if (association.nature === 'manyToMany' && details.dominant === true) {
-            return _.set(acc, current, property);
-          }
-
-          if (response[current] && _.isArray(response[current]) && current !== 'id') {
-            // Records to add in the relation.
-            const toAdd = _.differenceWith(property, response[current], (a, b) =>
-              (a[this.primaryKey] || a).toString() === (b[this.primaryKey] || b).toString()
-            );
-
-            // Records to remove in the relation.
-            const toRemove = _.differenceWith(response[current], property, (a, b) =>
-              (a[this.primaryKey] || a).toString() === (b[this.primaryKey] || b).toString()
-            )
-              .filter(x => toAdd.find(y => x.id === y.id) === undefined);
-
-            const model = getModel(details.model || details.collection, details.plugin);
-
-            // Push the work into the flow process.
-            toAdd.forEach(value => {
-              value = _.isString(value) ? { [this.primaryKey]: value } : value;
-
-              if (association.nature === 'manyToMany' && !_.isArray(params.values[this.primaryKey] || params[this.primaryKey])) {
-                value[details.via] = (value[details.via] || [])
-                  .concat([(params.values[this.primaryKey] || params[this.primaryKey])])
-                  .filter(x => {
-                    return x !== null && x !== undefined;
-                  });
-              } else {
-                value[details.via] = getValuePrimaryKey(params, this.primaryKey);
-              }
-
-              relationUpdates.push(
-                module.exports.addRelation.call(model, {
-                  id: getValuePrimaryKey(value, this.primaryKey),
-                  values: _.pick(value, [this.primaryKey, details.via]),
-                  foreignKey: current
-                })
-              );
-            });
-
-            toRemove.forEach(value => {
-              value = _.isString(value) ? { [this.primaryKey]: value } : value;
-
-              if (association.nature === 'manyToMany' && !_.isArray(params.values[this.primaryKey] || params[this.primaryKey])) {
-                value[details.via] = value[details.via].filter(x => _.toString(x) !== _.toString(params.values[this.primaryKey] || params[this.primaryKey]));
-              } else {
-                value[details.via] = null;
-              }
-
-              relationUpdates.push(
-                module.exports.removeRelation.call(model, {
-                  id: getValuePrimaryKey(value, this.primaryKey),
-                  values: _.pick(value, [this.primaryKey, details.via]),
-                  foreignKey: current
-                })
-              );
-            });
-
-            return acc;
-          }
         }
         case 'manyMorphToMany':
         case 'manyMorphToOne':

packages/strapi-plugin-content-manager/services/ContentManager.js

@@ -162,7 +162,8 @@ module.exports = {
     }
 
     params[primaryKey] = response[primaryKey];
-    params.values = Object.keys(JSON.parse(JSON.stringify(response))).reduce((acc, current) => {
+    
+    params.values = Object.keys(response).reduce((acc, current) => {
       const association = (strapi.models[params.model] || strapi.plugins[source].models[params.model]).associations.filter(x => x.alias === current)[0];
 
       // Remove relationships.

packages/strapi-plugin-graphql/services/Loaders.js

@@ -58,6 +58,7 @@ module.exports = {
             const { queries, map } = this.extractQueries(model, _.cloneDeep(keys));
             // Run queries in parallel.
             const results = await Promise.all(queries.map(query => this.makeQuery(model, query)));
+
             // Use to match initial queries order.
             const data = this.mapData(model, keys, map, results);
 

packages/strapi-plugin-graphql/services/Resolvers.js

@@ -369,8 +369,8 @@ const buildShadowCRUD = (models, plugin) => {
               ...Query.convertToQuery(queryParams.where),
             };
 
-            if (association.nature === 'manyToMany' && association.dominant) {
+            if (model.orm === 'mongoose' && association.nature === 'manyToMany' && association.dominant) {
-              _.set(queryOpts, ['query', ref.primaryKey], obj[association.alias] || []);
+              _.set(queryOpts, ['query', ref.primaryKey], obj[association.alias].map(val => val[ref.primaryKey] || val) || []);
             }
 
             _.set(queryOpts, ['query', association.via], obj[ref.primaryKey]);

packages/strapi-plugin-graphql/test/graphqlRelations.test.e2e.js

@@ -8,7 +8,7 @@ let rq;
 let graphqlQuery;
 
 // utils
-const selectFields = doc => _.pick(doc[('id', 'name')]);
+const selectFields = doc => _.pick(doc, ['id', 'name']);
 
 const documentModel = {
   attributes: [