From 49c175658389117cefc99e2723e8a71af804bb48 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Thu, 11 Jan 2024 23:36:29 -0800 Subject: [PATCH] MINOR - Test Case Ownership, keep severity, add test case result incidentId (#14691) * MINOR - Test Case Ownership, keep severity, add test case result incidentId * fix immutability * Fix tests * MINOR - Test Case Ownership, keep severity, add test case result incidentId --------- Co-authored-by: Pere Miquel Brull --- .../service/jdbi3/TestCaseRepository.java | 20 ++++++++++ .../TestCaseResolutionStatusRepository.java | 40 +++++++++---------- .../service/resources/EntityResourceTest.java | 16 +++++++- .../dqtests/TestCaseResourceTest.java | 1 + .../resources/json/schema/tests/basic.json | 4 ++ 5 files changed, 58 insertions(+), 23 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java index 9df58099896..0f8449bf2de 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java @@ -2,6 +2,7 @@ package org.openmetadata.service.jdbi3; import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty; +import static org.openmetadata.schema.type.Include.ALL; import static org.openmetadata.service.Entity.TEST_CASE; import static org.openmetadata.service.Entity.TEST_DEFINITION; import static org.openmetadata.service.Entity.TEST_SUITE; @@ -23,6 +24,7 @@ import org.jdbi.v3.sqlobject.transaction.Transaction; import org.openmetadata.schema.EntityInterface; import org.openmetadata.schema.api.feed.CloseTask; import org.openmetadata.schema.api.feed.ResolveTask; +import org.openmetadata.schema.entity.data.Table; import org.openmetadata.schema.entity.teams.User; import org.openmetadata.schema.tests.ResultSummary; import org.openmetadata.schema.tests.TestCase; @@ -89,6 +91,18 @@ public class TestCaseRepository extends EntityRepository { fields.contains(INCIDENTS_FIELD) ? getIncidentId(test) : test.getIncidentId()); } + @Override + public void setInheritedFields(TestCase testCase, Fields fields) { + EntityLink entityLink = EntityLink.parse(testCase.getEntityLink()); + Table table = Entity.getEntity(entityLink, "owner", ALL); + inheritOwner(testCase, fields, table); + } + + @Override + public EntityInterface getParentEntity(TestCase entity, String fields) { + return Entity.getEntity(entity.getTestSuite(), fields, Include.NON_DELETED); + } + @Override public void clearFields(TestCase test, Fields fields) { test.setTestSuites(fields.contains("testSuites") ? test.getTestSuites() : null); @@ -249,8 +263,14 @@ public class TestCaseRepository extends EntityRepository { UUID incidentStateId = null; if (TestCaseStatus.Failed.equals(testCaseResult.getTestCaseStatus())) { incidentStateId = getOrCreateIncidentOnFailure(testCase, updatedBy); + // Set the incident ID to the test case result to ensure we can link result <> incident when + // plotting the UI + // even after the incident has been closed. + testCaseResult.setIncidentId(incidentStateId); } + // We add the incidentStateId in the DQ table to quickly link Test Case <> Incident + // When we Resolve the incident, we'll clean up this incidentId column daoCollection .dataQualityDataTimeSeriesDao() .insert( diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java index 6a2ae22da4c..56d1150b248 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java @@ -21,7 +21,6 @@ import org.openmetadata.schema.tests.type.TestCaseResolutionStatus; import org.openmetadata.schema.tests.type.TestCaseResolutionStatusTypes; import org.openmetadata.schema.type.EntityReference; import org.openmetadata.schema.type.Include; -import org.openmetadata.schema.type.Relationship; import org.openmetadata.schema.type.TaskDetails; import org.openmetadata.schema.type.TaskStatus; import org.openmetadata.schema.type.TaskType; @@ -133,11 +132,9 @@ public class TestCaseResolutionStatusRepository throw IncidentManagerException.invalidStatus(lastStatus, newStatus); } } - case Resolved -> { - if (!newStatus.equals(TestCaseResolutionStatusTypes.Resolved)) { - throw IncidentManagerException.invalidStatus(lastStatus, newStatus); - } - } + // We only validate status if the last one is unresolved, so we should + // never land here + default -> throw IncidentManagerException.invalidStatus(lastStatus, newStatus); } } @@ -156,29 +153,36 @@ public class TestCaseResolutionStatusRepository // if we have an ongoing incident, set the stateId if the new record to be created // and validate the flow if (Boolean.TRUE.equals(unresolvedIncident(lastIncident))) { - recordEntity.setStateId(lastIncident.getStateId()); validateStatus( lastIncident.getTestCaseResolutionStatusType(), recordEntity.getTestCaseResolutionStatusType()); + // If there is an unresolved incident update the state ID + recordEntity.setStateId(lastIncident.getStateId()); + // If the last incident had a severity assigned and the incoming incident does not, inherit + // the old severity + recordEntity.setSeverity( + recordEntity.getSeverity() == null + ? lastIncident.getSeverity() + : recordEntity.getSeverity()); } switch (recordEntity.getTestCaseResolutionStatusType()) { - // When we create a NEW incident, we need to open a task with the test case owner as the - // assignee. We don't need to check any past history case New -> { // If there is already an unresolved incident, return it without doing any // further logic. if (Boolean.TRUE.equals(unresolvedIncident(lastIncident))) { return getLatestRecord(lastIncident.getTestCaseReference().getFullyQualifiedName()); } + // When we create a NEW incident, we need to open a task with the test case owner or + // the table owner as the assignee. openNewTask(recordEntity); } case Ack -> { - /* nothing to do for ACK. The Owner already has the task open. It will close it when reassigning it */ + /* nothing to do for ACK. The Owner already has the task open */ } case Assigned -> assignTask(recordEntity, lastIncident); - // When the incident is Resolved, we will close the Assigned task. case Resolved -> { + // When the incident is Resolved, we will close the Assigned task. resolveTask(recordEntity, lastIncident); // We don't create a new record. The new status will be added via the // TestCaseFailureResolutionTaskWorkflow @@ -193,16 +197,10 @@ public class TestCaseResolutionStatusRepository private void openNewTask(TestCaseResolutionStatus incidentStatus) { - List owners = - EntityUtil.getEntityReferences( - daoCollection - .relationshipDAO() - .findFrom( - incidentStatus.getTestCaseReference().getId(), - Entity.TEST_CASE, - Relationship.OWNS.ordinal(), - Entity.USER)); - createTask(incidentStatus, owners, "New Incident"); + TestCase testCase = + Entity.getEntity(incidentStatus.getTestCaseReference(), "owner", Include.NON_DELETED); + + createTask(incidentStatus, Collections.singletonList(testCase.getOwner()), "New Incident"); } private void assignTask( diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java index 7916d73c935..eeae3923a52 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java @@ -1480,7 +1480,9 @@ public abstract class EntityResourceTest testCaseResultMap = new HashMap<>(); for (TestCaseResult result : actualTestCaseResults.getData()) { + result.setIncidentId(null); testCaseResultMap.put(result.getTimestamp(), result); } for (TestCaseResult result : expectedTestCaseResults) { diff --git a/openmetadata-spec/src/main/resources/json/schema/tests/basic.json b/openmetadata-spec/src/main/resources/json/schema/tests/basic.json index a4174d7897b..9037e0b76fc 100644 --- a/openmetadata-spec/src/main/resources/json/schema/tests/basic.json +++ b/openmetadata-spec/src/main/resources/json/schema/tests/basic.json @@ -108,6 +108,10 @@ "failedRowsPercentage": { "description": "Percentage of rows that failed.", "type": "number" + }, + "incidentId": { + "description": "Incident State ID associated with this result. This association happens when the result is created, and will stay there even when the incident is resolved.", + "$ref": "../type/basic.json#/definitions/uuid" } } },