Opened 6 years ago

Closed 4 years ago

#16110 closed Bug (fixed)

GeometryField does not allow setting blank=True and null=False

Reported by: Paul Winkler Owned by: nobody
Component: GIS Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Sometimes you want to populate a required field's missing value with a value generated eg. from other form fields.
One common idiom for doing this is to have SomeField(blank=True, null=False) and provide the missing value in eg. the Form's clean() method.

For example, I've done variations on this a lot:

class Foo(Model):
    geom = PointField(blank=True, null=False)
    address = CharField(max_length=100)
    class Meta:
        app_label = 'myblock' # or whatever,

class FooForm(ModelForm):
    class Meta:
        model = Foo

    def clean(self):
        geom = self.cleaned_data.get('geom')
        if not geom:
            self.cleaned_data['geom'] = geocode(self.cleaned_data['address'])
        return super(FooForm, self).clean()

But with a GeometryField, that doesn't work, because of these lines in
GeometryField.clean() in django/contrib/gis/forms/fields.py :

        if not value:
            if self.null and not self.required:
                # The geometry column allows NULL and is not required.
                return None
            else:
                raise forms.ValidationError(self.error_messages['no_geom'])

Effectively, this makes blank=True meaningless.

As far as I know, GeometryField is the only FormField shipped with django that behaves this way.
This is surprising and inconsistent, and leads to odd workarounds to convince forms.GeometryField to leave you alone. Eg. hacking a ModelAdmin's formfield_for_dbfield() like so:

    def formfield_for_dbfield(self, db_field, **kwargs):
        if db_field.name == 'geom':
            kwargs['required'] = False
            kwargs['null'] = True
        return super(MyModelAdmin, self).formfield_for_dbfield(db_field, **kwargs)

Fixing this is trivial. Patch attached.

Attachments (2)

geodjango_16110.diff (2.2 KB) - added by Paul Winkler 6 years ago.
second attempt patch to fix 16610, with test. a typo fixed
16110-2.diff (3.5 KB) - added by Claude Paroz 4 years ago.
Updated patch

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by Paul Winkler

Attachment: geodjango_16110.diff added

second attempt patch to fix 16610, with test. a typo fixed

comment:1 Changed 6 years ago by Aymeric Augustin

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Note that #15271 is also about GeometryField.clean().

Something should be done about the first comment in the patch: # Never checked: stop passing this? => needs improvement.

comment:3 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

Changed 4 years ago by Claude Paroz

Attachment: 16110-2.diff added

Updated patch

comment:4 Changed 4 years ago by Claude Paroz

Patch needs improvement: unset

Just attached an updated patch, with some other choices, but basically the same effect (hopefully). I choose a quick deprecation timeline for the null argument, because this is mostly undocumented stuff.

comment:5 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In 18e990fa9639b5476b556c8de82717ebfc8b254e:

Fixed #16110 -- Fixed GeometryField odd behaviour regarding null values

Thanks slinkp for the report and the initial patch.

Note: See TracTickets for help on using tickets.
Back to Top