Fix PR comments

server/api/utils/query-builder.spec.js

@@ -7,6 +7,37 @@ const { filter: filterFactory, queryBuilder: queryBuilderFactory } = require('./
 
 describe('Query builder utilities', function() {
   describe('filter', function() {
+    it('should modify a query using a value and filter functions', async function() {
+
+      const Model = Abstract.extend({});
+
+      const mockQuery = new Model(); // The base query.
+      const updatedQuery = mockQuery.clone().where('foo', 'bar'); // The updated query.
+
+      const valueFunc = stub().returns(42); // The value function that retrieves the value that will be used by the filter function.
+      const filterFunc = stub().returns(updatedQuery); // The filter function.
+      const filter = filterFactory(valueFunc, filterFunc);
+
+      const mockContext = {
+        get: stub().returns(mockQuery),
+        set: stub()
+      };
+
+      await expect(filter(mockContext)).to.eventually.equal(undefined);
+
+      expect(valueFunc).to.have.been.calledWithExactly(mockContext);
+      expect(valueFunc).to.have.callCount(1);
+      expect(mockContext.get).to.have.been.calledWithExactly('query');
+      expect(mockContext.get).to.have.been.calledImmediatelyAfter(valueFunc);
+      expect(mockContext.get).to.have.callCount(1);
+      expect(filterFunc).to.have.been.calledWithExactly(mockQuery, 42, mockContext);
+      expect(filterFunc).to.have.been.calledImmediatelyAfter(mockContext.get);
+      expect(filterFunc).to.have.callCount(1);
+      expect(mockContext.set).to.have.been.calledWithExactly('query', updatedQuery); // Make sure the query was updated.
+      expect(mockContext.set).to.have.been.calledImmediatelyAfter(filterFunc);
+      expect(mockContext.set).to.have.callCount(1);
+    });
+
     it('should not call the filter function or modify the query if the value function returns undefined', async function() {
 
       const valueFunc = stub(); // Returns undefined.

server/models/abstract.js

@@ -25,9 +25,7 @@ const Abstract = db.bookshelf.Model.extend({
    *
    * @see {@link https://github.com/MediaComem/orm-query-builder}
    */
-  queryBuilder: function(options) {
-    return queryBuilder(_.extend(options, { baseQuery: this }));
-  },
+  queryBuilder: createQueryBuilder,
 
   /**
    * Returns a query builder initialized with this record as a base query and with pagination
@@ -41,9 +39,7 @@ const Abstract = db.bookshelf.Model.extend({
    *
    * @see {@link https://github.com/MediaComem/orm-query-builder}
    */
-  paginatedQueryBuilder: function(options = {}) {
-    return this.queryBuilder(_.omit(options, 'pagination')).use(pagination(options.pagination));
-  },
+  paginatedQueryBuilder: createPaginatedQueryBuilder,
 
   /**
    * Parses data from the specified source into this record's columns.
@@ -178,9 +174,7 @@ const Abstract = db.bookshelf.Model.extend({
    *
    * @see {@link https://github.com/MediaComem/orm-query-builder}
    */
-  queryBuilder: function(options) {
-    return queryBuilder(_.extend(options, { baseQuery: this }));
-  },
+  queryBuilder: createQueryBuilder,
 
   /**
    * Returns a query builder initialized with this model as a base query and with pagination
@@ -194,9 +188,15 @@ const Abstract = db.bookshelf.Model.extend({
    *
    * @see {@link https://github.com/MediaComem/orm-query-builder}
    */
-  paginatedQueryBuilder: function(options = {}) {
-    return this.queryBuilder(_.omit(options, 'pagination')).use(pagination(options.pagination));
-  }
+  paginatedQueryBuilder: createPaginatedQueryBuilder
 });
 
+function createQueryBuilder(options) {
+  return queryBuilder(_.extend(options, { baseQuery: this }));
+}
+
+function createPaginatedQueryBuilder(options = {}) {
+  return this.queryBuilder(_.omit(options, 'pagination')).use(pagination(options.pagination));
+}
+
 module.exports = Abstract;

server/models/abstract.spec.js

@@ -31,7 +31,7 @@ describe('Abstract model', function() {
 
     // An ORM Query Builder plugin that adds an "ORDER BY name" clause to the query.
     const orderByName = {
-      use: builder => builder.before('end', context => context.set('query', context.get('query').query(qb => qb.orderBy('name'))))
+      use: builder => builder.before('end', context => context.set('query', context.get('query').orderBy('name')))
     };
 
     /**

server/spec/fixtures/express.js

@@ -1,8 +1,24 @@
+/**
+ * Test utilities to mock Express objects.
+ *
+ * @module server/spec/fixtures/express
+ */
 const chance = require('chance').Chance();
 const { stub } = require('sinon');
 
 const app = require('../../app');
 
+/**
+ * Returns an object that looks like an Express request and that has various Sinon stubs ready for testing.
+ *
+ * @param {object} [data] - Optional request properties to override the defaults.
+ * @param {Application} [data.app] - The Express application, defaults to the project's application.
+ * @param {Function} [data.get] - A function that retrieves a header's value(s) by name (defaults to a stub).
+ * @param {string} [data.method] - The HTTP method (defaults to `GET`).
+ * @param {string} [data.path] - The path of the URL (defaults to a random path, e.g. `/foo`).
+ * @param {object} [data.query] - The query parameters (defaults to an empty object).
+ * @returns {object} A mocked Express request.
+ */
 exports.req = function(data = {}) {
   return {
     app: data.app || app,
@@ -14,6 +30,15 @@ exports.req = function(data = {}) {
   };
 };
 
+/**
+ * Returns an object that looks like an Express response and that has various Sinon stubs ready for testing.
+ *
+ * @param {object} [data] - Optional request properties to override the defaults.
+ * @param {Application} [data.app] - The Express application, defaults to the project's application.
+ * @param {Function} [data.send] - A function that sends the response to the client with an optional body (defaults to a stub).
+ * @param {Function} [data.set] - A function that sets a header's value(s) (defaults to a stub).
+ * @returns {object} A mocked Express response.
+ */
 exports.res = function(data = {}) {
   return {
     app: data.app || app,