Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[18] Split base_delivery_carrier_label and drop it #896

Open
florian-dacosta opened this issue Oct 28, 2024 · 1 comment
Open

[18] Split base_delivery_carrier_label and drop it #896

florian-dacosta opened this issue Oct 28, 2024 · 1 comment

Comments

@florian-dacosta
Copy link
Contributor

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 : #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 : [15.0][ADD] stock_picking_delivery_label_link: New module #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

  1. 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.

  2. Weight features

  • A new field weight on stock.move.line which is not filled anywhere => I guess we ignore it for now and decide when someone need it, if anyone does..
  • The override of stock.quant.package._compute_weight computed from stock.move.line.get_weight
    I am not really sure if it is still needed somehow and why. I guess we could first try to get rid of it and implement it somewhere if it is still needed, and document it.
    Maybe it was to be able to get the weight before the transfer of the picking (to be able to print a label before transfer). In that as, it could deserve a module doing it, along with the stock.picking.send_to_shipper override, not sure.
  1. 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
Copy link
Member

hbrunn commented Oct 28, 2024

sounds reasonable to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants