[16.0][MIG] sale_blanket_order: Migration to 16.0

[nguyenminhchien]

Supersedes #2407

Depend on:

• [16.0][MIG] web_action_conditionable: Migration to 16.0 web#2500

[sebalix]

LG

[sebalix]

I mixed the PRs, I think this one has a really big migration commit.

[sebalix]

Are we forced to split these files? It is hard to know what has been changed outside of the renaming, and it doesn't ease the port of commits I guess.
If there is no good reason to do so, I would restore them

[sebalix]

Also, commit [IMP] sale_blanket_order should be part of the migration commit no?

[nguyenminhchien]

Are we forced to split these files?

we should split these files because each python file should contain one class.

Also, commit [IMP] sale_blanket_order should be part of the migration commit no?

The migration commit was added by Jasper Jumelet, i have to add this commit separately to keep track

[sebalix]

we should split these files because each python file should contain one class.

Conventions allow to keep linked data models (like SO and SO line) in the same file, it's OK. But I understand the change, often I prefer to split them myself to avoid huge files, but I think this change is maybe not justified here. Or if you really want to split them (or keep the change of the original author), it should be done in its own commit as it's impossible (or difficult) to keep track of what has been changed outside of the renaming here without comparing the diff content side by side manually.
So I did it locally, and here is the result:

blanket_order.py:

--- blanket_orders.py.prev	2023-05-24 09:19:30.326850717 +0200
+++ blanket_order.py.next	2023-05-24 09:19:40.914984663 +0200
@@ -1,28 +1,18 @@
 # Copyright 2018 ACSONE SA/NV
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
-from odoo import SUPERUSER_ID, _, api, fields, models
+from odoo import _, api, fields, models
 from odoo.exceptions import UserError
 from odoo.tools import float_is_zero
-from odoo.tools.misc import format_date
+
+from odoo.addons.sale.models.sale_order import READONLY_FIELD_STATES
 
 
 class BlanketOrder(models.Model):
     _name = "sale.blanket.order"
     _inherit = ["mail.thread", "mail.activity.mixin"]
     _description = "Blanket Order"
-
-    @api.model
-    def _get_default_team(self):
-        return self.env["crm.team"]._get_default_team_id()
-
-    @api.model
-    def _default_currency(self):
-        return self.env.company.currency_id
-
-    @api.model
-    def _default_company(self):
-        return self.env.company
+    _check_company_auto = True
 
     @api.model
     def _default_note(self):
@@ -53,8 +43,7 @@
     partner_id = fields.Many2one(
         "res.partner",
         string="Partner",
-        readonly=True,
-        states={"draft": [("readonly", False)]},
+        states=READONLY_FIELD_STATES,
     )
     line_ids = fields.One2many(
         "sale.blanket.order.line", "order_id", string="Order lines", copy=True
@@ -73,24 +62,21 @@
         "product.pricelist",
         string="Pricelist",
         required=True,
-        readonly=True,
-        states={"draft": [("readonly", False)]},
+        states=READONLY_FIELD_STATES,
     )
     currency_id = fields.Many2one("res.currency", related="pricelist_id.currency_id")
     analytic_account_id = fields.Many2one(
         comodel_name="account.analytic.account",
         string="Analytic Account",
-        readonly=True,
         copy=False,
         check_company=True,
-        states={"draft": [("readonly", False)]},
+        states=READONLY_FIELD_STATES,
         domain="['|', ('company_id', '=', False), ('company_id', '=', company_id)]",
     )
     payment_term_id = fields.Many2one(
         "account.payment.term",
         string="Payment Terms",
-        readonly=True,
-        states={"draft": [("readonly", False)]},
+        states=READONLY_FIELD_STATES,
     )
     confirmed = fields.Boolean(copy=False)
     state = fields.Selection(
@@ -104,36 +90,32 @@
         store=True,
         copy=False,
     )
-    validity_date = fields.Date(readonly=True, states={"draft": [("readonly", False)]})
+    validity_date = fields.Date(
+        states=READONLY_FIELD_STATES,
+    )
     client_order_ref = fields.Char(
         string="Customer Reference",
         copy=False,
-        readonly=True,
-        states={"draft": [("readonly", False)]},
-    )
-    note = fields.Text(
-        readonly=True, default=_default_note, states={"draft": [("readonly", False)]}
+        states=READONLY_FIELD_STATES,
     )
+    note = fields.Text(default=_default_note, states=READONLY_FIELD_STATES)
     user_id = fields.Many2one(
         "res.users",
         string="Salesperson",
-        readonly=True,
-        states={"draft": [("readonly", False)]},
+        states=READONLY_FIELD_STATES,
     )
     team_id = fields.Many2one(
         "crm.team",
         string="Sales Team",
         change_default=True,
-        default=_get_default_team,
-        readonly=True,
-        states={"draft": [("readonly", False)]},
+        default=lambda self: self.env["crm.team"]._get_default_team_id(),
+        states=READONLY_FIELD_STATES,
     )
     company_id = fields.Many2one(
-        "res.company",
-        string="Company",
-        default=_default_company,
-        readonly=True,
-        states={"draft": [("readonly", False)]},
+        comodel_name="res.company",
+        required=True,
+        index=True,
+        default=lambda self: self.env.company,
     )
     sale_count = fields.Integer(compute="_compute_sale_count")
 
@@ -259,7 +241,7 @@
             ),
             "fiscal_position_id": self.env["account.fiscal.position"]
             .with_context(company_id=self.company_id.id)
-            .get_fiscal_position(self.partner_id.id),
+            ._get_fiscal_position(self.partner_id),
         }
 
         if self.partner_id.user_id:
@@ -341,7 +323,7 @@
 
     def action_view_sale_blanket_order_line(self):
         action = self.env.ref(
-            "sale_blanket_order" ".act_open_sale_blanket_order_lines_view_tree"
+            "sale_blanket_order.act_open_sale_blanket_order_lines_view_tree"
         ).read()[0]
         lines = self.mapped("line_ids")
         if len(lines) > 0:
@@ -355,7 +337,7 @@
             [("state", "=", "open"), ("validity_date", "<=", today)]
         )
         expired_orders.modified(["validity_date"])
-        expired_orders.recompute()
+        expired_orders.flush_recordset()
 
     @api.model
     def _search_original_uom_qty(self, operator, value):

blanket_order_line.py:

--- blanket_order_line.py.prev	2023-05-24 09:23:04.313554668 +0200
+++ blanket_order_line.py.next	2023-05-24 09:23:19.273743466 +0200
@@ -1,7 +1,16 @@
+# Copyright 2018 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+from odoo import SUPERUSER_ID, _, api, fields, models
+from odoo.exceptions import UserError
+from odoo.tools import float_is_zero
+from odoo.tools.misc import format_date
+
+
 class BlanketOrderLine(models.Model):
     _name = "sale.blanket.order.line"
     _description = "Blanket Order Line"
-    _inherit = ["mail.thread", "mail.activity.mixin"]
+    _inherit = ["mail.thread", "mail.activity.mixin", "analytic.mixin"]
 
     @api.depends(
         "original_uom_qty",
@@ -48,9 +57,7 @@
     )
     date_schedule = fields.Date(string="Scheduled Date")
     original_uom_qty = fields.Float(
-        string="Original quantity",
-        default=1,
-        digits="Product Unit of Measure",
+        string="Original quantity", default=1, digits="Product Unit of Measure"
     )
     ordered_uom_qty = fields.Float(
         string="Ordered quantity", compute="_compute_quantities", store=True
@@ -77,7 +84,7 @@
         copy=False,
     )
     company_id = fields.Many2one(
-        "res.company", related="order_id.company_id", store=True
+        related="order_id.company_id", store=True, index=True, precompute=True
     )
     currency_id = fields.Many2one("res.currency", related="order_id.currency_id")
     partner_id = fields.Many2one(related="order_id.partner_id", string="Customer")
@@ -92,11 +99,6 @@
     )
     price_total = fields.Monetary(compute="_compute_amount", string="Total", store=True)
     price_tax = fields.Float(compute="_compute_amount", string="Tax", store=True)
-    analytic_tag_ids = fields.Many2many(
-        comodel_name="account.analytic.tag",
-        string="Analytic Tags",
-        domain="['|', ('company_id', '=', False), ('company_id', '=', company_id)]",
-    )
     display_type = fields.Selection(
         [("line_section", "Section"), ("line_note", "Note")],
         default=False,
@@ -188,7 +190,7 @@
         pricelist = self.order_id.pricelist_id
         partner = self.order_id.partner_id
         if self.order_id.pricelist_id.discount_policy == "with_discount":
-            return product.with_context(pricelist=pricelist.id).price
+            return product.with_context(pricelist=pricelist.id).lst_price
         final_price, rule_id = pricelist.get_product_price_rule(
             self.product_id, self.original_uom_qty or 1.0, partner
         )

We are unable to see these changes only by checking the diff of this PR, so it becomes mandatory to put the renaming in its own commit.

[nguyenminhchien]

in fact, i don't split them but @jasperjumelet.
The commit [IMP] sale_blanket_order contains all the changes i did.

[sebalix]

Yes I know it's not in your commit, but as soon as someone is opening a PR for a migration, he is taking the responsibility of it, and as such this PR can't be merged IMO :)
For a renaming, you can take the right to edit the commit of @jasperjumelet as soon as you keep the other changes he did.

Or start from the beginning with your own commit(s) if you think it's easier.

[nguyenminhchien]

ok, wip

[nguyenminhchien]

@sebalix i have updated the migration commit, please review

[sebalix]

Requesting changes based on my comment above to keep a readable commit history.

[sebalix]

LG, thank you!

[rousseldenis]

/ocabot migration sale_blanket_order

[OCA-git-bot]

The migration issue (#2215) has been updated to reference the current pull request.
however, a previous pull request was referenced : #2407.
Perhaps you should check that there is no duplicate work.
CC : @jasperjumelet

[mostafabarmshory]

any new update?

[nguyenminhchien]

The migration issue (#2215) has been updated to reference the current pull request. however, a previous pull request was referenced : #2407. Perhaps you should check that there is no duplicate work. CC : @jasperjumelet

Because there is no action from @jasperjumelet on the PR #2407 for a long time, so i created this one to replace it.

[nilshamerlinck]

Hello @mostafabarmshory the module works fine, you can try it on runboat :) if you can, your technical review is also welcome, especially on the last commit that is the main purpose of this PR (migration commit)

[mostafabarmshory]

Hello @mostafabarmshory the module works fine, you can try it on runboat :) if you can, your technical review is also welcome, especially on the last commit that is the main purpose of this PR (migration commit)

I check this one on my own local version. It is ok.
I want to know when it will merge with the main branch (16.0)

[joepsanders]

@nguyenminhchien, to prevent permission error:

Suggested change

	action = self.env.ref("sale.action_orders").read()[0]
	action = self.env["ir.actions.act_window"]._for_xml_id(
	"sale.action_orders")

[joepsanders]

@nguyenminhchien, to prevent permission error

Suggested change

	action = self.env.ref(
	action = self.env["ir.actions.act_window"]._for_xml_id(
	"sale_blanket_order.act_open_sale_blanket_order_lines_view_tree")

[nguyenminhchien]

thanks, i have updated them

[dreispt]

Dependencies are there, trying a merge.

/ocabot merge nobump

[OCA-git-bot]

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-2509-by-dreispt-bump-nobump, awaiting test results.

[OCA-git-bot]

@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-2509-by-dreispt-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

[beagle-cloud]

Hi, I may have a remark. I'm new to (Odoo) development, so please bear with me if I'm wrong: I see get_product_price_rule( ) is called twice in blanket_orders.py, could it be that this should be _get_product_price_rule? I only see the method with the prefix _ in product_pricelist.py . Thanks to have a look at this.

[nguyenminhchien]

@beagle-cloud, thank you. i just updated.

[nguyenminhchien]

@OKT96 good points, pull request updated.

[matteoopenf]

@OKT96 good points, pull request updated.

thank you

[OKT96]

LGTM

[dreispt]

/ocabot merge nobump

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-2509-by-dreispt-bump-nobump, awaiting test results.

[OCA-git-bot]

It looks like something changed on 16.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 16.0-ocabot-merge-pr-2509-by-dreispt-bump-nobump, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at 7086307. Thanks a lot for contributing to OCA. ❤️