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][IMP] account_statement_import_online: add option to enable or disable statement creation. #754

Conversation

carlos-lopez-tecnativa
Copy link
Contributor

Starting from Odoo 16, bank statements are optional. This commit introduces the option to create statements automatically or skip their creation.

TT52481
@Tecnativa @pedrobaeza Could you please review this

@OCA-git-bot
Copy link
Contributor

Hi @alexey-pelykh,
some modules you are maintaining are being modified, check this out!

@pedrobaeza pedrobaeza added this to the 16.0 milestone Dec 30, 2024
@pedrobaeza pedrobaeza changed the title [IMP] account_statement_import_online: add option to enable or disable statement creation. [16.0][IMP] account_statement_import_online: add option to enable or disable statement creation. Dec 30, 2024
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. For avoiding too much diff being the same, what you can do is:

if not self.create_statement:
    return self._online_create_statement_lines(statement_values)

extracting the specific code of lines there, and leave the rest untouched.

@carlos-lopez-tecnativa carlos-lopez-tecnativa force-pushed the 16.0-account_statement_import_online-create-statement branch from 4043870 to 0dfe579 Compare December 30, 2024 20:00
@carlos-lopez-tecnativa
Copy link
Contributor Author

Thanks for the changes. For avoiding too much diff being the same, what you can do is:

if not self.create_statement:
    return self._online_create_statement_lines(statement_values)

extracting the specific code of lines there, and leave the rest untouched.

Thanks for your suggestion! I have applied it as you recommended. Could you please update your review?

@pedrobaeza
Copy link
Member

I think you should return the empty statement in the new method.

@carlos-lopez-tecnativa carlos-lopez-tecnativa force-pushed the 16.0-account_statement_import_online-create-statement branch from 0dfe579 to 5ac88ac Compare December 30, 2024 23:47
@carlos-lopez-tecnativa
Copy link
Contributor Author

I think you should return the empty statement in the new method.

Done

@carlos-lopez-tecnativa carlos-lopez-tecnativa force-pushed the 16.0-account_statement_import_online-create-statement branch from 5ac88ac to 74bb842 Compare December 31, 2024 11:25
…e statement creation.

Starting from Odoo 16, bank statements are optional. This commit introduces the option to create statements automatically or skip their creation.
@carlos-lopez-tecnativa carlos-lopez-tecnativa force-pushed the 16.0-account_statement_import_online-create-statement branch from 74bb842 to 44cf0c5 Compare December 31, 2024 12:25
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Thanks for the changes!

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

👍

@pedrobaeza
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-754-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 8b2f56d into OCA:16.0 Dec 31, 2024
7 checks passed
@pedrobaeza pedrobaeza deleted the 16.0-account_statement_import_online-create-statement branch December 31, 2024 13:04
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.

4 participants