Easy-digital-downloads: 3.0 - Issues with edd_order_adjustments table

Created on 12 Jun 2020  路  6Comments  路  Source: easydigitaldownloads/easy-digital-downloads

Bug Report

Expected behavior

I expect the order adjustments table data to be consistent and complete.

Actual behavior

I have migration related issues:

  • No discounts are migrated to the new adjustments table.
    Checking an order with a discount shows the correct amount in the customer details screen, but when viewing the order details screen, the discount does not display and the total is incorrect.
  • The adjustments which did migrate all use the migration date for the created/ modified date, instead of the order date (related: #7859).
  • In a storefront which collects sales tax, as I read the code, orders placed in the region associated with my tax rate should have a row for the tax rate; otherwise, they should not. Spot checking suggests that free orders have a tax rate associated with them, regardless of the order address. This doesn't seem to happen with orders placed within 3.0, so I'm listing as a migration issue.
  • Migrated orders with sales tax applied do correctly show the tax.

Possible table issues (all tax rate items):

  • The type_id is 0, instead of the adjustment id.
  • The description is empty, but should probably inherit the adjustment description, which for my store is the region (TN). For a storefront with multiple tax rates, I think it would be important to populate this.
  • The subtotal and tax data information are all 0.0000000, but this could be intentional.
  • The total is the tax rate.

Manually adding a new order with tax applied does not add a row to this table. Completing an order with tax applied on the front end does add a row.

Interestingly, the new order row shows the tax applied to the order_item, not the order, which is the case for the migrated orders. This may be a difference in accounting practice, but mentioning in case it is not.

Steps to reproduce the behavior

1) Migrate a 2.9 storefront with tax data to 3.0.
2) Check the edd_order_adjustments table. Mine shows only tax_rate, no discount, even though there should be.
3) Add a manual order with tax applied. The row for it will not be added to the database.

Information (if a specific version is affected):

PHP Version: 7.4.1
EDD Version (or branch): release/3.0
WordPress Version: 5.4.1

Any other relevant information:
A note for myself so I don't forget, but in the edd_order_adjustments table, the object_id refers to the specific ID of whatever the object_type is, which could be an order, an order_item, or maybe something else (so far this is what I have seen). Thanks to @JJJ for touring me through the adjustments tables so I could begin to wrap my head around them.

component-migration component-orders component-taxes type-bug

All 6 comments

Some notes as I'm looking into this:

  • It may not be possible to include description/more information for migrated tax_rate data, as it doesn't appear to be stored that specifically in 2.9.
  • It seems that the discount migration is failing because the orders are migrated before the discounts, so the edd_get_discount_by function is returning false because the new table is not yet populated. One option to resolve this would be to migrate discounts before orders; another would be to not use edd_get_discount_by, and instead just call EDD_Discount directly, as it is done in 2.9. @ashleyfae or @spencerfinnell, which approach do you think is better? I'm thinking the latter as it would be less of a change with the migration process.

ETA: the EDD_Discount class no longer returns a discount object when looking up a 2.9 discount--reordering the migration may be what's needed, actually. Testing that locally and moving the discounts and tax rates to be migrated first appears to work--discounts are now added to the adjustments table.

In terms of fixing this problem, I like changing the order of the migration. It feels cleaner. Changing the order should be easy in the CLI but maybe @spencerfinnell can weigh in on the UI implications?

I don't _believe_ there would be any issues but my knowledge of the batch/migration UI is relatively limited. @robincornett Are Discounts failing to migrate in the UI currently?

@spencerfinnell in release/3.0, Discounts and Tax Rates are properly migrated to the main edd_adjustments table, but because of the differences in how discounts are retrieved in 3.0, no discounts are migrated for orders at all (migration method does not matter), because if we migrate orders before discounts, the discounts don't exist to be retrieved.

@robincornett I think moving the migration order makes sense as well then.

This PR is likely going to cause some pretty large merge conflicts with #6810. I'll try and finalize that PR, so we can merge it and then update this branch with the changes so that we retain all the logic around improving the usability of the UI Migration.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mikeyhoward1977 picture mikeyhoward1977  路  5Comments

JJJ picture JJJ  路  5Comments

scottbuscemi picture scottbuscemi  路  5Comments

Ismail-elkorchi picture Ismail-elkorchi  路  3Comments

SDavisMedia picture SDavisMedia  路  3Comments