fix: always return primary keys of associated records

src/services/query-builder.js

@@ -3,8 +3,42 @@ import { Schemas } from 'forest-express';
 import Orm from '../utils/orm';
 import Database from '../utils/database';
 
+const HAS_ONE = 'HasOne';
+const BELONGS_TO = 'BelongsTo';
+
 const { getReferenceField } = require('../utils/query');
 
+/**
+ * @param {string[]} values
+ * @returns {string[]}
+ */
+function uniqueValues(values) {
+  return Array.from(new Set(values));
+}
+
+/**
+ * @param {string} key
+ * @param {import('sequelize').Association} association
+ * @returns {string}
+ */
+function getTargetFieldName(key, association) {
+  // Defensive programming
+  if (key && association.target.tableAttributes[key]) {
+    return association.target.tableAttributes[key].fieldName;
+  }
+
+  return undefined;
+}
+
+/**
+ * @param {import('sequelize').HasOne|import('sequelize').BelongsTo} association
+ * @returns {string[]}
+ */
+function getMandatoryFields(association) {
+  return association.target.primaryKeyAttributes
+    .map((attribute) => getTargetFieldName(attribute, association));
+}
+
 function QueryBuilder(model, opts, params) {
   const schema = Schemas.schemas[model.name];
 
@@ -30,7 +64,7 @@ function QueryBuilder(model, opts, params) {
     const includes = [];
 
     Object.values(modelForIncludes.associations)
-      .filter((association) => ['HasOne', 'BelongsTo'].includes(association.associationType))
+      .filter((association) => [HAS_ONE, BELONGS_TO].includes(association.associationType))
       .forEach((association) => {
         const targetFields = Object.values(association.target.tableAttributes)
           .map((attribute) => attribute.fieldName);
@@ -45,10 +79,10 @@ function QueryBuilder(model, opts, params) {
         || explicitAttributes.length) {
           // NOTICE: For performance reasons, we only request the keys
           //         as they're the only needed fields for the interface
-          const uniqueExplicitAttributes = Array.from(new Set([
-            association.targetKey,
+          const uniqueExplicitAttributes = uniqueValues([
+            ...getMandatoryFields(association),
             ...explicitAttributes,
-          ].filter(Boolean)));
+          ].filter(Boolean));
 
           const attributes = explicitAttributes.length
             ? uniqueExplicitAttributes

test/services/query-builder.test.js

@@ -28,64 +28,66 @@ describe('services > query-builder', () => {
   });
 
   describe('getIncludes', () => {
-    describe('with a hasOne relationship', () => {
-      function setup() {
-        const target = {
-          tableAttributes: {
-            id: { field: 'Uid', fieldName: 'uid' },
-            name: { field: 'Name', fieldName: 'name' },
-          },
-          unscoped: () => ({ name: 'user' }),
-        };
-
-        const association = {
-          associationType: 'HasOne',
-          as: 'user',
-          associationAccessor: 'userAccessor',
-          target,
-          targetKey: 'uid',
-          sourceKey: 'id',
-        };
-
-        const model = {
-          name: 'address',
-          associations: [association],
-        };
-
-        const sequelizeOptions = { sequelize: Sequelize };
-        Interface.Schemas = { schemas: { actor: { idField: 'id' } } };
-
-        const queryBuilder = new QueryBuilder(model, sequelizeOptions, {});
-
-        return {
-          association, model, target, queryBuilder,
-        };
-      }
-
-      it('should exclude field names that do not exist on the table', async () => {
-        expect.assertions(1);
-        const { model, queryBuilder } = setup();
-
-        const includes = queryBuilder.getIncludes(model, ['user.uid', 'user.name', 'user.id', 'user.badField']);
-
-        expect(includes).toStrictEqual([{
-          as: 'userAccessor',
-          attributes: ['uid', 'name'],
-          model: { name: 'user' },
-        }]);
-      });
-
-      it('should always include the target key even if not specified', () => {
-        expect.assertions(1);
-        const { model, queryBuilder } = setup();
-
-        const includes = queryBuilder.getIncludes(model, ['user.name', 'user.id', 'user.badField']);
-
-        expect(includes).toStrictEqual([{
-          as: 'userAccessor',
-          attributes: ['uid', 'name'],
-          model: { name: 'user' },
-        }]);
+    ['HasOne', 'BelongsTo'].forEach((associationType) => {
+      describe(`with a ${associationType} relationship`, () => {
+        function setup() {
+          const target = {
+            primaryKeyAttributes: ['id'],
+            tableAttributes: {
+              id: { field: 'Uid', fieldName: 'uid' },
+              name: { field: 'Name', fieldName: 'name' },
+            },
+            unscoped: () => ({ name: 'user' }),
+          };
+
+          const association = {
+            associationType,
+            as: 'user',
+            associationAccessor: 'userAccessor',
+            target,
+            sourceKey: 'id',
+          };
+
+          const model = {
+            name: 'address',
+            associations: [association],
+          };
+
+          const sequelizeOptions = { sequelize: Sequelize };
+          Interface.Schemas = { schemas: { actor: { idField: 'id' } } };
+
+          const queryBuilder = new QueryBuilder(model, sequelizeOptions, {});
+
+          return {
+            association, model, target, queryBuilder,
+          };
+        }
+
+        it('should exclude field names that do not exist on the table', async () => {
+          expect.assertions(1);
+          const { model, queryBuilder } = setup();
+
+          const includes = queryBuilder.getIncludes(model, ['user.uid', 'user.name', 'user.id', 'user.badField']);
+
+          expect(includes).toStrictEqual([{
+            as: 'userAccessor',
+            attributes: ['uid', 'name'],
+            model: { name: 'user' },
+          }]);
+        });
+
+        it('should always include the primary key even if not specified', () => {
+          expect.assertions(1);
+          const { model, queryBuilder } = setup();
+
+          const includes = queryBuilder.getIncludes(model, ['user.name', 'user.id', 'user.badField']);
+
+          expect(includes).toStrictEqual([{
+            as: 'userAccessor',
+            attributes: ['uid', 'name'],
+            model: { name: 'user' },
+          }]);
+        });
       });
     });
   });