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

Clear existing meta data to fix rewrite and republish with non-unique meta keys #192

Merged
merged 1 commit into from
Oct 28, 2021

Conversation

ocean90
Copy link
Contributor

@ocean90 ocean90 commented Aug 11, 2021

Context

As mentioned in #190, when using the Rewrite & Republish feature data of non-unique meta keys is lost due to the use of update_post_meta() in Post_Duplicator::copy_post_meta_info().
This is not an issue with the Clone feature because duplicate_post_copy_post_meta_info() is using add_post_meta(). That's why this PR suggests to use add_post_meta() in Post_Duplicator::copy_post_meta_info() too. But since the same method is also used by Post_Republisher::republish_post_meta() we have to make sure that existing meta data is removed first.

Summary

This PR can be summarized in the following changelog entry:

  • Fixes a bug where meta data went missing for duplicate posts created by the Rewrite & Republish feature. Props to @ocean90.

Relevant technical choices:

How to reproduce the bug

  • Install and activate Duplicate Post 4.1.2
  • Create a post which has at least two custom fields with same name (e.g. test_field) and different values
    • In the block editor, you'll probably need to visit the Preferences and enable "Custom fields":
      Schermata a 2021-10-28 13-50-51
  • Publish it
  • Create a Rewrite & Republish copy from it, do some edits, and republish.
  • Edit the rewritten post and see that there is only one custom field with the given name, while the others have disappeared.

Test the solution

  • Install and activate Duplicate Post trunk or RC with this PR merged in
  • Create a post which has at least two custom fields with same name (e.g. test_field) and different values
    • In the block editor, you'll probably need to visit the Preferences and enable "Custom fields":
      Schermata a 2021-10-28 13-50-51
  • Publish it
  • Create a Rewrite & Republish copy from it, do some edits, and republish.
  • Edit the rewritten post and see that you can still find every custom field you could see in the original post.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Documentation

  • I have written documentation for this change.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended

Fixes #190.

@karlijnbok
Copy link
Contributor

Thanks for your contribution @ocean90! Looks good, I am merging the patch.

@karlijnbok karlijnbok merged commit 1b790c1 into Yoast:trunk Oct 28, 2021
@grappler grappler deleted the fix/delete-meta-on-republish branch November 18, 2021 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non unqiue (not single) are not all copied
3 participants