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.

Sign up for GitHub

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

Jump to bottom

[17.0][MIG] account_global_discount: Migration to 17.0 #1687

Merged
merged 41 commits into from
Feb 4, 2025
Merged

[17.0][MIG] account_global_discount: Migration to 17.0 #1687

merged 41 commits into from
Feb 4, 2025

Conversation

miguel-S73
Copy link
Contributor

@miguel-S73 miguel-S73 commented Mar 18, 2024 •
edited
Loading

Standard migration to 17.0

Depends:

@miguel-S73 miguel-S73 mentioned this pull request Mar 18, 2024
Open
2 tasks
@pedrobaeza
Copy link
Member

/ocabot migration account_global_discount

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Jun 17, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Jun 17, 2024
Open
46 tasks
@miguel-S73 miguel-S73 force-pushed the 17.0-mig-account_global_discount branch 2 times, most recently from 20b1a28 to d6bd7b3 Compare August 14, 2024 16:11
@miguel-S73
Copy link
Contributor Author

@pedrobaeza I apply same change that OCA/server-backend#275 (comment)

@pedrobaeza pedrobaeza changed the title [MIG] account_global_discount: 17.0 [17.0][MIG] account_global_discount: Migration to 17.0 Aug 14, 2024
@pedrobaeza
Copy link
Member

OK, some reviewers should validate this.

@ferran-S73 ferran-S73 force-pushed the 17.0-mig-account_global_discount branch from d6bd7b3 to ad6b296 Compare September 27, 2024 06:36
@miguel-S73 miguel-S73 force-pushed the 17.0-mig-account_global_discount branch 3 times, most recently from ca1c359 to 77a9b48 Compare November 6, 2024 14:34
@miguel-S73
Copy link
Contributor Author

@pedrobaeza all check are green.
We've thoroughly tested this module with multiple clients, and it performs reliably.

@pedrobaeza
Copy link
Member

Great. Now some reviewers are needed. We don't need this module for now.

@miguel-S73
Copy link
Contributor Author

@etobella @gurneyalex @grindtildeath
Could one of you please review my changes?

We've thoroughly tested this module with multiple clients, and it performs reliably.

@Abimael1321
Copy link

Functionally reviewed. LGTM!

@miguel-S73
Copy link
Contributor Author

I’d like to thank @Abimael1321 for testing this PR and confirming that it works as expected.
With this validation, I believe the PR is ready to be merged.
@pedrobaeza, Please let me know if there’s anything else needed. Thanks!

@pedrobaeza
Copy link
Member

2 reviewers are needed, and one coming from a PSC: https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#review

@AinohaBH
Copy link

Functional: LGTM!

dalonsod
dalonsod suggested changes Jan 7, 2025
View reviewed changes
Copy link

@dalonsod dalonsod left a comment

Choose a reason for hiding this comment

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

See comments. Most of them look for commit noise reduction, and it seems that there are two errors in views.

account_global_discount/models/account_move.py Show resolved Hide resolved
account_global_discount/models/account_move.py Show resolved Hide resolved
account_global_discount/models/account_move.py Show resolved Hide resolved
account_global_discount/models/account_move.py Show resolved Hide resolved
account_global_discount/models/account_move.py Show resolved Hide resolved
account_global_discount/tests/test_global_discount.py Show resolved Hide resolved
account_global_discount/tests/test_global_discount.py Show resolved Hide resolved
account_global_discount/tests/test_global_discount.py Show resolved Hide resolved
account_global_discount/views/account_invoice_views.xml Outdated Show resolved Hide resolved
account_global_discount/views/account_invoice_views.xml Outdated Show resolved Hide resolved
@miguel-S73 miguel-S73 force-pushed the 17.0-mig-account_global_discount branch from 77a9b48 to ab1825f Compare January 8, 2025 11:24
@miguel-S73
Copy link
Contributor Author

The remaining test failures are caused by another module, but there’s an active PR that addresses this issue.

dalonsod
dalonsod suggested changes Jan 30, 2025
View reviewed changes
Copy link

@dalonsod dalonsod left a comment

Choose a reason for hiding this comment

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

See comment, and there are some other tree view fields with invisible="1" that should be updated as well, if I'm not wrong

account_global_discount/views/account_invoice_views.xml Outdated Show resolved Hide resolved
chienandalu and others added 4 commits February 3, 2025 09:58
@chienandalu @miguel-S73
[ADD][11.0] account_global_discount: New Module
fddb91e
@chienandalu @miguel-S73
[FIX+IMP] account_global_discount: link line taxes to discount move l…
fb18fc1 
…ine + Discounts in invoice report

[IMP] account_global_discount: Discounts in invoice report

Give discounts info in the invoice report.

[FIX] account_global_discount: link line taxes to discount move line
@pedrobaeza @miguel-S73
[FIX] account_global_discount: More on global discount/taxes link
cccbc7d 
- We need to take into account invoice lines with multiple taxes, so the link should
  be m2m.
- Migration scripts for preserving the best information on existing installations.
- Tests for checking new conditions.
- Perform sanity check for not ending in an incompatible situation.
- Some refactor done on onchanges for avoiding duplicating operations.
- Adjust UI for not allowing to edit computed invoice global discounts.
@pedrobaeza @miguel-S73
[MIG] acount_global_discount: Migration to 12.0
f5739c6 
- Standard procedure
- Make test more resilient
kirca and others added 12 commits February 3, 2025 09:58
@kirca @miguel-S73
[FIX] account_global_discount: multi-currency
54b42f1 
In a foreign currency invoice the amounts (amount_total,
amount_untaxed) are expressed in the `invoice` currency while the
accounting entry lines are expressed in the `company` currency.

The base_before_global_discounts has to be expressed in the invoice
currency as well in such invoices because the discounts need to be
computed in the same currency as the other amounts.

Also set the invoice currency_id on the tax line so that it doesn't
default to the company currency, and convert the discount journal entry
amount to the company currnecy.
@OCA-git-bot @miguel-S73
[FIX] account_global_discount: Make tests resilient
13f47f1 
- Depending on the installed set of modules, the company currency may
  be USD or EUR. If the second case, these tests will fail, so we make
  sure that the company currency is USD for our tests, doing the change
  by SQL, as there's a Python constraint that prevents it.

Not needed in v17 due to: odoo/odoo#107113.
@ChrisOForgeFlow @miguel-S73
[IMP] account_global_discount, added init hook to avoid memory issues…
814d2d8 
… on big database installation
@ferran-S73 @miguel-S73
[MIG] account_global_discount: 16.0
feed186
@Rferri44-S73 @miguel-S73
[IMP] account_global_discount: Remake amount global discount
72350d4
@miguel-S73
[UPD] Update account_global_discount.pot
895fc52
@OCA-git-bot @miguel-S73
[BOT] post-merge updates
d516e4a
@ferran-S73 @miguel-S73
[16.0][IMP] account_global_discount: exclude products from global disc
144c2d4
@OCA-git-bot @miguel-S73
[BOT] post-merge updates
5347952
@mymage @miguel-S73
Translated using Weblate (Italian)
9f72628 
Currently translated at 100.0% (36 of 36 strings)

Translation: account-invoicing-16.0/account-invoicing-16.0-account_global_discount
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-16-0/account-invoicing-16-0-account_global_discount/it/
@mymage @miguel-S73
Translated using Weblate (Italian)
3d76c83 
Currently translated at 100.0% (36 of 36 strings)

Translation: account-invoicing-16.0/account-invoicing-16.0-account_global_discount
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-16-0/account-invoicing-16-0-account_global_discount/it/
@miguel-S73
[IMP] account_global_discount: pre-commit auto fixes
753fac7
@miguel-S73 miguel-S73 force-pushed the 17.0-mig-account_global_discount branch from ab1825f to ca0b1fd Compare February 3, 2025 08:59
dalonsod
dalonsod suggested changes Feb 3, 2025
View reviewed changes
Copy link

@dalonsod dalonsod left a comment

Choose a reason for hiding this comment

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

There are more invisible that should be moved to column_invisible, please check.

account_global_discount/views/account_invoice_views.xml Outdated Show resolved Hide resolved
account_global_discount/views/account_invoice_views.xml Outdated Show resolved Hide resolved
@miguel-S73 miguel-S73 force-pushed the 17.0-mig-account_global_discount branch from ca0b1fd to 5c041f1 Compare February 4, 2025 07:33
@miguel-S73 miguel-S73 force-pushed the 17.0-mig-account_global_discount branch from 5c041f1 to 5feaf74 Compare February 4, 2025 10:09
dalonsod
dalonsod approved these changes Feb 4, 2025
View reviewed changes
Copy link

@dalonsod dalonsod left a comment

Choose a reason for hiding this comment

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

👍

@AaronHForgeFlow is this PR ready to be merged? Thanks!

@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). 🤖

@miguel-S73
Copy link
Contributor Author

Hi @pedrobaeza , this PR has received two approvals. Would it be possible to merge the changes?

@pedrobaeza
Copy link
Member

OK, let's go

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-1687-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 4c17371 into OCA:17.0 Feb 4, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AinohaBH AinohaBH AinohaBH approved these changes

@Abimael1321 Abimael1321 Abimael1321 approved these changes

@dalonsod dalonsod dalonsod approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
17.0
Development

Successfully merging this pull request may close these issues.