feat: improved handling of clone and group fields #193
Conversation
…roblem. . .but gets us heading in the right direction
…d to a field group - update CloneField.php to return 'NULL' type as the registry handles getting the interfaces now - update Group.php field type to return the Cloned Type if possible instead of registering a new Type
…refixed clone field groups
Pull Request Test Coverage Report for Build 819424c07072e8c3e12fa932f8ef4586a90d56a1-PR-193Details
💛 - Coveralls |
|
Having some build errors but will log what we did to fix them. |
josephfusco
left a comment
josephfusco
left a comment
There was a problem hiding this comment.
I was able to reproduce the original issue and confirm that the schema does not break with the changes in this PR 🙌🏻
|
@nhankla-kl thank you! 🙏🏻 Also happy to hop on a call if that would be helpful too. You can reach me in the WPGraphQL Slack if you would like to zoom or whatever. |
|
Gettin Schema issues still: Failure running getIntrospectionQuery: { |
|
@nhankla-kl can you provide an export of your ACF Field Groups so we can try and reproduce? |
|
oops. didn't mean to close. |
|
@nhankla-kl was your ACF Field Groups working in the Schema prior to this PR and breaking in this PR? |
|
@jasonbahl it works in .5.5.3 |
|
@nhankla-kl WPGraphQL for ACF v0.5.3? As in this version: https://github.com/wp-graphql/wp-graphql-acf/releases/tag/v0.5.3? |
|
@jasonbahl correct 
|
|
@nhankla-kl Ok so WPGraphQL for ACF v2.2.0 release also does not work, I assume? https://github.com/wp-graphql/wpgraphql-acf/releases/tag/v2.2.0 This specific PR did not break anything? If you are looking to migrate from v0.. to v2.*, I recommend checking the upgrade guide here: https://acf.wpgraphql.com/upgrade-guide/ It has some helpful information on things to look for in the Schema, such as avoiding special characters in GraphQL Type and Field names, avoiding field/type names starting with a number, etc. I did import your ACF field group and will see if I can help identify how to best support it, but it doesn't seem that this specific PR has caused any new issues compared to v2.2.0 |
|
@nhankla-kl One thing I noticed is that you had a couple instances of different ACF Field Groups with the same GraphQL Type Name:
Both had the GraphQL Type Name:
Both had the GraphQL Type Name
Both had the GraphQL Type Name I'm still looking through things, but these are some observations so far. |
|
@nhankla-kl Ok, I think I'm getting close to understanding another bug. It appears that there is a bug when it comes to cloning a field group that contains other cloned field groups. For example, you have the
It appears that somehow the double cloning isn't properly assigning GraphQL Interfaces to the schema. I'm working to better understand why this is and figure out a fix for it. |
|
@nhankla-kl I think I might have discovered the problem and am working on updating this PR with a solution. Hopefully will have something to test shortly 🤞🏻 |
…tion - fix call to possibly unset `$parent_field_group['ID']` - phpstan fixes - update acf_get_raw_fields to acf_get_fields - add adjustments to how group types are registered if they've been cloned
|
@nhankla-kl I've pushed up several changes to this PR and I believe we're getting close to having things work for the original scenario and the scenario you brought up if you want to give it another try. |
…WPGraphQL in Github
…//github.com/jasonbahl/wp-graphql/tree/fix/interface-recursion) but fails against WPGraphQL 1.23.0
| echo "Installing WPGraphQL from GitHub repo ${WPGRAPHQL_GIT_REPO}" | ||
| # Clone the repository | ||
| git clone -b ${WPGRAPHQL_GIT_BRANCH} ${WPGRAPHQL_GIT_REPO} "${PLUGINS_DIR}/wp-graphql" | ||
| # Navigate to the plugin directory | ||
| cd "${PLUGINS_DIR}/wp-graphql" | ||
| # Install dependencies with Composer | ||
| composer install --no-dev | ||
| # Optionally activate the plugin using wp-cli | ||
| wp plugin activate wp-graphql --allow-root |
There was a problem hiding this comment.
NOTE:
This allows us to run tests against arbitrary branches of WPGraphQL.
For example, this feature relies on some work in this PR (wp-graphql/wp-graphql#3100)
So, I was able to update my .env.testing file with these values:
WPGRAPHQL_GIT_REPO="https://github.com/jasonbahl/wp-graphql.git"
WPGRAPHQL_GIT_BRANCH="fix/interface-recursion"
And tests pass when running against that branch of WPGraphQL 👏🏻 👏🏻
|
@nhankla-kl I've updated #172 with a comment outlining steps to reproduce using the exported ACF Field Group you have provided. Using your ACF Field Group, I am now able to load the Schema. BeforeSchema doesn't load properly and errors out the GraphiQL IDE
AfterSchema loads and I can use the GraphiQL IDE:
Test added using the exported ACF Field Group provided above as an imported field group with assertions on querying the possible types and on executing an example query: 43d2b91 |
|
@jasonbahl this is amazing will test it on several dev sites that have similar ACF structure. Will let you know. Thanks! |
|
related: wp-graphql/wp-graphql#3102 (specifically, see: wp-graphql/wp-graphql#3100) |
…cloning-group-fields
|
And here we are, with all tests passing! 🥳 |
jasonbahl
changed the title
|
@nhankla-kl fwiw, it seems like there still are some more bugs related to cloning field groups that have nested group fields. I've broken it down into another issue that I'm continuing to troubleshoot: #201 |
What does this implement/fix? Explain your changes.
This is an attempt at fixing a bug related to cloning field groups that have a field of the "group" field within them.
For the situation described in #172 this gets the Schema to load and lets the fields be queried, but there are some things missing still.
For example: the Interface for the field group being cloned isn't being applied properly to the Type representing the Flexible Content Layout
Does this close any currently open issues?
closes #172
closes #197
related: wp-graphql/wp-graphql#3100
Any relevant logs, error output, GraphiQL screenshots, etc?
Given the ACF Field Groups in #172:
BEFORE:
Errors fetching the schema, as described in #172
AFTER:
The Schema loads, and I can query data
Any other comments?
I should be able to use the following fragment:
But this is showing invalid becauseBlockAccordion_Fieldsis being applied toContentBlocksBlocksAccordionLayoutAccordionand notContentBlocksBlocksAccordionLayoutI believe there's some correlation with how Flex Field Layouts are mapped to the schema in/src/FieldType/FlexibleContent.phpand how cloned groups apply their interfaces insrc/FieldType/CloneField.php, but I've not been able to pin it down quite yet.These fragments now work as expected.