Jump to content
thirty bees forum
  • 0

Edit order, update shipping


RabbitZzZ

Question

23 answers to this question

Recommended Posts

  • 0

Well I just looked at it for about an hour. But this looks more complex to me than expected.

- There are shipping costs saved ps_orders and ps_order_carrier. Why is this needed?
- There is total_shipping and total_shipping_tax_incl in ps_orders. What is the difference?

In orders class there is a method "updateShippingCost($amount)", but this doesn't look finished. The core doesn't seem to use this method at all. A question is also: should the core calculate the shipping cost newly or should there be an input field, which allows flexible input? A lot of tquestions :classic_rolleyes:

  • Thanks 1
Link to comment
Share on other sites

  • 0

Hmm, seems quite tricky, yes. Looks like the database could be designed less redundant in that matter.

In my opinion it would be good to have both options you mentioned. If there are products added to the order at least the total weight should be updated and the shipping cost should be calculated newly according to the configurated shipping table. The possibility to change the cost manually would be nice anyway. That would add flexibility in offering discounts in customer contact etc.

Is that something that could be added as a goal for an upcoming version?

Link to comment
Share on other sites

  • 0
15 minutes ago, RabbitZzZ said:

Looks like the database could be designed less redundant in that matter.

Quite the opposite, actually. When we make a sale, we always have to make a snapshot of the base information (product price, name, tax settings, shipping settings, etc...) so we could work with the order/invoice later, even when these changed already. In other word -- when you change the product price or shipping settings, your current orders and invoices can't change.

15 minutes ago, RabbitZzZ said:

In my opinion it would be good to have both options you mentioned. If there are products added to the order at least the total weight should be updated and the shipping cost should be calculated newly according to the configurated shipping table. The possibility to change the cost manually would be nice anyway. That would add flexibility in offering discounts in customer contact etc.

Is that something that could be added as a goal for an upcoming version?

The big problem is that the bulk of the calculation is performed in Cart class, and then cart is just converted to Order. That works good for front-office, but it's a terrible situation for back-office. There are few things we can do:

- duplicate the code from cart to order and adjust it. This is recipe for disaster, as we would end up with two similar implementations tackling the same problem

- create new / temporary cart when modifying the order - this is how it's done now, at least for adding new products to order. While it may sound like a good idea, it's really not. Cart work against current state of the database, but that's not what we want when updating order. We want to use historical data (product price valid at the time when product was originally ordered), we don't want cart to check available quantities for some products, but we would like to check quantities for products newly added to order,... It became real messy very soon. 

The proper solution would be to extract the business logic from cart, and make it reusable by both cart and order. But that's very hard and dangerous refactoring. I don't think this will be tackled anytime soon

  • Like 1
  • Thanks 1
Link to comment
Share on other sites

  • 0

Ok, tried the module. Installs alright, but when you try to change the shipping cost the update button does not work (console says: "Failed to load resource: the server responded with a status of 503 (temporarily overloaded)").
Also it's supposed to just allow you to manually edit the cost and does not recalculate it based on new articles/weight.

Edited by RabbitZzZ
other post in between
Link to comment
Share on other sites

  • 0

Yeah I also came to the conclusion that the shipping costs are calculated in the Cart Class. But actually I am not even sure, which method is for what purpose. There are multiple;

  • getTotalShippingCost
  • getPackageShippingCost
  • getCarrierCost

Deprecated:

  • getOrderShippingCost (it says use static::getPackageShippingCost) but this is not even a static method...

I believe that getPackageShippingCost is the interesting method. There it's also possible to use an array $productList. But I agree with @datakick. This can't be cleaned up so easily. That's a huge project. Probably similair as the rounding process, which is being under review by traumflug right now.

  • Like 1
Link to comment
Share on other sites

  • 0

@RabbitZzZ, @toplakd, @datakick

As I get mad almost once a week about the misssing "update shipping cost", I decided to tackle this myself. I couldn't wait any multiple years longer for this feature. In my opinion it's over due... As we pointed out above, there is no super clean solution to this problem, but I believe that my solution now is quite acceptable. At least it's much a cleaner way than the free PS module uses. 

Technically a new cart is created based on the order values. The cart is used then to recalculate the shipping cost.

I practice you can select the shipping method in an order and click on "recalculate shipping cost". This is very useful in case you have edited the order before. The select also allows to change the shipping method at all. This way you can change, in case your customer wishes so or you can also select a shipping method, that is only visible in the BO.

I haven't imlemented a amount input yet, but that would be possible too. I hope, that some expierenced user test my snipped (in a test store) and give a feedback. It's really hard to forseen every case. In my (quite simple) cases, the code seems to work properly.

https://github.com/eschiendorfer/thirtybees/commit/4ccd64465ff1e4b956e1caba1feb411b6ee834a3

  • Like 3
Link to comment
Share on other sites

  • 0

I have invested another day. It's now possible to add fixed a fixed amount/weight. Also the hardcoded transit email can be fixed that way. 

Still my code is not totally finished:

  • There needs to be validation check in the new updateShipping function.
  • RMA seems to use "submitShippingNumber" as well. That's maybe not problematic. I don't use RMA at all. So no idea what this should do. 

After a click on edit, the form looks like this:

shipping-update.JPG

Btw: For me this project is quite evident. My goal is clearly to push this into the core (after careful validations with merchants). If this doesn't workout, I won't supply anything in the future. Cause developping things for core, which doesn't get applied, is too costly:

  1. I invest a lot of time to be consistent with the core, if it's only for my own store, all is much faster and simpler.
  2. After a modification get declined, I have to rewrite again, since I need to use the code in an internal module instead of core.
  3. If this important (and much wanted) feature can't be implemented, I lose faith, that there will come any real progess on other topics (like asm, attributes on bundles/virtualProducts and so on...).
  • Like 3
Link to comment
Share on other sites

  • 0

It doesn't recalculate but you can change a lot on the order. It's this module I am using: 

https://globosoftware.net/product/prestashop-order-management-module-edit-order/

It do work pretty well for our needs but do have some bugs.

The one you are referring too is from a scam company "Module Buddy". I ordered another module from them but never got any download but the money was withdrawn. They do never answer emails either. The developers are from my country Sweden, I managed to find the phonenumber to one of them but still haven't seen the money or the module and this was some years ago.

  • Like 1
Link to comment
Share on other sites

  • 0

Thanks @wakabayashi for this initiative.

It will have to be thoroughly tested before it is integrated to the core, obviously. But hopefully there won't be any showstoppers.

One thing that would be nice to implement together with this functionality would be an audit log, so merchants could see why, when, and by whom was the order modified

  • Like 2
Link to comment
Share on other sites

  • 0
2 hours ago, datakick said:

It will have to be thoroughly tested before it is integrated to the core, obviously.

I understand ofc. We are already using it in our live store, but ofc other testers are needed.

 

2 hours ago, datakick said:

One thing that would be nice to implement together with this functionality would be an audit log, so merchants could see why, when, and by whom was the order modified

You mean in AdminLogs or something new? If it would be new, I believe we would better make a order history concept, that shows everything (like order status change, emails sent, customer messages, customer service cases) in a nice history tree or so.

 

Have you briefly looked at the concept. How do you think about this: 

"Technically a new cart is created based on the order values. The cart is used then to recalculate the shipping cost."

  • Like 1
Link to comment
Share on other sites

  • 0

I did look at your commits briefly, and I have some questions / comments:

  • even when Recalculate shipping is false, there is some recalculation performed. For example, newShippingCostTaxExcl will be updated according to current carrier settings tax settings. That shouldn't happen -- in fact, I would not call the new updateShipping at all in this situation
  • when shipping is modified, then existing invoice would be updated. At least in my country this is very serious issue -- invoices send to customers must be immutable, and all changes should be done by new (corrective) invoice only
  • There is an issue with carrier selection -- if carrier was modified since the order was created, it will have different id_carrier (but the same id_reference). The original id_carrier will not be part of rendered carrier list, and therefore will not be pre-selected 

 

Link to comment
Share on other sites

  • 0
50 minutes ago, datakick said:

even when Recalculate shipping is false, there is some recalculation performed. For example, newShippingCostTaxExcl will be updated according to current carrier settings tax settings. That shouldn't happen -- in fact, I would not call the new updateShipping at all in this situation

I will look again into this. But IMO this is not the case. It's true that 'updateShipping' is called always. This is because you can set a fixed amount of shipping. If you don't change this value and don't select recalculate, there is an update but with just the same values as you already had in DB.

52 minutes ago, datakick said:

when shipping is modified, then existing invoice would be updated. At least in my country this is very serious issue -- invoices send to customers must be immutable, and all changes should be done by new (corrective) invoice only

In my country this is a bit different. But I am aware that this can be an issue. That's why I created the form in the second commit. The form allows me to add an "invoice select". So every merchant can select the invoice, which should be updated.

54 minutes ago, datakick said:

There is an issue with carrier selection -- if carrier was modified since the order was created, it will have different id_carrier (but the same id_reference). The original id_carrier will not be part of rendered carrier list, and therefore will not be pre-selected 

Right. I completly forgot about this shitty id_carrier change. But I believe, if I just add the "deleted/changed" carrier to the selector as well, this should be solved.

Link to comment
Share on other sites

  • 0
1 hour ago, wakabayashi said:

I will look again into this. But IMO this is not the case. It's true that 'updateShipping' is called always. This is because you can set a fixed amount of shipping. If you don't change this value and don't select recalculate, there is an update but with just the same values as you already had in DB.

Unfortunately that's not true. For example, there is always recalculation of 

$newShippingCostTaxExcl = (float) $amountTaxIncl/(1+($newCarrier->getTaxesRate($deliveryAddress)/100));

which means that current tax rate will be used to figure out shipping cost tax excl

 

  • Thanks 1
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...