diff --git a/cdisc_rules_engine/enums/execution_status.py b/cdisc_rules_engine/enums/execution_status.py index d1b6b58bb..96ab25540 100644 --- a/cdisc_rules_engine/enums/execution_status.py +++ b/cdisc_rules_engine/enums/execution_status.py @@ -5,3 +5,10 @@ class ExecutionStatus(BaseEnum): SUCCESS = "success" SKIPPED = "skipped" EXECUTION_ERROR = "execution_error" + ISSUE_REPORTED = "issue_reported" + UNKNOWN_STATUS = "unknown_status" + + +class ExecutionError(BaseEnum): + AN_UNKNOWN_EXCEPTION_HAS_OCCURRED = "An unknown exception has occurred" + COLUMN_NOT_FOUND_IN_DATA = "Column not found in data" diff --git a/cdisc_rules_engine/services/reporting/base_report.py b/cdisc_rules_engine/services/reporting/base_report.py index 32798e401..2a4cf9201 100644 --- a/cdisc_rules_engine/services/reporting/base_report.py +++ b/cdisc_rules_engine/services/reporting/base_report.py @@ -4,7 +4,7 @@ from openpyxl import Workbook -from cdisc_rules_engine.enums.execution_status import ExecutionStatus +from cdisc_rules_engine.enums.execution_status import ExecutionStatus, ExecutionError from cdisc_rules_engine.models.rule_validation_result import RuleValidationResult from cdisc_rules_engine.models.validation_args import Validation_args @@ -47,24 +47,24 @@ def get_summary_data(self) -> List[List]: """ summary_data = [] for validation_result in self._results: - if validation_result.execution_status == "success": - for result in validation_result.results or []: - dataset = result.get("domain") - if ( - result.get("errors") - and result.get("executionStatus") == "success" - ): - summary_item = { - "dataset": dataset, - "core_id": validation_result.id, - "message": result.get("message"), - "issues": len(result.get("errors")), - } - - if self._item_type == "list": - summary_data.extend([[*summary_item.values()]]) - elif self._item_type == "dict": - summary_data.extend([summary_item]) + for result in validation_result.results or []: + dataset = result.get("domain") + if ( + result.get("errors") + and result.get("executionStatus") + == ExecutionStatus.ISSUE_REPORTED.value + ): + summary_item = { + "dataset": dataset, + "core_id": validation_result.id, + "message": result.get("message"), + "issues": len(result.get("errors")), + } + + if self._item_type == "list": + summary_data.extend([[*summary_item.values()]]) + elif self._item_type == "dict": + summary_data.extend([summary_item]) return sorted( summary_data, @@ -86,6 +86,70 @@ def get_detailed_data(self) -> List[List]: else (x["core_id"], x["dataset"]), ) + def _issue_details(self, validation_result: RuleValidationResult, result: dict): + errors = [] + variables = result.get("variables", []) + for error in [ + error + for error in result.get("errors") + if error.get("error") + not in [ + ExecutionError.AN_UNKNOWN_EXCEPTION_HAS_OCCURRED.value, + ExecutionError.COLUMN_NOT_FOUND_IN_DATA.value, + ] + ]: + error_item = { + "core_id": validation_result.id, + "message": result.get("message"), + "executability": validation_result.executability, + "dataset": result.get("domain") or "", + "USUBJID": error.get("USUBJID", ""), + "row": error.get("row", ""), + "SEQ": error.get("SEQ", ""), + } + + if self._item_type == "list": + error_item["variables"] = ", ".join(variables) + error_item["values"] = ", ".join( + [ + str(error.get("value", {}).get(variable)) + for variable in variables + ] + ) + errors = errors + [[*error_item.values()]] + elif self._item_type == "dict": + error_item["variables"] = variables + error_item["values"] = [ + str(error.get("value", {}).get(variable)) for variable in variables + ] + errors = errors + [error_item] + return errors + + def _error_details(self, validation_result: RuleValidationResult, result: dict): + errors = [] + for error in [ + error + for error in result.get("errors") + if error.get("error") + == ExecutionError.AN_UNKNOWN_EXCEPTION_HAS_OCCURRED.value + ]: + error_item = { + "core_id": validation_result.id, + "message": (f"{result.get('message')} - {error.get('error')}"), + "executability": validation_result.executability, + "dataset": result.get("domain") or "", + "USUBJID": "", + "row": "", + "SEQ": "", + "variables": "", + "values": error.get("message"), + } + if self._item_type == "list": + errors = errors + [[*error_item.values()]] + elif self._item_type == "dict": + errors = errors + [error_item] + return errors + def _generate_error_details( self, validation_result: RuleValidationResult ) -> List[List]: @@ -107,35 +171,11 @@ def _generate_error_details( """ errors = [] for result in validation_result.results or []: - if result.get("errors", []) and result.get("executionStatus") == "success": - variables = result.get("variables", []) - for error in result.get("errors"): - error_item = { - "core_id": validation_result.id, - "message": result.get("message"), - "executability": validation_result.executability, - "dataset": result.get("domain"), - "USUBJID": error.get("USUBJID", ""), - "row": error.get("row", ""), - "SEQ": error.get("SEQ", ""), - } - - if self._item_type == "list": - error_item["variables"] = ", ".join(variables) - error_item["values"] = ", ".join( - [ - str(error.get("value", {}).get(variable)) - for variable in variables - ] - ) - errors = errors + [[*error_item.values()]] - elif self._item_type == "dict": - error_item["variables"] = variables - error_item["values"] = [ - str(error.get("value", {}).get(variable)) - for variable in variables - ] - errors = errors + [error_item] + errors = ( + errors + + self._issue_details(validation_result, result) + + self._error_details(validation_result, result) + ) return errors def get_rules_report_data(self) -> List[List]: @@ -162,9 +202,9 @@ def get_rules_report_data(self) -> List[List]: "fda_rule_id": validation_result.fda_rule_id, "pmda_rule_id": validation_result.pmda_rule_id, "message": validation_result.message, - "status": ExecutionStatus.SUCCESS.value.upper() - if validation_result.execution_status == ExecutionStatus.SUCCESS.value - else ExecutionStatus.SKIPPED.value.upper(), + "status": ExecutionStatus( + validation_result.execution_status + ).value.upper(), } if self._item_type == "list": rules_report.append([*rules_item.values()]) diff --git a/cdisc_rules_engine/utilities/utils.py b/cdisc_rules_engine/utilities/utils.py index 4f9ae3754..c4312f8af 100644 --- a/cdisc_rules_engine/utilities/utils.py +++ b/cdisc_rules_engine/utilities/utils.py @@ -16,7 +16,7 @@ SUPPLEMENTARY_DOMAINS, ) from cdisc_rules_engine.constants.classes import SPECIAL_PURPOSE -from cdisc_rules_engine.enums.execution_status import ExecutionStatus +from cdisc_rules_engine.enums.execution_status import ExecutionError, ExecutionStatus from cdisc_rules_engine.interfaces import ConditionInterface from cdisc_rules_engine.models.base_validation_entity import BaseValidationEntity @@ -44,25 +44,75 @@ def mark_domain_as_validated(domain: str, validated_domains: Set[str]): def get_execution_status(results): """ - If all results have skipped status, return skipped. - Else return success + If any result has an execution error, return execution error + Else if any result has an issue reported, return issue reported + Else if any result is successful, return issue successful + Else, result should have all skips, return issue skipped """ if len(results) == 0: return ExecutionStatus.SUCCESS.value - if isinstance(results[0], BaseValidationEntity): - successful_results = [ - entity for entity in results if entity.status == ExecutionStatus.SUCCESS - ] - else: - successful_results = [ - result - for result in results - if result.get("executionStatus") == ExecutionStatus.SUCCESS.value - ] - if successful_results: + status = ( + { + ExecutionStatus.SUCCESS: [], + ExecutionStatus.EXECUTION_ERROR: [], + ExecutionStatus.ISSUE_REPORTED: results, + ExecutionStatus.SKIPPED: [], + } + if isinstance(results[0], BaseValidationEntity) + else { + ExecutionStatus.SUCCESS: [ + result + for result in results + if result.get("executionStatus") == ExecutionStatus.SUCCESS.value + ], + ExecutionStatus.EXECUTION_ERROR: [ + result + for result in results + if result.get("executionStatus") + == ExecutionStatus.EXECUTION_ERROR.value + and [ + error + for error in result.get("errors", []) + if error.get("error") + == ExecutionError.AN_UNKNOWN_EXCEPTION_HAS_OCCURRED.value + ] + ], + ExecutionStatus.ISSUE_REPORTED: [ + result + for result in results + if result.get("executionStatus") == ExecutionStatus.ISSUE_REPORTED.value + ], + ExecutionStatus.SKIPPED: [ + result + for result in results + if result.get("executionStatus") == ExecutionStatus.SKIPPED.value + or [ + error + for error in result.get("errors", []) + if error.get("error") + == ExecutionError.COLUMN_NOT_FOUND_IN_DATA.value + ] + ], + } + ) + print("break") + if len(results) != ( + len(status[ExecutionStatus.SUCCESS]) + + len(status[ExecutionStatus.EXECUTION_ERROR]) + + len(status[ExecutionStatus.ISSUE_REPORTED]) + + len(status[ExecutionStatus.SKIPPED]) + ): + return ExecutionStatus.UNKNOWN_STATUS.value + elif status[ExecutionStatus.EXECUTION_ERROR]: + return ExecutionStatus.EXECUTION_ERROR.value + elif status[ExecutionStatus.ISSUE_REPORTED]: + return ExecutionStatus.ISSUE_REPORTED.value + elif status[ExecutionStatus.SUCCESS]: return ExecutionStatus.SUCCESS.value - else: + elif status[ExecutionStatus.SKIPPED]: return ExecutionStatus.SKIPPED.value + else: + return ExecutionStatus.UNKNOWN_STATUS.value def get_standard_codelist_cache_key(standard: str, version: str) -> str: diff --git a/resources/templates/report-template.xlsx b/resources/templates/report-template.xlsx index 6c58ae0a8..77a004e21 100644 Binary files a/resources/templates/report-template.xlsx and b/resources/templates/report-template.xlsx differ diff --git a/tests/unit/test_rules_engine.py b/tests/unit/test_rules_engine.py index 713308c4c..00b31cb22 100644 --- a/tests/unit/test_rules_engine.py +++ b/tests/unit/test_rules_engine.py @@ -87,7 +87,7 @@ def test_validate_rule_invalid_suffix( ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": ["AESTDY"], "message": "Suffix of AESTDY is equal to test.", @@ -121,7 +121,7 @@ def test_validate_rule_invalid_prefix( ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": ["AESTDY"], "message": "Prefix of AESTDY is equal to test.", @@ -218,7 +218,7 @@ def test_validate_rule_cross_dataset_check( ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "EC", "variables": ["ECSTDY"], "message": "Value of ECSTDY is equal to AESTDY.", @@ -305,7 +305,7 @@ def test_validate_one_to_one_rel_across_datasets(dataset_rule_one_to_one_related ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "EC", "variables": ["VISITNUM"], "message": "VISITNUM is not one-to-one related to VISIT", @@ -339,7 +339,7 @@ def test_validate_rule_single_dataset_check(dataset_rule_greater_than: dict): ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "EC", "variables": ["ECCOOLVAR"], "message": "Value for ECCOOLVAR greater than 30.", @@ -373,7 +373,7 @@ def test_validate_rule_equal_length(dataset_rule_has_equal_length: dict): ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "EC", "variables": ["ECCOOLVAR"], "message": "Length of ECCOOLVAR is equal to 5.", @@ -404,7 +404,7 @@ def test_validate_is_contained_by_distinct(mock_rule_distinct_operation: dict): ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": ["AESTDY"], "message": "Value for AESTDY not in DM.USUBJID", @@ -435,7 +435,7 @@ def test_validate_rule_not_equal_length(dataset_rule_has_not_equal_length: dict) ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "EC", "variables": ["ECCOOLVAR"], "message": "Length of ECCOOLVAR is not equal to 5.", @@ -460,7 +460,7 @@ def test_validate_rule_multiple_conditions(dataset_rule_multiple_conditions: dic ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "EC", "variables": ["ECCOOLVAR"], "message": ( @@ -490,7 +490,7 @@ def test_validate_record_rule_numbers_separated_by_dash_pattern(): ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": ["AESTDY"], "message": "Records have the following pattern: ^\\d+\\-\\d+$", @@ -518,7 +518,7 @@ def test_validate_record_rule_semi_colon_delimited_pattern(): ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": ["AESTDY"], "message": "Records have the following pattern: [^,]*;[^,]*", @@ -548,7 +548,7 @@ def test_validate_record_rule_no_letters_numbers_underscores(): ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": ["AESTDY"], "message": "Records have the following pattern: ^((?![a-zA-Z0-9_]).)*$", @@ -630,7 +630,7 @@ def test_validate_dataset_metadata_wrong_metadata( assert validation_result == [ { "domain": "EC", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "variables": ["dataset_label", "dataset_name", "dataset_size"], "errors": [ { @@ -723,7 +723,7 @@ def test_validate_variable_metadata_wrong_metadata( { "domain": "EC", "variables": ["variable_data_type", "variable_label", "variable_name"], - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "errors": [ { "row": 1, @@ -788,7 +788,7 @@ def test_rule_with_domain_prefix_replacement(mock_get_dataset: MagicMock): ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": ["AESTDY"], "message": "Invalid AESTDY value", @@ -813,7 +813,7 @@ def test_rule_with_domain_prefix_replacement(mock_get_dataset: MagicMock): ], [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": ["AE"], "message": "Domain AE exists", @@ -888,7 +888,7 @@ def test_validate_single_rule(dataset_rule_equal_to_error_objects: dict): assert validation_result == [ { "domain": "AE", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "variables": ["AESTDY"], "errors": [ { @@ -963,7 +963,7 @@ def test_validate_single_rule_not_equal_to( assert validation_result == [ { "domain": "AE", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "variables": ["AESTDY"], "errors": [ { @@ -1017,7 +1017,7 @@ def test_validate_single_rule_not_equal_to( [ { "domain": "AE", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "variables": ["dataset_label", "dataset_location", "dataset_name"], "errors": [ { @@ -1126,7 +1126,7 @@ def test_validate_dataset_metadata_against_define_xml( [ { "domain": "AE", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "variables": ["variable_size"], "errors": [{"row": 1, "value": {"variable_size": 30}}], "message": ( @@ -1164,7 +1164,7 @@ def test_validate_dataset_metadata_against_define_xml( [ { "domain": "AE", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "variables": ["variable_size"], "errors": [{"row": 1, "value": {"variable_size": 30}}], "message": ( @@ -1258,7 +1258,7 @@ def filter_func(row): assert validation_result == [ { "domain": "AE", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "variables": [ "AETERM", ], @@ -1306,7 +1306,7 @@ def filter_func(row): [ { "domain": "AE", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "variables": ["AESTDY"], "errors": [ {"row": 1, "value": {"AESTDY": "test"}, "USUBJID": "1"}, @@ -1458,7 +1458,7 @@ def test_validate_split_dataset_metadata( assert validation_result == [ { "domain": "EC", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "errors": [ { "row": 2, @@ -1518,7 +1518,7 @@ def test_validate_split_dataset_variables_metadata( assert validation_result == [ { "domain": "EC", - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "variables": ["variable_data_type", "variable_label", "variable_name"], "errors": [ { @@ -1631,7 +1631,7 @@ def test_validate_record_in_parent_domain( ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "EC", "variables": ["ECPRESP", "QNAM"], "message": "Dataset contents is wrong.", @@ -1693,7 +1693,7 @@ def test_validate_additional_columns( ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "TS", "variables": ["TSVAL1", "TSVAL2", "TSVAL3"], "message": "Additional columns for TSVAL are empty.", @@ -1808,7 +1808,7 @@ def test_validate_dataset_contents_against_define_and_library_variable_metadata( ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": [ "AESER", @@ -1969,7 +1969,7 @@ def test_validate_extract_metadata_operation( ) assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "SUPPEC", "variables": [ "RDOMAIN", @@ -2047,7 +2047,7 @@ def test_dataset_references_invalid_whodrug_terms( assert validation_result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": [ "AEINA", @@ -2174,7 +2174,7 @@ def test_validate_variables_order_against_library_metadata( ) assert result == [ { - "executionStatus": "success", + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "domain": "AE", "variables": ["$column_order_from_dataset", "$column_order_from_library"], "message": RuleProcessor.extract_message_from_rule( diff --git a/tests/unit/test_services/test_reporting/test_excel_export.py b/tests/unit/test_services/test_reporting/test_excel_export.py index 9e56d2a36..856f4232c 100644 --- a/tests/unit/test_services/test_reporting/test_excel_export.py +++ b/tests/unit/test_services/test_reporting/test_excel_export.py @@ -63,7 +63,7 @@ { "domain": "AE", "variables": ["AESTDY", "DOMAIN"], - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "errors": [ { "row": 1, @@ -133,7 +133,7 @@ { "domain": "TT", "variables": ["TTVAR1", "TTVAR2"], - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "errors": [ { "row": 1, @@ -165,7 +165,7 @@ def test_get_rules_report_data(): result.fda_rule_id, result.pmda_rule_id, result.message, - ExecutionStatus.SUCCESS.value.upper(), + ExecutionStatus.ISSUE_REPORTED.value.upper(), ] ) expected_reports = sorted(expected_reports, key=lambda x: x[0]) diff --git a/tests/unit/test_services/test_reporting/test_json_export.py b/tests/unit/test_services/test_reporting/test_json_export.py index 85471ec53..7fe4c44af 100644 --- a/tests/unit/test_services/test_reporting/test_json_export.py +++ b/tests/unit/test_services/test_reporting/test_json_export.py @@ -56,7 +56,7 @@ { "domain": "AE", "variables": ["AESTDY", "DOMAIN"], - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "errors": [ { "row": 1, @@ -126,7 +126,7 @@ { "domain": "TT", "variables": ["TTVAR1", "TTVAR2"], - "executionStatus": ExecutionStatus.SUCCESS.value, + "executionStatus": ExecutionStatus.ISSUE_REPORTED.value, "errors": [ { "row": 1, @@ -157,7 +157,7 @@ def test_get_rules_report_data(): "fda_rule_id": result.fda_rule_id, "pmda_rule_id": result.pmda_rule_id, "message": result.message, - "status": ExecutionStatus.SUCCESS.value.upper(), + "status": ExecutionStatus.ISSUE_REPORTED.value.upper(), } ) expected_reports = sorted(expected_reports, key=lambda x: x["core_id"])