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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rbac): Custom mutations fallback to FooBaseInput when FooInput isn't available #375

Merged
merged 4 commits into from
Jan 10, 2019

Conversation

benjie
Copy link
Member

@benjie benjie commented Jan 10, 2019

Fixes #373

If insert(some_column) is not granted on foo then FooInput would have no fields, and thus is not generated.

In this case, a volatile function that accepts a foo parameter will not have a type for said parameter, and so will not add the function to the schema.

The current workaround for this is to use the smart comment in the error message:

COMMENT ON FUNCTION my_func(foo) IS E'@arg0variant base';

This tells PostGraphile to use the "base" input type (like a "super-patch" that contains all columns as nullable).

This patch makes the system fall back to the base input type automatically if no normal input type is available.

@benjie
Copy link
Member Author

benjie commented Jan 10, 2019

On second thoughts; doing this could lead to unexpected changes if you later GRANT INSERT(my_col) ON foo - suddenly the function mutation will use the FooInput (one column) rather than FooBaseInput (all columns).

I think it's better to make the user opt in to this functionality; using the comment that's already generated by the error:

  graphile-build:warn Error: Could not determine type for argument 0 ('post') of function 'post_with_suffix'; you might want to use smart comments, e.g. 'COMMENT ON FUNCTION "a"."post_with_suffix"("a"."post", "pg_catalog"."text") IS E'@arg0variant base';"
  graphile-build:warn     at argTypes.map (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build-pg/node8plus/plugins/makeProcField.js:122:13)
  graphile-build:warn     at Array.map (<anonymous>)
  graphile-build:warn     at Object.map (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build-pg/node8plus/plugins/makeProcField.js:116:32)
  graphile-build:warn     at makeProcField (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build-pg/node8plus/plugins/PgMutationProceduresPlugin.js:37:24)
  graphile-build:warn     at Array.reduce (<anonymous>)
  graphile-build:warn     at reduce (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build-pg/node8plus/plugins/PgMutationProceduresPlugin.js:28:64)
  graphile-build:warn     at SchemaBuilder.hook [as applyHooks] (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build/node8plus/SchemaBuilder.js:157:20)
  graphile-build:warn     at applyHooks (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build/node8plus/makeNewBuild.js:453:40)
  graphile-build:warn     at resolveThunk (/Users/benjiegillam/Dev/graphile-build/node_modules/graphql/type/definition.js:369:40)
  graphile-build:warn     at defineFieldMap (/Users/benjiegillam/Dev/graphile-build/node_modules/graphql/type/definition.js:551:18)
  graphile-build:warn     at GraphQLObjectType.getFields (/Users/benjiegillam/Dev/graphile-build/node_modules/graphql/type/definition.js:518:27)
  graphile-build:warn     at Object.getFields (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build/node8plus/makeNewBuild.js:544:36)
  graphile-build:warn     at newWithHooks (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build/node8plus/plugins/MutationPlugin.js:29:22)
  graphile-build:warn     at SchemaBuilder.hook [as applyHooks] (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build/node8plus/SchemaBuilder.js:157:20)
  graphile-build:warn     at Object.applyHooks (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build/node8plus/makeNewBuild.js:277:27)
  graphile-build:warn     at SchemaBuilder.newWithHooks [as buildSchema] (/Users/benjiegillam/Dev/graphile-build/packages/graphile-build/node8plus/SchemaBuilder.js:205:28)
  graphile-build:warn     at buildSchema (/Users/benjiegillam/Dev/graphile-build/packages/postgraphile-core/node8plus/index.js:200:28)
  graphile-build:warn     at process._tickCallback (internal/process/next_tick.js:68:7) +0ms

@benjie
Copy link
Member Author

benjie commented Jan 10, 2019

I've settled for improving the error message.

@benjie
Copy link
Member Author

benjie commented Jan 10, 2019

Collapsed:
screenshot 2019-01-10 12 17 42

Expanded:
screenshot 2019-01-10 12 18 20

@benjie benjie merged commit edca926 into master Jan 10, 2019
@benjie benjie deleted the rbac-fns branch January 10, 2019 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant