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

[18.0][MIG] account_reconcile_restrict_partner_mismatch: Migration to 18.0 #741

Open
wants to merge 18 commits into
base: 18.0
Choose a base branch
from

Conversation

LucasTran380381
Copy link

Base on #596

@LucasTran380381 LucasTran380381 mentioned this pull request Nov 12, 2024
4 tasks
Copy link

@henrybackman henrybackman left a comment

Choose a reason for hiding this comment

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

Can combine translation commits

@LucasTran380381
Copy link
Author

Can combine translation commits

will squash

ibuioli and others added 9 commits December 5, 2024 09:24
…on some journals / Enable on company level

In some cases, accounting users should be allowed to mix partners. This will be configured
on account journal level.

In order to not break standard behaviors, the restriction feature should be
enabled by a configuration parameter on company level. A new configuration
parameter has been added.
…tions / tests improvements

As accounting entries should be posted before reconciliation, test
correct exceptions.

Improvement to use BaseCommon class in tests
As the super() does not return 'True', if no self is defined,
return the super() method
@LucasTran380381 LucasTran380381 force-pushed the 18.0-mig-account_reconcile_restrict_partner_mismatch branch from c273d87 to 0a6b4b6 Compare December 5, 2024 02:24
in ("asset_receivable", "liability_payable")
)

def reconcile(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is not called when using this module with OEE. You should instead hook on account.move.line._reconcile_plan

Copy link
Author

Choose a reason for hiding this comment

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

I changed method _reconcile_plan to fixed with Odoo EE

@LucasTran380381 LucasTran380381 force-pushed the 18.0-mig-account_reconcile_restrict_partner_mismatch branch from 0a6b4b6 to 9ed55f8 Compare December 17, 2024 08:38
Copy link
Contributor

@grindtildeath grindtildeath left a comment

Choose a reason for hiding this comment

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

Would be good to improve the test because having them green is obviously wrong right now.

Also, functionally speaking, I don't see a test validating two move lines with same partner getting reconciled, and it would be great to use different journals. (that is not strictly required for the mig but maybe add a comment in the roadmap)

Comment on lines 26 to 28
if self:
partners = set()
for line in self:
Copy link
Contributor

Choose a reason for hiding this comment

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

_reconcile_plan is an api.model function, you should loop over the reconciliation_plan var instead (cf docstring to know how it must be structured)

Copy link
Author

Choose a reason for hiding this comment

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

improved test-case and loop over reconciliation_plan as suggested

@LucasTran380381 LucasTran380381 force-pushed the 18.0-mig-account_reconcile_restrict_partner_mismatch branch from 9ed55f8 to 4ee453e Compare December 18, 2024 04:01
[Fix] fix adapt ui for odoo18

[Fix] Fix res_config_view for odoo18

[Fix] fix review point
@LucasTran380381 LucasTran380381 force-pushed the 18.0-mig-account_reconcile_restrict_partner_mismatch branch from 4ee453e to 7f64e9e Compare December 18, 2024 04:04
Copy link
Contributor

@grindtildeath grindtildeath left a comment

Choose a reason for hiding this comment

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

Thanks you, that seems to work now 👍

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

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.