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

Simulate data liberation via hook #126

Open
wants to merge 1 commit into
base: docs/extend
Choose a base branch
from

Conversation

ashfame
Copy link
Member

@ashfame ashfame commented Nov 25, 2024

This PR attempts to explore the feedback provided in #125 for plugins wanting to participate in "post" liberation by not having to specify the same code twice.

Problem:

During liberation hook

add_action( 'data_liberated_product', function( $subject ) {
    // some code
} );

Post liberation hook

foreach( \DotOrg\TryWordPress\Subject_Repo::loop( 'product' ) as $subject ) {
    // same code here
}

Suggested solution as it was discussed on call was to: Just trigger the data_liberated_X hook again in post liberation handling. The plugin can indicate when it wants to do that by do_action( 'do_data_liberation', 'product' ); and we would fire the data_liberated_product hook to which the plugin is already attached. This PR has the code for it, but the problem with this approach is, there could be other plugins still attached to data_liberated_product and we don't want all of them to be executing, only the one triggering it. In order to achieve that, we would have to hack global state, which is an anti-pattern I would like to avoid.


So, how about the solution to be just this?

We ask the developers, to define a function to handle the subject type they are interested in:

function unique_prefix_subject_handler( $subject) {
     // all code goes here
}

and update docs to

During liberation hook

add_action( 'data_liberated_product', 'unique_prefix_subject_handler' );

Post liberation hook

foreach( \DotOrg\TryWordPress\Subject_Repo::loop( 'product' ) as $subject ) {
    unique_prefix_subject_handler( $subject );
}

Pretty simple solution! Thoughts?

@ashfame ashfame self-assigned this Nov 25, 2024
@ashfame ashfame requested review from psrpinto and akirk November 25, 2024 19:24
@ashfame
Copy link
Member Author

ashfame commented Dec 5, 2024

@psrpinto Current code change in the PR has a problem as I described in the PR description, which is followed by a solution that's quite a simple one. If we can agree that solves the feedback this PR was meant to address, then I can go ahead and make that change.

@akirk
Copy link
Member

akirk commented Dec 5, 2024

What about using remove_all_actions and ask the plugin to re-attach before rerunning the liberation?

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.

3 participants