feathersjs · daffl · Sep 22, 2016 · Sep 21, 2016 · Sep 22, 2016
diff --git a/package.json b/package.json
@@ -62,7 +62,7 @@
     "chai": "^3.4.1",
     "feathers": "^2.0.0-pre.4",
     "feathers-rest": "^1.3.0",
-    "feathers-service-tests": "^0.6.0",
+    "feathers-service-tests": "^0.8.0",
     "jshint": "^2.8.0",
     "knex": "^0.12.0",
     "mocha": "^3.0.0",

diff --git a/src/index.js b/src/index.js
@@ -37,6 +37,7 @@ class Service {
     this.id = options.id || 'id';
     this.paginate = options.paginate || {};
     this.table = options.name;
+    this.events = options.events || [];
 	}
 
   // NOTE (EK): We need this method so that we return a new query
@@ -50,26 +51,29 @@ class Service {
 	}
 
   knexify(query, params, parentKey) {
-    Object.keys(params || {}).forEach((key) => {
+    Object.keys(params || {}).forEach(key => {
       const value = params[key];
 
       if (isPlainObject(value)) {
         return this.knexify(query, value, key);
       }
 
+      // const self = this;
       const column = parentKey || key;
       const method = METHODS[key];
       const operator = OPERATORS[key] || '=';
 
-      // TODO (EK): Handle $or queries with nested specials.
-      // Right now they won't work and we'd need to start diving
-      // into nested where conditions.
       if (method) {
         if (key === '$or') {
-          return query.where(function() {
-            value.forEach(condition => this[method].call(this, condition));
-          });  
+          const self = this;
+
+          return value.forEach(condition => {
+            query[method](function() {
+              self.knexify(this, condition);
+            });
+          });
         }
+
         return query[method].call(query, column, value);
       }
 
@@ -78,13 +82,12 @@ class Service {
   }
 
 	_find(params, count, getFilter = filter) {
+    const { filters, query } = getFilter(params.query || {});
     let q = this.db().select(['*']);
-    let { filters, query } = getFilter(params.query || {});
 
 		// $select uses a specific find syntax, so it has to come first.
 		if (filters.$select) {
-      let fields = filters.$select;
-      q = this.db().select(... fields);
+      q = this.db().select(... filters.$select);
 		}
 
     // build up the knex query out of the query params
@@ -176,21 +179,32 @@ class Service {
     return this._create(data, params);
 	}
 
-	patch(id, data, params) {
-    params.query = params.query || {};
-    data = Object.assign({}, data);
+	patch(id, raw, params) {
+    const query = Object.assign({}, params.query);
+    const data = Object.assign({}, raw);
+    const patchQuery = {};
 
     if(id !== null) {
-      params.query[this.id] = id;
+      query[this.id] = id;
     }
 
-    let query = this.db();
-    this.knexify(query, params.query);
+    // Account for potentially modified data
+    Object.keys(query).forEach(key => {
+      if(query[key] !== undefined && data[key] !== undefined &&
+          typeof data[key] !== 'object') {
+        patchQuery[key] = data[key];
+      } else {
+        patchQuery[key] = query[key];
+      }
+    });
+
+    let q = this.db();
+    this.knexify(q, query);
 
     delete data[this.id];
 
-    return query.update(data).then(() => {
-      return this._find(params).then(page => {
+    return q.update(data).then(() => {
+      return this._find({ query: patchQuery }).then(page => {
         const items = page.data;
 
         if(id !== null) {

diff --git a/test/index.test.js b/test/index.test.js
@@ -4,7 +4,7 @@ import { expect } from 'chai';
 import assert from 'assert';
 import feathers from 'feathers';
 import knex from 'knex';
-import { base, orm, example } from 'feathers-service-tests';
+import { base, example } from 'feathers-service-tests';
 import { errors } from 'feathers-errors';
 import service from '../src';
 import server from '../example/app';
@@ -16,92 +16,74 @@ const db = knex({
   }
 });
 
-const app = feathers().use('/people', service({
-  Model: db,
-  name: 'people'
-}));
-
-let _ids = {};
-let people = app.service('people');
-
-function clean(done) {
-  db.schema.dropTableIfExists('people').then(() => {
-    db.schema.createTable('people', table => {
-      table.increments('id');
-      table.string('name');
-      table.integer('age');
-      table.integer('time');
-      table.boolean('created');
-    })
-    .then(() => {
-      done();
-    });
-  });
+function clean() {
+  return db.schema.dropTableIfExists('people')
+    .then(() => db.schema.dropTableIfExists('people-customid'))
+    .then(() =>
+      db.schema.createTable('people', table => {
+        table.increments('id');
+        table.string('name');
+        table.integer('age');
+        table.integer('time');
+        table.boolean('created');
+      }).then(() => db.schema.createTable('people-customid', table => {
+        table.increments('customid');
+        table.string('name');
+        table.integer('age');
+        table.integer('time');
+        table.boolean('created');
+      }))
+  );
 }
 
 describe('Feathers Knex Service', () => {
+  const app = feathers().use('/people', service({
+    Model: db,
+    name: 'people',
+    events: [ 'testing' ]
+  })).use('/people-customid', service({
+    Model: db,
+    id: 'customid',
+    name: 'people-customid',
+    events: [ 'testing' ]
+  }));
+
   before(clean);
   after(clean);
 
   describe('Initialization', () => {
     describe('when missing options', () => {
-      it('throws an error', () => {
-        expect(service.bind(null)).to.throw('Knex options have to be provided');
-      });
+      it('throws an error', () =>
+        expect(service.bind(null))
+          .to.throw('Knex options have to be provided')
+      );
     });
 
     describe('when missing a Model', () => {
-      it('throws an error', () => {
-        expect(service.bind(null, {})).to.throw(/You must provide a Model/);
-      });
+      it('throws an error', () =>
+        expect(service.bind(null, {}))
+          .to.throw(/You must provide a Model/)
+      );
     });
 
     describe('when missing a table name', () => {
-      it('throws an error', () => {
-        expect(service.bind(null, { Model: {} })).to.throw('No table name specified.');
-      });
-    });
-
-    describe('when missing the id option', () => {
-      it('sets the default to be id', () => {
-        expect(people.id).to.equal('id');
-      });
-    });
-
-    describe('when missing the paginate option', () => {
-      it('sets the default to be {}', () => {
-        expect(people.paginate).to.deep.equal({});
-      });
+      it('throws an error', () =>
+        expect(service.bind(null, { Model: {} }))
+          .to.throw('No table name specified.')
+      );
     });
   });
 
   describe('Common functionality', () => {
-    beforeEach(done => {
-      people.create({
-        name: 'Doug',
-        age: 32
-      }).then(data => {
-        _ids.Doug = data.id;
-        done();
-      }, done);
-    });
-
-    afterEach(done => {
-      people.remove(_ids.Doug, {}).then(() => done(), () => done());
-    });
+    it('is CommonJS compatible', () =>
+      assert.equal(typeof require('../lib'), 'function')
+    );
 
-    it('is CommonJS compatible', () => {
-      assert.equal(typeof require('../lib'), 'function');
-    });
-
-    base(people, _ids, errors);
+    base(app, errors, 'people');
+    base(app, errors, 'people-customid', 'customid');
   });
 });
 
-describe.skip('Knex service ORM errors', () => {
-  orm(people, _ids, errors);
-});
-
 describe('Knex service example test', () => {
   after(done => server.close(() => done()));