Need a way to exclude automatically generated columns from the update method.

[jayalfredprufrock]

It's fairly common to use automatically generated columns, i.e. datetimes like created_at and updated_at which knex has good support for. The problem is the knex service won't let you update tables like this, since it automatically includes NULL for those columns. I suppose you could argue PATCH should be used in this case, but I propose another way:

A simple extra option like nonUpdateableColumns that would default to an array of [this.id] could be used to prevent the overwriting of these columns during an update, more of a generalization of what is already being done to the "id" column. A hook really wouldn't work, since you'd have to apply the removal to the GET method, however in just about every other case you would want GET to return those columns. I think building support for this use case into the service makes since. If a maintainer agrees and wants to offer up an option name, I'm happy to create a pull request.

[jayalfredprufrock]

PR: #62

[daffl]

Thank you for the comment and the PR.

I'm always a little hesitant adding new options (which also need documentation, tests and demos to be maintained and updated) if it is possible to also workaround with Feathers patterns. Currently I can see two existing ways of doing this:

1. Use patch. The id property should also be already removed and not changeable in update already.
2. Use a small hook to remove the fields that should be excluded:

const _ = require('lodash');

app.service('myservice').before({
  update(hook) {
    hook.data = _.omit(hook.data, 'id', 'updatedAt', 'createdAt');
  }
});

[jayalfredprufrock]

Thanks for getting back, I totally understand not wanting to add new options. It's likely I'll maintain my own fork anyways since I'm already extending the knex Service quite a lot.

I also appreciate the suggestions, but I'm not sure your hook would work in the update case. The issue is that during an update, it first fetches the old record, and then replaces anything not included in the data parameter with NULL, essentially erasing the updated_at and created_at fields all together. If those columns are marked as NOT NULL (which is likely), this throws an error.

Using patch + the hook could work though, but then I'd need to completely disable the PUT method which I hate to do since I think it's more common to support PUT then PATCH.

Using hooks to generate the updated_at and created_at fields is probably the best bet, since it affords the flexibility of switching to a different DB service, however I think most people using Knex have no intention of moving to a NoSQL DB and knex already provides support for all the major relational DBs.

Anyways, I appreciate your time looking in to this. I do hope you'll consider #63 though, since I think that is an especially common use case for anybody generating their own primary keys on non-pgSQL databases.

[daffl]

Ah yes, I missed that it nulls everything else out. Having update and patch behave the same (which from what I understand is what you would basically like to do) could also be done with an update hook by setting hook.result:

const _ = require('lodash');

app.service('myservice').before({
  update(hook) {
    return this.patch(hook.id, _.omit(hook.data, 'id', 'updatedAt', 'createdAt'), hook.params)
      .then(result => {
        hook.result = result;
        return hook;
      });
  }
});

This will basically skip the old update and just use the result from patch instead without having to disable PUT.

[jayalfredprufrock]

Ahh that's a neat trick, however I wouldn't want update and patch to perform exactly the same, but I suppose the hook could first perform the get request so you could still NULL out values that aren't updatedAt and createdAt. A bit hacky, but I think this will work for me. Thanks for your help!

[daffl]

True but I think this could also be done by hooks. Since there are several ways to work around the issue without adding a new configuration option though I think we can close this one.