Postgraphile: "disableDefaultMutations" disable all mutations?

Created on 6 Sep 2017  ·  13Comments  ·  Source: graphile/postgraphile

Thank you for this awesome tool!

In my case, there are a lot of tables, but I use only a few mutations.
It seems that disableDefaultMutations disable all mutations.

Can I use default mutations only on some tables?

I created a function by myself, but I don't know how to use PersonPatch as an argument.
The following function creates PersonInput as the second argument.

CREATE FUNCTION update_person_by_id(id integer, person person)...

I want PersonPatch, because PersonInput must always requires columns with "not null" attribute.
If there is a way to easily create the same function as default mutation, please let me know.

💅 enhancement 📄 add-to-docs 😌 Fixed in v4
Source
picture knqyf263

Most helpful comment

The issue does not lie in SQL - I was talking about allowing custom mutations to utilise the FooPatch GraphQL type - currently custom mutations can only use the FooInput GraphQL type directly, which includes the not-null constraints inherent in that type. We currently have no way of knowing if a custom mutation wants FooInput or FooPatch, so we always use FooInput.

Recently we merged https://github.com/graphile/graphile-build/pull/128 which introduces smart comments into the Graphile stack. This should allow us to do something like:

comment on function update_person_by_id(id integer, person person) is
  '@usePatchTypeForArguments 2\nUpdates a person by their database id';

Then we can pull out the 'usePatchTypeForArguments' setting (please, someone suggest a better name for this!) see that it applies to the 2nd argument, and hence use PersonPatch rather than PersonInput.

Input welcome.

picture benjie on 4 Jan 2018
👍3

All 13 comments

Can I use default mutations only on some tables?

Currently it's all-or-nothing I'm afraid.

I don't know how to use PersonPatch as an argument

As far as I am aware there is no way to do this currently; it's an interesting issue though - not one I had considered. Perhaps we could add a naming convention so if the name ends in "_patch" or "Patch" we use the patch type?

CREATE FUNCTION update_person_by_id(id integer, person_patch person)...

Would love to get some feedback from other people on this.

benjie picture benjie on 6 Sep 2017

@benjie Thank you for the reply!

Perhaps we could add a naming convention so if the name ends in "_patch" or "Patch" we use the patch type?

I think this is a good idea.

Or, if possible, we could use enableDefaultMutations option.

enableDefaultMutations: ["table_a", "table_b"]

The mutations for tables other than table_a and table_b will be disabled.

knqyf263 picture knqyf263 on 6 Sep 2017

There's two feature requests here. I'd be happy to accept PRs to v4 that enabled these functionalities.

1. allow custom queries/mutations to use the patch type (perhaps with a naming convention)

For this, I think the code would go around here:

https://github.com/graphile/graphile-build/blob/6d7b4a3e4bf48bee76b952274f0a03a1be03a528/packages/graphile-build-pg/src/plugins/makeProcField.js#L92

Specifically checking !!argNames[idx].match(/.(_p|P)atch$/) and if so instead of using pgGqlInputTypeByTypeId[type.id] it'd use the patch type (via getTypeByName) from here:

https://github.com/graphile/graphile-build/blob/6d7b4a3e4bf48bee76b952274f0a03a1be03a528/packages/graphile-build-pg/src/plugins/PgTablesPlugin.js#L194-L211

(falling back to the input type if the patch type doesn't exist).

2. enable a whitelist of default mutations to enable

This could be implemented fairly simply by adding filters here:

https://github.com/graphile/graphile-build/blob/6d7b4a3e4bf48bee76b952274f0a03a1be03a528/packages/graphile-build-pg/src/plugins/PgMutationCreatePlugin.js#L46

and here:

https://github.com/graphile/graphile-build/blob/6d7b4a3e4bf48bee76b952274f0a03a1be03a528/packages/graphile-build-pg/src/plugins/PgMutationUpdateDeletePlugin.js#L58

benjie picture benjie on 6 Sep 2017

I'd love there to be a better solution for the patch thing than a naming convention though; so if anyone has any ideas please do speak up 👍

(e.g. we could add a "magic comment" to the function that changed its behaviour)

benjie picture benjie on 6 Sep 2017

@benjie Thank you for listing.
I need these features, so I will try implement No.2.
If it works, I will send PR!

knqyf263 picture knqyf263 on 6 Sep 2017

You'll also need to:

Parse the option here:

https://github.com/postgraphql/postgraphql/blob/benjie/graphql-build/src/postgraphql/cli.ts#L49

Add it to the config object here:

https://github.com/postgraphql/postgraphql/blob/benjie/graphql-build/src/postgraphql/cli.ts#L178

Which gets sent through to postgraphile-core. You then pass that through to the plugins by passing through the option here:

https://github.com/graphile/graphile-build/blob/6d7b4a3e4bf48bee76b952274f0a03a1be03a528/packages/postgraphile-core/src/index.js#L87

Add finally you extract the option in the relevant plugins from the second argument here:

https://github.com/graphile/graphile-build/blob/6d7b4a3e4bf48bee76b952274f0a03a1be03a528/packages/graphile-build-pg/src/plugins/PgMutationUpdateDeletePlugin.js#L14

Do have a look through the docs on https://www.graphile.org

benjie picture benjie on 6 Sep 2017

@benjie I am sorry for the basic question.
Should I work on the benjie/graphql-build?
For graphile-build, work on master?

knqyf263 picture knqyf263 on 7 Sep 2017

Yes 👍

benjie picture benjie on 7 Sep 2017

@benjie I'm sorry to be late due to my long business trip.

First, I tested graphile-build.

$ cd graphile/graphile-build
$ git log --oneline -n 1
d2da240 (HEAD -> master, origin/master, origin/HEAD) feat(inflector): add foreign table to manyRelationByKeys call (#80)
$ npm install -g lerna yarn
$ yarn
$ lerna bootstrap
$ npm run watch
$ lerna run test
...
Database reset successfully ✅
lerna success run Ran npm script 'test' in packages:
lerna success - graphile-build-pg
lerna success - graphile-build
lerna success - graphql-parse-resolve-info
lerna success - postgraphile-core

Then, I tried to run scripts/dev, but I got the following error.

$ cd ../../postgraphql/postgraphql/
$ git log --oneline -n 1
689fada (HEAD -> benjie/graphql-build, origin/benjie/graphql-build) Update README
$ npm link ../../graphile/graphile-build/packages/postgraphile-core/
$ npm install
$ scripts/run-kitchen-sink-sql
$ scripts/dev
[nodemon] 1.12.1
[nodemon] to restart at any time, enter `rs`
[nodemon] watching: /Users/teppei/src/github.com/postgraphql/postgraphql/src/**/*
[nodemon] starting `/Users/teppei/src/github.com/postgraphql/postgraphql/node_modules/.bin/ts-node --ignore node_modules --disableWarnings src/postgraphql/cli.ts --schema a,b,c --show-error-stack json --watch`
Error: You must provide a valid PG client configuration
    at _callee$ (/Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node7minus/withPgClient.js:119:19)
    at tryCatch (/Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node_modules/regenerator-runtime/runtime.js:65:40)
    at GeneratorFunctionPrototype.invoke [as _invoke] (/Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node_modules/regenerator-runtime/runtime.js:299:22)
    at GeneratorFunctionPrototype.prototype.(anonymous function) [as next] (/Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node_modules/regenerator-runtime/runtime.js:117:21)
    at step (/Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:30)
    at /Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node_modules/babel-runtime/helpers/asyncToGenerator.js:35:14
    at Promise.F (/Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node_modules/core-js/library/modules/_export.js:35:28)
    at /Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node_modules/babel-runtime/helpers/asyncToGenerator.js:14:12
    at withPgClient (/Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node7minus/withPgClient.js:157:17)
    at _callee2$ (/Users/teppei/src/github.com/graphile/graphile-build/packages/graphile-build-pg/node7minus/plugins/PgIntrospectionPlugin.js:69:78)
[nodemon] app crashed - waiting for file changes before starting...

I tried to find the cause, but I couldn't.
Please let me know if you know the cause of this error.
Thanks.

knqyf263 picture knqyf263 on 1 Oct 2017

It’s probably that there are multiple different instances of the pg module in use - PostGraphQL and postgraphile-core need to use the exact same pg module.

benjie picture benjie on 1 Oct 2017

As far as I am aware there is no way to do this currently;

if a table has foo, bar and tee columns and you want to patch a row with patchObj = {bar: 'bar'}

you could do 

.query(`UPDATE that_table SET
  foo = coalesce($1, foo),
  bar = coalesce($2, bar),
  tee = coalesce($3, tee)
WHERE ...`, [patchObj.foo, patchObj.bar, patchObj.tee, ...])
caub picture caub on 4 Jan 2018

The issue does not lie in SQL - I was talking about allowing custom mutations to utilise the FooPatch GraphQL type - currently custom mutations can only use the FooInput GraphQL type directly, which includes the not-null constraints inherent in that type. We currently have no way of knowing if a custom mutation wants FooInput or FooPatch, so we always use FooInput.

Recently we merged https://github.com/graphile/graphile-build/pull/128 which introduces smart comments into the Graphile stack. This should allow us to do something like:

comment on function update_person_by_id(id integer, person person) is
  '@usePatchTypeForArguments 2\nUpdates a person by their database id';

Then we can pull out the 'usePatchTypeForArguments' setting (please, someone suggest a better name for this!) see that it applies to the 2nd argument, and hence use PersonPatch rather than PersonInput.

Input welcome.

benjie picture benjie on 4 Jan 2018
👍3

This is now possible via two methods:

  1. Smart comments: https://www.graphile.org/postgraphile/smart-comments/#omitting

  2. --no-ignore-rbac / ignoreRBAC: false - and then revoking the relevant method on the table.

benjie picture benjie on 15 Aug 2018
Was this page helpful?
0 / 5 - 0 ratings

Related issues

tazsingh picture tazsingh  ·  3Comments
Venryx picture Venryx  ·  4Comments
marshall007 picture marshall007  ·  3Comments
giacomorebonato picture giacomorebonato  ·  3Comments
james-ff picture james-ff  ·  4Comments