Easy-digital-downloads: Move discount codes to custom table

[pippinsplugins]

For numerous reasons, including performance and flexibility, discount codes should be moved to a custom table.

This will help lay the foundation for moving payments and sale logs to custom tables.

Related:
https://github.com/easydigitaldownloads/easy-digital-downloads/issues/5217
https://github.com/easydigitaldownloads/easy-digital-downloads/issues/1427

4576

Pull Request: #6224

New database schema

There are two new tables being introduced:

• wp_edd_discounts
• wp_edd_discountmeta

Schema for wp_edd_discounts:

Field | Type | Null | Key | Default | Extra
-- | -- | -- | -- | -- | --
id | bigint(20) unsigned | NO | PRI | NULL | auto_increment
name | varchar(200) | NO |   | NULL |  
code | varchar(50) | NO | MUL | NULL |  
status | varchar(20) | NO |   | NULL |  
type | varchar(20) | NO |   | NULL |  
amount | mediumtext | NO |   | NULL |  
description | longtext | NO |   | NULL |  
max_uses | bigint(20) | NO |   | NULL |  
use_count | bigint(20) | NO |   | NULL |  
once_per_customer | int(1) | NO |   | NULL |  
min_cart_price | mediumtext | NO |   | NULL |  
scope | varchar(30) | NO |   | all |  
date_created | datetime | NO |   | |  
start_date | datetime | NO |   | NULL |  
end_date | datetime | NO |   | NULL |  

Schema for wp_edd_discountmeta:

Field | Type | Null | Key | Default | Extra
-- | -- | -- | -- | -- | --
meta_id | bigint(20) | NO | PRI | NULL | auto_increment
edd_discount_id | bigint(20) | NO | MUL | NULL |  
meta_key | varchar(255) | YES | MUL | NULL |  
meta_value | longtext | YES |   | NULL |  

New files

• includes/class-db-discounts.php
• includes/class-db-discount-meta.php

These files hold the DB interaction layers for discounts and discount meta.

Changes to EDD_Discount

The EDD_Discount class has been significantly updated in order to achieve a few goals:

1. Bring its properties in line with the DB structure. A number of properties have been renamed. For example, is_not_global is now called applies_globally and its value has been adjusted accordingly.
2. Reduce duplicate logic for adding and updating discounts.
3. Make it more consistent with EDD_Payment, EDD_Commission, EDD_SL_License, EDD_Subscription, etc.
4. Magic methods have been heavily utilized to provide a complete backwards compatibility layer for previous method and property names. For example, the expiration property no longer exists as it is now called end_date. Calling $discount->expiration will continue to function exactly the same, however, due to the BC layer that has been added.

The existing helper functions for discounts in includes/discount-functions.php have been left mostly unchanged. Only a few adjustments to account for data type enforcement.

Backwards compatibility layer

1. EDD_Discount. As a lot of properties and methods have changed, we need to ensure the old names still work when accessed directly. For example, $discount->start doesn't technically exist anymore because it's been renamed to start_date. Calling $discount->start returns the value of $discount->start_date. This has been done for properties and methods.

2. Post meta. I have implemented a BC layer that redirects calls to get_post_meta(), add_post_meta(), and update_post_meta().

3. All legacy parameters previously used in edd_store_discount() still work. Those parameters can be passed to edd_store_discount(), EDD_Discount->add(), and EDD_Discount->update() and they will be automatically converted to the new parameters.

4. A BC layer will be implemented with get_posts() and WP_Query. It will hijack the WP SQL query and instead query the wp_edd_discounts table but return WP_Post objects with the data filled from the wp_edd_discounts. A BC layer is not available for get_post() due to the way it's implemented in WordPress Core.

Upgrade routine

A full upgrade and migration routine has been added that moves discounts from the old wp_posts table to the new wp_edd_discounts table. This upgrade can be run via the UI or through WP CLI with wp edd migrate_discounts.

The upgrade routine includes a second step for removing legacy data.

Unit tests

The discount tests have been up into two sections:

• New tests that directly test EDD_Discount
• Legacy tests

So long as the legacy tests continue to pass without changes, we're pretty safe in knowing the BC layer is good.

tests-discounts-db.php and tests-discount-meta.php have been introduced that holds tests specific to the databases.

Todo

• [x] More unit tests that cover DB layer
• [x] New unit tests file for discount meta DB
• [x] More unit tests for EDD_Discount
• [x] Implement discount searching
• [x] Add debug log statements to migration routine
• [x] Change applies_globally to scope=global
• [x] Move date methods ahead of max_uses
• [x] Move excluded products into meta
• [ ] Add documentation to query discounts correctly (with reference to the message displayed on pre_get_posts)

Notes for testing

1. Please test on brand new installs
2. Please test on existing installs that need to be migrated
3. When testing, consider the following items as the most important
   a. Creating discounts
   b. Editing discounts
   c. Applying discounts to the cart (test all permutations of discount restrictions)
4. Please test the upgrade routine from the UI and from WP CLI (if able)
5. Please test any known extensions that interact with discount codes, such as Discount Code Generator and EDD Social Discounts

[pippinsplugins]

This is nearly ready for testing on new installs. I believe I have all functionality moved over and will now begin working on testing backwards compatibility layers.

To do that I'm going to first get all existing unit tests passing. Once those pass, I'll begin writing a bunch of unit tests that intentionally call old methods and intentionally interact with previous schemas so that I can account for those.

[pippinsplugins]

This is now ready for serious testing and review!

New database schema

There are two new tables being introduced:

• wp_edd_discounts
• wp_edd_discountmeta

Schema for wp_edd_discounts:

Field | Type | Null | Key | Default | Extra
-- | -- | -- | -- | -- | --
id | bigint(20) unsigned | NO | PRI | NULL | auto_increment
name | varchar(200) | NO |   | NULL |  
code | varchar(50) | NO | MUL | NULL |  
status | varchar(20) | NO |   | NULL |  
type | varchar(20) | NO |   | NULL |  
amount | mediumtext | NO |   | NULL |  
description | longtext | NO |   | NULL |  
max_uses | bigint(20) | NO |   | NULL |  
use_count | bigint(20) | NO |   | NULL |  
once_per_customer | int(1) | NO |   | NULL |  
min_cart_price | mediumtext | NO |   | NULL |  
product_reqs | mediumtext | NO |   | NULL |  
product_condition | varchar(3) | NO |   | all |  
excluded_products | mediumtext | NO |   | NULL |  
applies_globally | int(1) | NO |   | 1 |  
created_date | datetime | NO |   | CURRENT_TIMESTAMP |  
start_date | datetime | NO |   | NULL |  
end_date | datetime | NO |   | NULL |  
notes | longtext | NO |   | NULL |  

Schema for wp_edd_discountmeta:

Field | Type | Null | Key | Default | Extra
-- | -- | -- | -- | -- | --
meta_id | bigint(20) | NO | PRI | NULL | auto_increment
discount_id | bigint(20) | NO | MUL | NULL |  
meta_key | varchar(255) | YES | MUL | NULL |  
meta_value | longtext | YES |   | NULL |  

Two new files have been introduced:

• includes/class-db-discounts.php
• includes/class-db-discount-meta.php

These files hold the DB interaction layers for discounts and discount meta.

Changes to EDD_Discount

The EDD_Discount class has been significantly updated in order to achieve a few goals:

1. Bring its properties in line with the DB structure. A number of properties have been renamed. For example, is_not_global is now called applies_globally and its value has been adjusted accordingly.
2. Reduce duplicate logic for adding and updating discounts.
3. Make it more consistent with EDD_Payment, EDD_Commission, EDD_SL_License, EDD_Subscription, etc.
4. Magic methods have been heavily utilized to provide a complete backwards compatibility layer for previous method and property names. For example, the expiration property no longer exists as it is now called end_date. Calling $discount->expiration will continue to function exactly the same, however, due to the BC layer that has been added.

The existing helper functions for discounts in includes/discount-functions.php have been left mostly unchanged. Only a few adjustments to account for data type enforcement.

Backwards compatibility layer

So far I have focused backwards compatibility efforts for two primary parts:

1. EDD_Discount. As a lot of properties and methods have changed, we need to ensure the old names still work when accessed directly. For example, $discount->start doesn't technically exist anymore because it's been renamed to start_date. Calling $discount->start returns the value of $discount->start_date. This has been done for properties and methods.

2. Post meta. I have implemented a BC layer that redirects calls to get_post_meta(), add_post_meta(), and update_post_meta().

3. All legacy parameters previously used in edd_store_discount() still work. Those parameters can be passed to edd_store_discount(), EDD_Discount->add(), and EDD_Discount->update() and they will be automatically converted to the new parameters.

I still need to introduce a BC layer for queries to the wp_posts table.

Upgrade routine

A full upgrade and migration routine has been added that moves discounts from the old wp_posts table to the new wp_edd_discounts table. This upgrade can be run via the UI or through WP CLI with wp edd migrate_discounts.

The upgrade routine includes a second step for removing legacy data.

Unit tests

More than 50 new unit tests have been introduced to help ensure things work properly.

Our pre-existing unit tests for discount codes were pretty messy but I decided to leave them almost entirely as is in order to help test that the backwards compatibility layer works as expected.

I have broken the discount tests up into two sections:

• New tests that directly test EDD_Discount
• Legacy tests

So long as the legacy tests continue to pass without changes, we're pretty safe in knowing the BC layer is good.

I have also introduced a new test file tests-discounts-db.php that holds tests specific to the discounts DB.

Todo

I still have a number of items to address:

• [x] More unit tests that cover DB layer
• [x] New unit tests file for discount meta DB
• [x] More unit tests for EDD_Discount
• [ ] Backwards compatibility layer on get_post()
• [ ] Backwards compatibility layer on get_posts() and WP_Query for post_type='edd_discount'
• [x] Implement discount searching
• [ ] Add debug log statements to migration routine
• [x] Change applies_globally to scope=global
• [x] Move date methods ahead of max_uses
• [x] Move excluded products into meta
• [ ] Test and adjust for test resolves

Notes for testing

1. Please test on brand new installs
2. Please test on existing installs that need to be migrated
3. When testing, consider the following items as the most important
   a. Creating discounts
   b. Editing discounts
   c. Applying discounts to the cart (test all permutations of discount restrictions)
4. Please test the upgrade routine from the UI and from WP CLI (if able)
5. Please test any known extensions that interact with discount codes, such as Discount Code Generator and EDD Social Discounts

[JJJ]

A few cursory thoughts:

• Should there be a creator_id or author_id column to identify what user created the discount?
• Consistency on the id column should be decided. Is it ID or id or thing_id or something else?
• once_per_customer and max_uses are confusing together. What about uses_per_customer with a default of 1? That would allow some future flexibility here.
• applies_globally could be scope with a default global value. Would allow for future flexibility, and increase cardinality on the column index (1 vs. 0 won't index well.) This also moves all functional logic into PHP rather than pinning functionality to the literal database naming. Imagine a taxonomy-term scope, a post-status scope, a user-role scope, or countless others.
• product_condition as a VARCHAR(3) seems limiting. What other conditions are there?
• excluded_products should probably be a separate product relationship table. As is, it's not possible for a product to know what discounts it's excluded from, it's only possible for a discount to know what products it doesn't include. A pointer table makes white & black lists easier later. (Or, each excluded product ID could be stored as its own row in wp_edd_discountmeta; then two-way querying is possible, just not super quickly.)
• I'm sure a notes debate predates me, but a global wp_edd_notes table might be nice for a many-to-many notes to discounts type of thing.
• Non-functional recommendation - move created/start/end_date ahead of max_uses to create a natural separation between primary & secondary discount properties.
• Future consideration: start/end_date columns have overlap with other calendar database table work. Is a discount actually an event type? 🤐

[pippinsplugins]

1. I wouldn't be opposed to it though the use case has never come up in six+ years so I'm not sure it's really worth the effort nor future maintenance.

2. Our other custom tables use id. I have made ID work in respect to backwards compatibility as the discounts have been moved from wp_posts. discount_id is permitted because . . . I don't remember why but it was introduced at one point, perhaps during the initial construction of EDD_Discount. I would like id to be the standard.

3. once_per_customer is a boolean flag that indicates if customers are permitted to use individual discounts more than once. max_uses identifies the total number of times the discount can be redeemed, regardless of whether that is from one customer or 100 customers. max_uses is customer agnostic. I do see value in allowing the number of times a customer can use a discount being an actually integer instead of just a yes or no. My only concern with introducing that option is simply due to the lack of requests for it. I'm not sure I've ever seen a case where someone wanted to allow a customer to redeem a discount x times. It's always been unlimited or once.

4. Ah yes, I like that much better. Will work on adjusting it.

5. At this time it's only all and any.

6. I'd be fine with doing it in meta. A separate table seems overkill as it's something that's ever really been needed, though the example scenario makes perfect sense. Putting each product ID into its own meta row would be easy.

7. I agree, a notes table would be great. Customers, subscriptions, and payments already all have notes. Customers and subscriptions are stored in meta and payments is stored in comments. We'll need to make a decision on this before we complete the migration for payments. If we decide that payments needs to have its notes stored in a table, I vote we put discounts there too. If we do not use a separate table for payment notes, I think an individual column is fine. Most of the time notes are not too extensive so a single column is more than sufficient.

8. :+1:

9. Future considerations :)

[pippinsplugins]

Note: appears discount meta is not working yet.

[chriscct7]

Where is EDD_Download->discount_id defined?

I don't think that's a thing, at least it doesn't appear to be a thing for the last several major versions.

I think you might have misremembered copying customer_id from the customer's table here: https://github.com/easydigitaldownloads/easy-digital-downloads/commit/525e153394817818b608e0b48eb30f9d3e34d4c6#diff-d7c8b380123e5377812c31e1d293cfbbR31

Which in turn simply was named that way because of the way AffiliateWP which did the pre-cursor to EDD's custom tables did it in https://github.com/AffiliateWP/AffiliateWP/issues/537 here: https://github.com/AffiliateWP/AffiliateWP/pull/538/files#diff-a3b0aa41b683448eaa74de8ea168216aR120

and Ryan Duff just called it affiliate_id because there was a column for meta_id (which is/was the ID column value for the post meta from the meta table for each meta item transferred) thus removing the ambiguity in which ID (the affiliate or the post meta ID) it was referring to.

[chriscct7]

Notes can be really large. I would suggest storing notes in their own table, with an ID and timestamp so you can easily just query for the rows of notes for a specific discount. Also there's a github ticket for that #5794

Perhaps for a table suggestion to make a simple combined table:

Note ID | Date/time | Author | IP address | PostType | ID | Note

By using post type, we can allow extensions to easily use it to store their own notes by them just using the slug of their plugin name or something simple.

If you're thinking of moving payment logs to a custom table they should be in their own table. With the EDD bot auto created logs, that table is gonna get huge in a hurry for most stores.

So maybe 2 tables 1 for payments, one for everything else.

[chriscct7]

By the way, re Should there be a creator_id or author_id column to identify what user created the discount?, if, and I think we should, let discounts have notes, then {username} created discount at {datetime} should be the first auto-generated note if author ID isn't something going into the main table.

[JJJ]

An imaginary wp_edd_notes database table should have:

• id (int, unique, autoindex)
• object_id (int, related to post/user/discount/whatever ID)
• object_type (varchar, post/user/discount/product/event/whatever)
• author_id (int)
• author_ip (varchar)
• created_date (datetime)
• content (big dumb text)

This assumes the following:

• Notes are always "private" so no status is necessary
• Notes are not edited and/or no revision system is necessary
• Notes are not hierarchical, only relative
• Notes are always big dumb text blobs related to some other object
• Notes should probably have wp_edd_notemeta also

[JJJ]

Related: I like the use of product over download with this table. I think we should continue that trend and enforce it throughout everything else as we go.

[pippinsplugins]

I'm tempted to just remove notes for now rather than introduce it as a column and have to migrate it later. With as many other pieces of data being migrated in 3.0, I don't think it's a good time to introduce yet another migration for a dataset that is not currently problematic (and in the case of discounts) nor used.

[sunnyratilal]

⚠️ Issue found when adding a new discount code:

If the start and end date are left blank, they default to 1970-01-01 23:59:59 for the the start_date and end_date in the db

[sunnyratilal]

For reference, after discussion with @pippinsplugins, the discount_id column in the edd_discountmeta table has been renamed to edd_discount_id as we've changed the meta type to edd_discount. This was necessary as all meta writes were failing on this line.

[mindctrl]

I'm tempted to just remove notes for now rather than introduce it as a column and have to migrate it later. With as many other pieces of data being migrated in 3.0, I don't think it's a good time to introduce yet another migration for a dataset that is not currently problematic (and in the case of discounts) nor used.

I agree. There's enough stuff going on in 3.0, and since notes aren't part of discounts now, I see no problem punting that to a future release.

[sunnyratilal]

🎉 Product requirements and excluded products now both exist in edd_discountmeta and values are written to meta as well on save().

[sunnyratilal]

Having CURRENT_TIMESTAMP as the default value of the created_date column fails on Travis's Precise environment - most probably because it's using an older version of MySQL.

Also _technically_ CURRENT_TIMESTAMP is incorrect as a default value for a datetime field, it's only really correct for a field of type timestamp. I'm aware MySQL allows it as of version 5.6.5 but we're most probably going to have users who are still on a version of MySQL prior to 5.6.

[sunnyratilal]

🎉 Build is running and unit tests are passing!

[sunnyratilal]

Bug found in the count method. When calling the count() method with no parameters after inserting a new discount, it still returns the old count as the computed value still resides in the object cache.

[sunnyratilal]

Gone ahead and removed the caching from the count() as the same has been done in https://github.com/easydigitaldownloads/edd-software-Licensing/issues/515 by @cklosowski for probably the same reason

[sunnyratilal]

Unit tests have been added that cover the DB layer. All that's left is the search functionality.

[sunnyratilal]

The database schema has been altered to change created_date to date_created so as to maintain consistency with the wp_edd_licenses and wp_edd_customers table.

[JJJ]

@sunnyratilal owning it. Great work here so far!

[sunnyratilal]

Bug found with the unit tests, if update() is called with just updates to the meta and no updates to the columns in wp_edd_discounts, it causes an SQL error.

[sunnyratilal]

EDD_Discount now has full unit test coverage.

[sunnyratilal]

This commit introduces an error message for anyone trying to use get_post()/get_posts()/WP_Query to access discounts. We need to ensure that the link added to the messages references some sort of documentation to query discounts correctly.

[pippinsplugins]

@sunnyratilal looking good. Let's also please log a message with edd_debug_log() that includes a call stack.

[sunnyratilal]

@easydigitaldownloads/core-team I believe we are ready for testing here.

1. Pull and checkout issue/5277
2. Run wp edd create_discounts --number=1000 --legacy
3. Run the migration from the admin UI or the CLI (wp edd migrate_discounts, add --force if you've previously run the migration)

Notes for testing

1. Please test on brand new installs
2. Please test on existing installs that need to be migrated
3. When testing, consider the following items as the most important
   a. Creating discounts
   b. Editing discounts
   c. Applying discounts to the cart (test all permutations of discount restrictions)
4. Please test the upgrade routine from the UI and from WP CLI (if able)
5. Please test any known extensions that interact with discount codes, such as Discount Code Generator and EDD Social Discounts

[ashleyfae]

Just ran one quick migration test so far on some existing data.

1. A number of my discounts got imported without their names. I haven't quite figured out how this happened and I can't find anything odd/different about the ones with missing names.
2. The Amount column no longer shows the discount type. For example, it displays 50 instead of 50%, and it displays 35 instead of $35.
3. All of the expired discounts are missing. It looks like the table is using this query:

SELECT id 
FROM wp_edd_discounts 
WHERE 1=1 
AND `status` IN( 'active','inactive' ) 
ORDER BY id DESC 
LIMIT 0,30;

but perhaps it should be:

SELECT id 
FROM wp_edd_discounts 
WHERE 1=1 
AND `status` IN( 'active','inactive','expired' ) 
ORDER BY id DESC 
LIMIT 0,30;

[sunnyratilal]

@nosegraze Thanks for testing!

1. I've added a fallback in the migration routine, if _edd_discount_name is not present in the meta, it'll use the post_title from wp_posts.

2. This has been fixed.

3. When you say missing, did they not get migrated or are not being displayed? We don't display expired discounts on the table. Although, I can add it as a view.

[ashleyfae]

1. Ah that may have been it! I'll give it another test later.

2. They are migrated but not displayed anywhere in the table, whereas they were previously. Before I could see them listed under All. At first this made me think they hadn't been migrated at all because the All count went from like 46 to 21. I later realized it was excluding expired discount codes. Presumably this makes it impossible to edit or delete them, unless you do it manually via MySQL.

[sunnyratilal]

Ah, thanks for the clarification. I'll adjust the queries. :)

[sunnyratilal]

@nosegraze Gone ahead and adjusted the queries and added an Expired view.

[pippinsplugins]

Note, the upgrade routine that is currently used here will need to be updated once our other migrations are completed. See https://github.com/easydigitaldownloads/easy-digital-downloads/issues/6275.

[amdrew]

@sunnyratilal So far so good, can't fault it.

Not sure if you're ready for typos etc. But if you are:

---

On the initial upgrade notice:

This is a mandatory update that will migrate all discounts records

to

This is a mandatory update that will migrate all discount records

and

This upgrade should provider

to

This upgrade should provide

---

On the notice after the upgrade:

Easy Digital Downloads has finished migrating discount records, next step is to remove the legacy data. Learn more about this process.

to

Easy Digital Downloads has finished migrating discount records. Click here to remove the legacy data. Learn more about this process.

I think adding the click here text makes it more consistent with the first upgrade notice

All discountss records have been

to

All discount records have been

---

After the upgrade routine for the legacy data has completed, could we should another success notice? Edit: This is probably a bit premature considering it will be part of a much larger upgrade.

[ashleyfae]

I re-ran my earlier migration and can confirm the name issue is fixed!

One other thing I noticed is that not all the statuses quite match up. I _think_ this is because the status was previously being saved in two places? Not sure if I have some screwed up legacy data or what. But one of my old discounts has the post_status in the wp_posts table set to inactive and on the master branch the discount appears as inactive. However, in the wp_postmeta table for that same discount, there's a meta key for _edd_discount_status with a value active. The discount is then being migrated with the status active.

I'm not sure why I have conflicting data, but in master I have inactive discounts that are being migrated as active instead.

[sunnyratilal]

@nosegraze The status has actually been implemented incorrectly within EDD_Discount. We use post_status to track the status of the discount, but when an instance of EDD_Discount is instantiated, we're actually using the post_meta - not quite sure how we didn't pick this up earlier!

[pippinsplugins]

I've added a small piece of logic to migrate() to ensure non-EDD meta rows get moved over to the new meta table. This will help ensure that data coming from other plugins (like AffiliateWP) gets retained instead of lost into oblivion.

[amdrew]

Here's an example of what other integrations may need to do based on the discount migration. In this case, AffiliateWP: https://github.com/AffiliateWP/AffiliateWP/issues/2499

[sunnyratilal]

@JJJ I've gone ahead and remove the create_table() methods from EDD_DB_Discounts and EDD_DB_Discount_Meta to allow the classes introduced in #6277 to handle the table creation. I've also removed EDD_DB_Discount_Meta::register_table() as again the class in #6277 will register the meta table with $wpdb.

[pippinsplugins]

We need to either re-add the create_table() method (here and other new DBs) or get #6277 finished up asap and merged into release/3.0 as the lack of the create_table() method makes this PR untestable for anyone that doesn't already have the DB created.

[JJJ]

Obviously I vote to merge, since it’s mostly done-done, and those classes handle everything table related very well.

[JJJ]

Should product_condition be bigger than varchar(3)?

The only options today areany or all, but similar columns (like status) are varchar(20).

I think opening this up to 20 now avoids us needing to do it later, and sets up a future roadmap for a product-condition API for discounts to narrow the scope to taxonomies or other things.

@pippinsplugins @sunnyratilal @cklosowski Thoughts?

[pippinsplugins]

Product condition is always any or all so 3 is sufficient unless we want to proactively enable future options.

I'm not sure what we might add there but there are definitely possibilities.

:+1: to changing it to 20.

[sunnyratilal]

Once merged, the backwards compatibility code needs to be grouped with other migrations in #6324.

[sunnyratilal]

issue/5277 has been merged into release/3.0. 🎉

This issue and the issue branch will stay open as there it is very likely that we will things that need changing when we entering the testing phase.

[JJJ]

This issue is technically done, so let's close it! 🥇

If bugs are discovered, new issues should be created.

<3