[ADD] Module to merge draft or confirmed sales orders #341
Conversation
StefanRijnhart
force-pushed
the
8.0-sale_order_merge
branch
3 times, most recently
from
89c5894
to
b5586a4
Compare
StefanRijnhart
force-pushed
the
8.0-sale_order_merge
branch
from
b5586a4
to
afdea11
Compare
| This module allows the sale employee to merge draft or confirmed orders | ||
| from the same customer. | ||
|
|
||
| When orders are merged, draft invoices and unprocessed outgoing pickings |
There was a problem hiding this comment.
Why merging pickings? You can leave them unmerged as Odoo standard allows it.
There was a problem hiding this comment.
Why else merge the orders? This was the main use case of my customer.
There was a problem hiding this comment.
Yeah, you're right. You can't merge pickings without merging the "upstream" sales orders, or the links will not be totally correct (although the main control is through sales order lines, not through sales orders, but the main information/reports won't be correct).
|
I create a SO 1, I confirm and creates a DO1.
|
| for picking in self.sale_order.picking_ids: | ||
| if (picking.state not in ('done', 'cancel') and | ||
| picking.location_dest_id.usage == 'customer'): | ||
| key = (picking.picking_type_id, picking.location_id, |
There was a problem hiding this comment.
Looking towards extensibility, please build the key in a separate method. We work, for instance, on another model that various pickings for a sales order, based on the requested date maintained at sales order line level. In this case, when we merge orders we'd want to respect the scheduled date.
| ('partner_shipping_id', '=', sale.partner_shipping_id.id), | ||
| ('warehouse_id', '=', sale.warehouse_id.id), | ||
| ('state', 'in', self.MERGE_STATES), | ||
| ] + policy_clause |
There was a problem hiding this comment.
Looking towards extensibility, please build the domain in a separate method. For example, in sale_operating_unit (OCA/operating-unit#22) you would not want to merge sales orders that belong to separate operating units. Also, why is the company_id not included?
| target = pickings[0] | ||
| pickings -= target | ||
| pickings.move_lines.write({'picking_id': target.id}) | ||
| pickings.unlink() |
There was a problem hiding this comment.
In the context of real time inventory valuation this will cause that the journal entries created from the partial transfers of pickings will refer to a deleted picking (https://github.com/odoo/odoo/blob/8.0/addons/stock_account/stock_account.py#L278)
Perhaps it would be a good idea to add here a hook (at least) that would allow to update these journal entries.
There was a problem hiding this comment.
Only draft pickings are merged and deleted, so this is not an issue right?
|
@jbeficent cannot seem to reply directly to #341 (comment), but I cannot reproduce this. What are the order settings of SO1 and SO2? |
|
@jbeficent found them on the runbot |
|
@jbeficent You got multiple DO's for the merged order because the runbot instance has the module 'sale procurement group per line' installed. I uninstalled it and it works now. |
[FIX] Add company to merge domain [FIX] Fix syntax error in case multiple pickings are merged
| pickings -= target | ||
| pickings.mapped('move_lines').write({'picking_id': target.id}) | ||
| pickings.unlink() | ||
| return True |
There was a problem hiding this comment.
The final picking should have as Source Document the final sales order, not the original.
| Remove the orders that you do not want to merge, and click on the *Merge* | ||
| button in the footer of the pop-up window. The main window will then refresh | ||
| to the updated main sale order. | ||
|
|
There was a problem hiding this comment.
I suggest to add the following:
The mergeability criteria is defined as follows:
- Same customer, shipping address, warehouse and company.
- Orders in status 'Draft Quotation', 'Quotation Sent', 'Waiting Shedule', 'Sale Order', 'Sale to Invoice'.
- Once the order has already been confirmed, only draft orders or confirmed orders with the same invoice policy can be merged.
The criteria can easily be extended using method _get_merge_domain of model sale.order
There was a problem hiding this comment.
Done. Thanks for the review BTW.
[UPD] README, courtesy of Jordi Ballester
StefanRijnhart
force-pushed
the
8.0-sale_order_merge
branch
from
d7cb5dd
to
529ed06
Compare
|
Hi @jbeficient, would you like to revise your review based on my changes? |
|
@StefanRijnhart This has all the required approvals. Can I merge now? |
|
As it has the two require approvals, yes! |
|
Remember to squash on merge as this case there is no significant different commits |
|
Thanks ;-) |
No description provided.