[16.0][IMP] purchase_lot: remove sale_order_lot_selection dependency

[Tisho99]

Explained in the https://github.com/OCA/purchase-workflow/issues/2644 issue

T-7552

[OCA-git-bot]

Hi @florian-dacosta, some modules you are maintaining are being modified, check this out!

[metaminux]

Hello @florian-dacosta and @Tisho99

I'm answering on both #2247 and #2646 to be sure everyone is OK.

I totally agree with @Tisho99's vision.

The underlying concept is to have control on lot / serial numbers in stock moves and that's what stock_restrict_lot is offering. AFAIK, for the moment, only sale_order_lot_selection is triggering the restriction on moves (setting the restrict_lot_id field) From the beginning, purchase_lot is aiming to get the lot from the sale order lines that created the purchase... maybe it's not enough :)

We could imagine a module being the equivalent of sale_order_lot_selection on purchase side (maybe the need is not so common, but why not) Let's call it purchase_order_lot_selection, it would set restrictions on stock moves from the purchase order lines. The same way sale_order_lot_selection is depending only on sale_stock and stock_restrict_lot, purchase_order_lot_selection should depend on purchase_stock and stock_restrict_lot.

Then, to get the link between sales and purchases (provided by native Odoo glue modules sale_purchase and sale_purchase_stock), we can have a glue module called sale_purchase_lot_selection or whatever. This glue module would be what purchase_lot is doing nowadays, but would depend on sale_order_lot_selection and purchase_order_lot_selection with 'auto_install': True.

In that case, lot_id fields in sale and purchase order lines would become computed writable fields that should be kept in sync with restrict_lot_id fields on related stock moves.

Two questions stay in my mind :

• how do we deal with updates occurring after confirmation of one or more objects involved (sale, purchase) ?
• where should we store the glue module ? should it stay in OCA/purchase-workflow as purchase_lot ?

For the first question, the idea of opened PRs for "lot propagation" is to explicitly allow propagation or not and then if allowed, propagate on every objects as long as no stock move in the whole "chain of moves" is done. For the second one, keeping it in OCA/purchase-workflow and only renaming it seems good to me.

What do you think ? @emiliesoutiras @Kev-Roche

[florian-dacosta]

Thanks for your opinion @metaminux

This glue module would be what purchase_lot is doing nowadays, but would depend on sale_order_lot_selection and purchase_order_lot_selection

today purchase_lot allows to set a lot on the purchase from a from stock moves / procurements. The main use case (may the only one in OCA) is that this propagation is made from sale_order but nothing prevent to do it from a procurement (that does not come from a sale order) or from stock move, etc. I don't think we want to restrict this propagation logic in a module that depends on sale_order_lot_selection.

I mean, from a code point of view, the current module has no link with sale_order_lot_selection whatsoever. So I would prefer to keep the big part of the code in the base module and only what really depends on sale_order_lot_selection on the glue module.

Let's say I manufacture products, and depending on the lot I am producing, I want to buy a specific lot for my component. I want to be able to use the base module to create purchase orders with the restrict lot, and maybe I am not using sale order / sale_order_lot_selection.

I would rather have purchase_lot as it is (and eventually rename it if we find a better name), adding the feature of propatation And an auto-install glue module with only what is missing to make it work well with sale_order_lot_selection.

I mean, the module is small enough and works well enough for different cases without the need of the sale_order_lot_selection dependecy. it would be a shame to move most of it in a module that depends on sale_order_lot_selection IMO. (even now, the module works well with sale_order_lot_selection, without the need of the dependency).

That said, we should not change the name in this v16 version, if any change of name should occur, I guess it would be best to keep it for v18

[Tisho99]

Hello @metaminux

I agree with you it would be more useful to have a similar module to sale_order_lot_selection. But as @florian-dacosta says, I think it is better to edit the purchase_lot module to do that, removing the sale_order_lot_selection dependency, improving the lot propagation, and maybe renaming it in future versions; but keeping the compatible functions with sale_order_lot_selection.

Then, this base module will cover most use cases, even propagating lots from sale orders to purchase orders, and the new glue module should be smaller.

I'll try to improve this PR, and port it to v17

Thank you both

[Tisho99]

Test added

[florian-dacosta]

@metaminux I understand we all agree that having 2 modules would be better, to avoid the sale dependency on the base module. I am not sure we fully agree on how it should be splitted including the other PR, maybe we kind of are, not sure. But I think we have to move forward soon anyway, so we can have something cleaner for version 18, not yet migrated.

My statements are the following :

• We can't rename the module merged on a stable version. I think we should do it (if we decide to rename something) for v18.
• The current module (future base module) contains presently 4 methods. Considering the procurement source could be a manufacturing order or an orderpoint instead of a sale order, , all 4 methos may be usefull without the sale module installed, so they all belong to the module that does not depends on sale the base module.

Therefore I suggest that :

1. Ce merge this PR.
2. We rework the other improvement one : https://github.com/OCA/purchase-workflow/pull/2247 :

• Decide on a new module name which will depends on sale_order_lot_selection
• Decide on what (from this other PR) should go on which module
• Eventually add some missing code (option on company to allow or not the propagation?)
• Eventually decide if and how the base module should be rename in v18 or not. Once decided/agreed, I may help on the PR to get it done /merged quickly enough.

What do you think ? If you are ok with the approach, can you review / approve this one ?

[florian-dacosta]

Following my last comment, I think we should merge it now. This change is quite small and is already merged in v17. We should merge it now to avoid divergences and because this change is quite simple/logical when checking what is presently merged into the 16.0.

@metaminux I'll merge it next week if it is ok. It should not really impact https://github.com/OCA/purchase-workflow/pull/2247 on a project. But this PR should at least be lighly refactored to add a new module (could be auto-install) that manage the code dependent to sale.

[metaminux]

Hello @florian-dacosta, Hello @Tisho99

Sorry, I prepared an answer but never took time to send it...

For the moment, installing purchase_lot alone would be totally useless because there is no other way than sale_order_lot_selection to set restrict_lot_id field in stock moves. So, there is a FUNCTIONAL dependency for now if using only OCA modules... but anyone can use custom modules too, right ? So, I totally agree with both of you : there is no need for the technical dependency on sale_order_lot_selection.

In fact, the use of sale_line_id field in #2247, is not relevant... I just saw during tests we could set lot_id on a product with type = service. In that case there is no stock moves, but we could have some purchase generated using subcontracting, so I set the lot on purchase order line using only sale_line_id

Anyway, I think we're discussing 2 different cases here :

• the one retrieving lots on purchase from underlying stock moves / procurements
• the one wanting to set lots on purchase to restrict stock moves

It seems to me the first one is what purchase_lot was originally designed for (back to v7.0 in 2014 : see https://github.com/akretion/odoo-embed-external-configuration/tree/7.0/purchase_from_configuration) The second one is what @Tisho99 would like to achieve here, making lot_id not readonly on purchase order lines.

[florian-dacosta]

For the moment, installing purchase_lot alone would be totally useless because there is no other way than sale_order_lot_selection to set restrict_lot_id field in stock moves. So, there is a FUNCTIONAL dependency for now if using only OCA modules... but anyone can use custom modules too, right ? So, I totally agree with both of you : there is no need for the technical dependency on sale_order_lot_selection.

In fact, the use of sale_line_id field in #2247, is not relevant... I just saw during tests we could set lot_id on a product with type = service. In that case there is no stock moves, but we could have some purchase generated using subcontracting, so I set the lot on purchase order line using only sale_line_id

I am not sure I get what you mean. "the use of sale_line_id field is not relevant" but "I set the lot on purchase order line using only sale_line_id". So is it or is it not usefull ? Because if it is not, we can remove it from the PR 2247 and close the debate (keep one module without dependency on sale for now)

Anyway, I think we're discussing 2 different cases here :

* the one retrieving lots on purchase from underlying stock moves / procurements

* the one wanting to set lots on purchase to restrict stock moves

It seems to me the first one is what purchase_lot was originally designed for (back to v7.0 in 2014 : see https://github.com/akretion/odoo-embed-external-configuration/tree/7.0/purchase_from_configuration) The second one is what @Tisho99 would like to achieve here, making lot_id not readonly on purchase order lines.

Yes, I believe there are these 2 uses cases indeed. And I think there are not incompatible. If the only thing is that in one case we'd prefer to have the lot readonly and the other we need it editable, maybe we can live with it beeing always editable ? The more annoying option could be to add an extra field to make the lot editable or not. (the extra boolean field could be set to True if lot_id is set automatically from a procurement.)

Also not clear for me from your message : Are you ok to merge this PR as it is ? If not, what is blocking (the readonly attribute ?)

[metaminux]

Hi @florian-dacosta,

Yes, the readonly attribute is what makes me feel uncomfortable here... Anyway, as you said earlier, the same PR has already been merged in 17.0, so let's merge this one in 16.0 too and work on the others PRs.

[florian-dacosta]

/ocabot merge minor

[OCA-git-bot]

Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 16.0-ocabot-merge-pr-2646-by-florian-dacosta-bump-minor, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at a9e1f92b5d44065c3e34d5a27db37a22bec46965. Thanks a lot for contributing to OCA. ❤️