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

Added ability to control field visibility on schema definition #1434

Merged
merged 13 commits into from
Oct 4, 2023

Conversation

carlagouveia
Copy link
Contributor

@carlagouveia carlagouveia commented Aug 26, 2023

This PR aims to add the ability to control field visibility on the schema definition, using either a bool or a callable that has access to the context. This is not final in any way and I am open to other ideas on how to handle this.

Fixes #782

src/Type/Definition/FieldDefinition.php Outdated Show resolved Hide resolved
tests/Executor/ExecutorSchemaTest.php Outdated Show resolved Hide resolved
tests/Executor/ExecutorSchemaTest.php Show resolved Hide resolved
tests/Type/IntrospectionTest.php Outdated Show resolved Hide resolved
tests/Type/IntrospectionTest.php Outdated Show resolved Hide resolved
tests/Type/IntrospectionTest.php Outdated Show resolved Hide resolved
src/Executor/ReferenceExecutor.php Show resolved Hide resolved
@carlagouveia
Copy link
Contributor Author

@spawnia just added the field visibility validation. Let me know if that is what you had in mind. 👍

@spawnia
Copy link
Collaborator

spawnia commented Sep 26, 2023

I made some modifications to further align how a non-visible and a non-existent field would behave.

Due to the different call sites, we don't always have the same $context available. I think it would be of very limited use to provide whatever context is available, as the presented information can differ and the implementation of visibility would have to account for that. If you think something in particular would be useful or necessary to have available, we can discuss that further.

Please document this feature at https://github.com/webonyx/graphql-php/blob/master/docs/type-definitions/object-types.md#field-configuration-options. The note for visible should provide a short outline why this would be preferrable over conditional inclusion or exclusion of a field, and how exactly it behaves.

@carlagouveia
Copy link
Contributor Author

I made some modifications to further align how a non-visible and a non-existent field would behave.

Due to the different call sites, we don't always have the same $context available. I think it would be of very limited use to provide whatever context is available, as the presented information can differ and the implementation of visibility would have to account for that. If you think something in particular would be useful or necessary to have available, we can discuss that further.

Please document this feature at https://github.com/webonyx/graphql-php/blob/master/docs/type-definitions/object-types.md#field-configuration-options. The note for visible should provide a short outline why this would be preferrable over conditional inclusion or exclusion of a field, and how exactly it behaves.

@spawnia thanks for all the modifications. I've updated the documentation, adding information about this feature. Here is the PR #1456.

@spawnia
Copy link
Collaborator

spawnia commented Sep 28, 2023

You can push the docs update directly to this pull request.

@carlagouveia
Copy link
Contributor Author

You can push the docs update directly to this pull request.

Done @spawnia.

@spawnia spawnia changed the title Added ability to control field visibility on schema definition (Fixes #782) Added ability to control field visibility on schema definition Oct 4, 2023
@spawnia spawnia merged commit 2af585f into webonyx:master Oct 4, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide parts of the schema
2 participants