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] fetchReferencedMediaItemsAndCreateNodes resolves too early #31646

Closed
rburgst opened this issue May 29, 2021 · 9 comments · Fixed by #31652 or #32679
Closed

[gatsby-source-wordpress] fetchReferencedMediaItemsAndCreateNodes resolves too early #31646

rburgst opened this issue May 29, 2021 · 9 comments · Fixed by #31652 or #32679
Labels
topic: source-wordpress Related to Gatsby's integration with WordPress type: bug An issue or pull request relating to a bug in Gatsby

Comments

@rburgst
Copy link
Contributor

rburgst commented May 29, 2021

Description

I have an ACF that contains HTML content which contains multiple audio element which is referencing a wordpress hosted audio file. The sum of all referenced media items exceeds the configured schema.perPage configured page size.
In this case it seems that the promise for fetchReferencedMediaItemsAndCreateNodes resolves with only parts of the received data (IMHO the last page) rather than the combined list of media Items.

This leads to the case where you have only a few of the media items urls replaced, while the others still point to the wordpress server.

Steps to reproduce

  1. generate the starter project gatsby new my-wordpress-gatsby-site https://github.com/gatsbyjs/gatsby-starter-wordpress-blog
  2. configure the following options
     schema: {
          perPage: 2,
        },
    
  3. Create an empty wordpress instance, install, wp-graphql and update the automatically created post to contain 3 audio tags (mp3)
  4. run
    yarn clean
    yarn develop
    
  5. now go to http://localhost:8000/___graphql and execute the following query
     query MyQuery {
      allWpPost {
       nodes {
         slug
         content
         }
       }
     }
    
  6. observe the output HTML
      {
       "data": {
          "allWpPost": {
            "nodes": [
              {
                "slug": "hello-world",
                "content": "\n<p>Welcome to WordPress. This is your first post. Edit or delete it, then start writing!</p>\n\n\n\n<figure class=\"wp-block-audio\">
                        <audio controls src=\"http://dockerhost:8000/wp-content/uploads/2021/05/file1.mp3\"></audio></figure>\n\n\n\n<figure class=\"wp-block-audio\">
                       <audio controls src=\"http://dockerhost:8000/wp-content/uploads/2021/05/file2.mp3\"></audio></figure>\n\n\n\n<figure class=\"wp-block-audio\">
                        <audio controls src=\"http://dockerhost:8000/wp-content/uploads/2021/05/file3.mp3\"></audio></figure>\n"
              }
            ]
          }
        },
        "extensions": {} 
     }
    

Expected result

the 3 audio files should be rewritten to /static... urls

Actual result

The urls are not rewritten at all.

Environment


  System:
    OS: macOS 11.4
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.16.0 - /var/folders/dm/59s6_w2s3jl78c__p8j_cn1w0000gn/T/yarn--1622281277197-0.1413190554991537/node
    Yarn: 1.22.10 - /var/folders/dm/59s6_w2s3jl78c__p8j_cn1w0000gn/T/yarn--1622281277197-0.1413190554991537/yarn
    npm: 7.7.6 - ~/.nvm/versions/node/v14.16.0/bin/npm
  Languages:
    Python: 2.7.17 - /usr/local/bin/python
  Browsers:
    Chrome: 90.0.4430.212
    Firefox: 88.0.1
    Safari: 14.1.1
  npmPackages:
    gatsby: ^3.3.0 => 3.6.1 
    gatsby-image: ^3.3.0 => 3.6.0 
    gatsby-plugin-manifest: ^3.3.0 => 3.6.0 
    gatsby-plugin-offline: ^4.3.0 => 4.6.0 
    gatsby-plugin-react-helmet: ^4.3.0 => 4.6.0 
    gatsby-plugin-sharp: ^3.3.0 => 3.6.0 
    gatsby-source-filesystem: ^3.3.0 => 3.6.0 
    gatsby-source-wordpress: ^5.4.1 => 5.6.0 
    gatsby-transformer-sharp: ^3.3.0 => 3.6.0 
@rburgst rburgst added the type: bug An issue or pull request relating to a bug in Gatsby label May 29, 2021
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 29, 2021
@rburgst
Copy link
Contributor Author

rburgst commented May 29, 2021

Note that for repro you should always clear the cache (yarn clean) before starting yarn develop

@rburgst
Copy link
Contributor Author

rburgst commented May 29, 2021

created a sample repo (although there is not really anything to it other than setting schema.perPage to 2

https://github.com/rburgst/gatsby-wordpress-file-rewriting-bug

@rburgst
Copy link
Contributor Author

rburgst commented May 29, 2021

This is what the default wordpress post with 2 audio elements looks like:
image

@rburgst
Copy link
Contributor Author

rburgst commented May 29, 2021

Note that the rewriting works if I only add a single audio clip. It starts to fail with 2 audio clips.

@rburgst
Copy link
Contributor Author

rburgst commented May 30, 2021

I am fairly certain the problem is in fetchMediaItemsBySourceUrl it seems to me that the whole promise is being resolved when the first page is completed. Therefore, the await in replaceFileLinks is only receiving the data from the first page.
The problem is that audio tags in wordpress append a ?_=1 to the URL,

<audio
  class="wp-audio-shortcode"
  id="audio-2519-7"
  preload="none"
  style="width: 100%"
  controls="controls"
>
  <source
    type="audio/mpeg"
    src="https://wordpress.host/wp-content/uploads/2018/05/file1.mp3?_=7"
  />
  <a href="https://wordpress.host/wp-content/uploads/2018/05/file1.mp3">
    https://wordpress.host/wp-content/uploads/2018/05/file1.mp3
  </a>
</audio>

therefore, the graphql query that loads the media info data will produce empty results for https://wordpress.host/wp-content/uploads/2018/05/file1.mp3?_=7 but it will actually resolve the https://wordpress.host/wp-content/uploads/2018/05/file1.mp3 from the <a href> tag.

Thus, the first graphql query page data will contain many null fields and therefore, you need to wait until all pages are done and resolve the concatenated result.

rburgst added a commit to rburgst/gatsby that referenced this issue May 30, 2021
- in the case of either having a small `schema.perPage` setting
  or many many referenced resources on a single page only the first
  page of links were properly rewritten
- now we ensure that all pages are fully processed until we resolve the
  promise

fixes gatsbyjs#31646
@LekoArts LekoArts added topic: source-wordpress Related to Gatsby's integration with WordPress and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels May 31, 2021
@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Jun 20, 2021
@rburgst
Copy link
Contributor Author

rburgst commented Jun 21, 2021

I would really appreciate timely feedback to this important bugfix

@github-actions github-actions bot removed the stale? Issue that may be closed soon due to the original author not responding any more. label Jun 21, 2021
@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Jul 11, 2021
@rburgst
Copy link
Contributor Author

rburgst commented Jul 12, 2021

any news here? this is getting quite frustrating having to wait for months for a bugfix to be reviewed.

@github-actions github-actions bot removed the stale? Issue that may be closed soon due to the original author not responding any more. label Jul 12, 2021
gatsbybot pushed a commit that referenced this issue Jul 15, 2021
- in the case of either having a small `schema.perPage` setting
  or many many referenced resources on a single page only the first
  page of links were properly rewritten
- now we ensure that all pages are fully processed until we resolve the
  promise

fixes #31646

Co-authored-by: Ward Peeters <[email protected]>
rburgst added a commit to rburgst/gatsby that referenced this issue Aug 3, 2021
- in the case of either having a small `schema.perPage` setting
  or many many referenced resources on a single page only the first
  page of links were properly rewritten
- now we ensure that all pages are fully processed until we resolve the
  promise
- this is a recommit of gatsbyjs#31652

fixes gatsbyjs#31646
rburgst added a commit to rburgst/gatsby that referenced this issue Aug 3, 2021
- in the case of either having a small `schema.perPage` setting
  or many many referenced resources on a single page only the first
  page of links were properly rewritten
- now we ensure that all pages are fully processed until we resolve the
  promise
- this is an improved recommit of gatsbyjs#31652

fixes gatsbyjs#31646
TylerBarnes added a commit that referenced this issue Aug 9, 2021
* fix(wordpress): ensure all file links are rewritten

- in the case of either having a small `schema.perPage` setting
  or many many referenced resources on a single page only the first
  page of links were properly rewritten
- now we ensure that all pages are fully processed until we resolve the
  promise
- this is an improved recommit of #31652

fixes #31646

Co-authored-by: gatsbybot <[email protected]>
Co-authored-by: Tyler Barnes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: source-wordpress Related to Gatsby's integration with WordPress type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
2 participants