OCA / delivery-carrier

Odoo Carriers And Deliveries Management
https://odoo-community.org/psc-teams/logistics-18
GNU Affero General Public License v3.0
113 stars 359 forks source link

[18] Split base_delivery_carrier_label and drop it #896

Open florian-dacosta opened 1 month ago

florian-dacosta commented 1 month ago

Hello, base_delivery_carrier_label was meant to be a base module for shipping carrier label generation specific modules, but over the time it became quite hard to use, mainly because, it does too much stuff to be such a base module. Also, it was created before Odoo did start to manage carrier labels and has a lot less sense today. I had already started to split it in version 16, extracting the carrier account model : https://github.com/OCA/delivery-carrier/pull/570

I believe we should continue to split the used features so we can get rid of it and maybe avoid its migration to version 18. Here how I see this :

1) A delivery_carrier_option module, with all features about option you can choose on the picking about the delivery.

2) A stock_picking_delivery_label_link (or another name of course) for the shipping.label model which allow to identify the shipping labels from other attachments, mainly for further printing. A module is beeing implemented in v15 : https://github.com/OCA/delivery-carrier/pull/892 but with a totally different approach. This later module add many2many fields on picking to identify the shipping labels while base_delivery_carrier_label adds a shipping.label model inheriting attachment, to allow to find the labels. => About this one, it would be nice to have the opinion of the users of these feature, if it would be ok to go with the many2many way, that Tecnativa team seems to support or if it is important to keep the current way of base_delivery_carrier_label. I do prefer the shipping.label as for me it offers more possibility (adding field to help with the printing) but I could live with the other way, and I think it is important to converge and avoid having 2 concurrent modules for such a small feature.

I am not sure who use this but it would be nice to have some opinion before taking action. @hparfr @sebastienbeau @bguillot @yvaucher @hbrunn FYI @pedrobaeza @chienandalu

3)The manifest wizard => In a new dedicated module if needed (I do not use this one and do not know someone still does.) @ismaelcj

4) Tracking reference fields at package level + button to check the carrier tracking url on package As far as I know only delivery_roulier uses it, it may not be worth a dedicated module to just add to fields on stock.quant.package, unless other modules uses it => I will integrate it in delivery_roulier for now.

5) Weight features

6) Test features, there is a test class CarrierLabelCase which can be used by specfic carrier implementation modules to avoid some code duplication. I am not sure it worth to keep it or mainly where to put it... A dedicated module seems too much and it does not seem very used. As delivery_roulier is already a base module for all carrier modules using roulier, we could have some test helper there and for implementation not using roulier, it seems they do not use this for now so it is probably un-necessary to think about it.

I did not find these 2 methods : stock.picking._get_label_sender_address neither stock.picking._check_existing_shipping_label used anywhere, so I guess we can get rid of it for now.

To cut a long story short, my proposal is to get rid of base_delivery_carrier_label extracting mainly 2 modules delivery_carrier_option and stock_picking_delivery_label_link and throw the rest away. If someone misses one of the feature, then it should be quite simple to propose a brand new module with it.

hbrunn commented 1 month ago

sounds reasonable to me