[18.0][ADD] sale_order_add_byproduct

[bosd]

This module is designed for contract manufacturing scenarios where the
client provides raw materials or a base product for processing. Unlike
standard manufacturing where the output quantity of finished goods is
precisely known, this use case involves a process where the exact yield
of by-products is uncertain until after the manufacturing operation
is complete.

Scenario

A contract manufacturer processes materials supplied by a client. During
this process, in addition to any primary output, a variable quantity of
by-products is inevitably generated. The client is obligated to purchase
these by-products from the contract manufacturer.

[rousseldenis]

This has to be put at the beginning of the loop to avoid too much conditions evaluations.

[rousseldenis]

Moreover, I would have put this in a separate function like _get_byproduct_moves_to_add() or somethng like that.

[bosd]

Thanks for the review.
CI is still red, but that seems to come from a different module.

[Dranyel-Bosd]

LGTM

[alexey-pelykh]

Review: REQUEST_CHANGES

Thank you for the contribution -- the business case is well-documented and the test coverage is solid. However, there are several correctness issues and OCA convention gaps that should be addressed before merge.

Critical Findings

#	File	Severity	Finding
1	sale_order_line.py	Bug	_action_launch_stock_rule override breaks mixed recordsets -- returns True for the entire batch when ANY line is a byproduct, silently skipping procurement for legitimate lines
2	sale_order_line.py	Bug	create override may double-trigger procurement for non-byproduct lines added to confirmed SOs (Odoo already handles this)
3	mrp_production.py	Dead code	Duplicate SO search at L42-45 (origin.startswith("S") branch does the exact same query as L38-40)
4	mrp_production.py	Security	Note template uses Python str.format() with user-controlled template string from ir.config_parameter -- could expose object attributes via {0.__class__} style injection
5	sale_order_line.py	OCA	Missing license header
6	res_config_settings.py	Convention	Reviewer rousseldenis already flagged: prefer config_parameter= attribute or a proper company-level field with related= instead of manual compute/inverse

Suggestions

#	File	Finding
7	__manifest__.py	Remove commented-out "demo" key
8	Tests	Excessive invalidate_all() + re-browse suggests misunderstanding of ORM cache -- consider simplifying
9	mrp_production.py	bom_param argument in helper is accepted but never used -- remove or implement

[alexey-pelykh]

Bug (critical): This logic is inverted for mixed recordsets. If self contains 5 lines and 1 is a byproduct, this returns True immediately and skips procurement for the other 4 lines.

The correct pattern is to exclude byproduct lines and call super() on the remaining lines:

def _action_launch_stock_rule(self, previous_product_uom_qty=False):
    lines_to_procure = self.filtered(lambda line: not line.is_mrp_byproduct_line)
    if not lines_to_procure:
        return True
    return super(SaleOrderLine, lines_to_procure)._action_launch_stock_rule(
        previous_product_uom_qty=previous_product_uom_qty
    )

[alexey-pelykh]

Bug: Overriding create to manually call _action_launch_stock_rule() for non-byproduct lines is risky. Odoo's standard sale.order.line already triggers procurement on line creation for confirmed SOs via its own mechanism. This override may cause double procurement (two MOs created for one SO line).

I believe the _action_launch_stock_rule override above (once fixed) is sufficient to prevent byproduct lines from triggering procurement. This entire create override should likely be removed.

If it IS needed, please add a test that demonstrates the scenario it solves -- currently no test covers the create-triggered procurement path specifically.

[alexey-pelykh]

Dead code: This if branch (self.origin.startswith("S")) executes the exact same search as lines 38-40 above:

sale_order = self.env["sale.order"].search(
    [("name", "=", self.origin)], limit=1
)

This duplicate search can never find anything the first one missed. Remove this branch entirely.

[alexey-pelykh]

Security concern: Using str.format() with a user-controlled template string is a known Python injection vector. A malicious admin could craft a template like {product_name.__class__.__mro__} to leak internal Python object information.

Consider using safe_substitute from string.Template, or restrict the format to known placeholders via a simple .replace() approach:

note = config_param.replace("{product_name}", product_to_add.name).replace("{mo_name}", self.name)

[alexey-pelykh]

OCA convention: Missing license header. All Python files in OCA modules must have the copyright and license header:

# Copyright 2025 OBS Solutions BV
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

[alexey-pelykh]

Cleanup: Remove the commented-out "demo" line. OCA guidelines discourage committed commented-out code.

[alexey-pelykh]

As rousseldenis already noted: since you now have a res.company field, the settings field should use related="company_id.byproduct_note_template" rather than manual _compute / _inverse methods. This is the standard OCA pattern for company-level settings and removes ~15 lines of code.

[rousseldenis]

@bosd Could you improve your commit messages and squash some of them if needed. Thanks