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

Add / Media Credit Plugin Migrator #462

Merged
merged 18 commits into from
May 9, 2024

Conversation

ronchambers
Copy link
Collaborator

@ronchambers ronchambers commented Mar 23, 2024

This PR adds a general command to migrate the Media Credit Plugin to native [caption] shortcodes and post meta _media_credit for use with the Newspack Plugin media credits.

Caution: (FIXED WITH #498) This command can cause data to be lost. Please perform a --dry-run and examine the final "Report" section for data differences that will be removed from the post content image captions. The Media Credit Plugin not only stores media credits in the post meta table using the _media_credit key, it also allows Admins to add custom credits on images directly in the post editor. In many cases these custom strings match the value in post meta, but many times they do not. The CLI command in this PR will attempt to save the custom string into the post meta if the post meta is blank, but in the cases where the post meta already exists and is different than the custom string, the custom strings are removed from the post content. The only way to capture these for Publisher review is to do a --dry-run and view the "Report" section at the end of the log output for "by-hand" review.

Caution: This command must be run prior to converting post content to Blocks, otherwise the block converter will be unable to properly remove the Media Credit Plugin's [media-credit] shortcode. This command will convert [media-credit] shortcodes to native [caption] shortcodes that the Block converter can properly convert.

Command: wp newspack-content-migrator migrate-media-credit-plugin

Output: MediaCreditPluginMigrator_cmd_migrate_media_credit_plugin.log

Options: --dry-run will provide a data loss "Report" at the end of the log. (Data loss is fixed with PR)

Future ideas:

  • Convert [caption] and [media-credit] shortcodes directly into Blocks and stash the custom credit strings into the blocks meta. The Newspack Plugin allows custom meta, but only if post meta isn't set. What we need is the ability to add custom meta even if post meta exists. This line needs to move to this line. Keep in mind even if the plugin code is changed, and the CLI outputs Blocks, there is a chance that a single block inside classic HTML might make the block converter not convert the other content within the post (it appears that if a classic html post has a block as the first element, the remaining content might no be converted).
  • (FIXED WITH PR) Possibly save the --dry-run "Report" differences to additional post meta so text file differences don't need to be captured (ie: the lost data will be saved to the attachment image's post meta for safe keeping).

  • confirmed that PHPCS has been run

@ronchambers ronchambers requested a review from iuravic April 3, 2024 02:00
@ronchambers
Copy link
Collaborator Author

@iuravic - feel free to review. I worked with Andrew on the first "caution/dry-run" issue already and I posted to Slack in regards to the second "caution". So I feel confident on the logic here, but if you see any issues within the code feel free to let me know :-)

@ronchambers
Copy link
Collaborator Author

Commit f328cfb FIXES an issue with how the Media Credit Plugin wrote [caption] shortcodes vs the way Newspack Content Converter changes these into image blocks. This fix will remove un-needed line breaks from the shortcodes that were causing block conversion issues.

Old/wrong with line break:

[caption ...]
<img...>[/caption]

New/fixed without line break:

[caption ...]<img...>[/caption]

@ronchambers ronchambers requested a review from naxoc May 9, 2024 14:37
Copy link
Member

@naxoc naxoc left a comment

Choose a reason for hiding this comment

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

Looks good!
OK

Keep in mind even if the plugin code is changed, and the CLI outputs Blocks, there is a chance that a single block inside classic HTML might make the block converter not convert the other content within the post (it appears that if a classic html post has a block as the first element, the remaining content might no be converted).

In this PR #487 there are some CLI tools to "nudge" posts that start with a block so the NCC will pick it up, so that should not be a concern once we merge that.

@ronchambers ronchambers merged commit c85d678 into trunk May 9, 2024
@ronchambers ronchambers deleted the add/media-credit-plugin-migrator branch May 9, 2024 16:04
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.

None yet

2 participants