iommirocks / iommi

Your first pick for a django power cord
http://iommi.rocks
BSD 3-Clause "New" or "Revised" License
718 stars 47 forks source link

Fix EmailField / URLField required=False #434

Closed berycz closed 1 year ago

berycz commented 1 year ago

fix for #423

berycz commented 1 year ago

hmm, I'm not sure whether to fix the tests or set default field=None?

berycz commented 1 year ago

maybe rather field=None for backwards compatibility?

jlubcke commented 1 year ago

It would be nice with some test cases showing the different cases.

boxed commented 1 year ago

The existing code has return <something> or string_value but the or can never apply since <something> raises....

berycz commented 1 year ago

yea, I was also thinking what's the point of or string_value :) and I was also thinking if instead of field=None there shouldnt be rather something like allow_empty=False with self.invoke_callback(self.parse, string_value=raw_data, allow_empty=not self.required) on L870?

EDIT: probably not, choice_parse also takes field

berycz commented 1 year ago

oh I got the or string_value validators either raise exception or return "nothing" (None) so if the email/url is valid, then it's return None or string_value

boxed commented 1 year ago

Thanks!