The knex adapter's defaultTo doesn't provide default values to hooks

[jesstelford]

For example:

keystone.createList('Post', {
  fields: {
    createdAt: {
      type: DateTime,
      knexOptions: {
        defaultTo: knex => knex.raw(/* some SQL to create Date.now() */),
      },
      hooks: {
        validateInput: ({ resolvedData }) => {
          if (!resolvedData.createdAt) {
            throw new Error('No defaulted createdAt date set');
          }
        }
      }
    }
  }
});

Will always throw an error on create or update because the default isn't set until after the hooks are run.

On the other hand, using a defaultValue doesn't suffer from this issue:

keystone.createList('Post', {
  fields: {
    createdAt: {
      type: DateTime,
      // will be available as a function after #1136 is fixed
      defaultValue: () => Date.now(),
      hooks: {
        validateInput: ({ resolvedData }) => {
          if (!resolvedData.createdAt) {
            throw new Error('No defaulted createdAt date set');
          }
        }
      }
    }
  }
});

Which will not throw, because the resolvedData will have the default createdAt date.

The defaultTo option should most likely be removed from the Knex adapter.

[molomby]

You may have missed the point here.. this is entirely intended.

The defaultTo Knex option configures defaults in the DB schema. When values for a field (column) aren't specified on insert the database uses the value (or expression, etc.) specified by defaultTo to generate a value.

There's no way to expose these values to the Keystone hooks that are run before the insert is performed -- the values simply don't exist yet. These values will be available after though (in afterChange(), etc.).

There are cases where you could substitute the defaultTo option for defaultValue but mostly they solve different problems.

[jesstelford]

There's no way to expose these values to the Keystone hooks that are run before the insert is performed

The defaultValue option can now be a function (as shown in the example above). Which should enable all the same functionality as the knex-specific defaultTo (and possibly more as the defaultValue() function receives information on the input parameters, the currently logged in user, etc).

I guess to put it another way; Are there use cases for defaultTo which defaultValue() cannot handle?

My main motivation here is to move away from adapter-specific functionality where there's equivalent things available at a higher level within KS.

[molomby]

Yeah, these options looks similar from the outside but they lend themselves to very different solutions. There are two main points of differences I think -- where the value/function is evaluated and the effect defaultTo has on the DB schema.

defaultValue is evaluated on the app server (in node). So we can evaluate it before insert and used it to default values in the interface (for example). It has access other stuff in the Keystone instance (lists, config, session information, etc) and it's probably more accessible for most Keystone devs.

defaultTo is evaluated on the DB server -- a very different environment. From there we have local access to the entire DB (if we need it), ACID compliance and can be written in whatever languages the DB supports (eg. SQL, PL/pgSQL and others).

Evaluating things at the DB level also (usually) gives you a single source of truth. This is usually important for things like incrementing values and serials but can also effect things like the createdAt field in your example above -- if you're using Date.now() and also scaling your app servers, clock skew will cause createdAt values to be created out of order. Sometimes this isn't an issues, but imagine you're building a biding system (or similar). Even a 10 ms clock skew between web servers could allow one user to read a recently inserted item then insert a new item "before" it by (intentionally or otherwise) hitting a server with a slightly slower clock.

For me though, the most important difference is the resulting schema itself. Keystone defines the DB schema for it's lists but we can't assume Keystone is the only thing interacting with the database. Being able to trust the integrity of your data is incredibly valuable. To do this we need to be able to put rules and constraints around values enforced by the DB itself. Being able to assume, for example, that a record that's inserted (by Keystone or some other method) will always have a primary key, isn't something we want to give up. There's simply no way to ensure this with defaultValue alone.

TL;DR: these options are anything but "equivalent" :)

[stale]

It looks like you haven't had a response in over 3 months. Sorry about that! We've flagged this issue for special attention. It wil be manually reviewed by maintainers, not automatically closed. If you have any additional information please leave us a comment. It really helps! Thank you for you contributions. :)

[MadeByMike]

@timleslie you change the hooks function to get all the things... was this covered?

[singhArmani]

@MadeByMike @jesstelford : Hi guys, are there any updates on this? As per @molomby comment below:

There's no way to expose these values to the Keystone hooks that are run before the insert is performed -- the values simply don't exist yet.

do we still consider this as a bug?

[molomby]

The behaviour described is as intended and necessary/inherent in the way the functionality works, so not a bug, I'd say.

It is confusing there are multiple ways of defaulting things though -- the two methods discussed above but also in hooks, potentially in the interface, etc. It's possible we should make this clearer. Better docs? Clearer names for the config options? If there is a problem, I think those are the questions we should be asking.

[stale]

It looks like there hasn't been any activity here in over 6 months. Sorry about that! We've flagged this issue for special attention. It wil be manually reviewed by maintainers, not automatically closed. If you have any additional information please leave us a comment. It really helps! Thank you for you contribution. :)

[bladey]

Keystone 5 has officially moved into active maintenance mode as we push towards the next major new version Keystone Next, you can find out more information about this transition here.

In an effort to sustain the project going forward, we're cleaning up and closing old issues such as this one. If you feel this issue is still relevant for Keystone Next, please let us know.