Skip to content

[17.0] [MIG] sale_auto_remove_zero_quantity_lines: Migration to 17.0#3883

Open
bizzappdev wants to merge 14 commits intoOCA:17.0from
bizzappdev:17.0-mig-sale_auto_remove_zero_quantity_lines-BAD
Open

[17.0] [MIG] sale_auto_remove_zero_quantity_lines: Migration to 17.0#3883
bizzappdev wants to merge 14 commits intoOCA:17.0from
bizzappdev:17.0-mig-sale_auto_remove_zero_quantity_lines-BAD

Conversation

@bizzappdev
Copy link
Copy Markdown
Contributor

No description provided.

santostelmo and others added 12 commits September 1, 2025 19:02
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_auto_remove_zero_quantity_lines
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_auto_remove_zero_quantity_lines/
Currently translated at 100.0% (7 of 7 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_auto_remove_zero_quantity_lines
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_auto_remove_zero_quantity_lines/de/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_auto_remove_zero_quantity_lines
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_auto_remove_zero_quantity_lines/
Currently translated at 100.0% (7 of 7 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_auto_remove_zero_quantity_lines
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_auto_remove_zero_quantity_lines/it/
Currently translated at 100.0% (7 of 7 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_auto_remove_zero_quantity_lines
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_auto_remove_zero_quantity_lines/hr/
@bizzappdev bizzappdev marked this pull request as ready for review September 3, 2025 11:41
@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot migration sale_auto_remove_zero_quantity_lines

@bizzappdev bizzappdev force-pushed the 17.0-mig-sale_auto_remove_zero_quantity_lines-BAD branch from aa6bbac to e46c7bc Compare September 15, 2025 13:44
Copy link
Copy Markdown
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Reviewed the migration from 16.0 to 17.0. Overall this is a clean migration with one good improvement (batch unlink outside the loop). CI is green and the module structure follows OCA conventions correctly.

A couple of minor observations:

  1. i18n .pot file version: The Project-Id-Version in .pot and .po files still references Odoo Server 16.0. These should ideally be regenerated for 17.0, but Weblate typically handles this automatically upon merge, so this is non-blocking.

  2. Potential AttributeError on line.name.strip(): In the lambda filter, if line.name is False (which is the default for fields.Text), calling .strip() on it would raise AttributeError. This could be guarded with not (line.name or "").strip(). That said, this code is carried over from 16.0 and works in practice because line_note lines tend to always have a string value, so this is also non-blocking.

Nothing blocking here -- the migration looks correct.

if order._should_auto_remove_zero_quantity_lines():
zero_or_empty_lines = order.order_line.filtered(
lambda line: (line.product_id and line.product_uom_qty == 0)
or (line.display_type == "line_note" and not line.name.strip())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor/defensive: If line.name is ever False (the default for fields.Text), calling .strip() on it would raise AttributeError. A safer pattern would be:

or (line.display_type == "line_note" and not (line.name or "").strip())

This is inherited from 16.0 and works in practice (note lines typically have a string), so non-blocking.

Copy link
Copy Markdown
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

Thank you for the migration. LGTM

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.