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

[14.0][ADD] delivery_purchase_label #863

Open
wants to merge 12 commits into
base: 14.0
Choose a base branch
from

Conversation

TDu
Copy link
Member

@TDu TDu commented Jul 19, 2024

Print delivery labels for dropshipping purchase order. Useful when then vendor that will process the order does not have the capabilities to print the transporter labels needed for the delivery.

Print delivery labels for dropshipping purchase order.
Useful when then vendor that will process the order does not have the
capabilities to print the transporter labels needed for the delivery.
Copy link

@henrybackman henrybackman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of small comments

delivery_purchase_label/readme/CONFIGURE.rst Outdated Show resolved Hide resolved
delivery_purchase_label/readme/DESCRIPTION.rst Outdated Show resolved Hide resolved
delivery_purchase_label/models/purchase_order.py Outdated Show resolved Hide resolved
delivery_purchase_label/models/purchase_order.py Outdated Show resolved Hide resolved
delivery_purchase_label/models/purchase_order.py Outdated Show resolved Hide resolved
Copy link

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks !

It seems there are quite some lines not covered by unit tests. It might be a good idea to add a few more cases to them :)

LGTM

delivery_purchase_label/models/mail_compose_message.py Outdated Show resolved Hide resolved
Comment on lines 86 to 102
order = self.with_company(self.company_id)
# Create and process the transer to send the labels
values = order._get_purchase_delivery_label_picking_value(carrier)
picking = self.env["stock.picking"].with_user(SUPERUSER_ID).create(values)
moves = order.order_line._create_stock_moves(picking)
moves.location_id = picking.location_id
moves.location_dest_id = picking.location_dest_id
# Remove the link on the sale and purchase
# To not impact the delivered quantity on them
picking.sale_id = False
moves.sale_line_id = False
moves.purchase_line_id = False
picking.action_assign()
# Moves can change on action assign
moves = picking.move_lines
for move in moves:
move.quantity_done = move.product_uom_qty

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this please move into an own method.
Including also line 113 picking._action_done

delivery_purchase_label/models/purchase_order.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some remarks.
Canceling the PO should cancel the generated picking for label. That's a major issue that needs to be tackled.

if not self._is_valid_for_vendor_labels():
return
# Find the carrier that will be used
carrier = self.partner_id.purchase_delivery_carrier_id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a carrier on the PO as initially stated. Use cases showed that the vendor will not always ship with the same carrier. You can keep partner purchase_delivery_carrier_id as a fallback value but ideally we shouldn't configure any partner. We are generating drop-shipping PO and the PO carrier should come from the SO delivery carrier.
I would define a field on the PO like vendor_label_carrier_id.
This new field should be filled in when the PO is created/updated (done outside this module).

Suggested change
carrier = self.partner_id.purchase_delivery_carrier_id
carrier = self.vendor_label_carrier_id or self.partner_id.purchase_delivery_carrier_id

Comment on lines 84 to 90
if self._is_picking_label_uptodate():
return
order = self.with_company(self.company_id)
# Create and process the transer to send the labels
values = order._get_purchase_delivery_label_picking_value(carrier)
picking = self.env["stock.picking"].with_user(SUPERUSER_ID).create(values)
moves = order.order_line._create_stock_moves(picking)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low priority remark:
Instead of making a dry run with _is_picking_label_uptodate, you should create a savepoint, make the picking with the moves, then compare and rollback to savepoint if the moves content is the same.
Without this, you have no warranty your dry-run is doing the right thing as you are calling only a subset of the complete picking creation

delivery_purchase_label/models/purchase_order.py Outdated Show resolved Hide resolved
delivery_purchase_label/models/res_partner.py Outdated Show resolved Hide resolved
delivery_purchase_label/models/res_partner.py Outdated Show resolved Hide resolved
Comment on lines +86 to +88
carrier = self.vendor_label_carrier_id
if not carrier.purchase_label_picking_type:
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be part of _is_valid_for_vendor_labels ?

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

Successfully merging this pull request may close these issues.

6 participants