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

[16.0][MIG] account_invoice_triple_discount: Migration to 16.0 #1269

Merged

Conversation

ramiadavid
Copy link
Contributor

@ramiadavid ramiadavid commented Oct 19, 2022

Migration to v16

#1250

@ramiadavid ramiadavid changed the title [16][MIG] account_invoice_triple_discount: Migration to 16.0 [16.0][MIG] account_invoice_triple_discount: Migration to 16.0 Oct 19, 2022
@ramiadavid ramiadavid marked this pull request as ready for review October 19, 2022 18:22
@legalsylvain
Copy link
Contributor

Hi. Thanks for porting this module.
I have a functional question. In my company I receive many supplier invoices with Discount 1 & 2. (Odoo 12.)
Some computation are good (I mean Odoo computation = supplier computation), but some computation are bad. That is due to the fact that the supplier makes intermediate rounding.
Do you think, it could be possible to introduce some computation option (depending on the supplier ?)

If yes, I can try to implement that extra feature.

Note : I think that this module is very complex. We should try to talk to the Odoo SA dev team in the next odoo experience to try to have some hook to avoid all this code. Don't you think ?

Kind regard.

@ramiadavid ramiadavid force-pushed the 16.0-mig-account_invoice_triple_discount branch from c5c765c to 7fa0122 Compare October 26, 2022 19:56
@ramiadavid
Copy link
Contributor Author

Hi. Thanks for porting this module. I have a functional question. In my company I receive many supplier invoices with Discount 1 & 2. (Odoo 12.) Some computation are good (I mean Odoo computation = supplier computation), but some computation are bad. That is due to the fact that the supplier makes intermediate rounding. Do you think, it could be possible to introduce some computation option (depending on the supplier ?)

If yes, I can try to implement that extra feature.

Note : I think that this module is very complex. We should try to talk to the Odoo SA dev team in the next odoo experience to try to have some hook to avoid all this code. Don't you think ?

Kind regard.

Hi @legalsylvain
In this module there is a bug in the calculation of discounts, it is discussed here #1240 , and I have uploaded a PR with the fix, I am waiting for it to be merged

Does this solve your problem?

@legalsylvain
Copy link
Contributor

Hi @ramiadavid . thanks for your answer. it's différent topic. I talk about the fact that 2 results are valid depending on the supplier computation method.

Copy link

@FernandoRomera FernandoRomera left a comment

Choose a reason for hiding this comment

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

Review, OK.

Copy link

@MRomeera MRomeera left a comment

Choose a reason for hiding this comment

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

ok

chienandalu and others added 20 commits December 20, 2022 08:39
Currently translated at 100,0% (4 of 4 strings)

Translation: account-invoicing-11.0/account-invoicing-11.0-account_invoice_triple_discount
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-11-0/account-invoicing-11-0-account_invoice_triple_discount/de/
Currently translated at 100.0% (4 of 4 strings)

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

Translation: account-invoicing-14.0/account-invoicing-14.0-account_invoice_triple_discount
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-14-0/account-invoicing-14-0-account_invoice_triple_discount/it/
Currently translated at 100.0% (4 of 4 strings)

Translation: account-invoicing-15.0/account-invoicing-15.0-account_invoice_triple_discount
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-15-0/account-invoicing-15-0-account_invoice_triple_discount/ca/
Copy link

@sbejaoui sbejaoui left a comment

Choose a reason for hiding this comment

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

Hi,

Functional test

The invoice total don't match the lines subtotal.

image

@ramiadavid ramiadavid force-pushed the 16.0-mig-account_invoice_triple_discount branch 2 times, most recently from 8171b91 to cc65e60 Compare December 28, 2022 17:33
@ramiadavid
Copy link
Contributor Author

@sbejaoui @AntoniRomera @FernandoRomera @MRomeera

This is fixed, but I see that the discount_2 and discount_3 fields are only shown if we add the 'Technical / Discount on lines' group, while the discount field is always available, likewise the discount field is set to optional="hide" while than the other two as optional="show" and in the discount field the name is abbreviated "Disc." while in the others it says "Discount"

Wouldn't it be better to unify these things to follow the same criteria as the original field?

@sbejaoui
Copy link

Wouldn't it be better to unify these things to follow the same criteria as the original field?

+1,
Thank you

@ramiadavid ramiadavid force-pushed the 16.0-mig-account_invoice_triple_discount branch from cc65e60 to 0c165eb Compare January 2, 2023 20:58
@FernandoRomera
Copy link

@ramiadavid
In account_invoice_triple_discount/report/invoice.xml, the names are:
Disc.2 %
Disc.3 %
without so many blanks and parentheses

@ramiadavid ramiadavid force-pushed the 16.0-mig-account_invoice_triple_discount branch from 0c165eb to e628d2a Compare January 3, 2023 14:13
@ramiadavid
Copy link
Contributor Author

@ramiadavid In account_invoice_triple_discount/report/invoice.xml, the names are: Disc.2 % Disc.3 % without so many blanks and parentheses

You're right, I've already changed it.

Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

LGTM (Code review + functional) but the source term into the translation files must be updated with the new labels for the discount columns....

@ramiadavid ramiadavid force-pushed the 16.0-mig-account_invoice_triple_discount branch from e628d2a to 9f5ba13 Compare January 12, 2023 13:43
Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

Thank you @ramiadavid for all this work. LGTM (Code review + functional)

@ramiadavid ramiadavid force-pushed the 16.0-mig-account_invoice_triple_discount branch from 9f5ba13 to d2e12c0 Compare January 13, 2023 08:15
@lmignon
Copy link
Contributor

lmignon commented Jan 24, 2023

@OCA/accounting-maintainers Someone to merge this one?

@adrienpeiffer
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-1269-by-adrienpeiffer-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit fb78ef6 into OCA:16.0 Jan 24, 2023
@OCA-git-bot
Copy link
Contributor

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

@lmignon
Copy link
Contributor

lmignon commented Jan 24, 2023

Thank you @adrienpeiffer for the merge!

@ramiadavid ramiadavid deleted the 16.0-mig-account_invoice_triple_discount branch January 24, 2023 09:21
@tafaRU
Copy link
Member

tafaRU commented Sep 5, 2023

/ocabot migration acount_invoice_triple_discount

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Sep 5, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Sep 5, 2023
64 tasks
odooNextev pushed a commit to odooNextev/account-invoicing that referenced this pull request Sep 29, 2023
Signed-off-by AaronHForgeFlow
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.