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 <peremiquelbrull@gmail.com>

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<TestCase> {
         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<TestCase> {
     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(

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 -> {
+        // We only validate status if the last one is unresolved, so we should
-        if (!newStatus.equals(TestCaseResolutionStatusTypes.Resolved)) {
+        // never land here
-          throw IncidentManagerException.invalidStatus(lastStatus, newStatus);
+      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<EntityReference> owners =
+    TestCase testCase =
-        EntityUtil.getEntityReferences(
+        Entity.getEntity(incidentStatus.getTestCaseReference(), "owner", Include.NON_DELETED);
-            daoCollection
+
-                .relationshipDAO()
+    createTask(incidentStatus, Collections.singletonList(testCase.getOwner()), "New Incident");
-                .findFrom(
-                    incidentStatus.getTestCaseReference().getId(),
-                    Entity.TEST_CASE,
-                    Relationship.OWNS.ordinal(),
-                    Entity.USER));
-    createTask(incidentStatus, owners, "New Incident");
   }
 
   private void assignTask(

openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java

@@ -1480,7 +1480,9 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
     // Create entity without description, owner
     T entity = createEntity(createRequest(getEntityName(test), "", null, null), ADMIN_AUTH_HEADERS);
     // user will always have the same user assigned as the owner
-    if (!Entity.getEntityTypeFromObject(entity).equals(Entity.USER)) {
+    if (!Entity.getEntityTypeFromObject(entity).equals(Entity.USER)
+        && entity.getOwner() != null
+        && !entity.getOwner().getInherited()) {
       assertListNull(entity.getOwner());
     }
     entity = getEntity(entity.getId(), ADMIN_AUTH_HEADERS);
@@ -2898,6 +2900,11 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
 
   /** Compare entity Id and types in the entityReference */
   protected static void assertReference(EntityReference expected, EntityReference actual) {
+    // If the actual value is inherited, it will never match the expected
+    // We just ignore the validation in these cases
+    if (actual != null && actual.getInherited() != null && actual.getInherited()) {
+      return;
+    }
     if (expected != null) {
       assertNotNull(actual);
       TestUtils.validateEntityReference(actual);
@@ -3170,7 +3177,12 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
   public static EntityReference reduceEntityReference(EntityReference ref) {
     // In requests send minimum entity reference information to ensure the server fills rest of the
     // details
-    return ref != null ? new EntityReference().withType(ref.getType()).withId(ref.getId()) : null;
+    return ref != null && (ref.getInherited() == null || !ref.getInherited())
+        ? new EntityReference()
+            .withType(ref.getType())
+            .withId(ref.getId())
+            .withInherited(ref.getInherited())
+        : null;
   }
 
   public String getAllowedFields() {

openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java

@@ -1610,6 +1610,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
     assertEquals(expectedTestCaseResults.size(), actualTestCaseResults.getData().size());
     Map<Long, TestCaseResult> testCaseResultMap = new HashMap<>();
     for (TestCaseResult result : actualTestCaseResults.getData()) {
+      result.setIncidentId(null);
       testCaseResultMap.put(result.getTimestamp(), result);
     }
     for (TestCaseResult result : expectedTestCaseResults) {

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"
         }
       }
     },