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

Multiple EDD_Payments_Query's affect each other. #5346

Closed
mintplugins opened this issue Jan 5, 2017 · 13 comments
Closed

Multiple EDD_Payments_Query's affect each other. #5346

mintplugins opened this issue Jan 5, 2017 · 13 comments
Assignees
Milestone

Comments

@mintplugins
Copy link
Contributor

mintplugins commented Jan 5, 2017

If you instantiate 2 objects of EDD_Payments_Query, because the values are hooked to edd_pre_get_payments, any custom values you set up for the first object continue to be hooked for any subsequent objects.

For example:
The following code snippet will cause the payment history page in the WordPress dashboard to show no results (notice how we don't return any values or do anything with our returned data here - in theory, because of the word "get", this shouldn't be affecting anything):

function mess_with_edd_get_payments() {
	$args = array(
		'end_date' => time()
	);

	$payments = edd_get_payments( $args );
}
add_action( 'admin_init', 'mess_with_edd_get_payments' );

The problem is that in the EDD_Payments_Query class, the values are hooked to edd_pre_get_payments. So when the payment history page runs this in a separately instatiated object, the hook still runs which sets the "end_date" - even though we had no intention of setting the end_date for the Payment History page.

Proposed Solution:
Instead of hooking the custom values to edd_pre_get_payments, we should simply call each method from within the get_payments method in EDD_Payments_Query.

@pippinsplugins
Copy link
Contributor

Let's look at this soon after 2.7.

@mintplugins
Copy link
Contributor Author

mintplugins commented Jan 15, 2017

One other approach might be to unhook the edd_pre_get_payments hooks created by the class by hooking to edd_post_get_payments and unhooking the calls there.

@MythThrazz
Copy link
Contributor

Hi Pippin,
Could you please give it a slightly higher priority? We have a ready plugin which relies heavily on the fix for this issue.

@pippinsplugins pippinsplugins modified the milestones: 2.7.3, 2.7.5 Feb 10, 2017
@pippinsplugins
Copy link
Contributor

I've bumped it to 2.7.3.

@cklosowski cklosowski modified the milestones: 2.7.3, 2.7.4 Feb 20, 2017
@pippinsplugins pippinsplugins modified the milestones: 2.7.4, 2.7.5, 2.7.6 Feb 22, 2017
@evertiro
Copy link

@pippinsplugins Do we have a preferred method for fixing this? The method proposed by @mintplugins in the initial issue seems cleanest to me.

@pippinsplugins
Copy link
Contributor

@ghost1227 go with @mintplugins's proposal.

@mintplugins
Copy link
Contributor Author

With guidance from @ghost1227 I'm going to take a shot at this

@evertiro
Copy link

@mintplugins If you have any questions/need help feel free to ping me!

@mintplugins
Copy link
Contributor Author

This is working in my tests but could definitely use more tests. Try anything/everything that would use a payment query.

@sunnyratilal
Copy link
Contributor

I've run multiple tests and all good here but would still like prefer final review from @pippinsplugins or @cklosowski

@evertiro
Copy link

Another 👍 from me!

@pippinsplugins
Copy link
Contributor

👍 here

@sunnyratilal
Copy link
Contributor

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants