fix: handle regression when implementing interface with identical args. #3152
Conversation
![qlty-cloud-legacy[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| * | ||
| * @return array<string,array<string,mixed>> | ||
| */ | ||
| private function get_fields_from_implemented_interfaces( TypeRegistry $registry ): array { |
There was a problem hiding this comment.
Function get_fields_from_implemented_interfaces has a Cognitive Complexity of 11 (exceeds 5 allowed). Consider refactoring.
| * | ||
| * @return ?array<string,mixed> The field config with inherited values. Null if the field type cannot be determined. | ||
| */ | ||
| private function inherit_field_config_from_interface( string $field_name, array $field, array $interface_fields ): ?array { |
There was a problem hiding this comment.
Function inherit_field_config_from_interface has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
| * | ||
| * @return array<string,array<string,mixed>> The merged field args. | ||
| */ | ||
| private function merge_field_args( string $field_name, array $field_args, array $interface_args ): array { |
There was a problem hiding this comment.
Function merge_field_args has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
justlevine
force-pushed
the
fix/interface-identical-args-regression
branch
from
4961b58 to
7ee16f8
Compare
|
edit: fixed in 76a5dc9 now ready for review @jasonbahl / @josephfusco - I didn't have time yet to address wpengine/wp-graphql-content-blocks#240, but the other changes in this PR should make the class a lot easier to reason with. I'll hope to have some more time to wrap this up soon, but if you want to try and take this over the finish line before I can, please go ahead. |
| * | ||
| * @param string|array<string,mixed>|callable|\GraphQL\Type\Definition\Type $type The type to normalize. | ||
| */ | ||
| private function normalize_type_name( $type ): string { |
There was a problem hiding this comment.
Function normalize_type_name has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
justlevine
added
status: in review
@justlevine looks like the debug messages related to wpengine/wp-graphql-content-blocks#240 are resolved with this PR. 👏🏻 I'll work on a test to prevent future regressions. |
…th an implementing interface's fields
|
Code Climate has analyzed commit 12b1b14 and detected 4 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
3 participants
What does this implement/fix? Explain your changes.
The PR cleans up the
InterfaceTraitclass and addresses regressions in #3100 and #3112 in the following ways:graphql_debug()message when a field has no associated type. The fauxPostObject.ancestorsandPostObject.parenthave been fixed to be registered as deprecated Connections (that resolve null) to prevent schema breaks for people using those deprecated fields in the same query as aHierarchicalContentNode.ancestors/parent.$this->nameto$this->config['name']for getting the Interface name in the trait.InterfaceTest:testInvalidArgsOnInheritedObjectFieldsAreCapturedwhose errors should be explicitly caught.field_arg_type_to_string()tonormalize_type_name()and change the usage to handle callables/resolved types.Does this close any currently open issues?
closes #3106
closes wpengine/wp-graphql-content-blocks#240
Any relevant logs, error output, GraphiQL screenshots, etc?
Any other comments?
Where has this been tested?
Operating System: Ubuntu 20.04 (wsl2 + devilbox + php8.1.15)
WordPress Version: 6.5.3