diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestSuiteResourceIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestSuiteResourceIT.java index 64ee4ff76ed8..d548a03e4618 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestSuiteResourceIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestSuiteResourceIT.java @@ -8,6 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.fasterxml.jackson.databind.JsonNode; import es.co.elastic.clients.transport.rest5_client.low_level.Request; import es.co.elastic.clients.transport.rest5_client.low_level.Response; import es.co.elastic.clients.transport.rest5_client.low_level.Rest5Client; @@ -25,6 +26,7 @@ import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; import org.openmetadata.it.bootstrap.TestSuiteBootstrap; +import org.openmetadata.it.factories.UserTestFactory; import org.openmetadata.it.util.SdkClients; import org.openmetadata.it.util.TestNamespace; import org.openmetadata.schema.api.data.CreateTable; @@ -39,6 +41,7 @@ import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineStatus; import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineStatusType; import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineType; +import org.openmetadata.schema.entity.teams.User; import org.openmetadata.schema.metadataIngestion.SourceConfig; import org.openmetadata.schema.metadataIngestion.TestSuitePipeline; import org.openmetadata.schema.tests.DataQualityReport; @@ -386,6 +389,53 @@ void test_testSuiteWithOwner(TestNamespace ns) { assertFalse(fetched.getOwners().isEmpty()); } + @Test + void get_byName_honorsIncludeRelations_hidesSoftDeletedOwner(TestNamespace ns) { + // The Test Suite detail page fetches getByName with includeRelations=owners:non-deleted so a + // soft-deleted owner is hidden and the owner-edit diff stays aligned with the server's + // NON_DELETED PATCH base - otherwise a positional patch throws "array item index out of range". + // getByName previously ignored includeRelations (unlike getById); this guards that wiring. + // See issue #30117. + OpenMetadataClient client = SdkClients.adminClient(); + User active = UserTestFactory.createUser(ns, "aaa_active"); + User doomed = UserTestFactory.createUser(ns, "zzz_doomed"); + CreateTestSuite request = new CreateTestSuite(); + request.setName(ns.prefix("testsuite_getbyname_includerel")); + request.setOwners(List.of(active.getEntityReference(), doomed.getEntityReference())); + String fqn = createEntity(request).getFullyQualifiedName(); + client.users().delete(doomed.getId().toString()); + + // include=all with no includeRelations still surfaces the soft-deleted owner... + assertTrue( + ownerIdsByName(fqn, "").contains(doomed.getId()), + "include=all should surface the soft-deleted owner"); + + // ...but getByName must honor includeRelations=owners:non-deleted and filter it out. + List filtered = ownerIdsByName(fqn, "&includeRelations=owners:non-deleted"); + assertFalse( + filtered.contains(doomed.getId()), + "getByName must hide the soft-deleted owner when includeRelations=owners:non-deleted"); + assertTrue(filtered.contains(active.getId()), "the active owner must remain"); + } + + private List ownerIdsByName(String fqn, String extraQuery) { + String body = + SdkClients.adminClient() + .getHttpClient() + .executeForString( + HttpMethod.GET, + "/v1/dataQuality/testSuites/name/" + + fqn + + "?fields=owners&include=all" + + extraQuery, + null); + List ids = new ArrayList<>(); + for (JsonNode owner : JsonUtils.readTree(body).path("owners")) { + ids.add(UUID.fromString(owner.path("id").asText())); + } + return ids; + } + @Test void test_testSuiteFQNFormat(TestNamespace ns) { OpenMetadataClient client = SdkClients.adminClient(); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestSuiteResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestSuiteResource.java index 3c8964515743..8418d0fbfa5d 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestSuiteResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestSuiteResource.java @@ -426,8 +426,18 @@ public TestSuite getByName( schema = @Schema(implementation = Include.class)) @QueryParam("include") @DefaultValue("non-deleted") - Include include) { - return getByNameInternal(uriInfo, securityContext, name, fieldsParam, include); + Include include, + @Parameter( + description = + "Per-relation include control. Format: field:value,field2:value2. " + + "Example: owners:non-deleted,followers:all. " + + "Valid values: all, deleted, non-deleted. " + + "If not specified for a field, uses the entity's include value.", + schema = @Schema(type = "string", example = "owners:non-deleted,followers:all")) + @QueryParam("includeRelations") + String includeRelations) { + return getByNameInternal( + uriInfo, securityContext, name, fieldsParam, include, includeRelations); } @GET diff --git a/openmetadata-ui/src/main/resources/ui/src/rest/testAPI.test.ts b/openmetadata-ui/src/main/resources/ui/src/rest/testAPI.test.ts index 53de6da5e453..d24118da7206 100644 --- a/openmetadata-ui/src/main/resources/ui/src/rest/testAPI.test.ts +++ b/openmetadata-ui/src/main/resources/ui/src/rest/testAPI.test.ts @@ -883,7 +883,7 @@ describe('testAPI tests', () => { }); describe('getTestSuiteByName', () => { - it('should fetch test suite by name with params', async () => { + it('should default owners/experts to non-deleted so soft-deleted owners stay hidden (issue #30117)', async () => { const mockGet = jest.fn().mockResolvedValue({ data: mockTestSuite }); jest.mock('./index', () => ({ __esModule: true, @@ -894,17 +894,42 @@ describe('testAPI tests', () => { const { getTestSuiteByName } = require('./testAPI'); - const params = { fields: ['owners', 'tests'] }; - - const result = await getTestSuiteByName('test.suite.name', params); + const result = await getTestSuiteByName('test.suite.name', { + fields: ['owners', 'tests'], + }); expect(mockGet).toHaveBeenCalledWith( expect.stringContaining('/dataQuality/testSuites/name/'), - { params } + { + params: { + fields: ['owners', 'tests'], + includeRelations: 'owners:non-deleted,experts:non-deleted', + }, + } ); expect(result).toEqual(mockTestSuite); }); + it('should respect a caller-provided includeRelations', async () => { + const mockGet = jest.fn().mockResolvedValue({ data: mockTestSuite }); + jest.mock('./index', () => ({ + __esModule: true, + default: { + get: mockGet, + }, + })); + + const { getTestSuiteByName } = require('./testAPI'); + + await getTestSuiteByName('test.suite.name', { + includeRelations: 'owners:all', + }); + + expect(mockGet).toHaveBeenCalledWith(expect.any(String), { + params: { includeRelations: 'owners:all' }, + }); + }); + it('should encode FQN with special characters', async () => { const mockGet = jest.fn().mockResolvedValue({ data: mockTestSuite }); jest.mock('./index', () => ({ diff --git a/openmetadata-ui/src/main/resources/ui/src/rest/testAPI.ts b/openmetadata-ui/src/main/resources/ui/src/rest/testAPI.ts index fdee07369529..408915c0485a 100644 --- a/openmetadata-ui/src/main/resources/ui/src/rest/testAPI.ts +++ b/openmetadata-ui/src/main/resources/ui/src/rest/testAPI.ts @@ -413,7 +413,17 @@ export const getTestSuiteByName = async ( ) => { const response = await APIClient.get( `${testSuiteUrl}/name/${getEncodedFqn(name)}`, - { params } + { + // Resolve owners/experts as non-deleted (consistent with data-asset detail pages), so a + // soft-deleted owner is not surfaced and the owner-edit diff stays aligned with the server's + // NON_DELETED PATCH base - otherwise a positional patch throws "array item index out of + // range". See issue #30117. + params: { + ...params, + includeRelations: + params?.includeRelations ?? 'owners:non-deleted,experts:non-deleted', + }, + } ); return response.data;