Merge pull request #195 from open-data/fix/ds-text-int-filter

Backport DataStore Text/Int Filter Fix

Author: JVickery-TBS

changes/195.backport.bugfix

@@ -0,0 +1 @@
+You can now use non-string values in `datastore_search` and `datastore_delete` filters for text datatype fields.

ckanext/datastore/backend/postgres.py

@@ -595,8 +595,18 @@ def _where_clauses(
                 sa.column(field),
                 ','.join(f":{p}" for p in placeholders)
             ))
+            if fields_types[field] == 'text':
+                # pSQL can do int_field = "10"
+                # but cannot do text_field = 10
+                # this fixes parity there.
+                value = (str(v) for v in value)
             clause = (clause_str, dict(zip(placeholders, value)))
         else:
+            if fields_types[field] == 'text':
+                # pSQL can do int_field = "10"
+                # but cannot do text_field = 10
+                # this fixes parity there.
+                value = str(value)
             placeholder = f"value_{next(idx_gen)}"
             clause: tuple[Any, ...] = (
                 f'{sa.column(field)} = :{placeholder}',
@@ -1942,7 +1952,10 @@ def delete_data(context: Context, data_dict: dict[str, Any]):
         where_clause
     )
 
-    _execute_single_statement(context, sql_string, where_values)
+    try:
+        _execute_single_statement(context, sql_string, where_values)
+    except ProgrammingError as pe:
+        raise ValidationError({'filters': [_programming_error_summary(pe)]})
 
 
 def _create_triggers(connection: Any, resource_id: str,

ckanext/datastore/tests/test_delete.py

@@ -205,6 +205,41 @@ def test_delete_records_required_filters(self):
         err = ve.value.error_dict
         assert err == expected
 
+    @pytest.mark.ckan_config("ckan.plugins", "datastore")
+    @pytest.mark.usefixtures("clean_datastore", "with_plugins")
+    def test_delete_records_text_int_filter(self):
+        resource = factories.Resource()
+        data = {
+            "resource_id": resource["id"],
+            "force": True,
+            "fields": [
+                {"id": "text_field", "type": "text"},
+            ],
+            "records": [
+                {"text_field": 25},
+                {"text_field": 37},
+            ],
+        }
+        helpers.call_action("datastore_create", **data)
+
+        # can delete by int
+        data = {"resource_id": resource["id"], "force": True,
+                "filters": {"text_field": 25}}
+        helpers.call_action("datastore_records_delete", **data)
+        result = helpers.call_action("datastore_search",
+                                     resource_id=resource["id"],
+                                     include_total=True)
+        assert result["total"] == 1
+
+        # can delete by text
+        data = {"resource_id": resource["id"], "force": True,
+                "filters": {"text_field": "37"}}
+        helpers.call_action("datastore_records_delete", **data)
+        result = helpers.call_action("datastore_search",
+                                     resource_id=resource["id"],
+                                     include_total=True)
+        assert result["total"] == 0
+
 
 @pytest.mark.usefixtures("with_request_context")
 class TestDatastoreDeleteLegacy(object):

ckanext/datastore/tests/test_search.py

@@ -453,6 +453,62 @@ def test_search_filter_with_percent_in_column_name(self):
         result = helpers.call_action("datastore_search", **search_data)
         assert result["total"] == 1
 
+    @pytest.mark.ckan_config("ckan.plugins", "datastore")
+    @pytest.mark.usefixtures("clean_datastore", "with_plugins")
+    def test_search_sort_nulls_first_last(self):
+        resource = factories.Resource()
+        data = {
+            "resource_id": resource["id"],
+            "force": True,
+            "records": [{"a": 1, "b": "Y"}, {"b": "Z"}],
+        }
+        helpers.call_action("datastore_create", **data)
+
+        search_data = {
+            "resource_id": data["resource_id"],
+            "sort": [u"a desc nulls last"],
+        }
+        result = helpers.call_action("datastore_search", **search_data)
+        assert result["records"][0]['b'] == 'Y'
+
+        search_data = {
+            "resource_id": data["resource_id"],
+            "sort": [u"a desc nulls first"],
+        }
+        result = helpers.call_action("datastore_search", **search_data)
+        assert result["records"][0]['b'] == 'Z'
+
+    @pytest.mark.ckan_config("ckan.plugins", "datastore")
+    @pytest.mark.usefixtures("clean_datastore", "with_plugins")
+    def test_search_records_text_int_filter(self):
+        resource = factories.Resource()
+        data = {
+            "resource_id": resource["id"],
+            "force": True,
+            "fields": [
+                {"id": "text_field", "type": "text"},
+            ],
+            "records": [
+                {"text_field": 25},
+                {"text_field": 37},
+            ],
+        }
+        helpers.call_action("datastore_create", **data)
+
+        # can search by int
+        data = {"resource_id": resource["id"],
+                "include_total": True,
+                "filters": {"text_field": 25}}
+        result = helpers.call_action("datastore_search", **data)
+        assert len(result["records"]) == 1
+
+        # can search by text
+        data = {"resource_id": resource["id"],
+                "include_total": True,
+                "filters": {"text_field": "37"}}
+        result = helpers.call_action("datastore_search", **data)
+        assert len(result["records"]) == 1
+
 
 @pytest.mark.usefixtures("with_request_context")
 class TestDatastoreSearchLegacyTests(object):
@@ -770,25 +826,6 @@ def test_search_filters_get(self, app):
         assert result["total"] == 1
         assert result["records"] == [self.expected_records[0]]
 
-    @pytest.mark.ckan_config("ckan.plugins", "datastore")
-    @pytest.mark.usefixtures("clean_datastore", "with_plugins")
-    def test_search_invalid_filter(self, app):
-        data = {
-            "resource_id": self.data["resource_id"],
-            # invalid because author is not a numeric field
-            "filters": {u"author": 42},
-        }
-
-        auth = {"Authorization": self.sysadmin_token}
-        res = app.post(
-            "/api/action/datastore_search",
-            json=data,
-            extra_environ=auth,
-            status=409,
-        )
-        res_dict = json.loads(res.data)
-        assert res_dict["success"] is False
-
     @pytest.mark.ckan_config("ckan.plugins", "datastore")
     @pytest.mark.usefixtures("clean_datastore", "with_plugins")
     def test_search_sort(self, app):