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

feat(#524): Add hooks for subscription(s) post push #1043

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

aaronware
Copy link

@aaronware aaronware commented Apr 6, 2023

Description of the Change

This feature adds to hooks for to subscriptions.php dt_subscription_after_post_push and dt_subscriptions_after_post_push to be able to take action after a post has been pushed to a subscription and after the entire set of subscriptions have been pushed regardless of success or failure.

These hooks are not nearly as robust as dt_push_post as I tried to keep the change as slim as possible.

Closes #524 (as potentially stale issue or could be considered a separate issue)

How to test the Change

utilize

add_action( 'dt_subscriptions_after_post_push' ...
add_action( 'dt_subscription_after_post_push' ...

Changelog Entry

Added - New hooks to allow developers to extend post push subscriptions

Credits

Props @aaronware,

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly. I added docblocks, that should generate accordingly
  • I have added tests to cover my change. I need to research what I need to do here
  • All new and existing tests pass.

This feature adds to hooks for dt_subscription_after_post_push and dt_subscriptions_after_post_push to be able to take action after a subscribed post has been pushed and after the entire set of subscriptions have been pushed
@aaronware
Copy link
Author

I can review this draft request and the 2.0 project to make sure this is still accurate. It's been a while since I looked at this

@jeffpaul
Copy link
Member

jeffpaul commented Feb 7, 2024

@aaronware are you interested in continuing the extension work here (assuming primarily for deeper Woo support)?

@aaronware
Copy link
Author

@jeffpaul yes definitely. Now that 2.x has been out in the wild I can revisit. My apprehension before and why I drafted this was adding to the 1.9.x branch when so much was changing in 2.x. There are some quick wins to add hooks like what I drafted, however, I wasn't sure if you all wanted something more structured when integrating with Distributor for other plugins similar to Steam for example

@vikrampm1 vikrampm1 requested review from faisal-alvi and removed request for peterwilsoncc July 3, 2024 21:10
* @param {int} $post_id Post ID
* @param {array} $subscriptions Array of Subscriptions
*/
do_action( 'dt_subscriptions_after_post_push', $post_id, $subscriptions );
Copy link
Member

Choose a reason for hiding this comment

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

Should we also include $update_subscriptions as a third parameter so users can be informed of the success or failure status?

* @param {int} $subscription_id Subscription ID
* @param {array} $request Request array
*/
do_action( 'dt_subscription_after_post_push', $post_id, $subscription_id, $request );
Copy link
Member

Choose a reason for hiding this comment

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

Since the status is currently failure at this point, should we consider tweaking the filter name to reflect this?

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.

Return response from distribution request
3 participants