Allow `use_filters` change when creating post

xipasduarte commented 2 years ago

Context

This was an issue encountered while using the Distributor plugin, but fixing it can allow for greater compatibility with other plugins. When duplicating a post provide a way to enable filtering the original data that gets copied (post data, meta values and terms).

We opted not to include all instances of use_filters because one was already addressed in #244, even though the filter's name might need some adjusting.

Summary

This PR can be summarized in the following changelog entry: Adds a new filter duplicate_post_create_duplicate_use_filters to enable filtering the original data that gets copied (post data, meta values and terms).

Relevant technical choices:

The prefix for the filter name was kept consistent with the rest of the plugin, while adding specific details with function and property being filtered.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Enable data filtering on create:

// Uses WordPress' global functions.
add_filter( 'duplicate_post_create_duplicate_use_filters', '__return_true' );

Add some meta to be excluded


public function exclude_meta_keys( array $meta_keys ) : array {
// Replace this meta key according to your setup.
$meta_keys[] = 'excluded_meta_key';
return $meta_keys;
}

add_filter( 'duplicate_post_excludelist_filter', 'exclude_meta_keys' );


- Duplicate and check that the `'excluded_meta_key'` is not present in the new post.

#### Relevant test scenarios
* [ ] Changes should be tested with the browser console open
* [x] Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
* [ ] Changes should be tested on different editors (Block/Classic/Elementor/other)
* [ ] Changes should be tested on different browsers
* [ ] Changes should be tested on multisite
    <!--
    If you have checked any of the above cases, please add some context about the reason, what to check in the console,
    which type/editor/browser should be tested in particular, multisite with subfolders or subdomains, etc.
    -->

### Test instructions for QA when the code is in the RC
<!--
Sometimes some steps from the test instructions for the acceptance test aren't relevant anymore once the code has been merged or the feature is complete. If that is the case, do not check the checkbox below.
QA is our Quality Assurance team. The RC is the release candidate zip that is tested before a release
-->

* [x] QA should use the same steps as above.

## Impact check
<!--
Sometimes PRs have a bigger impact than is suggested in the user-facing changes. In such cases,
additional (regression) testing might be necessary. To make it clear what parts might need additional testing, please outline which parts of the plugin have been impacted by this PR.
-->
This PR affects the following parts of the plugin, which may require extra testing:

* None that we could find.

## UI changes

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

## Documentation

* [x] I have written documentation for this change.

## Quality assurance

* [x] I have tested this code to the best of my abilities
* [ ] I have added unittests to verify the code works as intended

Fixes #204

grappler commented 2 years ago

Thank you @xipasduarte for working on this PR.

I don't think this is the right solution as all of the filters will be activated. There is no way of only activating certain filters.

https://github.com/Yoast/duplicate-post/blob/1b790c12af24369f7cb73c628fafb53e3fc261ec/src/post-duplicator.php#L101-L111 https://github.com/Yoast/duplicate-post/blob/1b790c12af24369f7cb73c628fafb53e3fc261ec/src/post-duplicator.php#L207-L216 https://github.com/Yoast/duplicate-post/blob/1b790c12af24369f7cb73c628fafb53e3fc261ec/src/post-duplicator.php#L257-L266 https://github.com/Yoast/duplicate-post/blob/1b790c12af24369f7cb73c628fafb53e3fc261ec/src/post-duplicator.php#L283-L292

xipasduarte commented 2 years ago

@grappler Hello, I understand, but what you're saying is a level deeper. We could make this into a map that enables each use of $options['use_filters'] with more granularity. Something like:

[
    'use_case_1' => true,
    'use_case_2' => false,
]

This would then allow us to change the conditions to:

if ( $options['use_filters']['use_case_1'] ) {
    // Run use_case_1
}

Let me know if you have any other suggestions.

xipasduarte commented 1 month ago

Hello. Do you have any suggestions for this? Most 'use_filters' are disabled for operations that a user would expect to have the duplicate_post_excludelist_filter work. Copy for republish and rewrite afterwards are the main to concerns here. This PR only adds an option for the copy phase, but we could extend the same solution to the republish.

There is an external (outside duplicate post) solution to do this. By hooking everything in the wp_insert_post hook, checking that the post is a rewrite, and cleaning meta fields. The same filter could be used to be easier to remove custom code, in case the plugin adds support later.

Something like:

// Add excluded metas.
function my_custom_fields_filter( $meta_excludelist ) {
    // Merges the defaults array with our own array of custom fields.
    return array_merge( $meta_excludelist, [ 'my_custom_field1', 'my_custom_field2' ] );
}

add_filter( 'duplicate_post_excludelist_filter', 'my_custom_fields_filter' );

// Remove metas on insert.
add_action( 'wp_insert_post', function ( $post_id ) {

    // Check if this is a rewrite
    $is_rewrite = get_post_meta( '_dp_is_rewrite_republish_copy', $post_id, true );
    if ( $is_rewrite ) {

        // Get excluded metas.
        $excluded_metas = apply_filters( 'duplicate_post_excludelist_filter', [] );

        // Delete excluded metas.
        foreach ( $excluded_metas as $meta_key ) {
            delete_post_meta( $meta_key, $post_id );
        }
    }
}, 10 );

Yoast / duplicate-post