[16.0][ADD] sale_order_confirm_partial: add module

[dmitriypaulov]

This module allows to select which sale order lines you would like to confirm. It also can create a new quotation in the "Cancel" state with all the unconfirmed lines from the original quotation in case you need to keep that information.

[ivs-cetmix]

Can we use compute with readonly=False instead of onchange here?

[ivs-cetmix]

Check the comments and add tests pls.

[ivs-cetmix]

why not to set "cancel" state directly here?

[dmitriypaulov]

Because in the loop below I'm changing order lines of this order, but Odoo restricts such changes for orders in Cancel state

[ivs-cetmix]

Check the comment at L84

[angelinaanaki]

Code LGTM

[ivs-cetmix]

LGTM

[ivs-cetmix]

This is what I was talking about in the previous comment. You need to use direct reference. Eg

self.so_line_1.product_uom_qty,

[ivs-cetmix]

LGTM

[rousseldenis]

@dmitriypaulov What's the status of this ?

[ivs-cetmix]

Hi

@dmitriypaulov What's the status of this ?

Hi @rousseldenis we are already using this module. However would appreciate additional code review.

[Deriman-Alonso]

LGTM 👍.

[ivs-cetmix]

@dmitriypaulov What's the status of this ?

Hi @rousseldenis, could we have this checked and merged?

[ivs-cetmix]

Hey @OCA/crm-sales-marketing-maintainers I know that's summer holiday season and probably you are enjoying your drinks is some beach bars 😄
However would be great if someone could have a look at this in between of two "margaritas" 🍹

[ivs-cetmix]

Hey @OCA/crm-sales-marketing-maintainers I know that's summer holiday season and probably you are enjoying your drinks is some beach bars 😄 However would be great if someone could have a look at this in between of two "margaritas" 🍹

Hey @OCA/crm-sales-marketing-maintainers and @rousseldenis
Any chances for this to be merged?

[rousseldenis]

@dmitriypaulov @ivs-cetmix CI is red. Could you rebase ?

[ivs-cetmix]

Hi @rousseldenis , PR is rebased. Let me know what should be done next.

[rousseldenis]

@dmitriypaulov @ivs-cetmix you should fix tests

[mmequignon]

What about sale_order_split_strategy (available in 18.0) ?

[ivs-cetmix]

What about sale_order_split_strategy (available in 18.0) ?

That's absolutely different module. This one is user-focused: "I have an order, I want to confirm it right now, but only Item A and Item B".
The sale_order_split_strategy is about "Let's define some rules and apply them later".

P.S. this module was initially developed for a project that ... and it's gone... 😆 So if you would like to have this merge would appreciate your assistance with tests as it looks like they need to be addressed still.

[mmequignon]

That's absolutely different module. This one is user-focused: "I have an order, I want to confirm it right now, but only Item A and Item B". The sale_order_split_strategy is about "Let's define some rules and apply them later".

P.S. this module was initially developed for a project that ... and it's gone... 😆 So if you would like to have this merge would appreciate your assistance with tests as it looks like they need to be addressed still.

I would rather use the sale_order_split_strategy module, add a new field to_split on sale_order_line, and have a rule extracting lines with to_split == True.

I honestly would avoid having 2 modules doing "mostly" the same thing but with a slight difference.

[ivs-cetmix]

I would rather use the sale_order_split_strategy module, add a new field to_split on sale_order_line, and have a rule extracting lines with to_split == True.

I honestly would avoid having 2 modules doing "mostly" the same thing but with a slight difference.

I got your point, however they are absolutely different modules designed for different things. Yes, they both split quotations, but this is the only thing they have in common. I would recommend you to open the documentation for the both modules and check it.
Adding a rule may look good from the developer perspective, but it's a mess from the user's one. User's don't want to mess with technical things. They want it to work simple and straightforward.
Let's say a salesperson is in a call with a client discussing a quotation. The negotiations go so-so, but finally the client says "Ok, I will by Item 2 and 3 pcs of Item 7". How you suppose a salesperson to do this instantly with the sale_order_split_strategy ? "Hey, wait for a while, let me create a new strategy for your order?" 😄

[mmequignon]

I would rather use the sale_order_split_strategy module, add a new field to_split on sale_order_line, and have a rule extracting lines with to_split == True.
I honestly would avoid having 2 modules doing "mostly" the same thing but with a slight difference.

I got your point, however they are absolutely different modules designed for different things. Yes, they both split quotations, but this is the only thing they have in common. I would recommend you to open the documentation for the both modules and check it. Adding a rule may look good from the developer perspective, but it's a mess from the user's one. User's don't want to mess with technical things. They want it to work simple and straightforward. Let's say a salesperson is in a call with a client discussing a quotation. The negotiations go so-so, but finally the client says "Ok, I will by Item 2 and 3 pcs of Item 7". How you suppose a salesperson to do this instantly with the sale_order_split_strategy ? "Hey, wait for a while, let me create a new strategy for your order?" 😄

You are misunderstanding my point I think.
We can create a module based on sale_order_split_strategy.
The module adds the rule and a boolean field on sale_order_line, done.
After that the user just has to toggle lines to be confirmed, which is exactly what this module does.

I would rather backport sale_order_split_strategy and eventually create a module for your specific need.

[ivs-cetmix]

You are misunderstanding my point I think. We can create a module based on sale_order_split_strategy. The module adds the rule and a boolean field on sale_order_line, done. After that the user just has to toggle lines to be confirmed, which is exactly what this module does.

I would rather backport sale_order_split_strategy and eventually create a module for your specific need.

I see, got your point now. Yes, this might be reasonable. In this case I would prefer to abandon this PR but leave it in case someone would like to implement this later. Probably it makes no sense to keep it for 16.0 as the main focus shifted to 18.0 already.

[alexey-pelykh]

Thanks for the contribution. I looked through the code and I tend to agree with @mmequignon's suggestion -- building on top of sale_order_split_strategy (with a backport to 16.0 if needed) would be a cleaner path and avoid maintaining two modules with overlapping scope.

Since @ivs-cetmix also acknowledged this direction makes sense, it might be worth closing this PR in favor of that approach.

That said, a few code-level notes in case anyone picks this up or adapts it later:

1. action_confirm breaks multi-record calls. The override uses self.id (singular) to set default_sale_order_id on the wizard, but stock action_confirm supports multi-record. When partial confirmation is enabled, calling action_confirm() on a recordset of multiple orders would silently use only the last order's id. Should at minimum add self.ensure_one() or loop explicitly.

2. Tests reference fields that don't exist on the model. In test_confirmation_with_save_unconfirmed_selected, the assertion filters unconfirmed_so.order_line using lambda x: x.so_line_id == self.so_line_1 -- but sale.order.line has no so_line_id field (that's on the wizard line model). This is likely why CI is red.

3. attrs syntax is deprecated in Odoo 16+. The views use attrs="{'invisible': [...]}" which still works in 16.0 but newer OCA linting may flag it. Minor point for a 16.0 target.

4. String formatting with % in translatable strings. The message_post calls use _("...") % {...} -- while this works, OCA guidelines generally prefer _("... %(name)s ...") % {...} which is already used here, so that part is fine.

[rousseldenis]

@ivs-cetmix Could we close ?

[ivs-cetmix]

@rousseldenis yes, thank you!