From b86a9df36e4f4dfbef33f758fbf7b0f7317b8b80 Mon Sep 17 00:00:00 2001 From: viacheslav_kolesnyk Date: Wed, 15 Jul 2026 16:43:52 +0200 Subject: [PATCH 1/2] fix(access-types): Fix updater not returned on get collection Fix user not being queried for updatedByUserId Closes: MODKBEKBJ-842 --- .../accesstypes/AccessTypesServiceImpl.java | 2 +- ...oldingsAccessTypesImplIntegrationTest.java | 33 ++++--- .../AccessTypesServiceImplTest.java | 97 +++++++++++++++++++ .../org/folio/util/AccessTypesTestUtil.java | 11 ++- 4 files changed, 126 insertions(+), 17 deletions(-) create mode 100644 src/test/java/org/folio/service/accesstypes/AccessTypesServiceImplTest.java diff --git a/src/main/java/org/folio/service/accesstypes/AccessTypesServiceImpl.java b/src/main/java/org/folio/service/accesstypes/AccessTypesServiceImpl.java index 86b6f34c..bec0d040 100644 --- a/src/main/java/org/folio/service/accesstypes/AccessTypesServiceImpl.java +++ b/src/main/java/org/folio/service/accesstypes/AccessTypesServiceImpl.java @@ -239,7 +239,7 @@ private CompletableFuture> populateUserMetadata(Map accessTypes) { var usersIds = accessTypes.stream() .map(AccessType::getMetadata) - .map(metadata -> Arrays.asList(metadata.getCreatedByUserId(), metadata.getUpdatedByUsername())) + .map(metadata -> Arrays.asList(metadata.getCreatedByUserId(), metadata.getUpdatedByUserId())) .flatMap(List::stream) .filter(Objects::nonNull) .distinct() diff --git a/src/test/java/org/folio/rest/impl/EholdingsAccessTypesImplIntegrationTest.java b/src/test/java/org/folio/rest/impl/EholdingsAccessTypesImplIntegrationTest.java index 5ad279af..1d420f50 100644 --- a/src/test/java/org/folio/rest/impl/EholdingsAccessTypesImplIntegrationTest.java +++ b/src/test/java/org/folio/rest/impl/EholdingsAccessTypesImplIntegrationTest.java @@ -1,5 +1,8 @@ package org.folio.rest.impl; +import static com.github.tomakehurst.wiremock.client.WireMock.containing; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import static org.folio.HttpStatus.SC_BAD_REQUEST; import static org.folio.HttpStatus.SC_CREATED; import static org.folio.HttpStatus.SC_NOT_FOUND; @@ -9,6 +12,7 @@ import static org.folio.repository.accesstypes.AccessTypeMappingsTableConstants.ACCESS_TYPES_MAPPING_TABLE_NAME; import static org.folio.repository.accesstypes.AccessTypesTableConstants.ACCESS_TYPES_TABLE_NAME; import static org.folio.repository.kbcredentials.KbCredentialsTableConstants.KB_CREDENTIALS_TABLE_NAME; +import static org.folio.service.users.UsersLookUpService.USERS_ENDPOINT; import static org.folio.util.AccessTypesTestUtil.ACCESS_TYPES_PATH; import static org.folio.util.AccessTypesTestUtil.KB_CREDENTIALS_ACCESS_TYPES_ENDPOINT; import static org.folio.util.AccessTypesTestUtil.KB_CREDENTIALS_ACCESS_TYPE_ID_ENDPOINT; @@ -71,23 +75,26 @@ void tearDown() { @Test void shouldReturnAccessTypeCollectionOnGet() { - List testAccessTypes = testData(credentialsId); - final String id0 = insertAccessType(testAccessTypes.get(0), vertx); - final String id1 = insertAccessType(testAccessTypes.get(1), vertx); + var testAccessTypes = testData(credentialsId); + // First access type is created by John and last updated by Jane (both served by the mocked /users lookup). + final var id0 = insertAccessType(testAccessTypes.get(0), JOHN_ID, JANE_ID, vertx); + final var id1 = insertAccessType(testAccessTypes.get(1), vertx); - AccessTypeCollection actual = getWithStatus(ACCESS_TYPES_PATH, SC_OK) - .as(AccessTypeCollection.class); + var actual = getWithStatus(ACCESS_TYPES_PATH, SC_OK).as(AccessTypeCollection.class); assertEquals(Integer.valueOf(2), actual.getMeta().getTotalResults()); assertEquals(2, actual.getData().size()); - assertThat(actual.getData().get(0), allOf( - hasProperty("id", equalTo(id0)), - allOf(hasProperty("attributes", notNullValue()), hasProperty("metadata", notNullValue())) - )); - assertThat(actual.getData().get(1), allOf( - hasProperty("id", equalTo(id1)), - allOf(hasProperty("attributes", notNullValue()), hasProperty("metadata", notNullValue())) - )); + var first = actual.getData().get(0); + assertThat(first, allOf(hasProperty("id", equalTo(id0)), + hasProperty("attributes", notNullValue()), hasProperty("metadata", notNullValue()))); + assertThat(actual.getData().get(1), allOf(hasProperty("id", equalTo(id1)), + hasProperty("attributes", notNullValue()), hasProperty("metadata", notNullValue()))); + assertEquals("John", first.getCreator().getFirstName()); + assertEquals("Doe", first.getCreator().getLastName()); + assertEquals("firstname_test", first.getUpdater().getFirstName()); + assertEquals("lastname_test", first.getUpdater().getLastName()); + // Regression guard: the updater's id (updatedByUserId) must be part of the /users query. + wm.verify(getRequestedFor(urlPathEqualTo(USERS_ENDPOINT)).withQueryParam("query", containing(JANE_ID))); } @Test diff --git a/src/test/java/org/folio/service/accesstypes/AccessTypesServiceImplTest.java b/src/test/java/org/folio/service/accesstypes/AccessTypesServiceImplTest.java new file mode 100644 index 00000000..16139530 --- /dev/null +++ b/src/test/java/org/folio/service/accesstypes/AccessTypesServiceImplTest.java @@ -0,0 +1,97 @@ +package org.folio.service.accesstypes; + +import static java.util.concurrent.CompletableFuture.completedFuture; +import static org.assertj.core.api.Assertions.assertThat; +import static org.folio.util.TestUtil.result; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; +import java.util.UUID; +import org.apache.commons.collections4.map.CaseInsensitiveMap; +import org.folio.holdingsiq.model.RequestContext; +import org.folio.okapi.common.XOkapiHeaders; +import org.folio.repository.accesstypes.AccessTypesRepository; +import org.folio.repository.accesstypes.DbAccessType; +import org.folio.rest.jaxrs.model.AccessType; +import org.folio.rest.jaxrs.model.AccessTypeCollection; +import org.folio.rest.jaxrs.model.Metadata; +import org.folio.service.users.User; +import org.folio.service.users.UsersLookUpService; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.springframework.core.convert.converter.Converter; + +@MockitoSettings(strictness = Strictness.WARN) +@ExtendWith(MockitoExtension.class) +class AccessTypesServiceImplTest { + + private static final String TENANT_ID = "test"; + private static final Map HEADERS = new CaseInsensitiveMap<>(Map.of( + XOkapiHeaders.TENANT, TENANT_ID, + XOkapiHeaders.URL, "http://test.url" + )); + + @InjectMocks + private AccessTypesServiceImpl accessTypesService; + + @Mock + private UsersLookUpService usersLookUpService; + @Mock + private AccessTypesRepository repository; + @Mock + private Converter, AccessTypeCollection> accessTypeCollectionConverter; + @Mock + private Converter accessTypeFromDbConverter; + + @Test + void findByCredentialsIdShouldPopulateUpdaterFromUpdatedByUserId() { + var credentialsId = UUID.randomUUID(); + var creatorId = UUID.randomUUID(); + var updaterId = UUID.randomUUID(); + + // Access type created by one user and last updated by a different user. + var accessType = new AccessType().withMetadata(new Metadata() + .withCreatedByUserId(creatorId.toString()) + .withUpdatedByUserId(updaterId.toString())); + var dbAccessType = DbAccessType.builder().id(UUID.randomUUID()).credentialsId(credentialsId).build(); + when(repository.findByCredentialsId(credentialsId, TENANT_ID)) + .thenReturn(completedFuture(List.of(dbAccessType))); + when(accessTypeFromDbConverter.convert(dbAccessType)).thenReturn(accessType); + when(usersLookUpService.lookUpUsers(anyList(), any(RequestContext.class))) + .thenReturn(completedFuture(List.of(user(creatorId, "creator"), user(updaterId, "updater")))); + when(accessTypeCollectionConverter.convert(anyList())).thenReturn(new AccessTypeCollection()); + + result(accessTypesService.findByCredentialsId(credentialsId.toString(), HEADERS)); + + // The updater's id must be part of the users lookup - regression guard for the + // updatedByUsername/updatedByUserId mix-up that left updater unresolved. + var idsCaptor = ArgumentCaptor.forClass(List.class); + verify(usersLookUpService).lookUpUsers(idsCaptor.capture(), any(RequestContext.class)); + assertThat(idsCaptor.getValue()).contains(creatorId, updaterId); + // Enrichment mutates the access types in place before they are converted to the collection. + assertThat(accessType.getUpdater()).isNotNull() + .extracting("firstName", "lastName").containsExactly("updater_firstname", "updater_lastname"); + assertThat(accessType.getMetadata().getUpdatedByUsername()).isEqualTo("updater"); + assertThat(accessType.getCreator()).isNotNull() + .extracting("firstName").isEqualTo("creator_firstname"); + } + + private User user(UUID id, String prefix) { + return User.builder() + .id(id.toString()) + .userName(prefix) + .firstName(prefix + "_firstname") + .lastName(prefix + "_lastname") + .build(); + } +} diff --git a/src/test/java/org/folio/util/AccessTypesTestUtil.java b/src/test/java/org/folio/util/AccessTypesTestUtil.java index e18da156..76ffa4d8 100644 --- a/src/test/java/org/folio/util/AccessTypesTestUtil.java +++ b/src/test/java/org/folio/util/AccessTypesTestUtil.java @@ -80,6 +80,11 @@ public static List insertAccessTypes(List items, Vertx v } public static String insertAccessType(AccessType accessType, Vertx vertx) { + return insertAccessType(accessType, JOHN_ID, null, vertx); + } + + public static String insertAccessType(AccessType accessType, String createdByUserId, String updatedByUserId, + Vertx vertx) { CompletableFuture> future = new CompletableFuture<>(); String query = prepareQuery(upsertAccessTypeQuery(), accessTypesTestTable()); @@ -91,9 +96,9 @@ public static String insertAccessType(AccessType accessType, Vertx vertx) { accessType.getAttributes().getName(), accessType.getAttributes().getDescription(), OffsetDateTime.now(), - toUUID(JOHN_ID), - null, - null + toUUID(createdByUserId), + updatedByUserId == null ? null : OffsetDateTime.now(), + toUUID(updatedByUserId) ); PostgresClient.getInstance(vertx).execute(query, params, event -> future.complete(null)); From 15cf9a198e242c01f67aa1c503e660ecf868169a Mon Sep 17 00:00:00 2001 From: viacheslav_kolesnyk Date: Thu, 16 Jul 2026 13:10:36 +0200 Subject: [PATCH 2/2] Update NEWS --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index cbc4eee0..4c62d5e0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -13,6 +13,7 @@ ### Bug fixes * Fix offset handling when retrieving holdings from HoldingsIQ. ([MODKBEKBJ-825](https://folio-org.atlassian.net/browse/MODKBEKBJ-825)) * Fix error messages format received from HoldingsIQ. ([MODKBEKBJ-826](https://folio-org.atlassian.net/browse/MODKBEKBJ-826)) +* Fix updater not returned on get access-types collection. ([MODKBEKBJ-842](https://folio-org.atlassian.net/browse/MODKBEKBJ-842)) ### Tech Dept * Description ([ISSUE](https://folio-org.atlassian.net/browse/ISSUE))