Skip to content

fix: clone fields not respecting prefix #136

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.

Sign up for GitHub

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

Jump to bottom
Merged
jasonbahl merged 14 commits into from
Dec 19, 2023
Merged

fix: clone fields not respecting prefix #136

jasonbahl merged 14 commits into from
Dec 19, 2023

Conversation

@jasonbahl
Copy link
Contributor

@jasonbahl jasonbahl commented Dec 8, 2023 •
edited
Loading

What does this implement/fix? Explain your changes.

This fixes a bug where multiple clone fields using the "prefix_name" setting weren't being added to the Schema properly or resolving properly.

Does this close any currently open issues?

Closes #125

Any relevant logs, error output, GraphiQL screenshots, etc?

Given the following ACF Field Group export:
acf-export-2023-12-07.json.zip

And a page with the following content entered using these fields:

CleanShot 2023-12-07 at 17 00 51

BEFORE

Inspecting the CustomContent Type in the Schema shows that it has 3 fields:

  • content: String
  • sections: [ContentSectionsSections_Layout]
  • sidebar: String

CleanShot 2023-12-07 at 16 54 38

The following query can be executed successfully:

query GetPage {
  page(id: 12, idType: DATABASE_ID) {
    id
    title
    ...CustomContent
  }
}

fragment CustomContent on WithAcfCustomContent {
  customContent {
    sections {
      __typename
      ...HeroLayout
    }
  }
}

fragment HeroLayout on LayoutHero_Fields {
  text
  image {
    node {
      sourceUrl
    }
  }
}

However, this is only returning the "Content" field that cloned "All fields from the Content Sections" field group.

The "Sidebar" field that also cloned "All fields from the Content Sections" field group is unable to be queried. 😢

AFTER

Now, we can execute the following query to get both the "Content" sections that were cloned with no prefix, AND we can query the "Sidebar" sections which cloned with "prefix_name" true.

query GetPage {
  page(id: 12, idType: DATABASE_ID) {
    id
    title
    ...CustomContent
  }
}

fragment CustomContent on WithAcfCustomContent {
  customContent {
    sections {
      __typename
      ...HeroLayout
    }
    sidebar {
      sections {
        __typename
        ...HeroLayout
      }
    }
  }
}

fragment HeroLayout on LayoutHero_Fields {
  text
  image {
    node {
      sourceUrl
    }
  }
}

And this yields the following results:

{
  "data": {
    "page": {
      "id": "cG9zdDoxMg==",
      "title": "Custom Content",
      "customContent": {
        "sections": [
          {
            "__typename": "ContentSectionsSectionsHeroLayout",
            "text": "content hero text",
            "image": {
              "node": {
                "sourceUrl": "http://acf-clone-debug.local/wp-content/uploads/2023/12/profile.jpeg"
              }
            }
          },
          {
            "__typename": "ContentSectionsSectionsHeroWithCtaButtonLayout",
            "text": "Hero with CTA Text",
            "image": {
              "node": {
                "sourceUrl": "http://acf-clone-debug.local/wp-content/uploads/2023/12/categories-mutation-update-not-allowed.png"
              }
            }
          }
        ],
        "sidebar": {
          "sections": [
            {
              "__typename": "ContentSectionsSectionsHeroLayout",
              "text": "sidebar hero text",
              "image": {
                "node": {
                  "sourceUrl": "http://acf-clone-debug.local/wp-content/uploads/2023/12/application-data-graph.png"
                }
              }
            }
          ]
        }
      }
    }
  }
}

Any other comments?

This is a possible breaking change for users that were using clone fields and/or flexible content fields.

It's highly recommended that you test your queries on a staging environment and modify as needed.

@jasonbahl
- initial exploration of fixing the issue. . .debugging
3a22eba
@jasonbahl
Revert "- initial exploration of fixing the issue. . .debugging"
64a3358 
This reverts commit 3a22eba.
@jasonbahl
- exploring having a Clone field type and moving the logic for what T…
c499cd6 
…ypes should inherit which Interfaces to the clone field type

- @todo remove some debug code and test further
@jasonbahl
- fix php error
238d702
@jasonbahl
- add clone fields to the "should_format" logic
7eb9bea 
- add missing translators comment
- remove some debug code
- move logic for clone field interfaces out of the Registry and into the CloneField.php
- modify logic for how flex field layouts are mapped to the schema
@jasonbahl jasonbahl requested a review from josephfusco December 8, 2023 00:02
@jasonbahl jasonbahl self-assigned this Dec 8, 2023
@jasonbahl jasonbahl mentioned this pull request Dec 8, 2023
Closed
3 tasks
@jasonbahl
- allow Field Types to return 'NULL' to bail from mapping to the Schema
b9db850 
- update logic for how clone fields are mapped to the schema when individual fields are cloned and when entire field groups are cloned
- update AdminSettingsCest to account for multiple field groups existing at the same time
@jasonbahl
- modify logic for get_parent_graphql_type_name to account for fields…
8f299a4 
… within a flex layout

- remove commented out code from CloneField.php
- modify tests to account for multiple field groups being imported in the test suite
- update test to account for flex field layout type name change
@coveralls
Copy link

coveralls commented Dec 13, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build e92b625fbee198f62089d7a64d18e33e3fead666-PR-136

  • 67 of 129 (51.94%) changed or added relevant lines in 5 files are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.2%) to 62.369%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/FieldConfig.php 16 17 94.12%
src/FieldType/Repeater.php 2 7 28.57%
src/Registry.php 16 31 51.61%
src/FieldType/CloneField.php 31 51 60.78%
src/FieldType/FlexibleContent.php 2 23 8.7%
Files with Coverage Reduction New Missed Lines %
src/FieldType/FlexibleContent.php 1 58.33%
src/FieldConfig.php 4 89.83%
Totals Coverage Status
Change from base Build b56bef59902ac5327c7a9b97909a13bcc488b8b1: -0.2%
Covered Lines: 2017
Relevant Lines: 3234
💛 - Coveralls

@jasonbahl jasonbahl merged commit 777d53a into main Dec 19, 2023
@jasonbahl jasonbahl mentioned this pull request Dec 19, 2023
Merged
@jasonbahl jasonbahl deleted the fix/125-clone-fields-not-respecting-prefix branch January 24, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@josephfusco josephfusco Awaiting requested review from josephfusco

Assignees

@jasonbahl jasonbahl

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Clone Fields inherit the name of the cloned field

3 participants

@jasonbahl @coveralls