[15.0][ADD] stock_picking_delivery_label_link: New module

[chienandalu]

cc @sergio-teruel @Tecnativa TT51077

[florian-dacosta]

There is a way to identify shipping label since a long time https://github.com/OCA/delivery-carrier/blob/16.0/base_delivery_carrier_label/models/shipping_label.py

I believe the base_delivery_carrier_label should be refactore to be splitted into multiple modules and one of the module would be this shipping label class. Could we not converge toward something like this ?

[pedrobaeza]

@florian-dacosta not in this stable version...

[florian-dacosta]

@pedrobaeza Sure, but I was wondering if something similar could be done instead of doing it differently ? I mean, we could introduce in v15 this stock_picking_delivery_label_link that would implement the shipping.label inherits instead of a many2many on pickings? And then in v18, we could migrate it and remove this part from base_delivery_carrier_label somehow ?

I don't know what is feasible on your side, I am just wondering if we could avoid having, at term, 2 concurrent modules...

[pedrobaeza]

We can converge in 18 if that ugly module is split.

[chienandalu]

Hi @florian-dacosta precisely the intention here is to avoid pulling new deps into the current modules. For the future, a base module could add a hook, maybe a filed, and not much more IMO... for that base_delivery_carrier_label it's too bloated in its current shape...

[florian-dacosta]

Well we agree about the current state of base_delivery_carrier_label What I am trying to say, is that, beeing able to determine which attachment are shipping label and which are not is important. This is done in base_delivery_carrier_label by adding a new model (shipping.label) inheriting ir.attachment and by this module adding a many2many field.

The approach is totally different. Let's say we extract the shipping.label feature from base_delivery_carrier_label to have a brand new v18 stock_picking_delivery_label_link module, it would mean a data migration from the many2many toward this new shipping.label model.

Maybe the approach to have a many2many field is best and should be kept, and we'll then have to migrate from shipping.label toward this many2many way, but I'd like to be sure it has been thought.

Do you think the many2many approach is best and why? I don't have a strong opinion on the matter, but the shipping.label inherits approach seemed fine. And it seems more powerful as some field could be added if needed. Example in base_delivery_carrier_label are package_id. If filled, it could for instance allow to re-print a single package label if needed instead of printing all labels related to the picking. Or the file_type (pdf, zpl...) that could be used for instance to send the shipping labels to a different printer depending on the type (zebra...)

I mean, if I start a work to extract the shipping.label stuff from base_delivery_carrier_label in v18 now, would it be ok or would you stay with this implementation anyway ?

[pedrobaeza]

For me, bloating the DB schema with more and more models for something so simple as an attachment is not the correct approach.

[florian-dacosta]

Well anyway, it is nice to try to make the module work on its own, independently of any code in the carrier specific module.

But it is not reliable enough for now. Indeed, during the call to send_to_shipper attachment that are not labels are sometimes created. I have multiple use cases like this. The main one beeing when you export some good, like, a french company sending goods to united state or any country outside EU. The call to carrier webservices give back some labels, but it also give back some customs documents which usually are A4 formats, not compatible at all with the label formats. In this case, I will find myself with my customs A4 documents in the shipping_label_ids which is not good obviously.

I think we should at least implement something to be able to bypass the current way to compute these shipping labels so the specific carrier impl modules could override it and compute it their own way if needed.

For instance :

    def _all_carrier_document_are_shipping_label(self):
        # Override this method for your carrier if not-label attachment could be created during the call to carrier webservice
        self.ensure_one()
        return True

    def send_to_shipper(self):
        # Shipping labels are attached to the record during this method. There's no
        # core hook method for this, and we want to avoid pulling a dependency in
        # every carrier implementation.
        if self._all_carrier_document_are_label():
            previous_attachments = self.allowed_shipping_attachement_ids
            result = super().send_to_shipper()
            self._compute_allowed_shipping_attachement_ids()
            self.shipping_label_ids = (
                self.allowed_shipping_attachement_ids - previous_attachments
        )
            return result
        else:
            return super().send_to_shipper()

So that the specific carrier implementation module, if we know we are in the case I described (send_to_shipper generate not-label documents), I could depend on this module and implement :

    def _all_carrier_document_are_label(self):
        if self.carrier_id.delivery_type == 'my_type':
            return False
        return super()._all_carrier_document_are_label()

And then implement its own logic to fill the shipping_label_ids fields. It is a bit less pretty and it adds a few lines of code but at least it becomes usable for all cases.

I am way more in favor of having one or multiple simple dependencies that are kind of required for all carrier specific impl like this module or delivery_carrier_account modules but I know that is not the way you see it, so I guess I would settle for that...

I would also hide the shipping_label_ids or set it to readonly at least, to avoid any mistake. I am not sure why we need it displayed here since we can see all attachments in the chatter and labels are generally easy to spot from the name of the attachments. And I think it is confusing to be able to delete it from the many2many field but that it does not change anything to the attachment in the chatter.