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

Avoid fatal error when Pulling an item that has been Pulled already on a different site #1159

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Nov 21, 2023

Description of the Change

Within a multisite instance, if you Pull an item from one site to another (say from Site A to Site B), the original item has the connection details stored in the dt_connection_map meta.

If you then pull that same item into a different site (say from Site A into Site C) we check if the original item has that meta and if so, we check to see if the current site ID exists within that meta (which would indicate a duplicate item).

The problem here is the code we run looks for a different structure than what we actually have. Unsure if this is a new bug coming out of the v2 release or if this is a long standing issue that just hasn't been found. EDIT: Tested further and seems to be something that was always an issue but PHP 8.1 (maybe 8.0 but didn't test that) started throwing a fatal error instead of just a warning. So anyone running PHP 7.4 or less wouldn't see this.

This PR fixes this by using the correct data structure.

Closes #1158

How to test the Change

  1. Set up a WordPress multisite with Distributor active and at least three sites in place
  2. Create a piece of content on the first site
  3. On the latest released version of Distributor, Pull this content into the second site
  4. Then try pulling this content into the third site. A PHP fatal error should happen
  5. Check out this PR, create a new piece of content on the first site and run through the steps again
  6. No PHP fatals should happen

Changelog Entry

Fixed - Avoid a PHP fatal error when pulling content that has previously been pulled into a different network site

Credits

Props @dkotter, @jeffpaul

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@dkotter dkotter added this to the 2.0.2 milestone Nov 21, 2023
@dkotter dkotter self-assigned this Nov 21, 2023
@dkotter dkotter requested a review from a team as a code owner November 21, 2023 21:53
@dkotter dkotter requested review from peterwilsoncc and removed request for a team November 21, 2023 21:53
Copy link
Collaborator

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me and resolves the error listed in the ticket.

While testing, I did notice that these lines in the same method can also trigger a fatal error

$post = $this->remote_get( [ 'id' => $item_array['remote_post_id'] ], $insert_args );
if ( is_wp_error( $post ) ) {
$created_posts[] = $post;
continue;
}

remote_get() can return false if the post is an invalid ID and the unexpected value isn't caught by the is_wp_error() condition. This subsequently passes the value to a number of functions that do not handle booleans, triggers a few warnings and finally a fatal error.

I'm happy if you want to merge this and follow up in another PR for the issue of an invalid post ID.

@dkotter dkotter merged commit b8e5c5a into develop Nov 22, 2023
12 of 17 checks passed
@dkotter dkotter deleted the fix/1158 branch November 22, 2023 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fatal error due to using array_key_exists on a non-array value
2 participants