Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MIG] purchase_analytic: Migration to 17.0 #731

Closed

Conversation

CLaurelB
Copy link
Contributor

@CLaurelB CLaurelB commented Jan 9, 2025

Supersed #707

Laetitia Gangloff and others added 30 commits January 9, 2025 16:18
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: account-analytic-12.0/account-analytic-12.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-12-0/account-analytic-12-0-purchase_analytic/
Some of the "tricks" done in this module are no longer needed
and can be easily implemented with newest framework features:

* No need for an auxiliar `project_id2` field. User can set
  an analytic account with no lines and it is respected.
* Simplify onchange. Now update analytic line on the go (no
  need to save) which is a better UX because avoid unexpected
  changes on save.

Also re-label the field `project_id` to "Analytic Account" to
align with the typical label in newer versions of Odoo.
Tha label "Contract / Analytic" was last used in v8 (
https://github.com/odoo/odoo/blob/8.0/addons/sale/sale.py#L217).
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: account-analytic-15.0/account-analytic-15.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-15-0/account-analytic-15-0-purchase_analytic/
Currently translated at 100.0% (3 of 3 strings)

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/hr/
Currently translated at 100.0% (3 of 3 strings)

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/es/
Currently translated at 100.0% (3 of 3 strings)

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/fr/
AungKoKoLin1997 and others added 23 commits January 9, 2025 16:18
Currently translated at 100.0% (4 of 4 strings)

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/pt_BR/
Currently translated at 100.0% (4 of 4 strings)

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/es/
Co-authored-by: Alexis de Lattre <[email protected]>
Currently translated at 100.0% (4 of 4 strings)

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/
Currently translated at 100.0% (5 of 5 strings)

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/
Currently translated at 100.0% (5 of 5 strings)

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/it/
Currently translated at 100.0% (5 of 5 strings)

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/pt_BR/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

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

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

Translation: account-analytic-16.0/account-analytic-16.0-purchase_analytic
Translate-URL: https://translation.odoo-community.org/projects/account-analytic-16-0/account-analytic-16-0-purchase_analytic/pt_BR/
@CLaurelB
Copy link
Contributor Author

CLaurelB commented Jan 9, 2025

Hello @luisg123v @StefanRijnhart

Could you please review this PR?

It already includes the additional tests to increase the coverage.

Best regards.

Changelog:
- Set the purchase analytic field's readonly attribute on the views
  based on its state, instead of defining it in the model.
- Update methods docstrings to be more descriptive.
- Add test cases for setting the analytic distribution on a purchase
  order without lines and unsetting the analytic distribution on a
  purchase order with lines.

Co-authored-by: Wodran Van de Sande <[email protected]>
@CLaurelB CLaurelB force-pushed the 17.0-mig-purchase_analytic-CLaurelB branch from 5371f88 to 73c470e Compare January 13, 2025 20:17
@@ -30,14 +26,17 @@ def _compute_analytic_distribution(self):
po.analytic_distribution = al

def _inverse_analytic_distribution(self):
"""When set analytic_distribution set analytic distribution on all order lines"""
"""When setting analytic_distribution, apply the analytic distribution to all
order lines."""

Choose a reason for hiding this comment

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

Nitpick:

Move closing quotes to the next lines, to follow PEP8 recommendations regarding multi-line docstrings

@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8" ?>
<?xml version="1.0" ?>

Choose a reason for hiding this comment

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

Why? I'd avoid this, AFAIK it's preferred to provide explicitly the encoding.

# Test setting analytic distribution with an order line
po.order_line = self.add_new_order_line()
po.analytic_distribution = self.analytic_distribution_manual2
po._onchange_analytic_distribution()

Choose a reason for hiding this comment

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

If you need to cover an onchange, it's preferred to use the Form class

"product_id": self.product_id.id,
"product_qty": 1.0,
"product_uom": self.uom_id.id,
"price_unit": 121.0,

Choose a reason for hiding this comment

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

I'd suggest making these params configurable and provide default values, so it may be called without params but also being able to provide some of them.

def test_analytic_distribution(self):
def add_new_order_line(self):
return [
Command.create(

Choose a reason for hiding this comment

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

Why don't you pass the order as param?

],
}
)
po = self.env["purchase.order"].new({"partner_id": self.partner_id.id})

Choose a reason for hiding this comment

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

Te Form class?

analytic_plan = self.env["account.analytic.plan"].create(
{"name": "Plan Test", "company_id": False}
)
analytic_plan = self.env["account.analytic.plan"].create({"name": "Plan Test"})

Choose a reason for hiding this comment

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

Why this? Document it in commit description.

@CLaurelB
Copy link
Contributor Author

Closing because the original PR is active again.

@CLaurelB CLaurelB closed this Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.