lat9 / edit_orders

Edit Orders: Updates for continued operation on Zen Cart v1.5.8 and later
GNU General Public License v2.0
5 stars 9 forks source link

Sales-tax miscalculation when using ot_group_pricing #207

Closed lat9 closed 2 years ago

lat9 commented 2 years ago

As noted in this support-thread posting.

daphilli224 commented 2 years ago

I disagree with the "Penny Off" title. The correct tax for the test order is $3.37 (6% of $66.05 - discount of $9.91) that was displayed entering EO. The tax shown after EO update is $3.96 (6% of $66.05). The update was a credit of $0.01 post tax which should not have changed the tax. The tax error is a bit more than a penny!

lat9 commented 2 years ago

OK, but since you didn't indicate (from the forum) what results were expected, that was my best guess.

lat9 commented 2 years ago

I also didn't notice that the issue occurs with ot_quantity_discount (I thought you were referencing ot_group_pricing). That add-on order-total, as you identified, has some inter-operational issues with EO due to, I'm guessing, its reliance on storefront operation.

That's an issue you'll need to take up with that order-total's author.

daphilli224 commented 2 years ago

Hi Cindy,

Yes, both discount modules were enabled, but the order in question did not qualify for quantity discount, only group pricing. Thank you for looking into this. I’m in the process of running a series of tests to see the extent of the problem on my system and the exact conditions/steps to reproduce it.

Would you prefer further communication on this issue via email, github comments, or forum?

All the best, Dave

On Oct 15, 2021, at 6:53 AM, lat9 @.***> wrote:

I also didn't notice that the issue occurs with ot_quantity_discount (I thought you were referencing ot_group_pricing). That add-on order-total, as you identified, has some inter-operational issues with EO due to, I'm guessing, its reliance on storefront operation.

That's an issue you'll need to take up with that order-total's author.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/lat9/edit_orders/issues/207#issuecomment-944204021, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVXNN2GVGQNOHNXVGDS2YWLUHAB2XANCNFSM5F7SKF6A. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

lat9 commented 2 years ago

Dave, when I had more coffee (!) and re-read that log, I saw that it was just ot_group_pricing in the mix. Communications best via GitHub (that keeps me on track).

I'll look closer over the next few days. Note, however, that I don't have a zc155 installation on which to run any tests so I'll be using a zc157c test site as my checking-basis.

daphilli224 commented 2 years ago

I can not reproduce the problem on my v155f development system but it keeps happening on the production system. The software has been compared and any differences are expected, e.g., configuration files, unrelated files I'm working on, etc. I'm currently checking the database, specifically the configuration table. I'll get back to you soon.

daphilli224 commented 2 years ago

I was able to reproduce the problem on my development system, which makes things much easier. I found that the problem occurs when the original order shipping method is set to store pickup. If the original order shipping method is USPS, EO works correctly whether the EO update is added product, changed price, or post-tax credit. But if the shipping method is store pickup on the original order, an EO update calculates the tax without subtracting the group discount.

Fortunately, the problem does not occur with EO 4.6.0 and zc 157c, but unfortunately I have to find a solution for EO 4.5.7 and zc155f.

daphilli224 commented 2 years ago

In addition, I found that when the original order shipping method is store pickup, the shipping method pull down menu in EO only shows store pickup. Other shipping methods (e.g., USPS) available for the original order are not displayed. If the original order shipping method is USPS, EO's pulldown menu displays other methods available in the original order, including store pickup. Again, this problem is for zc155f and EO 4.5.7.

Does this information provide any clues?

daphilli224 commented 2 years ago

Seeking a workaround to the problem, I changed the shipping_method and shipping_module_code in table orders and title in order_total from store pickup to USPS using phpmyadmin. The EO shipping module pulldown still only shows store pickup, although the title changed to the correct value in the adjacent box. Tax calculation upon EO update still incorrect. Ugh.

daphilli224 commented 2 years ago

Some background is in order at this point. The store normally operates only online. Once or twice a year, we have a live event where store products are available for purchase in person for cash, checks, or credit cards. Some products are offered at a special price that differs from the online price. Since the also store operates online during the events, price changes are not changed in the database. After the event, it's my job to enter the purchases from sales slips and edit the orders to reflect the amount collected for each order. I enter each order with a check/money order payment method and store pickup shipping method. Most purchases are cash, so after the order is entered, I change the payment_method and payment_module_code to COD in the orders table. Then I change the few check or credit card orders as appropriate. So right now I'm using EO to edit COD orders with store pickup shipping method.

Looking through the EO code where the pulldown menu for shipping method is generated, I see that the default value is $order->info['shipping_module_code']. Further checking revealed a comment where $order->info is generated that for COD, the payment module must be active. But in my case, the COD module was never active, as the payment method was changed via phpmyadmin.

So there are multiple things complicating the problem. Changing payment method to check/money order for these orders does not solve the problem. And changing the shipping method to USPS does not solve the problem either. I really don't want to enter all those orders again. Suggestions?

lat9 commented 2 years ago

Thanks for all the investigations; I'll try to find some time over the weekend to see what (if anything) I can see.

You haven't mentioned debug-logs ... are there any admin logs generated during EO's tax calculations -- those would be indicative. If you don't have it installed, I'll suggest installing my "Report All Errors" plugin (https://github.com/lat9/report_all_errors) and enabling the logging of "All non-duplicate definitions" (IgnoreDups) on the admin side.

If, after installing and re-running an order with the tax issue, there are debug logs generated, please attach one of the logs (obfuscating your admin directory name) for review.

daphilli224 commented 2 years ago

Comparing two cases, one with shipping of USPS and the other with store pickup, for both the original orders on the store side and the orders on the admin side with EO updates, shows that 'tax_groups' is different for the store pickup admin/EO case. 'tax_groups' for the two store-side orders and the USPS shipping case in admin/EO are all identical. These comparisons were made using data extracted from the ot_group_discount method "calculate_deductions".

daphilli224 commented 2 years ago

Examining the logs, the same errors appear whether EO's tax calculation is correct or not.

There is an undefined index notice: [21-Oct-2021 10:37:01 America/New_York] PHP Notice: Undefined index: messageToStack in /admin/includes/classes/message_stack.php on line 58 [21-Oct-2021 10:37:01 America/New_York] PHP Stack trace: [21-Oct-2021 10:37:01 America/New_York] PHP 1. {main}() /admin/edit_orders.php:0 [21-Oct-2021 10:37:01 America/New_York] PHP 2. messageStack->add_session() /admin/edit_orders.php:686

and two deprecated class constructor notices: [21-Oct-2021 10:31:01 America/New_York] Request URI: /admin/edit_orders.php?page=1&oID=1977&action=edit, IP address: ::1

1 include_once() called at [/includes/classes/shipping.php:64]

2 shipping->__construct() called at [/admin/includes/functions/extra_functions/edit_orders_functions.php:1707]

3 eo_get_available_shipping_modules() called at [/admin/edit_orders.php:1608]

[21-Oct-2021 10:31:01 America/New_York] PHP Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; flat has a deprecated constructor in /includes/modules/shipping/flat.php on line 13 [21-Oct-2021 10:31:01 America/New_York] PHP Stack trace: [21-Oct-2021 10:31:01 America/New_York] PHP 1. {main}() /admin/edit_orders.php:0 [21-Oct-2021 10:31:01 America/New_York] PHP 2. eo_get_available_shipping_modules() /admin/edit_orders.php:1608 [21-Oct-2021 10:31:01 America/New_York] PHP 3. shipping->__construct() /admin/includes/functions/extra_functions/edit_orders_functions.php:1707 [21-Oct-2021 10:31:01 America/New_York] Request URI: /admin/edit_orders.php?page=1&oID=1977&action=edit, IP address: ::1

1 include_once() called at [/includes/classes/shipping.php:64]

2 shipping->__construct() called at [/admin/includes/functions/extra_functions/edit_orders_functions.php:1707]

3 eo_get_available_shipping_modules() called at [/admin/edit_orders.php:1608]

[21-Oct-2021 10:31:01 America/New_York] PHP Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; flatClone has a deprecated constructor in /includes/modules/shipping/flatClone.php on line 27 [21-Oct-2021 10:31:01 America/New_York] PHP Stack trace: [21-Oct-2021 10:31:01 America/New_York] PHP 1. {main}() /admin/edit_orders.php:0 [21-Oct-2021 10:31:01 America/New_York] PHP 2. eo_get_available_shipping_modules() /admin/edit_orders.php:1608 [21-Oct-2021 10:31:01 America/New_York] PHP 3. shipping->__construct() /admin/includes/functions/extra_functions/edit_orders_functions.php:1707

zc155f, EO 4.5.7, PHP 7.3.x

daphilli224 commented 2 years ago

I can provide the EO logs and error logs should you really need them. Dave

daphilli224 commented 2 years ago

Weird! I previously told you 'tax_groups' differed between correct tax calculation case and incorrect tax case. Comparing the EO logs earlier in the process, there is a line getProductTaxes(PA Sales tax) in the correct tax case, and getProductTaxes(Tax) in the incorrect case. The code generating the log entry is at line 397 in admin/includes/classes/editOrders.php. The text in parenthesis in the log is $products_tax_description which is set by a field in a query beginning at line 377. The query results are printed out in the next line of the log, and that entire line is identical for both cases! The product is the same, the query results are the same, yet $products_tax_description differs. How can that be?

daphilli224 commented 2 years ago

I've attached two comparison logs for your study over a cup of coffee! The log for order 1971 has a correct tax calculation after an EO update of changing the product price from $50.00 to $100.00. Payment method is check/money order. Shipping method is USPS. The log for order 1972 has the incorrect tax calculation for the same EO update as 1971. Payment method is check/money order and the shipping method is store pickup. Dave

debug_edit_orders_1971_sanitized.log debug_edit_orders_1972_sanitized.log .

proseLA commented 2 years ago

... and two deprecated class constructor notices: ... 21-Oct-2021 10:31:01 America/New_York] PHP Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; flat has a deprecated constructor in /includes/modules/shipping/flat.php on line 13 ... [21-Oct-2021 10:31:01 America/New_York] PHP Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; flatClone has a deprecated constructor in /includes/modules/shipping/flatClone.php on line 27

zc155f, EO 4.5.7, PHP 7.3.x

dave, i have NOT dug into the tax calculation, but these errors have nothing to do with EO. flatClone is not a part of the base ZC package. and the error in the shipping module class flat would not be there if you had the v155 version of said file. the v154 version of this file (the same file going all the way back to v150) would throw this error, but not the v155. in addition, that error would be on line 15 not line 13.

v154: https://github.com/zencart/zencart/blob/a82e4423a4794b9b4deeec4c98529a4cdcebb41b/includes/modules/shipping/flat.php#L15

v155: https://github.com/zencart/zencart/blob/v155/includes/modules/shipping/flat.php

daphilli224 commented 2 years ago

I apologize if I implied the errors had anything to do with EO or the problem being discussed. They obviously do not. My only intent in revealing those notices to you was to demonstrate that I did look in the PHP logs.

lat9 commented 2 years ago

Random thought: Could the storepickup discrepancy for taxes be due to the fact that storepickup taxes are calculated based on that shipping-method's configuration? Normally, the storepickup taxation is based on the store's location rather than the customer's chosen ship-to location.

daphilli224 commented 2 years ago

The flat rate shipping module and store pickup shipping module were configured as follows: flat: tax class: taxable goods, tax basis: shipping, shipping zone: United States store pickup: tax class: none (because shipping cost is set to 0.0), tax basis: shipping, shipping zone: United States

However, I set store pickup tax class to taxable goods for a test this morning. EO tax calculation still incorrect after price update, shipping pulldown still shows only store pickup both before and after update. The EO logs for USPS case and store pickup case are now identical (except for shipping costs, taxes, shipping method - things that should be different. I had been trying to find out why the USPS case log showed Checking order for virtual status and the store pickup did not. (around line 16 in the EO log). Now both cases show checking for virtual status.

I'm still chasing down why $products_tax_description in the EO log entry for getProductTaxes around line 18 in the logs show PA Sales tax for USPS and Tax for store pickup.

The EO log for the store pickup case with tax class set to taxable products is attached (order 1981). Dave

debug_edit_orders_1981 sanitized.log .

daphilli224 commented 2 years ago

I think I'm getting closer. zone_id is wrong in calls to EO function zen_get_tax_description for store pickup. That causes the query in that function to not return any valid results, and hence the unknown tax rate description ("Tax") is displayed.

daphilli224 commented 2 years ago

The problem was mine, not something in EO! I set a define for the pickup state for store pickup orders to "PA" instead of "Pennsylvania". That caused zen_get_zone_id in function zen_get_tax_locations to set the zone_id to 0 which caused the unknown tax rate description, incorrect tax calculation on EO updates, and the pull down menu issue.

The tax calculation is now correct for EO updates for store pickup and USPS orders and the pull down menu displays all available shipping methods. All due to a lousy define! Thanks again for a great plugin!

lat9 commented 2 years ago

@daphilli224, thanks for the update on the root-cause of the issue ... as well as the in-progress notes! The steps that you took to determine what was going on will certainly help others in the future.