3.0 - Allow order and discount IDs to be directly migrated, without the need for legacy IDs using auto_increment.

[mintplugins]

I am opening this as a place to have a conversation and record of the thoughts behind how the id column will operate in EDD3, what its purpose is, and what the expectations are for it going forward from EDD3.

Previously to EDD3.0, payment IDs were automatically assigned by WordPress, as they were stored in the wp_posts table. Any blog post or revision (or any other post_type) that would happen "in the middle" of payments, would cause their IDs to become non sequential.

For example, imagine this series of events:

1. Payment comes in (ID is 1)
2. Blog post is published (ID is 2)
3. Another blog post is published (ID is 3) 4) Another payment comes in (ID is 4)

You now have 2 payments:

• Payment 1
• Payment 4

EDD 3.0 will migrate those payments to their own unique table, and also change their terminology from "payments" into "orders". That new table has an ID column which is sequential. So after migration, you will have 2 payments:

• Order 1 (previously was Payment 1)
• Order 2 (previously was Payment 4)

The problem here is that the ID of Payment/Order 4 has changed to 2. Since the ID has been used for the ID of the payment, the invoice, and receipts, customers have records of their payment with the id 4 attached. We do not know the legality of changing an order number in all countries and regions. Aside from legality, we do not know if this will cause issues in 3rd party book-keeping integrations like Xero and others.

For this reason, a second ID column is being proposed which would keep the "legacy" IDs exactly the same after migration as before. While the id column would still be incremental (1,2,3), the order_id column would be considered as the "official" id of the order. After migration, using the example above, the next payment that comes in would sequentially start where the payments were left off. So it would look like this:

• Order 1 (previously was Payment 1)
• Order 4 (previously was Payment 4)
• Order 5 (new payment after migration to EDD3.0)
• Order 6 (new payment after migration to EDD3.0)
• Order 7 (new payment after migration to EDD3.0)
• Order 8 (new payment after migration to EDD3.0)

The id column would be ignored by any code that intends to query post the ID of the payment.

[mintplugins]

A few other issues that could arise if IDs change (props @chriscct7):

• Software licensing generates the license based on the payment ID
• Google Analytics integrations could be thrown off
• Rest API calls using the ID will be affected
• Bookkeeping integrations could be affected
• Admin URLs will be affected (#7209)
• Frontend URLs that do not use pretty permalinks will be affected
• Stripe metadata stores the payment ID, but has no indication of whether the ID is a legacy ID. Could be problematic if troubleshooting payments

[pippinsplugins]

I would really like to avoid adding another ID column. We already have three ID-type fields:

• id (the true ID of the order)
• order_number
• legacy_id

While I do feel the case for adding another column, introducing another ID will ultimately lead to too much confusion. It will cause questions like:

• Which one should be used for accounting?
• Which one is used to look up an order?
• How are search results affected?
• How do functions and APIs behave with the different IDs?
• How are collisions handled?

I think there are too many issues that would be introduced by an additional ID column to warrant it.

We cannot possibly know what all of the legal ramifications are for changing order IDs, but I do know that store owners change systems (be it their eCommerce or book keeping tool) and very few of them have extensive toolsets for keeping the previous order IDs in tact. If the legal ramifications were too great, store owners would never change systems, but we know that's not true because we see it all the time.

I propose that we:

1. Not add an additional ID column
2. Allow the sequential order numbers feature be the solution here for book keeping
3. Continue to update all extensions and features (such as REST API) to use distinct parameters (order_id vs payment_id) for looking up records. The name of the parameter will tell us if we should look up by the new ID or the legacy ID.

[JJJ]

https://dev.mysql.com/doc/refman/8.0/en/example-auto-increment.html

It appears that we can pass the old ID if we wanted to allow for that.

The Database API I wrote would need to be modified to support this, as right now it slices the ID column off on insert.

The safest thing to do would be to check if that ID exists before inserting, and fail the insert if it does. (Update uses that ID so it’s required there.) This brings some parity to update & insert, as they’ll both now be looking for an existing entry, just for different reasons.

Retaining the ID from table to table seems like it would absolve a lot of concerns.

@alexstandiford worked on a related PR for Affiliate WP that included auto_incrementor manipulators, and I think that would be a valuable addition to the Table class in our 3.0 database API. (That repo is private, so I won’t link to that here.)

I’ll put up a PR asap and reference this issue, so we can see how viable this approach is.

[pippinsplugins]

I had no idea auto increment could be done that way! That's awesome.

If that works the way described, that could potentially mean that all extensions will continue to work as is without changing any payment IDs. How freakin' cool.

[JJJ]

If that works the way described

Confirmed that it works. 🎉

In my InnoDB setup:

• inserting a new row with an ID higher than the current auto-increment value will automatically bump the value
• inserting a new row with an ID lower than the current auto-increment value will automatically insert the row in numeric order without bumping the auto-increment value

Both of the above scenarios match my expectations and the previously linked-to MySQL documentation. Heckin' cool.

---

A few things:

• The database API Query class requires a very small change to support this
• The migration API will require more tweaks and more testing to make sure it correctly addresses all of our above concerns
• The database API Table class should get auto-incrementor manipulation methods, in the event someone wants to (or needs to) manually adjust them

PR with the database class alterations imminent. I think the migration alterations might need some tag-teaming and deeper review when @cklosowski is back around. <3

[JJJ]

Addendum: it's also (thankfully) impossible to set the auto-increment value to anything lower than the highest ID in the relative column.

This means when we introduce a method to set it, it will be impossible for them to screw it up; MySQL internally prevents the table contents and the auto-increment value from being bass-ackwards.

For example:

• your table has 100 rows
• you set the auto-increment to 1000
• you add 5 rows
• you set auto-increment back to 500

Auto-increment will be 1006, without throwing any errors or notices. Pretty sweet.

[mintplugins]

Amazing

[JJJ]

Pull request #7216 is ready for a first-pass review.

I've added @cklosowski and @mintplugins as potential reviewers. 🎈

[cklosowski]

Thank you all for investigating this while I was out. I'll get a review on the PR.

This also means, as @mintplugins has already done, that the issues set to migrate these IDs in extensions are unnecessary...and added bonus we don't have to add a meta value.

[mintplugins]

After adding in https://github.com/easydigitaldownloads/easy-digital-downloads/commit/cf03cc215bc3cec75f634bd4bd4545fa8716f9ea I was able to migrate successfully, and all order IDs remain the same. 🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉.

I'm going to do some searching around and see if I find any issues. But I think this concept is proven.

[cklosowski]

I just thought about something that might derail this whole thing...

Stores cannot be open to taking new orders while this migration happens with this method unless we can make sure any new orders that are inserted during the migration will have an ID greater than the highest Post ID in wp_posts that is an edd_payment

The scenario would be: 1) Start migration 2) Post ID 123 is migrated to order_id 123 (for this example, assume this is the first edd_payment post type in the wp_posts table. 3) Auto increment is bumped to 124 when this happens. 4) Post ID 128 is migrated to order_id 128 5) Auto increment is bumped to 129 6) A new order comes in from the live site and is now order_id 129 7) The next Post ID is migrated and is 129 8) :boom:

I believe the only way around this would to set the auto_increment upon table creation...to whatever the highest post ID value of edd_payments is + 1, and then starting the migration. The table will allow us to inset items with an ID lower than the auto_increment will it not? @JJJ @mintplugins

[JJJ]

I like the idea of setting the auto_increments as step 1 of the upgrade process.

It is a pretty graceful way of keeping the store open to new orders while the upgrader is chugging through the old ones.

We should do this for discounts also.

If we want to put more shine on this, continuing to do all of this under the guise of better reporting, we should “disable” a bunch of admin screens while the upgrade is happening so busy/impatient store owners don’t open a second browser tab and manually do anything that would unexpectedly write to the database.

We’d want to avoid them from changing anything, while still allowing them to navigate around the store and see the progress being made. People are going to want to make darn sure it’s working, and opening another tab and clicking around is the easiest way to do that.

[JJJ]

The database API does tie into roles and caps, and we could filter the capabilities arrays of the various tables while the upgrade is taking place, allowing reads and disallowing, updates, inserts, and deletes.

And we’d need to make sure the upgrade itself remains unfiltered.

[cklosowski]

So can we set the initial auto_increment and insert lower numbers after that without issue?

[JJJ]

Yup! We totally can. No issues.

New orders (that do not pass an ID in) will get new auto-increments values.

Old orders (that do pass an ID in) will use the ID that gets passed in.

The auto-increment is only used and bumped when no ID is passed in.

Your suggestion is a great way to “reserve” the space necessary for the migrated data to fit inside of. 👍

[mintplugins]

Sounds like the right approach to reserve space for all of the "legacy" posts. Good thought @cklosowski

[cklosowski]

Ok I've been looking through how we should do this today quite a bit. I think the most ideal situation is going to be to add a call to a method in the EDD\Database\Table class called something like post_creation_steps/actions.

Then we can use the create method as normal, and if the table is successfully created, run the post_creation_steps/actions method (which can be a stub of nothing in EDD\Database\Table class, but in specific table classes that extend it like orders and discounts we can create a method that handles this auto_increment step.

Does that seem like the best logic? I originally was thinking actions, but I don't want actions this deep into things...I like the idea of it being part of the class inheritance.

[cklosowski]

After some discussion with @jjj in Slack, we're going to avoid the approach above and just use the inheritance to have a create method that calls parent::create in the child class (like orders) that will do anything extra needed, like setting auto_increment after creation.

:+1:

[cklosowski]

@jjj @mintplugins ready for a look over.

[cklosowski]

Just updated this from release/3.0 to include #7236

[cklosowski]

I've pushed up some changes to do this same reservation for discounts as they are migrated to the adjustments table.