Easy-digital-downloads: Introduce EDD_Discount

[cklosowski]

In the spirit of abstracting all the object data we can. We need an EDD_Discount object that allows us to work through a custom table.

[sunnyratilal]

Initial work for this has begun. An EDD_Discount class has been created and methods have been written for all the discount meta.

[pippinsplugins]

WIP PR: https://github.com/easydigitaldownloads/easy-digital-downloads/pull/5318

[pippinsplugins]

@sunnyratilal let's make each of the class properties protected to enforce the usage of the __get() magic method.

[pippinsplugins]

That will make it more consistent with EDD_Payment as well.

[sunnyratilal]

All the discount functions have been ported over to make use of EDD_Discount. I am now going to begin initial testing of all the functions.

Once I've done the base testing, I'll mark this issue as Needs Testing.

[sunnyratilal]

@easydigitaldownloads/core-devs Ready for general testing.

[sunnyratilal]

PR at #5318

[pippinsplugins]

This is looking really good. Left a small change request on the PR.

Now, a question that could result in sizable change.

In EDD_Payment we support setting individual properties and then calling save(), like this:

$payment = new EDD_Payment( 34 );
$payment->total = 35;
$payment->save();

It shouldn't take a ton of work to make that work with EDD_Discount and it would make the behavior consistent. So question for @sunnyratilal @cklosowski is "should we?"

[sunnyratilal]

Definitely agree that it the behaviour should be similar to that of EDD_Payment.

It shouldn't be too big of a change - it's effectively just creating another add() method which makes use of object variables in lieu of a passed array of arguments.

[pippinsplugins]

Let's keep add() and update() as is so they can also be used, unless either of you disagree?

[sunnyratilal]

Agreed - we should keep them as is for backwards compatibility reasons.

edd_store_discount() currently calls the add() method which requires an array to be passed. We should only alter the add() method when we move to custom tables.

[sunnyratilal]

save() method has been created and EDD_Discount now has similar behaviour to that of EDD_Payment

[pippinsplugins]

@sunnyratilal I'm getting two PHP warnings on the discount edit screen:

[pippinsplugins]

Fixed. get_meta() needed to have $single set to true for product requirements and exclusions.

[pippinsplugins]

@sunnyratilal save() doesn't appear to be working for me. Tried this but no change was made:

$d = new EDD_Discount( 2188 );
$d->name = 'Boo Code';
$d->save();

[pippinsplugins]

We have some notice:

PHP Notice: Trying to get property of non-object in /app/public/wp-content/plugins/easy-digital-downloads/includes/admin/discounts/discount-actions.php on line 143
PHP Stack trace:
PHP 1. {main}() /app/public/wp-admin/edit.php:0
PHP 2. require_once() /app/public/wp-admin/edit.php:10
PHP 3. do_action() /app/public/wp-admin/admin.php:154
PHP 4. WP_Hook->do_action() /app/public/wp-includes/plugin.php:453
PHP 5. WP_Hook->apply_filters() /app/public/wp-includes/class-wp-hook.php:323
PHP 6. edd_process_actions() /app/public/wp-includes/class-wp-hook.php:298
PHP 7. do_action() /app/public/wp-content/plugins/easy-digital-downloads/includes/admin/admin-actions.php:24
PHP 8. WP_Hook->do_action() /app/public/wp-includes/plugin.php:453
PHP 9. WP_Hook->apply_filters() /app/public/wp-includes/class-wp-hook.php:323
PHP 10. edd_edit_discount() /app/public/wp-includes/class-wp-hook.php:298

PHP Warning: strtotime() expects parameter 1 to be string, object given in /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-discount.php on line 1421
PHP Stack trace:
PHP 1. {main}() /app/public/wp-admin/admin-ajax.php:0
PHP 2. do_action() /app/public/wp-admin/admin-ajax.php:91
PHP 3. WP_Hook->do_action() /app/public/wp-includes/plugin.php:453
PHP 4. WP_Hook->apply_filters() /app/public/wp-includes/class-wp-hook.php:323
PHP 5. edd_ajax_apply_discount() /app/public/wp-includes/class-wp-hook.php:298
PHP 6. edd_is_discount_valid() /app/public/wp-content/plugins/easy-digital-downloads/includes/ajax-functions.php:276
PHP 7. EDD_Discount->is_valid() /app/public/wp-content/plugins/easy-digital-downloads/includes/discount-functions.php:554
PHP 8. EDD_Discount->is_started() /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-discount.php:1771
PHP 9. strtotime() /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-discount.php:1421


PHP Notice: Object of class WP_Error could not be converted to float in /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-discount.php on line 1527
PHP Stack trace:
PHP 1. {main}() /app/public/wp-admin/admin-ajax.php:0
PHP 2. do_action() /app/public/wp-admin/admin-ajax.php:91
PHP 3. WP_Hook->do_action() /app/public/wp-includes/plugin.php:453
PHP 4. WP_Hook->apply_filters() /app/public/wp-includes/class-wp-hook.php:323
PHP 5. edd_ajax_apply_discount() /app/public/wp-includes/class-wp-hook.php:298
PHP 6. edd_is_discount_valid() /app/public/wp-content/plugins/easy-digital-downloads/includes/ajax-functions.php:276
PHP 7. EDD_Discount->is_valid() /app/public/wp-content/plugins/easy-digital-downloads/includes/discount-functions.php:554
PHP 8. EDD_Discount->is_min_price_met() /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-discount.php:1774


PHP Warning: array_key_exists(): The first argument should be either a string or an integer in /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-html-elements.php on line 93
PHP Stack trace:
PHP 1. {main}() /app/public/wp-admin/edit.php:0
PHP 2. require_once() /app/public/wp-admin/edit.php:10
PHP 3. do_action() /app/public/wp-admin/admin.php:222
PHP 4. WP_Hook->do_action() /app/public/wp-includes/plugin.php:453
PHP 5. WP_Hook->apply_filters() /app/public/wp-includes/class-wp-hook.php:323
PHP 6. edd_discounts_page() /app/public/wp-includes/class-wp-hook.php:298
PHP 7. require_once() /app/public/wp-content/plugins/easy-digital-downloads/includes/admin/discounts/discount-codes.php:24
PHP 8. EDD_HTML_Elements->product_dropdown() /app/public/wp-content/plugins/easy-digital-downloads/includes/admin/discounts/edit-discount.php:91
PHP 9. array_key_exists() /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-html-elements.php:93


PHP Warning: explode() expects parameter 2 to be string, array given in /app/public/wp-content/plugins/easy-digital-downloads/includes/download-functions.php on line 1287
PHP Stack trace:
PHP 1. {main}() /app/public/wp-admin/edit.php:0
PHP 2. require_once() /app/public/wp-admin/edit.php:10
PHP 3. do_action() /app/public/wp-admin/admin.php:222
PHP 4. WP_Hook->do_action() /app/public/wp-includes/plugin.php:453
PHP 5. WP_Hook->apply_filters() /app/public/wp-includes/class-wp-hook.php:323
PHP 6. edd_discounts_page() /app/public/wp-includes/class-wp-hook.php:298
PHP 7. require_once() /app/public/wp-content/plugins/easy-digital-downloads/includes/admin/discounts/discount-codes.php:24
PHP 8. EDD_HTML_Elements->product_dropdown() /app/public/wp-content/plugins/easy-digital-downloads/includes/admin/discounts/edit-discount.php:91
PHP 9. edd_parse_product_dropdown_value() /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-html-elements.php:95
PHP 10. explode() /app/public/wp-content/plugins/easy-digital-downloads/includes/download-functions.php:1287

[sunnyratilal]

@pippinsplugins

1. The line number the notice is giving doesn't seem to make sense - it's referencing this line: https://github.com/easydigitaldownloads/easy-digital-downloads/blob/issue/5217/includes/admin/discounts/discount-actions.php#L143

2. Fixed in bcd99d1227d969ecfacf77d86f57fe3e720611f3

3. Fixed in 16c53b13a41ad3b6782408945b9284f9a98b25e5

4. Fixed in 51c71229fb372e2d1a13a99505ebeee4753608e6

5. Fixed in 51c71229fb372e2d1a13a99505ebeee4753608e6

[sunnyratilal]

@pippinsplugins Ignore the commits for 4 and 5. I just realised you fixed and I did the opposite.

[sunnyratilal]

51c71229fb372e2d1a13a99505ebeee4753608e6 reverted in efa93fe1de8fc7d590e6570c30abf5738a285b83

[pippinsplugins]

Great, will test again shortly.

[sunnyratilal]

@pippinsplugins save() issue is fixed now too. It was updating the meta _edd_discount_name was changing to Boo Code. The change just wasn't reflecting in the post_title column in the wp_posts table.

[pippinsplugins]

@sunnyratilal works great now!

[sunnyratilal]

@pippinsplugins Are we good to merge?

[sunnyratilal]

Fixes introduced in 152b4dbb20d81c81d10203ff5dc1d898819276a6, 4374b503ec67eacd8ae19d01a2f47edb67d021fa, 2476358e4e56ceed9d9e02bcfe03e4ddf31d4540 and a0bd9cbb0100b8873574d1bfc290bbfd75815aa6 fix the unit tests failing.

[pippinsplugins]

@sunnyratilal Looking good here but I'd like to get a full review from @cklosowski and also a full set of testing from @SDavisMedia.

[pippinsplugins]

For testing, we just need some standard functionality tests of discount codes to ensure nothing broke.

[cklosowski]

Seems to work fine for me, left a small comment on the PR about some redundancy, but great work @sunnyratilal

[sunnyratilal]

Merged into release/2.7

[pippinsplugins]

❤
On Thu, Jan 12, 2017 at 8:00 PM Sunny Ratilal [e-mail]
wrote:

Merged into release/2.7

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/easydigitaldownloads/easy-digital-downloads/issues/5217#issuecomment-272340893,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA_HfR5Ed-sSne1BXST4BOV6yqzo730rks5rRtqlgaJpZM4KzFv1
.

[SDavisMedia]

Just FYI, I did full testing of this anyway (on release/2.7), even though I'm late. I can't break it. I only found one bug but I replicated it on master: #5392 So that's unrelated. All seems well on my end. 🎉 👍