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

Fixed #26834 -- Made IntegerField forward min/max validators to form … #6964

Closed
wants to merge 1 commit into from

Conversation

sgzsh269
Copy link

…field

The change forwards the user set limit values of MinValueValidator/MaxValueValidator
defined in the model IntegerField to the form field using the formfield() method.

Thanks Markus Holtermann for help with the patch.

…field

The change forwards the user set limit values of MinValueValidator/MaxValueValidator
defined in the model IntegerField to the form field using the formfield() method.

Thanks Markus Holtermann for help with the patch.
MaxValueValidator to the form fields using formfield() method.
"""
if1 = models.IntegerField()
if2 = models.IntegerField(validators=[validators.MinValueValidator(10),
Copy link
Member

Choose a reason for hiding this comment

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

Could you wrap this as

if2 = models.IntegerField(
    validators=[validators.MinValueValidator(10), validators.MaxValueValidator(100)]
)

please.

@MarkusH
Copy link
Member

MarkusH commented Jul 22, 2016

What happens when a field has more than one MaxValueValidator (or MinValueValidator) where the boundary values differ, e.g.

field = IntegerField(validators=[MaxValueValidator(5), MaxValueValidator(10)])

@timgraham
Copy link
Member

I think when we talked at the sprints, you indicated there was another issue that needed to be fixed before this one. Do you have any follow up with details about that? Should we close this in the meantime?

@sgzsh269
Copy link
Author

sgzsh269 commented Aug 5, 2016

@timgraham yes, the issue is related to the validators property, it needs further evaluation. Haven't gotten a chance to work on it but plan to do it soon.
I think @MarkusH mentioned at the time that the fix for the validators property issue could be included in this same PR.
Should I create a separate ticket and subsequently a separate PR for the validators property issue?

@timgraham
Copy link
Member

A separate ticket/PR is fine. Sometimes a PR will include multiple commits but separating them if they stand on their own is fine too.

@timgraham
Copy link
Member

Closing since this isn't suitable for merge yet.

@timgraham timgraham closed this Aug 26, 2016
@mluszczyk
Copy link

@sgzsh269 are you still working on this? I'd like to see this feature included into Django and I'll be happy to help with this patch. Could you elaborate on the "issue related to the validators property", is there a ticket for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants