Skip to content

[18.0][MIG] sale_attached_product#3925

Open
bjouini-acsone wants to merge 15 commits intoOCA:18.0from
acsone:18.0-mig_sale_attached_product
Open

[18.0][MIG] sale_attached_product#3925
bjouini-acsone wants to merge 15 commits intoOCA:18.0from
acsone:18.0-mig_sale_attached_product

Conversation

@bjouini-acsone
Copy link
Copy Markdown

No description provided.

@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot migration sale_attached_product

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Oct 9, 2025
@rousseldenis rousseldenis changed the title 18.0 mig sale attached product [18.0][MIG] sale attached product Oct 9, 2025
@rousseldenis rousseldenis changed the title [18.0][MIG] sale attached product [18.0][MIG] sale_attached_product Oct 9, 2025
@anthonissen-a
Copy link
Copy Markdown
Contributor

See Issue : #4060 for my last commit.

Copy link
Copy Markdown

@alexey-pelykh alexey-pelykh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: [18.0][MIG] sale_attached_product

Thanks for the migration. The CI is green (tests pass with both Odoo and OCB, pre-commit passes, runboat builds). One existing approval from @rousseldenis. The code is a straight port from 17.0 with only the version bump in __manifest__.py.

I found a few issues worth addressing:

1. README and generated docs still reference 17.0 (moderate)

The badge URLs in README.rst and static/description/index.html still point to 17.0:

:target: https://github.com/OCA/sale-workflow/tree/17.0/sale_attached_product
:target: https://translation.odoo-community.org/projects/sale-workflow-17-0/...
:target: https://runboat.odoo-community.org/builds?repo=OCA/sale-workflow&target_branch=17.0

These should reference 18.0. Typically oca-gen-addon-readme regenerates them; please re-run the tool or update the branch references.

2. Onchange-based approach may need review for Odoo 18 (informational)

The _get_attached_line_values_product method uses _onchange_methods to trigger field computations on new lines:

def _execute_onchanges(records, field_name):
    for onchange in records._onchange_methods.get(field_name, []):
        for record in records:
            onchange(record)

Odoo has been progressively moving from onchange to compute methods since v14+. In 18.0, _onchange_methods still exists but some fields may have migrated to compute-only. Since the tests pass, this works today, but it is worth noting as a fragility point for future maintainability. A more resilient approach would be to rely on _compute_* methods or Form simulation.

3. String concatenation bug in help text (minor, pre-existing)

In product_template.py:

help="Similar to optional products, although they're added automatically to the"
"sale order and optionally removed when the main product goes away.",

The implicit string concatenation produces "...to thesale order..." (missing space). This is carried over from 17.0 and also present in the .pot and .po files. Low priority but worth fixing since it is user-visible.

4. codecov/project check is failing

The codecov/project check shows failure, while codecov/patch passes. This likely means overall project coverage dipped slightly. This is typically not blocking for OCA migrations but worth noting.

5. No migration script needed (confirmed)

Since the module existed in 17.0 with identical schema (same fields, same relations), no pre-migrate or post-migrate scripts are needed. The version bump alone is correct for this case.


Summary: This is a clean version-bump migration. The code is identical to the 17.0 version and the tests pass. The main actionable item is regenerating the README to fix the 17.0 branch references. The string concatenation bug and onchange pattern are pre-existing concerns, not regressions.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.