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

[17.0][IMP] account_credit_control: Allow to create credit control lines with the maximum level for one control run #432

Conversation

sergio-teruel
Copy link
Contributor

  • Some ux improvements and minor fixes

cc @Tecnativa TT55185

ping @carlosdauden @carlos-lopez-tecnativa

@sergio-teruel sergio-teruel changed the title [17.0][IMP] account_credit_control: Allow create credit control lines with the maximum level for one control run [17.0][IMP] account_credit_control: Allow to create credit control lines with the maximum level for one control run Feb 20, 2025
Copy link
Contributor

@carlosdauden carlosdauden left a comment

Choose a reason for hiding this comment

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

Minor changes

@@ -32,6 +35,11 @@ class CreditControlPolicy(models.Model):
help="This policy will be active only for the selected accounts",
)
active = fields.Boolean(default=True)
apply_max_policy_level = fields.Boolean(
string="Apply max policy level",
help="Apply max policy lavel for one partner in a credit control run execution "
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
help="Apply max policy lavel for one partner in a credit control run execution "
help="Apply max policy level for one partner in a credit control run execution "

default_apply_max_policy_level = fields.Boolean(
string="Apply max policy level",
default_model="credit.control.policy",
help="Apply max policy lavel for one partner in a credit control run execution "
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
help="Apply max policy lavel for one partner in a credit control run execution "
help="Apply max policy level for one partner in a credit control run execution "

<field name="invoice_id" />
<field name="partner_id" />
<field name="partner_id" readonly="true" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<field name="partner_id" readonly="true" />
<field name="partner_id" readonly="True" />

It probably works correctly, but the usual way is to use it with a capital T or readonly="1".

Copy link

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

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

Could you please add a test to cover this use case?

@sergio-teruel sergio-teruel force-pushed the 17.0-IMP-account_credit_control-some-improvements branch 2 times, most recently from f2df5f6 to 10cac52 Compare February 20, 2025 22:17
@sergio-teruel
Copy link
Contributor Author

Changes done!!

Copy link

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

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

Tested and working as expected, just a minor change.

…the maximum level for one control run

Some ux improvements and minor fixes
@sergio-teruel sergio-teruel force-pushed the 17.0-IMP-account_credit_control-some-improvements branch from 10cac52 to ed0fbf4 Compare February 21, 2025 13:55
@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). 🤖

@pedrobaeza pedrobaeza added this to the 17.0 milestone Feb 25, 2025
@pedrobaeza
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-432-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit e634efe into OCA:17.0 Feb 25, 2025
7 checks passed
@pedrobaeza pedrobaeza deleted the 17.0-IMP-account_credit_control-some-improvements branch February 25, 2025 15:01
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.

5 participants