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.

Sign up for GitHub

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

Jump to bottom

Fixed #28992 -- added validate into GenericIPAddressField's prep value #11682

Closed
wants to merge 1 commit into from
Closed

Fixed #28992 -- added validate into GenericIPAddressField's prep value #11682

wants to merge 1 commit into from

Conversation

cansarigol
Copy link
Contributor

Ticket: #28992

claudep
claudep reviewed Aug 17, 2019
View reviewed changes
tests/model_fields/test_genericipaddressfield.py
@@ -38,6 +42,6 @@ def test_blank_string_saved_as_null(self):
self.assertIsNone(o.ip)

def test_save_load(self):
instance = GenericIPAddress.objects.create(ip='::1')
instance = GenericIPAddress.objects.create(ip='1.1.1.1')
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 explain this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GenericIPAddress ip field's protocol=ipv4 because of that ::1 value is not valid

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes, obviously!

@carltongibson
Copy link
Member

carltongibson commented Aug 20, 2019 •
edited

It's nearly my lunchtime so 🤯 a bit but... I'm not seeing quite how this solves the reported issue, which was a 500 from search_fields in the admin.

@carltongibson carltongibson self-assigned this Aug 20, 2019
@cansarigol
Copy link
Contributor Author

you're right about admin side. I will look asap. I wanted to take an approved about which exception will be raised for field-side from you.

@carltongibson
Copy link
Member

It’s not immediately clear to me that we get that much benefit raising the ValueError here. I guess it’s quicker than a trip to the DB...

Thinking about this, it seems we’ll need to expose the form, via the ChangeList class..., so users can add their own validators. We need to have a little think about how that’ll look, but first thing is to get #10895 in.

@carltongibson carltongibson removed their assignment Aug 29, 2019
Base automatically changed from master to main March 9, 2021 06:21
@cansarigol cansarigol closed this Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claudep claudep claudep left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@cansarigol @carltongibson @claudep