MODDCB-86 Handling invalid barcode issue in Lender role (#70)

* MODDCB-86 Handling invalid barcode issue in Lender role * MODDCB-86 Restricting cancellation from CLOSED status
folio-org · Dec 18, 2023 · a30d79e · a30d79e
1 parent cd1f6b1
commit a30d79e
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 46 deletions.
diff --git a/src/main/java/org/folio/dcb/client/feign/InventoryItemStorageClient.java b/src/main/java/org/folio/dcb/client/feign/InventoryItemStorageClient.java
@@ -5,14 +5,11 @@
 import org.folio.spring.model.ResultList;
 import org.springframework.cloud.openfeign.FeignClient;
 import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RequestParam;
 
 @FeignClient(name = "item-storage", configuration = FeignClientConfiguration.class)
 public interface InventoryItemStorageClient {
-  @GetMapping("/items/{itemId}")
-  InventoryItem findItem(@PathVariable("itemId") String itemId);
 
   @GetMapping("/items")
-  ResultList<InventoryItem> fetchItemByBarcode(@RequestParam("query") String query);
+  ResultList<InventoryItem> fetchItemByQuery(@RequestParam("query") String query);
 }
diff --git a/src/main/java/org/folio/dcb/controller/ExceptionHandlingController.java b/src/main/java/org/folio/dcb/controller/ExceptionHandlingController.java
@@ -7,6 +7,7 @@
 import org.folio.spring.exception.NotFoundException;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.converter.HttpMessageNotReadableException;
+import org.springframework.web.bind.MethodArgumentNotValidException;
 import org.springframework.web.bind.MissingServletRequestParameterException;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.ResponseStatus;
@@ -75,7 +76,8 @@ public Errors handleBadGatewayException(FeignException.BadGateway ex) {
     HttpMessageNotReadableException.class,
     IllegalArgumentException.class,
     StatusException.class,
-    FeignException.BadRequest.class
+    FeignException.BadRequest.class,
+    MethodArgumentNotValidException.class
   })
   public Errors handleValidationErrors(Exception ex) {
     logExceptionMessage(ex);

diff --git a/src/main/java/org/folio/dcb/service/ItemService.java b/src/main/java/org/folio/dcb/service/ItemService.java
@@ -4,12 +4,6 @@
 import org.folio.spring.model.ResultList;
 
 public interface ItemService {
-  /**
-   * Get item details of an inventory by itemId
-   * @param itemId - id of an item
-   * @return InventoryItem
-   */
-  InventoryItem fetchItemDetailsById(String itemId);
 
   /**
    * Provides material type Id by material type name.
@@ -24,4 +18,11 @@ public interface ItemService {
    * @return InventoryItem
    */
   ResultList<InventoryItem> fetchItemByBarcode(String itemBarcode);
+  /**
+   * Get item details of an inventory by item id and Barcode
+   * @param itemBarcode - barcode of an item
+   * @param id - id of an item
+   * @return InventoryItem
+   */
+  InventoryItem fetchItemByIdAndBarcode(String id, String itemBarcode);
 }
diff --git a/src/main/java/org/folio/dcb/service/impl/ItemServiceImpl.java b/src/main/java/org/folio/dcb/service/impl/ItemServiceImpl.java
@@ -1,6 +1,5 @@
 package org.folio.dcb.service.impl;
 
-import feign.FeignException;
 import lombok.AllArgsConstructor;
 import lombok.extern.log4j.Log4j2;
 import org.folio.dcb.client.feign.InventoryItemStorageClient;
@@ -19,16 +18,6 @@ public class ItemServiceImpl implements ItemService {
   private final InventoryItemStorageClient inventoryItemStorageClient;
   private final MaterialTypeClient materialTypeClient;
 
-  @Override
-  public InventoryItem fetchItemDetailsById(String itemId) {
-    log.debug("fetchItemDetailsById:: Trying to fetch item details for itemId {}", itemId);
-    try {
-      return inventoryItemStorageClient.findItem(itemId);
-    } catch (FeignException.NotFound ex) {
-      throw new NotFoundException(String.format("Item not found for itemId %s ", itemId));
-    }
-  }
-
   @Override
   public String fetchItemMaterialTypeIdByMaterialTypeName(String materialTypeName) {
     log.debug("fetchItemMaterialTypeIdByMaterialTypeName:: Fetching ItemMaterialTypeId by MaterialTypeName={}", materialTypeName);
@@ -43,7 +32,17 @@ public String fetchItemMaterialTypeIdByMaterialTypeName(String materialTypeName)
   @Override
   public ResultList<InventoryItem> fetchItemByBarcode(String itemBarcode) {
     log.debug("fetchItemByBarcode:: fetching item details for barcode {} ", itemBarcode);
-    return inventoryItemStorageClient.fetchItemByBarcode("barcode==" + itemBarcode);
+    return inventoryItemStorageClient.fetchItemByQuery("barcode==" + itemBarcode);
+  }
+
+  @Override
+  public InventoryItem fetchItemByIdAndBarcode(String id, String barcode) {
+    log.debug("fetchItemByBarcode:: fetching item details for id {} , barcode {} ", id, barcode);
+    return inventoryItemStorageClient.fetchItemByQuery("barcode==" + barcode + " and id==" + id)
+      .getResult()
+      .stream()
+      .findFirst()
+      .orElseThrow(() -> new NotFoundException(String.format("Unable to find existing item with id %s and barcode %s.", id, barcode)));
   }
 
 }
diff --git a/src/main/java/org/folio/dcb/service/impl/RequestServiceImpl.java b/src/main/java/org/folio/dcb/service/impl/RequestServiceImpl.java
@@ -34,7 +34,7 @@ public class RequestServiceImpl implements RequestService {
   public CirculationRequest createPageItemRequest(User user, DcbItem item, String pickupServicePointId) {
     log.debug("createPageItemRequest:: creating a new page request for userBarcode {} , itemBarcode {}",
       user.getBarcode(), item.getBarcode());
-    var inventoryItem = itemService.fetchItemDetailsById(item.getId());
+    var inventoryItem = itemService.fetchItemByIdAndBarcode(item.getId(), item.getBarcode());
     var inventoryHolding = holdingsService.fetchInventoryHoldingDetailsByHoldingId(inventoryItem.getHoldingsRecordId());
     var circulationRequest = createCirculationRequest(PAGE, user, item, inventoryItem.getHoldingsRecordId(), inventoryHolding.getInstanceId(), pickupServicePointId);
     return circulationClient.createRequest(circulationRequest);

diff --git a/src/main/java/org/folio/dcb/service/impl/TransactionsServiceImpl.java b/src/main/java/org/folio/dcb/service/impl/TransactionsServiceImpl.java
@@ -53,8 +53,9 @@ public TransactionStatusResponse updateTransactionStatus(String dcbTransactionId
           "Current transaction status equal to new transaction status: dcbTransactionId: %s, status: %s", dcbTransactionId, transactionStatus.getStatus()
         ));
       } else if (transactionStatus.getStatus() == TransactionStatus.StatusEnum.CANCELLED
-                  && (dcbTransaction.getStatus() == TransactionStatus.StatusEnum.ITEM_CHECKED_IN ||
-                        dcbTransaction.getStatus() == TransactionStatus.StatusEnum.ITEM_CHECKED_OUT)) {
+        && (dcbTransaction.getStatus() == TransactionStatus.StatusEnum.ITEM_CHECKED_IN ||
+        dcbTransaction.getStatus() == TransactionStatus.StatusEnum.ITEM_CHECKED_OUT) ||
+        dcbTransaction.getStatus() == TransactionStatus.StatusEnum.CLOSED) {
         throw new StatusException(String.format(
           "Cannot cancel transaction dcbTransactionId: %s. Transaction already in status: %s: ", dcbTransactionId, dcbTransaction.getStatus()
         ));

diff --git a/src/main/resources/swagger.api/schemas/InventoryItem.yaml b/src/main/resources/swagger.api/schemas/InventoryItem.yaml
@@ -5,6 +5,9 @@ InventoryItem:
     id:
       description: Item id
       $ref: "uuid.yaml"
+    barcode:
+      description: Barcode of the item
+      type: string
     holdingsRecordId:
       description: ID of the holdings record the item is a member of
       $ref: "uuid.yaml"

diff --git a/src/test/java/org/folio/dcb/controller/TransactionApiControllerTest.java b/src/test/java/org/folio/dcb/controller/TransactionApiControllerTest.java
@@ -89,6 +89,40 @@ void createLendingCirculationRequestTest() throws Exception {
     );
   }
 
+  @Test
+  void createLendingCirculationRequestWithValidIdAndInvalidBarcode() throws Exception {
+    removeExistedTransactionFromDbIfSoExists();
+    var dcbTransaction = createDcbTransactionByRole(LENDER);
+    dcbTransaction.getItem().setBarcode("DCB_ITEM1");
+
+    this.mockMvc.perform(
+        post("/transactions/" + DCB_TRANSACTION_ID)
+          .content(asJsonString(dcbTransaction))
+          .headers(defaultHeaders())
+          .contentType(MediaType.APPLICATION_JSON)
+          .accept(MediaType.APPLICATION_JSON))
+      .andExpectAll(status().is4xxClientError(),
+        jsonPath("$.errors[0].code", is("NOT_FOUND_ERROR")));
+
+  }
+
+  @Test
+  void createCirculationRequestWithInvalidUUID() throws Exception {
+    removeExistedTransactionFromDbIfSoExists();
+    var dcbTransaction = createDcbTransactionByRole(LENDER);
+    //Setting a non UUID for itemId
+    dcbTransaction.getItem().setId("1234");
+
+    this.mockMvc.perform(
+        post("/transactions/" + DCB_TRANSACTION_ID)
+          .content(asJsonString(dcbTransaction))
+          .headers(defaultHeaders())
+          .contentType(MediaType.APPLICATION_JSON)
+          .accept(MediaType.APPLICATION_JSON))
+      .andExpectAll(status().is4xxClientError(),
+        jsonPath("$.errors[0].code", is("VALIDATION_ERROR")));
+  }
+
   @Test
   void createBorrowingPickupCirculationRequestTest() throws Exception {
     removeExistedTransactionFromDbIfSoExists();
@@ -227,6 +261,38 @@ void transactionStatusUpdateFromOpenToCheckedInTest() throws Exception {
       .andExpect(status().isOk())
       .andExpect(jsonPath("$.status").value("ITEM_CHECKED_IN"));
   }
+
+  @Test
+  void transactionStatusCancelledFromClosedTest() throws Exception {
+
+    var transactionID = UUID.randomUUID().toString();
+    var dcbTransaction = createTransactionEntity();
+    dcbTransaction.setStatus(TransactionStatus.StatusEnum.OPEN);
+    dcbTransaction.setRole(BORROWER);
+    dcbTransaction.setId(transactionID);
+
+    systemUserScopedExecutionService.executeAsyncSystemUserScoped(TENANT, () -> transactionRepository.save(dcbTransaction));
+
+    this.mockMvc.perform(
+        put("/transactions/" + transactionID + "/status")
+          .content(asJsonString(createTransactionStatus(TransactionStatus.StatusEnum.CLOSED)))
+          .headers(defaultHeaders())
+          .contentType(MediaType.APPLICATION_JSON)
+          .accept(MediaType.APPLICATION_JSON))
+      .andExpect(status().isOk())
+      .andExpect(jsonPath("$.status").value("CLOSED"));
+
+    // Trying to Cancel the closed transaction
+    this.mockMvc.perform(
+        put("/transactions/" + transactionID + "/status")
+          .content(asJsonString(createTransactionStatus(TransactionStatus.StatusEnum.CANCELLED)))
+          .headers(defaultHeaders())
+          .contentType(MediaType.APPLICATION_JSON)
+          .accept(MediaType.APPLICATION_JSON))
+      .andExpectAll(status().is4xxClientError(),
+        jsonPath("$.errors[0].code", is("VALIDATION_ERROR")));
+  }
+
   @Test
   void transactionStatusUpdateFromCreatedToOpenForPickupLibTest() throws Exception {
 
@@ -794,9 +860,7 @@ private void removeExistedTransactionFromDbIfSoExists() {
   private void removeExistingTransactionsByItemId(String itemId) {
     systemUserScopedExecutionService.executeAsyncSystemUserScoped(TENANT, () ->
         transactionRepository.findTransactionsByItemIdAndStatusNotInClosed(UUID.fromString(itemId))
-          .forEach(transactionEntity -> {
-            transactionRepository.deleteById(transactionEntity.getId());
-          })
+          .forEach(transactionEntity -> transactionRepository.deleteById(transactionEntity.getId()))
     );
   }
 }
diff --git a/src/test/java/org/folio/dcb/service/InventoryItemServiceTest.java b/src/test/java/org/folio/dcb/service/InventoryItemServiceTest.java
@@ -1,6 +1,5 @@
 package org.folio.dcb.service;
 
-import feign.FeignException;
 import org.folio.dcb.client.feign.InventoryItemStorageClient;
 import org.folio.dcb.service.impl.ItemServiceImpl;
 import org.folio.spring.exception.NotFoundException;
@@ -18,8 +17,6 @@
 import static org.folio.dcb.utils.EntityUtils.createInventoryItem;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
@@ -31,27 +28,30 @@ class InventoryItemServiceTest {
   private InventoryItemStorageClient inventoryItemStorageClient;
 
   @Test
-  void fetchItemDetailsByIdTest() {
+  void fetchItemDetailsByIdAndBarcodeTest() {
     var itemId = UUID.randomUUID().toString();
+    var barcode = "DCB_ITEM";
     var inventoryItem = createInventoryItem();
-    when(inventoryItemStorageClient.findItem(itemId)).thenReturn(inventoryItem);
-    var response = itemService.fetchItemDetailsById(itemId);
-    verify(inventoryItemStorageClient).findItem(itemId);
+    when(inventoryItemStorageClient.fetchItemByQuery("barcode==" + barcode + " and id==" + itemId)).thenReturn(ResultList.of(1, List.of(inventoryItem)));
+
+    var response = itemService.fetchItemByIdAndBarcode(itemId, barcode);
     assertEquals(inventoryItem, response);
   }
 
   @Test
-  void fetchItemDetailsByInvalidIdTest() {
+  void fetchItemByIdAndInvalidBarcode() {
     var itemId = UUID.randomUUID().toString();
-    doThrow(FeignException.NotFound.class).when(inventoryItemStorageClient).findItem(itemId);
-    assertThrows(NotFoundException.class, () -> itemService.fetchItemDetailsById(itemId));
+    var barcode = "DCB_ITEM";
+    when(inventoryItemStorageClient.fetchItemByQuery("barcode==" + barcode + " and id==" + itemId)).thenReturn(ResultList.of(0, List.of()));
+
+    assertThrows(NotFoundException.class, () -> itemService.fetchItemByIdAndBarcode(itemId, barcode));
   }
 
   @Test
   void fetchItemByBarcode() {
     var item = createDcbItem();
     var inventoryItem = createInventoryItem();
-    when(itemService.fetchItemByBarcode(item.getBarcode())).thenReturn(ResultList.of(1, List.of(inventoryItem)));
+    when(inventoryItemStorageClient.fetchItemByQuery("barcode==" + item.getBarcode())).thenReturn(ResultList.of(1, List.of(inventoryItem)));
 
     var response = itemService.fetchItemByBarcode(item.getBarcode());
     assertEquals(1, response.getTotalRecords());

diff --git a/src/test/java/org/folio/dcb/service/RequestServiceTest.java b/src/test/java/org/folio/dcb/service/RequestServiceTest.java
@@ -33,10 +33,10 @@ class RequestServiceTest {
 
   @Test
   void createPageItemRequestTest() {
-    when(itemService.fetchItemDetailsById(any())).thenReturn(createInventoryItem());
+    when(itemService.fetchItemByIdAndBarcode(any(), any())).thenReturn(createInventoryItem());
     when(holdingsService.fetchInventoryHoldingDetailsByHoldingId(any())).thenReturn(createInventoryHolding());
     requestService.createPageItemRequest(createUser(), createDcbItem(), createDcbPickup().getServicePointId());
-    verify(itemService).fetchItemDetailsById(any());
+    verify(itemService).fetchItemByIdAndBarcode(any(), any());
     verify(holdingsService).fetchInventoryHoldingDetailsByHoldingId(any());
     verify(circulationClient).createRequest(any());
   }

diff --git a/src/test/java/org/folio/dcb/utils/EntityUtils.java b/src/test/java/org/folio/dcb/utils/EntityUtils.java
@@ -195,6 +195,7 @@ public static InventoryItem createInventoryItem() {
     return InventoryItem.builder()
       .id(UUID.randomUUID().toString())
       .holdingsRecordId(UUID.randomUUID().toString())
+      .barcode("DCB_ITEM")
       .build();
   }
 

diff --git a/src/test/resources/mappings/inventory.json b/src/test/resources/mappings/inventory.json
@@ -3,11 +3,11 @@
     {
       "request": {
         "method": "GET",
-        "url": "/item-storage/items/5b95877d-86c0-4cb7-a0cd-7660b348ae5a"
+        "url": "/item-storage/items?query=barcode%3D%3DDCB_ITEM%20and%20id%3D%3D5b95877d-86c0-4cb7-a0cd-7660b348ae5a"
       },
       "response": {
         "status": 200,
-        "body": "{\"id\":\"5b95877d-86c0-4cb7-a0cd-7660b348ae5a\",\"holdingsRecordId\": \"fb7b70f1-b898-4924-a991-0e4b6312bb5f\",\"status\": {\n \"name\": \"Available\",\n \"date\": \"2023-09-21T01:49:50.620+00:00\"\n}}",
+        "body": "{\"items\": [{ \"id\":\"5b95877d-86c0-4cb7-a0cd-7660b348ae5a\",\"barcode\":\"DCB_ITEM\",\"holdingsRecordId\": \"fb7b70f1-b898-4924-a991-0e4b6312bb5f\",\"status\": {\n \"name\": \"Available\",\n \"date\": \"2023-09-21T01:49:50.620+00:00\"\n}}],\n\"totalRecords\": 1,\n    \"resultInfo\": {\n        \"totalRecords\": 1,\n        \"facets\": [],\n        \"diagnostics\": []\n    }}",
         "headers": {
           "Content-Type": "application/json"
         }
@@ -16,10 +16,11 @@
     {
       "request": {
         "method": "GET",
-        "url": "/item-storage/items/5b95877d-86c0-4cb7-a0cd-7660b348ae5b"
+        "url": "/item-storage/items?query=barcode%3D%3DDCB_ITEM1%20and%20id%3D%3D5b95877d-86c0-4cb7-a0cd-7660b348ae5a"
       },
       "response": {
-        "status": 404,
+        "status": 200,
+        "body": "{\n\"items\": [],\n\"totalRecords\": 0,\n \"resultInfo\": {\n \"totalRecords\": 0,\n \"facets\": [],\n  \"diagnostics\": []\n    }\n}",
         "headers": {
           "Content-Type": "application/json"
         }