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

[17.0][MIG] account_invoice_pricelist: Migration to 17.0 #1619

Merged
merged 61 commits into from
Sep 30, 2024

Conversation

peluko00
Copy link

@peluko00 peluko00 commented Nov 27, 2023

Module migrated to version 17.0

cc https://github.com/APSL 148829

@miquelalzanillas @lbarry-apsl @javierobcn @mpascuall please review

legalsylvain and others added 30 commits November 27, 2023 12:27
OCA Transbot updated translations from Transifex

OCA Transbot updated translations from Transifex
Currently translated at 100,0% (5 of 5 strings)

Translation: account-invoicing-11.0/account-invoicing-11.0-account_invoice_pricelist
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-11-0/account-invoicing-11-0-account_invoice_pricelist/de/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_pricelist
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_pricelist/
Currently translated at 100.0% (5 of 5 strings)

Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_pricelist
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_pricelist/fr/
Currently translated at 100.0% (5 of 5 strings)

Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_pricelist
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_pricelist/pt_BR/
Currently translated at 100.0% (5 of 5 strings)

Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_pricelist
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_pricelist/es_CL/
Currently translated at 100.0% (5 of 5 strings)

Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_pricelist
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_pricelist/hr/
OCA-git-bot and others added 2 commits November 27, 2023 12:27
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: account-invoicing-16.0/account-invoicing-16.0-account_invoice_pricelist
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-16-0/account-invoicing-16-0-account_invoice_pricelist/
@peluko00 peluko00 mentioned this pull request Nov 27, 2023
44 tasks
@pedrobaeza pedrobaeza changed the title [17.0][MIG]1account_invoice_pricelist: Migration to 17.0 [17.0][MIG] account_invoice_pricelist: Migration to 17.0 Nov 27, 2023
@peluko00 peluko00 force-pushed the 17.0-mig-account_invoice_pricelist branch from 2c5c7fe to 14fa4cb Compare May 14, 2024 05:40
@peluko00 peluko00 marked this pull request as ready for review May 14, 2024 07:12
Copy link

@mpascuall mpascuall left a comment

Choose a reason for hiding this comment

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

Working correctly, checked in runboat

Copy link
Member

@lbarry-apsl lbarry-apsl left a comment

Choose a reason for hiding this comment

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

Works, tested on Runboat

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@hildickethan
Copy link
Member

I've been having some issues installing this PR alongside l10n_es in an environment with demo data, though i think any non-USD accounting would have the same issue. Creating a db with odoo -i l10n_es,account_invoice_pricelist with without_demo=False will error when installing the latter.
image

From my digging the problem seems to be that in this commit odoo/odoo@83c5257 the way pricelists work in 17.0 was altered to not be required and to use the company's currency if it isn't set.
image

This is causing the demo sales/invoices to have EUR currency and then error in the _check_currency due to _compute_pricelist_id computing the default USD pricelist for the partners, which in turn changes the pricelist but the _check_currency happens before the _compute_currency_id fixes the currency 🫠

But the reason this only happens in 17.0 is due to aforementioned commit, it removes the basic "Public Pricelist", which is what would be the default computed EUR pricelist in previous versions (I checked on 16.0).
image

Since this no longer exists, it can only default to USD.
The easiest way to handle this is to avoid computing pricelist_id on existing invoices altogether. It also has the benefit of speeding up installation times which in big databases could be a few minutes. I don't think it's unreasonable to not touch existing invoices either.

This is the simple pre_init_hook I added locally that fixes the issue

Patch

diff --git a/account_invoice_pricelist/__init__.py b/account_invoice_pricelist/__init__.py
index 0650744..548c73e 100644
--- a/account_invoice_pricelist/__init__.py
+++ b/account_invoice_pricelist/__init__.py
@@ -1 +1,2 @@
+from .hooks import pre_init_hook
 from . import models
diff --git a/account_invoice_pricelist/__manifest__.py b/account_invoice_pricelist/__manifest__.py
index 7e365ba..b30a7cf 100644
--- a/account_invoice_pricelist/__manifest__.py
+++ b/account_invoice_pricelist/__manifest__.py
@@ -11,4 +11,5 @@
     "depends": ["account", "sale_management"],
     "data": ["views/account_invoice_view.xml"],
     "installable": True,
+    "pre_init_hook": "pre_init_hook",
 }
diff --git a/account_invoice_pricelist/hooks.py b/account_invoice_pricelist/hooks.py
new file mode 100644
index 0000000..ce5fe6a
--- /dev/null
+++ b/account_invoice_pricelist/hooks.py
@@ -0,0 +1,15 @@
+# Copyright 2024 Studio73 - Ethan Hildick <[email protected]>
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+from odoo.tools.sql import column_exists, create_column
+
+
+def pre_init_hook(env):
+    cr = env.cr
+    # Speed up the installation of the module on an existing Odoo instance
+    # by not computing the pricelist for every invoice. Also avoids
+    # a possible computation error of
+    # computing all pricelists -> _check_currency constraint error before the
+    # currency can recompute itself
+    if not column_exists(cr, "account_move", "pricelist_id"):
+        create_column(cr, "account_move", "pricelist_id", "int4")

@hildickethan
Copy link
Member

This is the simple pre_init_hook I added locally that fixes the issue

Patch

@peluko00 sorry for the long message but could you add this hook to the migration?

@peluko00 peluko00 force-pushed the 17.0-mig-account_invoice_pricelist branch from b4a2915 to ea9c746 Compare July 12, 2024 06:25
@peluko00 peluko00 force-pushed the 17.0-mig-account_invoice_pricelist branch from ea9c746 to 365a034 Compare July 12, 2024 06:27
@peluko00
Copy link
Author

This is the simple pre_init_hook I added locally that fixes the issue
Patch

@peluko00 sorry for the long message but could you add this hook to the migration?

Thanks for the fix! Can you review now please

Copy link

@ecino ecino left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@grindtildeath
Copy link
Contributor

Can you please cherry-pick #1751 ? 🙏

@yvaucher
Copy link
Member

I'll merge this and cherry-pick #1751 in a new PR

@yvaucher
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-1619-by-yvaucher-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit d06e514 into OCA:17.0 Sep 30, 2024
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@yvaucher
Copy link
Member

@grindtildeath I created the PR here: #1806

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.