-
-
Notifications
You must be signed in to change notification settings - Fork 619
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_tax_balance: Migration to 16.0 #937
[16.0][MIG] account_tax_balance: Migration to 16.0 #937
Conversation
* Tests * PEP8 * Use invoice._convert_to_write(invoice._cache). This way, the onchange will be inheritable and will add here also the added values * better get_context_values * unify method compute_balance * open move lines linked to balance
Allow to explore all move lines Spanish translation Bugfixes Show negative lines too Show move type in account.move views Tests include new fields
4328ed2
to
5cd930d
Compare
Hi @pedrobaeza |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM except for ramiadavid#1
--edit--: please also squash administrative commits (if any) with the previous commit for reducing commit noise.
Thanks a lot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested
@ramiadavid thanks for merging my PR! Please also squash administrative commits, then your PR is ready for the merge 👍 |
/ocabot migration account_tax_balance |
@ramiadavid let me know if you need help for merging the squash administrative commits. |
I'm improving the guide for this. I'm pasting the link when finish. |
If you can explain me how to do it correctly, I would appreciate it, because I don't want to do it wrong |
Uf, it has taken more time than expected, but I hope they are good enough: https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests Please tell any suggestion to it. |
* Test * README * add extra parent menu. If not, with the web_responsive module is mixed between other menus
Let the database do some computations (sum) and do not put large lists of ids in action domain.
This indexes improve the display time of account_tax_balance by a factor of 10 on a database with 1M move lines.
Due to the change at odoo/odoo@15a1b3a it is now required to run tests that require CoA after install.
78e6237
to
6ad3e20
Compare
@tafaRU @pedrobaeza I think it is ready |
@ramiadavid you missed one: |
I think that one of the cases where a conflict arises. |
@tafaRU I'm not sure how to do this, since it causes a conflict |
@tafaRU can you please update your review? |
/ocabot merge nobump |
What a great day to merge this nice PR. Let's do it! |
This PR has the |
Congratulations, your PR was merged at 8ab16dd. Thanks a lot for contributing to OCA. ❤️ |
Hi @dreispt, This module is merged, however the translations are locked: Thanks in advance, |
Unlocked. |
No description provided.