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

[IMP] account: Adding a method to check reconcilability on bank statements #32527

Closed

Conversation

hbto
Copy link
Contributor

@hbto hbto commented Apr 8, 2019

[IMP] account: Adding a method to check reconcilability on
account.bank.statement.line and make it inheritable.

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@hbto hbto force-pushed the 12.0-task_32371-check_reconcilability-hbto branch from 783d7ed to 7eea73c Compare April 8, 2019 18:36
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Apr 8, 2019
Copy link
Contributor

@moylop260 moylop260 left a comment

Choose a reason for hiding this comment

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

@nim-odoo
It looks more inheritable
We need to add custom validations here then it could be good if it is applied this patch then we can apply our custom behaviour.

What do you think?

@moylop260
Copy link
Contributor

@nim-odoo
Could you help us to check it, please?

Copy link
Contributor

@nim-odoo nim-odoo left a comment

Choose a reason for hiding this comment

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

What is the use case you are trying to solve?

By the way, counterpart_aml_dicts, payment_aml_rec and new_aml_dicts could also be sent to the method. So that it receives all available information to extend the behavior.

addons/account/models/account_bank_statement.py Outdated Show resolved Hide resolved
@hbto
Copy link
Contributor Author

hbto commented Apr 16, 2019

@nim-odoo

What is the use case you are trying to solve?

Users want to add validations at the reconciliation of Journal Items of future or past dates than that of the Bank Statement Line date.

By adding this method it will be possible to add those validations without having to override the whole method.

Regards.

account.bank.statement.line and make it inheritable.
@hbto hbto force-pushed the 12.0-task_32371-check_reconcilability-hbto branch from 7eea73c to ae80cc4 Compare April 16, 2019 15:21
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Apr 16, 2019
@nim-odoo
Copy link
Contributor

I don't think this is in any way necessary. You can do:

def process_reconciliation(self, counterpart_aml_dicts=None, payment_aml_rec=None, new_aml_dicts=None):
    for rec in payment_aml_rec or self.env['account.move.line']:
        [do whatever check you want here]
    super().process_reconciliation(counterpart_aml_dicts, payment_aml_rec, new_aml_dicts)

No valuable information is computed before the piece of code you want to override, only a couple of variable assignments. Making this inheritable won't make an override cleaner or easier.

@mart-e
Copy link
Contributor

mart-e commented Apr 6, 2020

Closing following the reasons given by Nicolas.

@mart-e mart-e closed this Apr 6, 2020
@robodoo robodoo added closed 💔 and removed CI 🤖 Robodoo has seen passing statuses labels Apr 6, 2020
@luisg123v luisg123v deleted the 12.0-task_32371-check_reconcilability-hbto branch July 14, 2022 00:55
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.

6 participants