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

External connections do not distribute media #975

Closed
1 task done
peterwilsoncc opened this issue Nov 22, 2022 · 12 comments
Closed
1 task done

External connections do not distribute media #975

peterwilsoncc opened this issue Nov 22, 2022 · 12 comments
Labels
help wanted needs:engineering This requires engineering to resolve. nice-to-have type:bug Something isn't working.
Milestone

Comments

@peterwilsoncc
Copy link
Collaborator

peterwilsoncc commented Nov 22, 2022

Describe the bug

When pushing or pulling content from an external connection, no media is distributed regardless of the settings in the admin.

This occurs regardless of whether the featured image is attached to the post or not.

Internal/network connections do distribute the featured images.

Updates:

Steps to Reproduce

  1. Create two sites single sites running Distributor
  2. Connect them bidirectionally (ie, so each site has the other as an external connection)
  3. Create a post on one site
  4. Add and in-content image(s)
  5. publish the post
  6. Either
    a. Push the post from the first sites screen
    b. Pull the post from the second sites admin
  7. Open the pulled post in the editor, and observe there is no in-content image(s).

Screenshots, screen recording, code snippet

Source site
Screen Shot 2022-11-22 at 2 09 03 pm

Pulled content
Screen Shot 2022-11-22 at 2 10 15 pm

Pushed content
Screen Shot 2022-11-22 at 2 11 10 pm

Environment information

No response

WordPress information

  • Distributor Develop (at 3bb3e3f )
  • WordPress 6.1.1

Code of Conduct

  • I agree to follow this project's Code of Conduct
@peterwilsoncc peterwilsoncc added the type:bug Something isn't working. label Nov 22, 2022
@vikrampm1 vikrampm1 moved this to Incoming in Open Source Practice Nov 22, 2022
@jeffpaul jeffpaul added help wanted needs:engineering This requires engineering to resolve. labels Dec 1, 2022
@jeffpaul jeffpaul moved this from Incoming to To Do in Open Source Practice Dec 1, 2022
@jeffpaul jeffpaul added this to the 2.0.0 milestone Dec 1, 2022
@gleysen
Copy link

gleysen commented Jan 11, 2023

Hope this gets fixed soon.. really need this as well...

@faisal-alvi
Copy link
Member

@peterwilsoncc I've performed the steps you suggested above, and I've seen that media is distributed between external connections without any problems (Tested on the "develop" branch). Please have a look at the video and let me know if there is anything I missed.

dist-975-working.mp4

@gleysen
Copy link

gleysen commented Jan 12, 2023

Hey Faisal,
Thank you for the response, I looked at the video, and this is also working for me.However, the problem I had is with images inside ACF fields. Most of my clients posts are extended with ACF fields to add photo's or have ACF wysiwyg fields to add content (with the possibility to add images in there)And those images don't get synced.

@gleysen
Copy link

gleysen commented Jan 12, 2023 via email

@faisal-alvi
Copy link
Member

faisal-alvi commented Jan 12, 2023

@gleysen thank you for the details. This seems to be a duplicate of #81. Closing this one, but if you have any further information that isn't included in #81, feel free to open it again.

@github-project-automation github-project-automation bot moved this from To Do to Merged in Open Source Practice Jan 12, 2023
@dkotter
Copy link
Collaborator

dkotter commented Jan 13, 2023

So a few things here:

  • The issue title here mentions how external connections aren't distributing media but the test steps only mention how featured images aren't working. From the tests @faisal-alvi has run, it seems like this (featured images) are working as expected now (can you verify this @peterwilsoncc?)
  • We've had a few separate issues (most recently Attachment images not getting transferred #993) that mention in-content images aren't getting transferred though the featured image is
  • And the most recent follow up on this issue is around ACF, which is a known issue with Distributor right now (so something we want to eventually solve but not necessarily part of the investigation here)

I have run some tests myself with just in-content images and I've found some issues (see below). As such I'm reopening this issue. Note we may want to update the description of this issue to make it more clear that it seems to be more of an issue with in-content images and not featured images.

All tests were run with the setting Process the featured image and any attached images turned on for both sites as well as running Distributor v1.9.0

Internal Connections

  • Push
    • Block editor
      • Image was added to the media library ✅ but the URL in the content still referenced the original site ❌
    • Classic editor
      • Image was added to the media library ✅ but the URL in the content still referenced the original site ❌
  • Pull
    • Block editor
      • Image was added to the media library ✅ but the URL in the content still referenced the original site ❌
    • Classic editor
      • Image was added to the media library ✅ but the URL in the content still referenced the original site ❌

External Connections

  • Push
    • Block editor
      • Image not pushed at all ❌
    • Classic editor
      • Image not pushed at all ❌
  • Pull
    • Block editor
      • Image was added to the media library ✅ but the URL in the content still referenced the original site ❌
    • Classic editor
      • Image was added to the media library ✅ but the URL in the content still referenced the original site ❌

So from my testing, not a single scenario passed entirely. Most of them distributed the image as expected but just didn't update the URL in the content to reference the new site. But Pushing content to an external connection didn't do anything with media.

I still need to investigate why this is happening (unless someone else gets to this first) but definitely seems to be a problem. For reference, this setting was added in #184, so may be worth looking at that to get a sense of where to start.

@dkotter dkotter reopened this Jan 13, 2023
@github-project-automation github-project-automation bot moved this from Merged to In Progress in Open Source Practice Jan 13, 2023
@dkotter
Copy link
Collaborator

dkotter commented Jan 25, 2023

Following up on the ^ above, I refreshed my memory even further on this and most of the above failures are expected (though would still be great to get fixed).

See #152 as well as this comment for more details but the current approach will send images over (if the setting is in place) but won't actually update the image URLs in the content (as there's more to still figure out there on how best to handle this).

So looks like the main failure here that isn't already known is Pushing to External Connections, as that doesn't seem to send images at all.

@kirtangajjar
Copy link
Member

@dkotter I had a look at it and I found that for pushing, the featured image does not appear while debugging locally, but it does appear if both sites are publically available. The root cause of the issue seemed to be the fact that the SSL certificates on localhost are self-signed (which does not get recognised by the PHP container). To test this issue, I have created two different WP instances on my server.

The credentials are available here. (Both sites and the server is temporary so it's okay to expose them publically for time being).

@peterwilsoncc peterwilsoncc modified the milestones: 2.0.0, 2.0.1 May 25, 2023
@peterwilsoncc
Copy link
Collaborator Author

@faisal-alvi @dkotter I came back to this while looking at another PR.

The reason I was not seeing features images in an external pull request was because my local was set up to use content URLs with a relative scheme //example.com/wp-content/upload/.../ben-platt.jpg

In WordPress this fails because WordPress isn't a browser so there is no scheme to be relative to (code reference).

In this block of code, Distributor should check for relative URLs and add a scheme if there isn't one set

if ( $download_url ) {
// Allows to pull media from local IP addresses
// Uses a "magic number" for priority so we only unhook our call, just in case.
add_filter( 'http_request_host_is_external', '__return_true', 88 );
// Download file to temp location.
$file_array['tmp_name'] = download_url( $url );
remove_filter( 'http_request_host_is_external', '__return_true', 88 );
}

Technically, there is no reason an http URL needs to produce the same result as its https counterpart. However, I think it is safe to assume that they will match for a typical WordPress site.

I suggest we default to an http URL as download_url follows redirects and will download the final destination.

@roshniahuja roshniahuja self-assigned this Jul 21, 2023
@faisal-alvi
Copy link
Member

@roshniahuja I see you self-assigned this ticket. Please note that I have updated the description to mention that the issue is with the in-content images and not featured images. Also, please see #975 (comment) for more details if you haven't already.

@roshniahuja roshniahuja removed their assignment Aug 8, 2023
@roshniahuja
Copy link
Contributor

@faisal-alvi , I am not working on this task anymore so I removed my self from this task.

Thanks..!!

@jeffpaul
Copy link
Member

Closing via #1110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted needs:engineering This requires engineering to resolve. nice-to-have type:bug Something isn't working.
Projects
Archived in project
Status: Done
Development

No branches or pull requests

7 participants