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

Consolidate "graphql_blacklisted_fields" and "exclude" directives #322

Closed
chillu opened this issue Nov 12, 2020 · 3 comments
Closed

Consolidate "graphql_blacklisted_fields" and "exclude" directives #322

chillu opened this issue Nov 12, 2020 · 3 comments

Comments

@chillu
Copy link
Member

chillu commented Nov 12, 2020

We have two mechanisms for excluding fields from model operations: graphql_blacklisted_fields on DataObject, and exclude on operations.

In the following example, both password and autoLoginHash would be excluded from the update operation on the default schema.

Member:
    graphql_blacklisted_fields:
        password: true

SilverStripe\GraphQL\Schema\Schema:
  schemas:
    default:
        models:
            Member:
                fields: '*'
                operations:
                    read: true
                    update:
                        exclude:
                            autoLoginHash: true

Wouldn't this be better placed in the global schema? Particularly if we provide a way to show the merged config tree, that's going to be a lot more transparent.

SilverStripe\GraphQL\Schema\Schema:
  schemas:
    '*':
      modelConfig:
        DataObject:
            exclude:
                password: true

SilverStripe\GraphQL\Schema\Schema:
  schemas:
    default:
        models:
            Member:
                fields: '*'
                operations:
                    read: true
                    update:
                        exclude:
                            autoLoginHash: true
@unclecheese
Copy link

unclecheese commented Feb 23, 2021

I think the difference is that graphql_blacklisted_fields is a DataObject's way to declaring which fields should never be added to the schema, no matter what, across all schemas.

The exclude directive used on input types, because by default, when no fields is defined, you get fields: *, so you nee d away to override that.

It should work similarly to the standard types:

update:
  fields:
    '*': true
    autoLoginHash: false

That '*': true bit likely didn't exist when this was written.

Will remove the exclude directive from input types and the related FieldReconciler trait.

@unclecheese
Copy link

PR is here: #366

@unclecheese
Copy link

Docs are here: silverstripe/silverstripe-framework#9870

@chillu chillu closed this as completed Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants