Fixing count default sort (feathersjs-ecosystem#151)

* Clone params.knex before adding default sort. Fixes feathersjs-ecosystem#141 * Pass `hook.error` to `trx.rollback()` for more descriptive errors * fixed customizing query section * added test for customized params.knex * commented out console.log in customized hook * Linter fixed formatting issues, added missing catch for countQuery
Vasilcenko · Mar 26, 2018 · cd84f45 · cd84f45
1 parent 609377d
commit cd84f45
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 29 deletions.
diff --git a/README.md b/README.md
@@ -247,12 +247,13 @@ Combined with `.createQuery({ query: {...} })`, which returns a new KnexJS query
 app.service('mesages').hooks({
   before: {
     find(context) {
-      const query = this.createQuery({ query: context.params.query });
+      const query = this.createQuery(context.params);
 
       // do something with query here
       query.orderBy('name', 'desc');
 
       context.params.knex = query;
+      return context;
     }
   }
 });

diff --git a/lib/hooks.js b/lib/hooks.js
@@ -48,7 +48,7 @@ const rollback = (options) => {
   return hook => {
     if (hook.params.transaction) {
       const { trx, id } = hook.params.transaction;
-      return trx.rollback()
+      return trx.rollback(hook.error)
         .then(() => debug('rolling back transaction %s', id))
         .then(() => hook);
     }

diff --git a/lib/index.js b/lib/index.js
@@ -133,19 +133,19 @@ class Service {
 
   _find (params, count, getFilter = filterQuery) {
     const { filters, query } = getFilter(params.query || {});
-    const q = params.knex || this.createQuery(params);
+    const q = params.knex ? params.knex.clone() : this.createQuery(params);
 
     // Handle $limit
     if (filters.$limit) {
       q.limit(filters.$limit);
     }
-    
+
     // Handle $skip
     if (filters.$skip) {
       q.offset(filters.$skip);
 
       // provide default sorting if its not set
-      if(!filters.$sort){
+      if (!filters.$sort) {
         q.orderBy(this.id, 'asc');
       }
     }
@@ -175,13 +175,13 @@ class Service {
     if (count) {
       let countQuery =
         (params.knex || this.db(params))
-        .clone()
-        .clearSelect()
-        .count(`${this.table}.${this.id} as total`);
+          .clone()
+          .clearSelect()
+          .count(`${this.table}.${this.id} as total`);
 
-      this.knexify(countQuery, query);
+      if (!params.knex) { this.knexify(countQuery, query); }
 
-      return countQuery.then(count => count[0].total).then(executeQuery);
+      return countQuery.then(count => count[0].total).then(executeQuery).catch(errorHandler);
     }
 
     return executeQuery().catch(errorHandler);
@@ -243,7 +243,7 @@ class Service {
     // we create a list of the ids of all items that will be changed
     // to re-query them after the update
     const ids = id === null ? this._find(params)
-        .then(mapIds) : Promise.resolve([ id ]);
+      .then(mapIds) : Promise.resolve([ id ]);
 
     if (id !== null) {
       query[this.id] = id;

diff --git a/test/index.test.js b/test/index.test.js
@@ -31,24 +31,28 @@ const peopleId = service({
 });
 
 function clean () {
-  return people.init({}, (table) => {
-    table.increments('id');
-    table.string('name');
-    table.integer('age');
-    table.integer('time');
-    table.boolean('created');
-    return table;
-  })
-  .then(() => {
-    return peopleId.init({}, (table) => {
-      table.increments('customid');
-      table.string('name');
-      table.integer('age');
-      table.integer('time');
-      table.boolean('created');
-      return table;
-    });
-  });
+  return Promise.all([
+    db.schema.dropTableIfExists('people').then(() => {
+      return people.init({}, (table) => {
+        table.increments('id');
+        table.string('name');
+        table.integer('age');
+        table.integer('time');
+        table.boolean('created');
+        return table;
+      });
+    }),
+    db.schema.dropTableIfExists('people-customid').then(() => {
+      return peopleId.init({}, (table) => {
+        table.increments('customid');
+        table.string('name');
+        table.integer('age');
+        table.integer('time');
+        table.boolean('created');
+        return table;
+      });
+    })
+  ]);
 }
 
 describe('Feathers Knex Service', () => {
@@ -95,6 +99,36 @@ describe('Feathers Knex Service', () => {
     base(app, errors, 'people-customid', 'customid');
   });
 
+  describe('custom queries', () => {
+    before(clean);
+    before(() => {
+      app.hooks({}).service('people').hooks({
+        before: {
+          find (context) {
+            const query = this.createQuery(context.params);
+
+            // do something with query here
+            query.orderBy('name', 'desc');
+            // console.log(query.toSQL().toNative());
+
+            context.params.knex = query;
+            return context;
+          }
+        }
+      });
+    });
+    after(clean);
+    after(() => {
+      app.hooks({
+        before: transaction.start(),
+        after: transaction.end(),
+        error: transaction.rollback()
+      }).service('people').hooks({});
+    });
+
+    base(app, errors, 'people');
+  });
+
   describe('$like method', () => {
     beforeEach(() => app.service('/people').create({
       name: 'Charlie Brown',