Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding db level config for knex adapter #1383

Merged
merged 3 commits into from Jul 10, 2019

Conversation

@molomby
Copy link
Member

commented Jul 9, 2019

Adding knexOptions to the KnexFieldAdapter to support DB-level config for nullability (isNotNullable) and defaults (defaultTo). These are defaulted based on their KS-level equivalents (isRequired and defaultValue).

Also updated existing Knex field adapters to use new db-level config options. This switches isRequired flag for isNotNullable and adds db-level defaulting to all fields (for which it makes sense).

I'm expecting some of the naming to be a little spicy for some 馃尪

@changeset-bot

This comment has been minimized.

Copy link

commented Jul 9, 2019

馃 Changeset is good to go

Latest commit: 1806416

We got this.

Not sure what this means? Click here to learn what changesets are.

@molomby molomby requested review from timleslie, JedWatson and jesstelford Jul 9, 2019

@molomby molomby force-pushed the molomby/db-level-config-for-knex-adapter branch from e9821a0 to fb463f9 Jul 10, 2019

@molomby

This comment has been minimized.

Copy link
Member Author

commented Jul 10, 2019

This is now dependant on #1385; it should be merged first

@molomby molomby force-pushed the molomby/db-level-config-for-knex-adapter branch from fb463f9 to 600aea3 Jul 10, 2019

@molomby

This comment has been minimized.

Copy link
Member Author

commented Jul 10, 2019

Usage

This..

keystone.createList('User', {
  fields: {
    name: { type: Text },

    // Here we're setting a default in KS but preventing the adapter from also using
    // the value as column default
    email: {
      type: Text,
      defaultValue: 'noone@example.com',
      knexOptions: { defaultTo: null }
    },

    // Both isRequired and the defaultValue will carry though to the DB as `not null` 
    // and `default false`
    isEnabled: {
      type: Checkbox,
      isRequired: true,
      defaultValue: true,
    },

    // This field is required according to KS but we'll keep the field nullable 
    // (eg. say there's a legacy integration that adds users to the DB directly)
    password: {
      type: Password,
      isRequired: true,
      knexOptions: { isNotNullable: false }
    },

    // This field isn't required by KS but the knexOptions mean it will always 
    // be populated in the DB
    registeredAt: {
      type: DateTime,
      isRequired: false,
      knexOptions: {
        defaultTo: knex => knex.raw('localtimestamp'),
        isNotNullable: true
      }
    },

    // This field is being defaulted using a DB-side function
    // defaultTo needs to be a function in this case so we can get the knex referrence
    externalId: {
      type: Uuid,
      knexOptions: { defaultTo: knex => knex.raw('gen_random_uuid()') }
    },
  },
});

Produces a schema like this...

CREATE TABLE keystone."User" (
    id integer DEFAULT nextval('keystone."User_id_seq"'::regclass) PRIMARY KEY,
    name text,
    email text,
    "isEnabled" boolean NOT NULL DEFAULT true,
    password character varying(60),
    "registeredAt_utc" timestamp without time zone NOT NULL DEFAULT LOCALTIMESTAMP,
    "registeredAt_offset" text NOT NULL,
    "externalId" uuid DEFAULT gen_random_uuid()
);
molomby added 3 commits Jul 9, 2019
Adding `knexOptions` to the KnexFieldAdapter to support DB-level conf鈥
鈥g for nullability (`isNotNullable`) and defaults (`defaultTo`). These default back to the relevant Keystone equivalents when not provided.
Updating existing knex field adapters to use new db-level config opti鈥
鈥ns. This switches `isRequired` flag for `isNotNullable` and adds db-level defaulting (`defaultTo`) to all fields type for which it makes sense.

@molomby molomby force-pushed the molomby/db-level-config-for-knex-adapter branch from 134fae8 to 1806416 Jul 10, 2019

@timleslie
Copy link
Contributor

left a comment

Yep, this all works for me 馃憤

@molomby molomby merged commit 0d84792 into master Jul 10, 2019

9 of 12 checks passed

Header rules No header rules processed
Details
Pages changed 21 new files uploaded
Details
Redirect rules No redirect rules processed
Details
DeepScan 0 new and 0 fixed issues
Details
Mixed content No mixed content detected
Details
WIP Ready for review
Details
ci/circleci: integration_tests_accesscontrol Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_basic Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_clientvalidation Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_login Your tests passed on CircleCI!
Details
ci/circleci: simple_tests Your tests passed on CircleCI!
Details
deploy/netlify Deploy preview ready!
Details

@molomby molomby deleted the molomby/db-level-config-for-knex-adapter branch Jul 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.