[16.0] [MIG] Migration: sale_isolated_quotation#3679
[16.0] [MIG] Migration: sale_isolated_quotation#3679
Conversation
|
/ocabot migration sale_isolated_quotation |
|
@Tiago370 could you rebase it please? |
f528148 to
8a6d739
Compare
|
This PR has the |
BhaveshHeliconia
left a comment
There was a problem hiding this comment.
Functional review LGTM!
alexey-pelykh
left a comment
There was a problem hiding this comment.
Review: [16.0] [MIG] Migration: sale_isolated_quotation
This PR ports sale_isolated_quotation from 15.0 to 16.0. However, the Python code, XML views, hooks, and tests are byte-for-byte identical to the 15.0 version (only the version string in __manifest__.py changed from 15.0.1.0.0 to 16.0.1.0.0). Several Odoo 16.0 breaking changes have not been addressed.
Key Findings
| # | File | Finding | Severity |
|---|---|---|---|
| 1 | hooks.py:38 |
api.Environment.manage() removed in Odoo 16.0. The uninstall_hook uses with api.Environment.manage(): which was removed in Odoo 15.0+. This context manager is a no-op since 15.0 and fully removed in 16.0 -- the with block and manual api.Environment(cr, SUPERUSER_ID, {}) construction should be replaced. |
Critical |
| 2 | views/sale_views.xml |
attrs dict syntax deprecated in Odoo 16.0. Lines using attrs="{'invisible': [('order_sequence','=',True)]}" and similar should be migrated to the new invisible="order_sequence == True" domain-expression syntax introduced in Odoo 16.0. While attrs may still partially work in some 16.0 builds, OCA migration guidelines require using the new syntax. |
Moderate |
| 3 | views/sale_views.xml |
states attribute deprecated in Odoo 16.0. Button attributes like states="draft,sent" should be converted to invisible expressions (e.g., invisible="state not in ('draft', 'sent')"). |
Moderate |
| 4 | models/sale.py:59 |
action_done() may not exist in Odoo 16.0 sale.order. In Odoo 16.0 the sale order workflow changed -- action_done() was renamed/removed. The method call self.action_done() in action_convert_to_order needs verification against the 16.0 sale module API and likely needs to be replaced with _action_done() or the appropriate 16.0 equivalent. |
Critical |
| 5 | models/sale.py:63 |
open_duplicated_sale_order uses @api.model but accesses self.order_id. This method is decorated with @api.model (no recordset) yet references self.order_id on the last line. Since it is called from action_convert_to_order (a recordset method), this works by accident but the decorator is semantically incorrect. It should be a regular method (remove @api.model). |
Minor |
| 6 | models/sale.py:71 |
nodestroy key is not a valid key for ir.actions.act_window dict returns in Odoo 16.0 (it was removed long ago). It is silently ignored but should be cleaned up during migration. |
Minor |
| 7 | i18n/*.po |
Translation files reference Odoo 10.0 Project-Id-Version. These .po files were carried forward unchanged from 10.0/14.0 era. While functional, the .pot file references Odoo Server 15.0 rather than 16.0. This should be regenerated with 16.0 references. |
Minor |
Summary
This is essentially a direct copy of the 15.0 module without addressing Odoo 16.0 API changes. The two critical items (api.Environment.manage() removal and action_done() removal/rename) will likely cause runtime errors on Odoo 16.0. The attrs/states deprecation items, while potentially still functional, do not meet OCA migration standards for 16.0.
Please address at minimum the critical items and the deprecated view syntax before merging.
sale_isolated_quotation/hooks.py
Outdated
| dom = ast.literal_eval(action.domain or "{}") | ||
| dom = [x for x in dom if x[0] != "order_sequence"] | ||
| if action_id == "sale.action_orders": | ||
| dom.append(("state", "not in", ("draft", "sent", "cancel"))) |
There was a problem hiding this comment.
Critical: api.Environment.manage() context manager was removed in Odoo 16.0. This entire with api.Environment.manage(): block should be refactored. In 16.0, you can construct the Environment directly without the context manager:
def uninstall_hook(cr, registry):
env = api.Environment(cr, SUPERUSER_ID, {})
for action_id in ACTIONS:
...Alternatively, for 16.0 hooks, consider using the env parameter signature if the hook API supports it.
| order = self.copy(self._prepare_order_from_quotation()) | ||
| self.order_id = order.id # Reference from this quotation to order | ||
| if self.state == "draft": | ||
| self.action_done() |
There was a problem hiding this comment.
Critical: action_done() was removed/renamed in Odoo 16.0's sale.order. In 16.0, the equivalent is _action_done() (private method). Please verify against the Odoo 16.0 sale module source and update accordingly.
Also, note that action_done in older versions locked the SO; in 16.0 the lock workflow changed. This needs careful testing.
There was a problem hiding this comment.
action_done() can still be found in v16:
https://github.com/odoo/odoo/blob/16.0/addons/sale/models/sale_order.py#L885
| <xpath expr="/form/header" position="after"> | ||
| <header | ||
| name="quotation" | ||
| attrs="{'invisible': [('order_sequence','=',True)]}" |
There was a problem hiding this comment.
Moderate: The attrs dict syntax (attrs="{'invisible': [...]}") is deprecated in Odoo 16.0. Per OCA migration guidelines, this should be converted to the new inline syntax:
<header name="quotation" invisible="order_sequence">This applies to all attrs usages in this file (lines for quote_id, order_id, and the quotation header).
| <button | ||
| name="action_convert_to_order" | ||
| states="draft,sent" | ||
| class="oe_highlight" |
There was a problem hiding this comment.
Moderate: The states attribute on buttons is deprecated in Odoo 16.0. Replace with invisible expressions:
<button
name="action_convert_to_order"
invisible="state not in ('draft', 'sent')"
class="oe_highlight"
type="object"
string="Convert to Order"
/>Same applies to the Cancel button below.
| "view_id": False, | ||
| "res_model": "sale.order", | ||
| "context": {"default_order_sequence": True, "order_sequence": True}, | ||
| "type": "ir.actions.act_window", |
There was a problem hiding this comment.
Minor: @api.model is semantically incorrect here. This method accesses self.order_id (line 72), which only makes sense on a recordset. The @api.model decorator should be removed since the method is always called from a record context (self.open_duplicated_sale_order() in action_convert_to_order).
Also, nodestroy (line 69) is not a recognized key in 16.0 action dicts and should be removed.
35d3410 to
32a80ad
Compare
* Add README.rst
Currently translated at 90.9% (10 of 11 strings) Translation: sale-workflow-14.0/sale-workflow-14.0-sale_isolated_quotation Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-14-0/sale-workflow-14-0-sale_isolated_quotation/de/
…ble: they should not be installed nor tested together
This is to resolve conflict with sale_quotation_number.
Currently translated at 27.2% (3 of 11 strings) Translation: sale-workflow-14.0/sale-workflow-14.0-sale_isolated_quotation Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-14-0/sale-workflow-14-0-sale_isolated_quotation/it/
Currently translated at 27.2% (3 of 11 strings) Translation: sale-workflow-14.0/sale-workflow-14.0-sale_isolated_quotation Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-14-0/sale-workflow-14-0-sale_isolated_quotation/it/
9cac90f to
8344a6c
Compare
[WIP] sale_isolated_quotation: Migration to 16.0 - fixes
ced0867 to
011d278
Compare
|
Depends on #4204 |
alexey-pelykh
left a comment
There was a problem hiding this comment.
Good progress — 4 of 7 items from the previous review are now addressed. The api.Environment.manage() removal, attrs/states migration to inline expressions, and the @api.model decorator fix are all done correctly.
Still outstanding (critical):
models/sale.py:self.action_done()— this method was removed/renamed in Odoo 16.0. The equivalent is_action_done(). This will raiseAttributeErrorat runtime when converting a draft quotation to an order.
Still outstanding (minor):
"nodestroy": Trueis not a recognized key in 16.0 action dicts — silently ignored but should be cleaned up..pofiles still referenceProject-Id-Version: Odoo Server 10.0— should be regenerated for 16.0.
Co-Reviewed-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
…e .pot for 16.0 - Replace self.action_done() with self._action_done() (renamed in Odoo 16.0) - Remove "nodestroy": True from action dict (not recognized in 16.0) - Update .po file headers from "Odoo Server 10.0" to "Odoo Server 16.0" - Regenerate .pot via click-odoo-makepot
fb97ac2 to
00fc29e
Compare
No description provided.