-
-
Notifications
You must be signed in to change notification settings - Fork 353
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
base: 14.0
Are you sure you want to change the base?
Conversation
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.
There was a problem hiding this 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
There was a problem hiding this 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
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 |
There was a problem hiding this comment.
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
There was a problem hiding this 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 |
There was a problem hiding this comment.
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).
carrier = self.partner_id.purchase_delivery_carrier_id | |
carrier = self.vendor_label_carrier_id or self.partner_id.purchase_delivery_carrier_id |
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) |
There was a problem hiding this comment.
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
…livery_purchase_label
… Add delivery_purchase_label
carrier = self.vendor_label_carrier_id | ||
if not carrier.purchase_label_picking_type: | ||
return |
There was a problem hiding this comment.
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
?
… fixup! Add delivery_purchase_label
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.