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

[gatsby-source-wordpress] ACF Flexible content in Flexible content returns duplicates #8910

Closed
jordyvg opened this issue Oct 8, 2018 · 5 comments
Labels
type: bug An issue or pull request relating to a bug in Gatsby

Comments

@jordyvg
Copy link
Contributor

jordyvg commented Oct 8, 2018

Description

Flexible content in Flexible Content returns an array with only the first unique row times the amount of rows. So all the rows after row 1 are duplicates of row 1 (even if every row are different field types).

Only one level Flexible Content works just fine.

Steps to reproduce

Add another ACF Flexible Content field inside the parent Flexible Content field.

Expected result

All rows to be unique.

Actual result

Rows are duplicates of the first row.

The WordPress Rest API:

...
"acf": {
  "builder": [
    {
      "acf_fc_layout": "grid",
      "grid": [
        {
          "acf_fc_layout": "text",
          "text": "<p>Text 1</p>\n",
        },
        {
          "acf_fc_layout": "text",
          "text": "<p>Another row, Text 2</p>\n",
        },
        {
          "acf_fc_layout": "text",
          "text": "<p>Another row, Text 3</p>\n",
        }
      ],
    }
  ],
},
...

GraphiQL query:

{
  allWordpressPage(filter: { slug: { eq: "test"} }) {
    edges {
      node {
        id
        title
        slug
        acf {
          builder_page {
           __typename
            ...on WordPressAcf_grid {
	      id
              grid {
                id
                text
              }
            }
          }
        }
      }
    }
  }
}

GraphiQL response:

{
  "data": {
    "allWordpressPage": {
      "edges": [
        {
          "node": {
            "id": "270e0b30-05f3-5c06-87bc-fa47a0b9ed03",
            "title": "Test",
            "slug": "test",
            "acf": {
              "builder_page": [
                {
                  "__typename": "WordPressAcf_grid",
                  "id": "270e0b30-05f3-5c06-87bc-fa47a0b9ed030builderWordPressAcf_grid",
                  "grid": [
                    {
                      "id": "270e0b30-05f3-5c06-87bc-fa47a0b9ed030builderWordPressAcf_gridgrid",
                      "text": "<p>Text 1</p>\n"
                    },
                    {
                      "id": "270e0b30-05f3-5c06-87bc-fa47a0b9ed030builderWordPressAcf_gridgrid",
                      "text": "<p>Text 1</p>\n"
                    },
                    {
                      "id": "270e0b30-05f3-5c06-87bc-fa47a0b9ed030builderWordPressAcf_gridgrid",
                      "text": "<p>Text 1</p>\n"
                    }
                  ]
                }
              ]
            }
          }
        }
      ]
    }
  }
}

Environment

 System:
    OS: macOS 10.14
    CPU: x64 Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 8.11.1 - /usr/local/bin/node
    Yarn: 1.6.0 - /usr/local/bin/yarn
    npm: 6.4.1 - /usr/local/bin/npm
  Browsers:
    Chrome: 69.0.3497.100
    Firefox: 61.0.2
    Safari: 12.0
  npmPackages:
    gatsby: ^2.0.0 => 2.0.18
    gatsby-image: ^2.0.0-rc.4 => 2.0.0-rc.4
    gatsby-plugin-google-fonts: 0.0.4 => 0.0.4
    gatsby-plugin-manifest: ^2.0.2 => 2.0.4
    gatsby-plugin-offline: ^2.0.5 => 2.0.5
    gatsby-plugin-react-helmet: ^3.0.0 => 3.0.0
    gatsby-plugin-sharp: ^2.0.0-rc.7 => 2.0.0-rc.7
    gatsby-plugin-stylus: next => 2.0.0-rc.1
    gatsby-source-filesystem: ^2.0.1 => 2.0.1
    gatsby-source-wordpress: ^3.0.1 => 3.0.1
    gatsby-transformer-sharp: ^2.1.1-rc.3 => 2.1.1-rc.3
  npmGlobalPackages:
    gatsby-cli: 2.4.2
@jordyvg jordyvg changed the title WordPress ACF Flexible content in Flexible content returns duplicates [gatsby-source-wordpress] ACF Flexible content in Flexible content returns duplicates Oct 8, 2018
@pieh
Copy link
Contributor

pieh commented Oct 10, 2018

@jordyvg Can you create reproduction repo? If this happens there is probably bug somewhere here

const prepareACFChildNodes = (
obj,
entityId,
topLevelIndex,
type,
children,
childrenNodes
) => {
// Replace any child arrays with pointers to nodes
_.each(obj, (value, key) => {
if (_.isArray(value) && value[0] && value[0].acf_fc_layout) {
obj[`${key}___NODE`] = value.map(
v =>
prepareACFChildNodes(
v,
entityId,
topLevelIndex,
type + key,
children,
childrenNodes
).id
)
delete obj[key]
}
})
const acfChildNode = {
...obj,
id: entityId + topLevelIndex + type,
parent: entityId,
children: [],
internal: { type, contentDigest: digest(JSON.stringify(obj)) },
}
children.push(acfChildNode.id)
// We recursively handle children nodes first, so we need
// to make sure parent nodes will be before their children.
// So let's use unshift to put nodes in the beginning.
childrenNodes.unshift(acfChildNode)
return acfChildNode
}
and id is getting reused for all nested children (which will overwrite them)

@pieh pieh added the type: bug An issue or pull request relating to a bug in Gatsby label Oct 10, 2018
@jordyvg
Copy link
Contributor Author

jordyvg commented Oct 10, 2018

@pieh Thank you, the reused id is the problem, acfChildNode returns the following:

{ acf_fc_layout: 'text',
  text: '<p>Text 1</p>\n',
  id: '37faeda9-a006-5368-afff-c5d7b13d79940builderWordPressAcf_gridgrid',
  parent: '37faeda9-a006-5368-afff-c5d7b13d79940',
  children: [],
  internal:
   { type: 'WordPressAcf_gridgrid',
     contentDigest: '6a30381167e7ae819989db8bd01e0057' } }
{ acf_fc_layout: 'text',
  text: '<p>Text 2</p>\n',
  id: '37faeda9-a006-5368-afff-c5d7b13d79940builderWordPressAcf_gridgrid',
  parent: '37faeda9-a006-5368-afff-c5d7b13d79940',
  children: [],
  internal:
   { type: 'WordPressAcf_gridgrid',
     contentDigest: '3d439974f00abc1181b39fff7d6ba2fa' } }
{ acf_fc_layout: 'text',
  text: '<p>Text 3</p>\n',
  id: '37faeda9-a006-5368-afff-c5d7b13d79940builderWordPressAcf_gridgrid',
  parent: '37faeda9-a006-5368-afff-c5d7b13d79940',
  children: [],
  internal:
   { type: 'WordPressAcf_gridgrid',
     contentDigest: '18a1117cefaf94226c53b5993c22df0d' } }

It is probably not the best way to do it, but an unique id will fix the issue:

const acfChildNode = { 
  ...obj, 
  id: _.random(10000) + '-'' + entityId + topLevelIndex + type, 
  parent: entityId, 
  children: [], 
  internal: { type, contentDigest: digest(JSON.stringify(obj)) }, 
} 

The code above returns:

{ acf_fc_layout: 'text',
  text: '<p>Text 1</p>\n',
  id: '695-37faeda9-a006-5368-afff-c5d7b13d79940builderWordPressAcf_gridgrid',
  parent: '37faeda9-a006-5368-afff-c5d7b13d79940',
  children: [],
  internal:
   { type: 'WordPressAcf_gridgrid',
     contentDigest: '6a30381167e7ae819989db8bd01e0057' } }
{ acf_fc_layout: 'text',
  text: '<p>Text 2</p>\n',
  id: '3937-37faeda9-a006-5368-afff-c5d7b13d79940builderWordPressAcf_gridgrid',
  parent: '37faeda9-a006-5368-afff-c5d7b13d79940',
  children: [],
  internal:
   { type: 'WordPressAcf_gridgrid',
     contentDigest: '3d439974f00abc1181b39fff7d6ba2fa' } }
{ acf_fc_layout: 'text',
  text: '<p>Text 3</p>\n',
  id: '8187-37faeda9-a006-5368-afff-c5d7b13d79940builderWordPressAcf_gridgrid',
  parent: '37faeda9-a006-5368-afff-c5d7b13d79940',
  children: [],
  internal:
   { type: 'WordPressAcf_gridgrid',
     contentDigest: '18a1117cefaf94226c53b5993c22df0d' } }

GraphiQL response:

{
  "data": {
    "allWordpressPage": {
      "edges": [
        {
          "node": {
            "id": "270e0b30-05f3-5c06-87bc-fa47a0b9ed03",
            "title": "Test",
            "slug": "test",
            "acf": {
              "builder_page": [
                {
                  "__typename": "WordPressAcf_grid",
                  "id": "4682-270e0b30-05f3-5c06-87bc-fa47a0b9ed030builderWordPressAcf_grid",
                  "grid": [
                    {
                      "acf_fc_layout": "text",
                      "id": "9390-270e0b30-05f3-5c06-87bc-fa47a0b9ed030builderWordPressAcf_gridgrid",
                      "text": "<p>Text 1</p>\n",
                    },
                    {
                      "acf_fc_layout": "text",
                      "id": "6791-270e0b30-05f3-5c06-87bc-fa47a0b9ed030builderWordPressAcf_gridgrid",
                      "text": "<p>Text 2</p>\n",
                    },
                    {
                      "acf_fc_layout": "text",
                      "id": "3104-270e0b30-05f3-5c06-87bc-fa47a0b9ed030builderWordPressAcf_gridgrid",
                      "text": "<p>Text 3</p>\n",
                    }
                  ],
                }
              ]
            }
          }
        }
      ]
    }
  }
}

@pieh
Copy link
Contributor

pieh commented Oct 10, 2018

It is probably not the best way to do it, but an unique id will fix the issue:

const acfChildNode = { 
  ...obj, 
  id: _.random(10000) + '-'' + entityId + topLevelIndex + type, 
  parent: entityId, 
  children: [], 
  internal: { type, contentDigest: digest(JSON.stringify(obj)) }, 
} 

Yeah, if possible we should avoid using _.random. I think in the code I linked above, when we mapping over flexible content items we would need to use index and pass it over to prepareACFChildNodes - maybe something like this?

_.each(obj, (value, key) => { 
     if (_.isArray(value) && value[0] && value[0].acf_fc_layout) { 
       obj[`${key}___NODE`] = value.map( 
-         v => 
+         (v, itemIndex) => 
           prepareACFChildNodes( 
             v, 
-             entityId, 
+             `${entityId}_${itemIndex}`, 
             topLevelIndex, 
             type + key, 
             children, 
             childrenNodes 
           ).id 
       ) 
       delete obj[key] 
     } 
   }) 

@jordyvg
Copy link
Contributor Author

jordyvg commented Oct 10, 2018

Yeah, if possible we should avoid using _.random. I think in the code I linked above, when we mapping over flexible content items we would need to use index and pass it over to prepareACFChildNodes - maybe something like this?

_.each(obj, (value, key) => { 
     if (_.isArray(value) && value[0] && value[0].acf_fc_layout) { 
       obj[`${key}___NODE`] = value.map( 
-         v => 
+         (v, itemIndex) => 
           prepareACFChildNodes( 
             v, 
-             entityId, 
+             `${entityId}_${itemIndex}`, 
             topLevelIndex, 
             type + key, 
             children, 
             childrenNodes 
           ).id 
       ) 
       delete obj[key] 
     } 
   }) 

Works, seems fine to me.

@pieh
Copy link
Contributor

pieh commented Oct 10, 2018

@jordyvg would you be interested in opening PR with that change?

jordyvg added a commit to jordyvg/gatsby that referenced this issue Oct 10, 2018
jordyvg added a commit to jordyvg/gatsby that referenced this issue Oct 10, 2018
@jordyvg jordyvg closed this as completed Oct 11, 2018
pieh pushed a commit that referenced this issue Oct 11, 2018
* Fix: added unique id for ACF childeNodes (#8910)

* Fix: added unique id for ACF childeNodes (#8910)
jedrichards pushed a commit to jedrichards/gatsby that referenced this issue Nov 1, 2018
)

* Fix: added unique id for ACF childeNodes (gatsbyjs#8910)

* Fix: added unique id for ACF childeNodes (gatsbyjs#8910)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

No branches or pull requests

3 participants