Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alex.neblett’s picture

Status: Active » Needs review

It has been about a month without hearing anything back.

Is this the normal behaviour?

Does anyone else experiance the same thing if they replicate the process?

In short, my issue is as follows:

If an order is created by a customer, all works well.

If an order is modified or created via /admin/store/orders/edit, the sales tax is not created for new orders and not updated for edits. For example, if new line items are added or if the line items are changed, the sales tax will not change. It is as if the sales tax rules do not work while in /admin/store/orders...

I would like to get the issue resolved. So, please let me know if this is the normal behaviour, if this happens for me, but not for other folks, or if this is something that is broken for others as well.

I was looking for a needs feedback status, but could only find this.

Thanks,

Alex

wodenx’s picture

I believe that when an order is past the "in checkout" state, the taxes and rates that were applied at the time the order was made are frozen. This is intentional, so that a completed order can be edited without losing historical tax information. However, those old taxes and rates should apply to new products if you click the "submit changes" button (they do for me). If you want to have the current tax rules applied, adjust the order status to "in checkout" (or something in the "in checkout" state) and then make your changes.

alex.neblett’s picture

Wodenx,

Thanks. I changed the status to in checkout. That is what I was looking for.

One issue: I have a $55.00 product with a $55.00 discount coupon applied. So, the tax and total should be zero, but they are not. Instead, this is what I get.

Products Subtotal: $55.00
Coupon 10005047381: -$55.00
Subtotal excluding taxes: $0.00
Texas Sales Tax: $4.54
------
Total for this Order: $4.54

If the order is created by the user, the tax is zero and the total is zero. I am guessing that the tax rule is not looking at the discount.

Any thoughts?

Thanks,

Alex

wodenx’s picture

Status: Needs review » Active

Can you post the exact series of steps to produce the problem - i.e. are you editing an existing order created through checkout or a new order created through the admin screen? And are you trying to add the coupon as a line-item on the order-admin screen? If so, I'm pretty sure that doesn't work at the moment - the support for adding/removing coupons on the order admin screen is fairly limited. Please open an issue in the uc_coupon queue if necessary.

alex.neblett’s picture

Sorry.

I am editing an existing order crated through checkout.

The coupon had been added via the admin screen when it was in pending status.

I followed your steps and changed the status to in checkout. When I switched back to Edit from View, the sales tax appeared as I showed you.

Another strange thing I noticed is that you can delete the sales tax item, but I did not see a way to add it back in.

I am going to test some more scenarios and then will get back to you.

Thanks again,

Alex

wodenx’s picture

Yeah, adding a coupon on the admin screen isn't really supported - you're really just adding a "blank line" line item that's called a coupon (i.e. the actual coupon is not referenced in any way), so it won't register as a "taxable" line item. This is on my list to fix, but not sure when I will get to it.

DuaelFr’s picture

Priority: Major » Critical

There is a real issue with tax line items from the admin interface.

Steps to reproduce :
1- go to admin/store/orders/create
2- choose a client, put him addresses that are covered by your taxes conditions
The tax line item is not added to the order.
3- add a product, save the order
The tax line item is always not added to the order.
4- Do whatever you want, the tax line item is not there and will never be.

Here is an other use case :
1- your client make an order via the checkout form
2- edit this order from the admin panel
3- make a mistake and remove the tax line itemp by clicking the little cross near to it
4- cry because you will never find any way to get it back

longwave’s picture

Priority: Critical » Major
Jawi’s picture

any progress?

aadityawalawalkar’s picture

Please find the patch for the first use case as commented by DuaelFr in comment #7

Dan Z’s picture

Changing status in order to run the testbot on the patch in #10.

Version: 7.x-3.0 » 7.x-3.x-dev
Status: Active » Needs work

The last submitted patch, 1425978_admin_create_order.patch, failed testing.

Dan Z’s picture

Status: Needs work » Needs review

#10: 1425978_admin_create_order.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1425978_admin_create_order.patch, failed testing.

Dan Z’s picture

Wrong format in the patch. Here's one way to do it that should work:

Start from the -dev version of the file (7.x-3.2 might work).
Make a copy of the file, something like uc_taxes.module.orig, containing the original version.
Modify uc_taxes.module with your fix.
cd to your sites/all/modules directory.
diff -up ubercart/uc_taxes/uc_taxes.module.orig ubercart/uc_taxes/uc_taxes.module.orig > 1425978-1_admin_create_order.patch
Make a new reply here and attach 1425978-1_admin_create_order.patch. Set the status to "needs review" on the reply.

aadityawalawalkar’s picture

Status: Needs work » Needs review
FileSize
614 bytes

Please find the new patch, created as suggested by DanZ in comment #15
Thanks a lot DanZ for you help/tips.

Status: Needs review » Needs work

The last submitted patch, 1425978-1_admin_create_order.patch, failed testing.

aadityawalawalkar’s picture

Status: Needs work » Needs review

#16: 1425978-1_admin_create_order.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1425978-1_admin_create_order.patch, failed testing.

TR’s picture

That's a real test failure so retesting isn't going to help; you have to fix the patch so it doesn't break the existing test. The easiest way to work on this is to run the tests on your local machine by enabling the core Drupal "Testing" module then going to admin/config/development/testing and selecting the failing test to re-run. The local test output is a lot more verbose, and you can re-run as often as you like until you get it working.

aadityawalawalkar’s picture

Status: Needs work » Needs review
FileSize
610 bytes

Please find the new patch

Status: Needs review » Needs work

The last submitted patch, 1425978-1_admin_create_order.patch, failed testing.

aadityawalawalkar’s picture

Status: Needs work » Needs review
FileSize
1.13 KB

Please find the new patch for the first use case as commented by DuaelFr in comment #7

Dan Z’s picture

Ah, it passed. Lovely.

Two minor coding issues:

"if (rules_invoke_component('uc_taxes_' . $tax->id, $order)) {" is indented too far.

You don't need "$tax = clone $rate;" Just use $rate->id instead of $tax->id and "$taxes[] = clone $rate;" instead of "$taxes[] = $tax;" No sense in spending the CPU and memory to clone it if you don't need to use the clone.

Yes, I see that you're copying the block of code below that. That block is not efficient.

aadityawalawalkar’s picture

Please find the optimized patch as suggested by DanZ in the previous post.
Thanks a lot again Dan for your inputs.

Dan Z’s picture

This fix is a big deal. I'll have to give it a test when I get a chance.

For now, though, three small things.

First, the patch file name should start with the issue number, 1425978 and probably include the comment number (which was 25). So, something like "1425978-25-admin_order.patch".

Second, the indenting is still off. Instead of:

        foreach (uc_taxes_rate_load() as $rate) {
            if (rules_invoke_component('uc_taxes_' . $rate->id, $order)) {
              $taxes[] = clone $rate;
          }
        }

It should be:

        foreach (uc_taxes_rate_load() as $rate) {
          if (rules_invoke_component('uc_taxes_' . $rate->id, $order)) {
            $taxes[] = clone $rate;
          }
        }

Also,

        if ($item['type'] == 'tax') {
            $flag = FALSE;
          if (!empty($item['data']['tax'])) {

should be

        if ($item['type'] == 'tax') {
          $flag = FALSE;
          if (!empty($item['data']['tax'])) {

Third, the logic here is just complex enough that it isn't easy to understand what the code is doing. I'd recommend a little comment to explain what is going on. Under what circumstances does it re-calculate? Maybe give "$flag" a more descriptive name. Maybe "$tax_recalculate_needed" is too long, but it's better than "$flag". Since you know what it's doing, you can probably come up with a better name than I can.

longwave’s picture

Let's rewind a little and check what the comment at the top of uc_taxes_filter_rates() says:

The taxes depend on the order status. For orders which are still in checkout, any tax can apply. For orders out of checkout, only taxes originally saved as line items can apply.

As far as I can tell, this works as expected. This is by design, so that if you change your tax configuration after an order has been submitted (say, you change the tax rate), the same tax configuration is still used on future changes to the order (as the tax rate should not be changed on old orders if you update the products).

So, the problem we are fixing here is from comment #7:

Steps to reproduce :
1- go to admin/store/orders/create
2- choose a client, put him addresses that are covered by your taxes conditions
The tax line item is not added to the order.
3- add a product, save the order
The tax line item is always not added to the order.
4- Do whatever you want, the tax line item is not there and will never be.

If you add an extra step in this anywhere after step 1, which is to change the order status to "in checkout", then go back to the order edit page, you will see that the taxes update as expected, and as noted by the code comment above.

So to fix this, we should default the "new order" status to "in checkout". However, "in checkout" orders are currently abandoned after 10 minutes, which is probably not very useful for an administrator. So perhaps we should add a new order status of "New admin order" (or similar), with the *state* set to "in checkout", so the taxes will update as expected but the order will not automatically be marked as abandoned.

Other suggestions are welcome; note that I suspect the above patch won't work correctly where there are multiple tax rates applied to an order, with different sets of conditions (though I don't know how widely this is used, if at all).

Dan Z’s picture

I'd suggest that this not apply just to new orders on the admin screen. An administrator might want to edit a customer order.

longwave’s picture

That works as expected for me; if I edit an existing order that has taxes applied, add a new product, then click "Submit changes", the tax line item is updated to include the tax of the new product - at the rate specified when the order was originally made, if the tax rate has changed since. This could be improved by updating the line items immediately by not having to click "Submit changes", but it does work.

Jawi’s picture

#29 Thanks for the input longwave!

aadityawalawalkar’s picture

Please find the patch that also handles multiple taxes scenario.
The patch in the post #25 fails when two or more taxes are applied.
Suppose for an order two taxes are applied.
Then go to admin UI for editing an order. Delete one of the taxes using the cancel/cross button besides each tax line item. Then even after clicking on "Save Changes" button deleted tax line item is not added to the order (until we delete all the tax line items from the order and then click on "Save Changes" button).
The deleted tax is not added to the order since the flag in the patch is set to false even if one tax is applied to the order but actually two taxes must be applied to current order and only one tax is saved & displayed.
The attached patch handles such a scenario.

Rastløs’s picture

Hi,
thanks to everyone working on this. Looks like patch in #31 works for me. I do however have one problem, but it might not be related.

When I add a discount to the order, the subtotal doesn't change, and the tax doesn't recalculate. Does anybody have any clues as to how I can solve, work around this?

I can change the price of all the products, but this makes for a lot of extra work and doesn't give me the option to show the discount.

DanZ’s picture

Rastløs: Provide a step-by-step procedure on exactly how you can reproduce this problem. Even if you think it's obvious and simple, list every bit.

ebo1958’s picture

Status: Needs review » Reviewed & tested by the community

#31 works for me as well, thanks.

ebo1958’s picture

Status: Reviewed & tested by the community » Needs work

Sorry, this patch did work for me. Then suddenly it didn't. I have 2 tax rates, 7% and 19%. When I look up the orders or on the invoices the 7% is ok. On mixed orders, the sums come up correctly. When only 19% products are ordered false sums show up — the taxes seem to be added twice. I removed the patch and the sums are corrected. More info under http://www.ubercart.org/forum/support/32759/taxes_seem_be_added_twice_or...

DanZ’s picture

Not sure if this is related, but....

A customer ordered the wrong product. My store administrator changed an order to remove the single product in it, then put in a different product. The log shows that she removed the tax line item, even though she swears that she did not. She then completed the order as usual. There's no tax on it, though, which is wrong.

I tried manually repeating this. Sure enough, the tax line item disappeared, probably because there was nothing in the cart. This should probably be changed.

Now, the order doesn't have the tax item. It's in completed status, but there doesn't seem to be any way to get the tax item back. In particular, there's no manual way to add a tax line item.

Looking at the instructions here, I put the order into "In checkout" status (from the view page), went to the order edit page, and hit "Submit changes". Sure enough, the tax re-appeared. I then went back to the order view page, and put the order into "Completed" status. The tax stayed.

ebo1958’s picture

As far as calculating the tax goes, the patch (#31) works. The line items are perfect and I can create or change an order by placing it "In Checkout." The Grand Total is also correct. The problem is the price of the product. With the 3,99€ tax included, the price shown should be 25€ but it's 28,99€ when the order is in any status other than "In Checkout". When I remove the patch, the sum is 25€ as it should be. As said, this only happens when all products are the 19% tax rate. Unfortunately, when I remove the patch I can't create or change orders properly. Any ideas on what could be happening here?

ajFernandez’s picture

Hi ebo1958,

I have the same problem. Were you able to fix the problem? Thanks!

ebo1958’s picture

Hi ajFernandez,
Sorry, I'm not a programmer so haven't a clue on how to fix this. I was wondering if it has something to do with using "included tax." Are you using included tax? This could be helpful.

ajFernandez’s picture

Hi ebo1958,

I am using included tax in the products and it works fine but when the administrator creates an order from admin interface the patch (#31) increments the tax in all line items twice because I am using included tax setting. I think it should check the products already have the tax included and not apply again if they had them.

Regards,

coolhandlukek2’s picture

#27 Longwaves comment on changing the order status to 'In Checkout' was very helpful. I couldn't for the life of me work out why my Taxes were not being added.

mandreato’s picture

  • ORDER FROM USER
    1. login as a customer
    2. drop a 10€ (+VAT) product on your cart
    3. complete checkout
    4. on admin/store/orders/... you see one product line which total is 10€ (+VAT) (OK1)
    5. a tax subtotal line is visible before Order total (OK2)
  • ORDER FROM ADMIN
    1. login as administrator
    2. navigate to admin/store/orders/create and choose a customer
    3. create an order and fill in the addresses fields
    4. add the 10€ (+VAT) product to order then submit
    5. on admin/store/orders/... you see one product line which total is 10€ (without VAT) (KO1)
    6. no tax subtotal line is visible before Order total (KO2)
  • ORDER FROM ADMIN with additional step suggested in #27
    1. login as administrator
    2. navigate to admin/store/orders/create and choose a customer
    3. create an order and fill in the addresses fields
    4. submit changes, view order on admin/store/orders/... then change status to "In checkout"
    5. edit the order and add the 10€ (+VAT) product to order then submit
    6. on admin/store/orders/... you see one product line which total is 10€ (+VAT) (OK1)
    7. a tax subtotal line is visible before Order total (OK2)

I didn't tried patch #31 and maybe another specific default status would be better when creating orders from admin, but I second the @longwave suggestion of "In checkout" default status as a quick workaround.

rvb’s picture

Indeed, I can get a tax line with the correct total amount being displayed as mentioned in #42 when creating an order in the back office, but it's very clumsy to explain to my employees.

Is there a way that rules can help with this to put an order in "In Checkout" status instead of "In progress" when an admin creates a new order?

longwave’s picture

Title: Editing an order does not update tax » Allow tax to be edited for newly created orders
Category: bug » feature
Priority: Major » Normal

So there is no bug here really, just we should make it more streamlined and intuitive for admin users.

rvb’s picture

Indeed, it all works as intended, yet I am currently sponsoring the development of a module that should make this an easy step (read: add an updated tax line to an order created in the backoffice). Hopefully I'll be able to share the (good) code.

beautifulmind’s picture

If you have added any conditions which creating the tax, it will not update the total cost. Also, apply this patch https://drupal.org/comment/6942844#comment-6942844

Regards.

rvb’s picture

ok, so I had a developer creating this for me and it works great:

Goal: To apply the tax when an order is created by an administrator in the back office.
(This already works out of the box, but then you have to switch between tabs and change the status of your order etc. Now it will just update the tax and the total of your order on the spot)

Here are the changes I have made:

File: ubercart/uc_taxes/uc_taxes.module
1) Modified function: uc_taxes_filter_rates($order)
   Code line number: 397
   Modified code: 
   if (isset($order->line_items)) {
      $order_tax_count = 0;
       foreach ($order->line_items as $item) {
         if ($item['type'] == 'tax') {
          $order_tax_count++;
           if (!empty($item['data']['tax'])) {
             // Use the rate stored in the line-item.
             $taxes[] = clone $item['data']['tax'];

2) Modified function:function uc_taxes_filter_rates($order)
   Code line number: 416
   Modified code:
      // count the actual no of taxes 
      // that can be applied to an order
      $actual_tax_count = uc_taxes_get_tax_count();

      // Check if all the taxes are applied to current order
      // if not apply all the taxes that are applicable to current order
      if ($order_tax_count < $actual_tax_count) {
        foreach (uc_taxes_rate_load() as $rate) {
          if (rules_invoke_component('uc_taxes_' . $rate->id, $order)) {
            $taxes[] = clone $rate;
          }
        }
      }
     }
   }

3) Modified function: function uc_taxes_get_included_tax
    Code line number: 681
    Modified code: 
       return array($amount, $suffixes);
      }
     
/**
* Counts the number of taxes that can be applied to an order
*
* @return 
*   The number of taxes that can be applied to an order
*/
function uc_taxes_get_tax_count() {
  $tax_count = 0;

  // Get all the rate data from the database.
  $result = db_query("SELECT * FROM {uc_taxes} ORDER BY weight");

  // Loop through each returned row.
  foreach ($result as $rate) {
    $tax_count++;
  }
    
  return $tax_count;
}


File: /uc_order/uc_order.line_item.inc
1) Modifid function: uc_line_item_total($op, $order)
    Code line number: 35
    Modified code: 
    lines[] = array(
         'id' => 'total',
         'title' => t('Order total'),
        'amount' => $order->order_total,
       );
       return $lines;
   }


File: uc_taxes/uc_taxes.module
Modified function:  uc_taxes_uc_line_item
Code Line number: 159
Modified code:
$items['tax_display'] = array(
     'title' => t('Tax'),
     'callback' => 'uc_line_item_tax_display',
    'weight' => 9,
     'stored' => FALSE,
     'calculated' => TRUE,
     'display_only' => TRUE,

All comments are welcome of course.

longwave’s picture

@rvb: It would greatly assist us and anyone who wants to test your changes if you uploaded them as a patch file. See https://drupal.org/patch/create

hanoii’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
2.33 KB

Attached is a patch that implements what @longwave suggested on #27.

#2129243: rules condition for admin created orders also modifies common bits of code, I wonder if both could be included?

Status: Needs review » Needs work

The last submitted patch, 49: admin-orders-in-checkout-status-1425978-49-D7.patch, failed testing.

hanoii’s picture

Status: Needs work » Needs review
FileSize
3.05 KB

Fix the failing test, I added 'admin_in_checkout' status to the "active" select on the views filter, I thought about where to add this and that seemed as the best place to do so, as I assume admin_in_checkout is an active order, isn't it?

mandreato’s picture

Status: Needs review » Reviewed & tested by the community

Applied patch #51 and tried on testcase #42.

  • ORDER FROM ADMIN
    1. navigate to admin/store/orders/create and choose a customer
    2. create an order and fill in the addresses fields
    3. add the 10€ (+VAT) product to order then submit
    4. on admin/store/orders/... you see one product line which total is 10€ (+VAT) (OK1)
    5. a tax subtotal line is visible before Order total (OK2)
hanoii’s picture

Worth noting that patch ads an update code, so it needs update.php or drush updb to run before trying, but glad it worked.

mandreato’s picture

Indeed, I runned update.php and cleared cache as usual.
Thank you !

mandreato’s picture

Status: Reviewed & tested by the community » Needs review

I've found a regression on stocks:

  1. navigate to admin/store/orders/create and choose a customer
  2. create an order and fill in the addresses fields
  3. add a product and choose the shipping method, then save
  4. the order is created with status "Admin - in checkout"
  5. the stock level for the product has been decreased
  6. add a payment and the status changes to "Payment received"
  7. the stock level for the product has been decreased AGAIN
Colin @ PCMarket’s picture

We are running the patch provided by hanoii and much prefer this functionality over the default workflow.

Unfortunately we found out the hard (expensive) way about how to get the GST to apply to admin created orders and some orders shipped without any tax applied.

Thank you hanoii, i hope this patch gets reviewed further and committed as it could save much confusion for others.

zrpnr’s picture

Status: Needs review » Reviewed & tested by the community

Applied patch in #51, tested on 7.x-3.8
Create order as admin, enter address, add product. Tax is added correctly, tested with State and City taxes.
Changing address, state/province or country will apply the new tax rate after clicking "submit changes".

Running update.php provides the new "Admin - in checkout" status.

I tried to recreate the bug described in #55, but I didn't see the stock level decrease any further after editing payment info on an existing order.

jdhildeb’s picture

Here is rvb's modification (from #47 above) as a proper patch file.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 58: apply-tax-when-order-created-by-admin.patch, failed testing.

jdhildeb’s picture

Status: Needs work » Needs review

Initial testing failed (25 May 2017) but two more recent runs have passed. So I'm changing the status to 'Needs review'.