From 8dcdae2590fa4d5077be4f879f8147c63382d3c8 Mon Sep 17 00:00:00 2001 From: Kevin Burnett <18027+burnettk@users.noreply.github.com> Date: Wed, 15 Nov 2023 06:20:00 -0800 Subject: [PATCH] Feature/fix filters (#670) * work in progress * respect multiple filters for the same field --------- Co-authored-by: burnettk --- .../process_instance_report_service.py | 54 +++++++++---------- .../helpers/base_test.py | 14 +++-- .../integration/test_process_api.py | 13 +++++ 3 files changed, 51 insertions(+), 30 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py index 8d39b2d3..d1b55283 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py @@ -626,39 +626,39 @@ class ProcessInstanceReportService: instance_metadata_alias = aliased(ProcessInstanceMetadataModel) instance_metadata_aliases[column["accessor"]] = instance_metadata_alias - filter_for_column = None + filters_for_column = [] if "filter_by" in report_metadata: - filter_for_column = next( - (f for f in report_metadata["filter_by"] if f["field_name"] == column["accessor"]), - None, - ) + filters_for_column = [f for f in report_metadata["filter_by"] if f["field_name"] == column["accessor"]] isouter = True join_conditions = [ ProcessInstanceModel.id == instance_metadata_alias.process_instance_id, instance_metadata_alias.key == column["accessor"], ] - if filter_for_column: - isouter = False - if "operator" not in filter_for_column or filter_for_column["operator"] == "equals": - join_conditions.append(instance_metadata_alias.value == filter_for_column["field_value"]) - elif filter_for_column["operator"] == "not_equals": - join_conditions.append(instance_metadata_alias.value != filter_for_column["field_value"]) - elif filter_for_column["operator"] == "greater_than_or_equal_to": - join_conditions.append(instance_metadata_alias.value >= filter_for_column["field_value"]) - elif filter_for_column["operator"] == "less_than": - join_conditions.append(instance_metadata_alias.value < filter_for_column["field_value"]) - elif filter_for_column["operator"] == "contains": - join_conditions.append(instance_metadata_alias.value.like(f"%{filter_for_column['field_value']}%")) - elif filter_for_column["operator"] == "is_empty": - # we still need to return results if the metadata value is null so make sure it's outer join - isouter = True - process_instance_query = process_instance_query.filter( - or_(instance_metadata_alias.value.is_(None), instance_metadata_alias.value == "") - ) - elif filter_for_column["operator"] == "is_not_empty": - join_conditions.append( - or_(instance_metadata_alias.value.is_not(None), instance_metadata_alias.value != "") - ) + if len(filters_for_column) > 0: + for filter_for_column in filters_for_column: + isouter = False + if "operator" not in filter_for_column or filter_for_column["operator"] == "equals": + join_conditions.append(instance_metadata_alias.value == filter_for_column["field_value"]) + elif filter_for_column["operator"] == "not_equals": + join_conditions.append(instance_metadata_alias.value != filter_for_column["field_value"]) + elif filter_for_column["operator"] == "greater_than_or_equal_to": + join_conditions.append(instance_metadata_alias.value >= filter_for_column["field_value"]) + elif filter_for_column["operator"] == "less_than": + join_conditions.append(instance_metadata_alias.value < filter_for_column["field_value"]) + elif filter_for_column["operator"] == "contains": + join_conditions.append( + instance_metadata_alias.value.like(f"%{filter_for_column['field_value']}%") + ) + elif filter_for_column["operator"] == "is_empty": + # we still need to return results if the metadata value is null so make sure it's outer join + isouter = True + process_instance_query = process_instance_query.filter( + or_(instance_metadata_alias.value.is_(None), instance_metadata_alias.value == "") + ) + elif filter_for_column["operator"] == "is_not_empty": + join_conditions.append( + or_(instance_metadata_alias.value.is_not(None), instance_metadata_alias.value != "") + ) process_instance_query = process_instance_query.join( instance_metadata_alias, and_(*join_conditions), isouter=isouter ).add_columns(func.max(instance_metadata_alias.value).label(column["accessor"])) diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py b/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py index 09d72468..04836821 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py @@ -19,6 +19,7 @@ from spiffworkflow_backend.models.process_group import ProcessGroup from spiffworkflow_backend.models.process_group import ProcessGroupSchema from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.process_instance_metadata import ProcessInstanceMetadataModel +from spiffworkflow_backend.models.process_instance_report import FilterValue from spiffworkflow_backend.models.process_instance_report import ProcessInstanceReportModel from spiffworkflow_backend.models.process_instance_report import ReportMetadata from spiffworkflow_backend.models.process_model import NotificationType @@ -495,8 +496,14 @@ class BaseTest: process_instance: ProcessInstanceModel, operator: str, filter_field_value: str = "", + filters: list[FilterValue] | None = None, expect_to_find_instance: bool = True, ) -> None: + if filters is None: + filters = [] + + first_filter: FilterValue = {"field_name": "key1", "field_value": filter_field_value, "operator": operator} + filters.append(first_filter) report_metadata: ReportMetadata = { "columns": [ {"Header": "ID", "accessor": "id", "filterable": False}, @@ -504,7 +511,7 @@ class BaseTest: {"Header": "Key two", "accessor": "key2", "filterable": False}, ], "order_by": ["status"], - "filter_by": [{"field_name": "key1", "field_value": filter_field_value, "operator": operator}], + "filter_by": filters, } process_instance_report = ProcessInstanceReportModel.create_report( identifier=f"{process_instance.id}_sure", @@ -520,9 +527,10 @@ class BaseTest: assert response.json["results"][0]["id"] == process_instance.id else: if len(response.json["results"]) == 1: + first_result = response.json["results"][0] assert ( - response.json["results"][0]["id"] != process_instance.id - ), "expected not to find a specific process instance, but we found it" + first_result["id"] != process_instance.id + ), f"expected not to find a specific process instance, but we found it: {first_result}" else: assert len(response.json["results"]) == 0 db.session.delete(process_instance_report) diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py index eae7fa60..856f4487 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py @@ -2957,6 +2957,19 @@ class TestProcessApi(BaseTest): operator="greater_than_or_equal_to", filter_field_value="value1", ) + + # these two filters are conflicting, hence the expect_to_find_instance=False + # this test was written because, at the time, only one filter per field was being applied, + # so the code ignored the less_than and the test passed. + self.assert_report_with_process_metadata_operator_includes_instance( + client=client, + user=with_super_admin_user, + process_instance=process_instance_one, + operator="less_than", + filter_field_value="value1", + filters=[{"field_name": "key1", "field_value": "value1", "operator": "greater_than_or_equal_to"}], + expect_to_find_instance=False, + ) self.assert_report_with_process_metadata_operator_includes_instance( client=client, user=with_super_admin_user, process_instance=process_instance_two, operator="is_empty" )