# Geometry error message fix

* fixes bug where errors on geometry form were not rendered properly
* fixes bug where invalid geometry was written as read-only back into form (could not be corrected by user)
* adds explanatory comments to SimpleGeomForm is_valid() checks
* reorders code snippets for better understanding
* adds correcting logic to _set_geojson_properties() in case of missing properties element

konova/forms/geometry_form.py

@@ -35,6 +35,7 @@ class SimpleGeomForm(BaseForm):
         disabled=False,
     )
     _num_geometries_ignored: int = 0
+    empty = False
 
     def __init__(self, *args, **kwargs):
         self.read_only = kwargs.pop("read_only", True)
@@ -49,7 +50,7 @@ class SimpleGeomForm(BaseForm):
                 raise AttributeError
 
             geojson = self.instance.geometry.as_feature_collection(srid=DEFAULT_SRID_RLP)
-            self._set_geojson_properties(geojson, title=self.instance.identifier or None)
+            geojson = self._set_geojson_properties(geojson, title=self.instance.identifier or None)
             geom = json.dumps(geojson)
         except AttributeError:
             # If no geometry exists for this form, we simply set the value to None and zoom to the maximum level
@@ -62,21 +63,29 @@ class SimpleGeomForm(BaseForm):
         super().is_valid()
         is_valid = True
 
-        # Get geojson from form
+        # Make sure invalid geometry is properly rendered again to the user
+        # Therefore: write submitted data back into form field
+        # (does not matter whether we know if it is valid or invalid)
+        submitted_data = self.data["output"]
+        submitted_data = json.loads(submitted_data)
+        submitted_data = self._set_geojson_properties(submitted_data)
+        self.initialize_form_field("output", json.dumps(submitted_data))
+
+        # Get geojson from form for validity checking
         geom = self.data.get("output", None)
-        if geom is None or len(geom) == 0:
+        geom_is_empty = geom is None or len(geom) == 0
-            # empty geometry is a valid geometry
+        if geom_is_empty:
+            # If no geometry has been submitted, we create an empty geometry object since
+            # an empty geometry is a valid geometry
             self.cleaned_data["output"] = MultiPolygon(srid=DEFAULT_SRID_RLP).ewkt
             return is_valid
-        geom = json.loads(geom)
-
-        # Write submitted data back into form field to make sure invalid geometry
-        # will be rendered again on failed submit
-        self.initialize_form_field("output", self.data["output"])
 
         # Initialize features list with empty MultiPolygon, so that an empty input will result in a
         # proper empty MultiPolygon object
         features = []
+
+        # Prepare geometry for validity checks (create iterable dict)
+        geom = json.loads(geom)
         features_json = geom.get("features", [])
         accepted_ogr_types = [
             "Polygon",
@@ -84,20 +93,23 @@ class SimpleGeomForm(BaseForm):
             "MultiPolygon",
             "MultiPolygon25D",
         ]
+        # Check validity for each feature of the geometry
         for feature in features_json:
             feature_geom = feature.get("geometry", feature)
             if feature_geom is None:
                 # Fallback for rare cases where a feature does not contain any geometry
                 continue
 
+            # Try to create a geometry object from the single feature
             feature_geom = json.dumps(feature_geom)
             g = gdal.OGRGeometry(feature_geom, srs=DEFAULT_SRID_RLP)
 
-            flatten_geometry = g.coord_dim > 2
+            geometry_has_unwanted_dimensions = g.coord_dim > 2
-            if flatten_geometry:
+            if geometry_has_unwanted_dimensions:
                 g = self.__flatten_geom_to_2D(g)
 
-            if g.geom_type not in accepted_ogr_types:
+            geometry_type_is_accepted = g.geom_type not in accepted_ogr_types
+            if geometry_type_is_accepted:
                 self.add_error("output", _("Only surfaces allowed. Points or lines must be buffered."))
                 is_valid &= False
                 return is_valid
@@ -109,18 +121,23 @@ class SimpleGeomForm(BaseForm):
                 self._num_geometries_ignored += 1
                 continue
 
+            # Whatever this geometry object is -> try to create a Polygon from it
+            # The resulting polygon object automatically detects whether a valid polygon has been created or not
             g = Polygon.from_ewkt(g.ewkt)
             is_valid &= g.valid
             if not g.valid:
                 self.add_error("output", g.valid_reason)
                 return is_valid
 
+            # If the resulting polygon is just a single polygon, we add it to the list of properly casted features
             if isinstance(g, Polygon):
                 features.append(g)
             elif isinstance(g, MultiPolygon):
+                # The resulting polygon could be of type MultiPolygon (due to multiple surfaces)
+                # If so, we extract all polygons from the MultiPolygon and extend the casted features list
                 features.extend(list(g))
 
-        # Unionize all geometry features into one new MultiPolygon
+        # Unionize all polygon features into one new MultiPolygon
         if features:
             form_geom = MultiPolygon(*features, srid=DEFAULT_SRID_RLP).unary_union
         else:
@@ -129,7 +146,7 @@ class SimpleGeomForm(BaseForm):
         # Make sure to convert into a MultiPolygon. Relevant if a single Polygon is provided.
         form_geom = Geometry.cast_to_multipolygon(form_geom)
 
-        # Write unioned Multipolygon into cleaned data
+        # Write unionized Multipolygon back into cleaned data
         if self.cleaned_data is None:
             self.cleaned_data = {}
         self.cleaned_data["output"] = form_geom.ewkt
@@ -252,6 +269,8 @@ class SimpleGeomForm(BaseForm):
         """
         features = geojson.get("features", [])
         for feature in features:
+            if not feature.get("properties", None):
+                feature["properties"] = {}
             feature["properties"]["editable"] = not self.read_only
             if title:
                 feature["properties"]["title"] = title

templates/map/geom_form.html

@@ -13,9 +13,9 @@
     </div>
 {% endif %}
 
-{% if geom_form.geom.errors %}
+{% if geom_form.output.errors %}
     <div class="alert-danger p-2">
-    {% for error in geom_form.geom.errors %}
+    {% for error in geom_form.output.errors %}
             <strong class="invalid">{{ error }}</strong>
             <br>
     {% endfor %}