Skip to content

[FIX] purchase_request: bind name before use in onchange_product_id - #3136

Open
mav-adhoc wants to merge 1 commit into
OCA:18.0from
adhoc-dev:18.0-fix-purchase_request-onchange-unbound-name
Open

[FIX] purchase_request: bind name before use in onchange_product_id#3136
mav-adhoc wants to merge 1 commit into
OCA:18.0from
adhoc-dev:18.0-fix-purchase_request-onchange-unbound-name

Conversation

@mav-adhoc

Copy link
Copy Markdown
Contributor

Bug

onchange_product_id on the make-purchase-order wizard item raised
UnboundLocalError: cannot access local variable 'name' when
keep_description was set.

name was only assigned inside if not self.keep_description:, so when
keep_description is True it stayed unbound and was then read:

  • on the supplier-info branch, f"[...] {p_name if p_name else name}",
    when the matched product.supplierinfo had no product_name;
  • and by the final if name: guard when there was no supplier info and
    the product had no internal reference.

Fix

Always bind name: keep the description already provided on the wizard
line when keep_description is set, otherwise fall back to the product
name. Behaviour for keep_description = False is unchanged.

Test

Added test_onchange_product_id_keep_description: a product without an
internal reference plus a product.supplierinfo with an empty
product_name, keep_description=True, then calls the onchange. It
raises UnboundLocalError on the previous code and passes with the fix.

@mav-adhoc
mav-adhoc force-pushed the 18.0-fix-purchase_request-onchange-unbound-name branch 3 times, most recently from 4a6ff3d to 96b5060 Compare July 24, 2026 16:48
When keep_description was set, the local variable `name` was only
assigned inside the `if not self.keep_description` branch, leaving it
unbound. It was then read on the supplier info branch (and by the
final `if name:` guard), raising UnboundLocalError as soon as the
matched product.supplierinfo had no product_name (or there was no
supplier info and the product had no internal reference).

Always bind `name`: when keep_description is set keep the description
already provided on the wizard line, otherwise fall back to the
product name.
@mav-adhoc
mav-adhoc force-pushed the 18.0-fix-purchase_request-onchange-unbound-name branch from 96b5060 to ff730eb Compare July 24, 2026 16:52
@ACBRI

ACBRI commented Aug 4, 2026

Copy link
Copy Markdown

Functional review on 18.0 — tested on a clean database (Odoo 18.0-20260528, purchase_request installed on its own).

First, a small thing: I could not find test_onchange_product_id_keep_description anywhere. It is not in this PR (the diff is one file, +3/-2), it is not on 18.0, and it is not on any other branch. I suspect the test file was left out of the commit.

I wrote one to check the fix anyway, and I can confirm both the bug and the fix.

Before the patch, two paths raise:

Scenario (keep_description = True) Line Result
supplierinfo found, product_name empty 400, p_name if p_name else name UnboundLocalError
no supplierinfo, product without internal reference 407, if name: UnboundLocalError

The remaining keep_description = True paths survive by accident: line 403 both reads and assigns name, and the conditional expressions short-circuit whenever p_name or code is set.

That also explains why CI stays green today. test_purchase_request_to_rfq does set keep_description = True and does call onchange_product_id(), but on two different wizard items, so the failing combination is never exercised.

After the patch, the three tests below pass and the full purchase_request suite is green (42 tests, 0 failures, 0 errors).

Here is the test I used, in case it is useful:

# Copyright 2026
# License LGPL-3.0 or later (https://www.gnu.org/licenses/lgpl-3.0)

from odoo import SUPERUSER_ID
from odoo.tests import common


class TestOnchangeProductIdKeepDescription(common.TransactionCase):
    @classmethod
    def setUpClass(cls):
        super().setUpClass()
        cls.vendor = cls.env["res.partner"].create({"name": "Vendor OCA Review"})
        cls.product_supinfo_no_name = cls.env["product.product"].create(
            {
                "name": "Product With Nameless Supplierinfo",
                "type": "consu",
                "default_code": "TEST-SUP",
            }
        )
        cls.env["product.supplierinfo"].create(
            {
                "partner_id": cls.vendor.id,
                "product_tmpl_id": cls.product_supinfo_no_name.product_tmpl_id.id,
                "product_name": False,
                "product_code": False,
            }
        )
        cls.product_no_code = cls.env["product.product"].create(
            {
                "name": "Product Without Internal Reference",
                "type": "consu",
                "default_code": False,
            }
        )

    def _build_item(self, product, keep_description=True, name="Existing description"):
        request = self.env["purchase.request"].create(
            {
                "picking_type_id": self.env.ref("stock.picking_type_in").id,
                "requested_by": SUPERUSER_ID,
                "line_ids": [
                    (
                        0,
                        0,
                        {
                            "product_id": product.id,
                            "product_uom_id": product.uom_id.id,
                            "product_qty": 1.0,
                        },
                    )
                ],
            }
        )
        request.button_approved()
        wiz = (
            self.env["purchase.request.line.make.purchase.order"]
            .with_context(
                active_model="purchase.request.line",
                active_ids=request.line_ids.ids,
            )
            .create({"supplier_id": self.vendor.id})
        )
        return self.env["purchase.request.line.make.purchase.order.item"].new(
            {
                "wiz_id": wiz.id,
                "line_id": request.line_ids[0].id,
                "name": name,
                "product_qty": 1.0,
                "product_uom_id": product.uom_id.id,
                "keep_description": keep_description,
            }
        )

    def test_01_supplierinfo_without_product_name(self):
        item = self._build_item(self.product_supinfo_no_name)
        item.onchange_product_id()
        self.assertEqual(item.name, "[TEST-SUP] Existing description")

    def test_02_no_supplierinfo_and_no_internal_reference(self):
        item = self._build_item(self.product_no_code)
        item.onchange_product_id()
        self.assertEqual(item.name, "Existing description")

    def test_03_keep_description_false_is_unchanged(self):
        item = self._build_item(self.product_no_code, keep_description=False)
        item.onchange_product_id()
        self.assertEqual(item.name, "Product Without Internal Reference")

The code change itself looks right to me: for keep_description = False the behaviour is byte-for-byte the same, and for keep_description = True it now falls back to the description already present on the wizard line, which is what the option promises.

I am happy to approve as soon as a test lands, either the one above or your own version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants