FINERACT-1926: Fix wrong type of transfer in cancellation event + fix NPE

fineract-investor/src/main/java/org/apache/fineract/investor/cob/loan/LoanAccountOwnerTransferBusinessStep.java

@@ -28,6 +28,7 @@ import org.apache.fineract.cob.loan.LoanCOBBusinessStep;
 import org.apache.fineract.infrastructure.core.service.DateUtils;
 import org.apache.fineract.infrastructure.event.business.domain.loan.LoanAccountSnapshotBusinessEvent;
 import org.apache.fineract.infrastructure.event.business.service.BusinessEventNotifierService;
+import org.apache.fineract.interoperation.util.MathUtil;
 import org.apache.fineract.investor.config.InvestorModuleIsEnabledCondition;
 import org.apache.fineract.investor.data.ExternalTransferStatus;
 import org.apache.fineract.investor.data.ExternalTransferSubStatus;
@@ -135,7 +136,7 @@ public class LoanAccountOwnerTransferBusinessStep implements LoanCOBBusinessStep
             accountingService.createJournalEntriesForSaleAssetTransfer(loan, newExternalAssetOwnerTransfer);
         } else {
             ExternalTransferSubStatus subStatus = ExternalTransferSubStatus.BALANCE_ZERO;
-            if (loan.getTotalOverpaid().compareTo(BigDecimal.ZERO) > 0) {
+            if (MathUtil.nullToDefault(loan.getTotalOverpaid(), BigDecimal.ZERO).compareTo(BigDecimal.ZERO) > 0) {
                 subStatus = ExternalTransferSubStatus.BALANCE_NEGATIVE;
             }
             newExternalAssetOwnerTransfer = createNewEntry(settlementDate, externalAssetOwnerTransfer, ExternalTransferStatus.DECLINED,
@@ -169,7 +170,7 @@ public class LoanAccountOwnerTransferBusinessStep implements LoanCOBBusinessStep
     }
 
     private boolean isTransferable(final Loan loan) {
-        return loan.getLoanSummary().getTotalOutstanding().compareTo(BigDecimal.ZERO) > 0;
+        return MathUtil.nullToDefault(loan.getLoanSummary().getTotalOutstanding(), BigDecimal.ZERO).compareTo(BigDecimal.ZERO) > 0;
     }
 
     private void handleSameDaySaleAndBuyback(final LocalDate settlementDate, final List<ExternalAssetOwnerTransfer> transferDataList,

fineract-investor/src/main/java/org/apache/fineract/investor/data/ExternalTransferData.java

@@ -32,6 +32,7 @@ public class ExternalTransferData {
     private String purchasePriceRatio;
     private LocalDate settlementDate;
     private ExternalTransferStatus status;
+    private ExternalTransferSubStatus subStatus;
     private LocalDate effectiveFrom;
     private LocalDate effectiveTo;
 }

fineract-investor/src/main/java/org/apache/fineract/investor/domain/ExternalAssetOwnerTransferRepository.java

@@ -49,4 +49,6 @@ public interface ExternalAssetOwnerTransferRepository
 
     @Query("SELECT t FROM ExternalAssetOwnerTransfer t WHERE t.loanId = :loanId AND t.effectiveDateTo > :effectiveDate")
     List<ExternalAssetOwnerTransfer> findEffectiveTransfers(@Param("loanId") Long loanId, @Param("effectiveDate") LocalDate effectiveDate);
+
+    Optional<ExternalAssetOwnerTransfer> findFirstByExternalIdOrderByIdAsc(ExternalId externalTransferId);
 }

fineract-investor/src/main/java/org/apache/fineract/investor/exception/ExternalAssetOwnerTransferNotFoundException.java

@@ -31,6 +31,12 @@ public class ExternalAssetOwnerTransferNotFoundException extends AbstractPlatfor
                 externalId.getValue(), externalTransferStatus);
     }
 
+    public ExternalAssetOwnerTransferNotFoundException(ExternalId externalId) {
+        super("error.msg.external.asset.owner.transfer.external.id",
+                String.format("External asset owner transfer with external id: %s does not found", externalId.getValue()),
+                externalId.getValue());
+    }
+
     public ExternalAssetOwnerTransferNotFoundException(Long id) {
         super("error.msg.external.asset.owner.transfer.id", String.format("External asset owner transfer with id: %s does not found", id),
                 id);

fineract-investor/src/main/java/org/apache/fineract/investor/service/ExternalAssetOwnersReadService.java

@@ -18,6 +18,7 @@
  */
 package org.apache.fineract.investor.service;
 
+import org.apache.fineract.infrastructure.core.domain.ExternalId;
 import org.apache.fineract.investor.data.ExternalOwnerJournalEntryData;
 import org.apache.fineract.investor.data.ExternalOwnerTransferJournalEntryData;
 import org.apache.fineract.investor.data.ExternalTransferData;
@@ -25,14 +26,16 @@ import org.springframework.data.domain.Page;
 
 public interface ExternalAssetOwnersReadService {
 
-    Page<ExternalTransferData> retrieveTransferData(Long loanId, String externalLoanId, String transferExternalId, Integer offset,
+    Page<ExternalTransferData> retrieveTransferData(Long loanId, String externalLoanId, String externalTransferId, Integer offset,
             Integer limit);
 
-    ExternalTransferData retrieveTransferData(Long transferExternalId);
+    ExternalTransferData retrieveTransferData(Long transferId);
 
     ExternalTransferData retrieveActiveTransferData(Long loanId, String externalLoanId, String transferExternalId);
 
     ExternalOwnerTransferJournalEntryData retrieveJournalEntriesOfTransfer(Long transferId, Integer offset, Integer limit);
 
     ExternalOwnerJournalEntryData retrieveJournalEntriesOfOwner(String ownerExternalId, Integer offset, Integer limit);
+
+    ExternalTransferData retrieveFirstTransferByExternalId(ExternalId externalTransferId);
 }

fineract-investor/src/main/java/org/apache/fineract/investor/service/ExternalAssetOwnersReadServiceImpl.java

@@ -21,6 +21,7 @@ package org.apache.fineract.investor.service;
 import java.util.Optional;
 import lombok.RequiredArgsConstructor;
 import org.apache.fineract.accounting.journalentry.JournalEntryMapper;
+import org.apache.fineract.infrastructure.core.domain.ExternalId;
 import org.apache.fineract.infrastructure.core.service.ExternalIdFactory;
 import org.apache.fineract.investor.data.ExternalOwnerJournalEntryData;
 import org.apache.fineract.investor.data.ExternalOwnerTransferJournalEntryData;
@@ -53,7 +54,7 @@ public class ExternalAssetOwnersReadServiceImpl implements ExternalAssetOwnersRe
     private final JournalEntryMapper journalEntryMapper;
 
     @Override
-    public Page<ExternalTransferData> retrieveTransferData(Long loanId, String externalLoanId, String transferExternalId, Integer offset,
+    public Page<ExternalTransferData> retrieveTransferData(Long loanId, String externalLoanId, String externalTransferId, Integer offset,
             Integer limit) {
         Page<ExternalAssetOwnerTransfer> result;
         PageRequest pageRequest = getPageRequest(offset, limit);
@@ -61,8 +62,8 @@ public class ExternalAssetOwnersReadServiceImpl implements ExternalAssetOwnersRe
             result = externalAssetOwnerTransferRepository.findAllByLoanId(loanId, pageRequest);
         } else if (externalLoanId != null) {
             result = externalAssetOwnerTransferRepository.findAllByExternalLoanId(ExternalIdFactory.produce(externalLoanId), pageRequest);
-        } else if (transferExternalId != null) {
+        } else if (externalTransferId != null) {
-            result = externalAssetOwnerTransferRepository.findAllByExternalId(ExternalIdFactory.produce(transferExternalId), pageRequest);
+            result = externalAssetOwnerTransferRepository.findAllByExternalId(ExternalIdFactory.produce(externalTransferId), pageRequest);
         } else {
             throw new IllegalArgumentException(
                     "At least one of the following parameters must be provided: loanId, externalLoanId, transferExternalId");
@@ -113,6 +114,18 @@ public class ExternalAssetOwnersReadServiceImpl implements ExternalAssetOwnersRe
         return mappedResult;
     }
 
+    @Override
+    public ExternalTransferData retrieveFirstTransferByExternalId(ExternalId externalTransferId) {
+        return externalAssetOwnerTransferRepository.findFirstByExternalIdOrderByIdAsc(externalTransferId).map(mapper::mapTransfer)
+                .orElseThrow(() -> new ExternalAssetOwnerTransferNotFoundException(externalTransferId));
+    }
+
+    @Override
+    public ExternalTransferData retrieveTransferData(Long transferId) {
+        return externalAssetOwnerTransferRepository.findById(transferId).map(mapper::mapTransfer)
+                .orElseThrow(() -> new ExternalAssetOwnerTransferNotFoundException(transferId));
+    }
+
     private PageRequest getPageRequest(Integer offset, Integer limit) {
         if (offset == null) {
             offset = 0;
@@ -123,10 +136,4 @@ public class ExternalAssetOwnersReadServiceImpl implements ExternalAssetOwnersRe
         return PageRequest.of(offset, limit, Sort.by("id"));
     }
 
-    @Override
-    public ExternalTransferData retrieveTransferData(Long externalTransferId) {
-        return externalAssetOwnerTransferRepository.findById(externalTransferId).map(mapper::mapTransfer)
-                .orElseThrow(() -> new ExternalAssetOwnerTransferNotFoundException(externalTransferId));
-    }
-
 }

fineract-investor/src/main/java/org/apache/fineract/investor/service/serialization/serializer/investor/InvestorBusinessEventSerializer.java

@@ -23,15 +23,11 @@ import static org.apache.fineract.investor.data.ExternalTransferStatus.ACTIVE;
 import static org.apache.fineract.investor.data.ExternalTransferStatus.BUYBACK;
 import static org.apache.fineract.investor.data.ExternalTransferStatus.CANCELLED;
 import static org.apache.fineract.investor.data.ExternalTransferStatus.DECLINED;
-import static org.apache.fineract.investor.data.ExternalTransferSubStatus.BALANCE_NEGATIVE;
-import static org.apache.fineract.investor.data.ExternalTransferSubStatus.BALANCE_ZERO;
-import static org.apache.fineract.investor.data.ExternalTransferSubStatus.SAMEDAY_TRANSFERS;
 
 import java.math.BigDecimal;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import lombok.RequiredArgsConstructor;
 import org.apache.avro.generic.GenericContainer;
 import org.apache.fineract.avro.generator.ByteBufferSerializable;
@@ -42,11 +38,12 @@ import org.apache.fineract.infrastructure.event.business.domain.BusinessEvent;
 import org.apache.fineract.infrastructure.event.external.service.serialization.serializer.BusinessEventSerializer;
 import org.apache.fineract.investor.data.ExternalTransferData;
 import org.apache.fineract.investor.data.ExternalTransferStatus;
-import org.apache.fineract.investor.domain.ExternalAssetOwnerTransfer;
+import org.apache.fineract.investor.data.ExternalTransferSubStatus;
 import org.apache.fineract.investor.domain.InvestorBusinessEvent;
 import org.apache.fineract.investor.service.ExternalAssetOwnersReadService;
 import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
 import org.apache.fineract.portfolio.loanaccount.domain.LoanCharge;
+import org.jetbrains.annotations.NotNull;
 import org.springframework.stereotype.Component;
 
 @Component
@@ -76,15 +73,21 @@ public class InvestorBusinessEventSerializer implements BusinessEventSerializer
     public <T> ByteBufferSerializable toAvroDTO(BusinessEvent<T> rawEvent) {
         InvestorBusinessEvent event = (InvestorBusinessEvent) rawEvent;
         ExternalTransferData transferData = externalAssetOwnersReadService.retrieveTransferData(event.get().getId());
+        String transferType = getType(transferData.getStatus());
+        if (ExternalTransferStatus.DECLINED.equals(transferData.getStatus()) || CANCELLED.equals(transferData.getStatus())) {
+            ExternalTransferData originalTransferData = externalAssetOwnersReadService
+                    .retrieveFirstTransferByExternalId(event.get().getExternalId());
+            transferType = getType(originalTransferData.getStatus());
+        }
 
         LoanOwnershipTransferDataV1.Builder builder = LoanOwnershipTransferDataV1.newBuilder().setLoanId(transferData.getLoan().getLoanId())
                 .setLoanExternalId(transferData.getLoan().getExternalId()).setTransferExternalId(transferData.getTransferExternalId())
                 .setAssetOwnerExternalId(transferData.getOwner().getExternalId())
                 .setPurchasePriceRatio(transferData.getPurchasePriceRatio()).setCurrency(getCurrencyFromEvent(event))
                 .setSettlementDate(transferData.getSettlementDate().format(DEFAULT_DATE_FORMATTER))
-                .setSubmittedDate(transferData.getSettlementDate().format(DEFAULT_DATE_FORMATTER))
+                .setSubmittedDate(transferData.getSettlementDate().format(DEFAULT_DATE_FORMATTER)).setType(transferType)
-                .setType(transferData.getStatus() == BUYBACK ? "BUYBACK" : "SALE").setTransferStatus(getStatus(event.get()))
+                .setTransferStatus(getStatus(transferData.getStatus()))
-                .setTransferStatusReason(getTransferStatusReason(event.get()).orElse(null));
+                .setTransferStatusReason(getTransferStatusReason(transferData.getSubStatus()));
 
         if (transferData.getDetails() != null) {
             builder.setTotalOutstandingBalanceAmount(transferData.getDetails().getTotalOutstanding())
@@ -98,6 +101,11 @@ public class InvestorBusinessEventSerializer implements BusinessEventSerializer
         return builder.build();
     }
 
+    @NotNull
+    private static String getType(ExternalTransferStatus transferStatus) {
+        return transferStatus == BUYBACK ? "BUYBACK" : "SALE";
+    }
+
     private List<UnpaidChargeDataV1> getUnpaidChargeData(InvestorBusinessEvent event) {
         java.util.Map<Long, UnpaidChargeDataV1> map = new HashMap<>();
         event.getLoan().getLoanCharges().forEach(loanCharge -> addToMap(map, loanCharge));
@@ -117,29 +125,21 @@ public class InvestorBusinessEventSerializer implements BusinessEventSerializer
         }
     }
 
-    private String getStatus(ExternalAssetOwnerTransfer externalAssetOwnerTransfer) {
+    private String getStatus(ExternalTransferStatus status) {
-        ExternalTransferStatus status = externalAssetOwnerTransfer.getStatus();
         if (ACTIVE.equals(status) || BUYBACK.equals(status)) {
             return "EXECUTED";
-        } else if (DECLINED.equals(status)) {
+        } else if (DECLINED.equals(status) || CANCELLED.equals(status)) {
-            return "DECLINED";
+            return status.name();
-        } else if (CANCELLED.equals(status)) {
-            return "CANCELLED";
         } else {
             return "UNKNOWN";
         }
     }
 
-    private Optional<String> getTransferStatusReason(ExternalAssetOwnerTransfer externalAssetOwnerTransfer) {
+    private String getTransferStatusReason(ExternalTransferSubStatus subStatus) {
-        ExternalTransferStatus status = externalAssetOwnerTransfer.getStatus();
+        if (subStatus != null) {
-        if (DECLINED.equals(status) && BALANCE_ZERO.equals(externalAssetOwnerTransfer.getSubStatus())) {
+            return subStatus.name();
-            return Optional.of("BALANCE ZERO");
-        } else if (DECLINED.equals(status) && BALANCE_NEGATIVE.equals(externalAssetOwnerTransfer.getSubStatus())) {
-            return Optional.of("BALANCE NEGATIV");
-        } else if (CANCELLED.equals(status) && SAMEDAY_TRANSFERS.equals(externalAssetOwnerTransfer.getSubStatus())) {
-            return Optional.of("SAMEDAY TRANSFER");
         } else {
-            return Optional.empty();
+            return null;
         }
     }
 }

fineract-investor/src/test/java/org/apache/fineract/investor/service/serialization/serializer/investor/InvestorBusinessEventSerializerTest.java

@@ -27,6 +27,7 @@ import static org.apache.fineract.investor.data.ExternalTransferSubStatus.BALANC
 import static org.apache.fineract.investor.data.ExternalTransferSubStatus.SAMEDAY_TRANSFERS;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -40,6 +41,7 @@ import java.util.stream.Collectors;
 import org.apache.fineract.avro.generator.ByteBufferSerializable;
 import org.apache.fineract.avro.loan.v1.LoanOwnershipTransferDataV1;
 import org.apache.fineract.avro.loan.v1.UnpaidChargeDataV1;
+import org.apache.fineract.infrastructure.core.domain.ExternalId;
 import org.apache.fineract.investor.data.ExternalTransferData;
 import org.apache.fineract.investor.data.ExternalTransferDataDetails;
 import org.apache.fineract.investor.data.ExternalTransferLoanData;
@@ -61,7 +63,7 @@ public class InvestorBusinessEventSerializerTest {
     public static final long LOAN_ID = 222L;
     public static final String ASSET_OWNER_EXTERNAL_ID = "1ad87015-8b05-49de-9ed8-e9c214fda7eb";
     public static final String LOAN_EXTERNAL_ID = "29641fe0-0ac6-409a-bb8b-24fdf08d2891";
-    public static final String TRANSFER_EXTERNAL_ID = "ac303982-46a5-4cea-9f71-67b69063a2b7";
+    public static final ExternalId TRANSFER_EXTERNAL_ID = new ExternalId("ac303982-46a5-4cea-9f71-67b69063a2b7");
 
     @Test
     public void testSerializationSellOK() {
@@ -75,24 +77,25 @@ public class InvestorBusinessEventSerializerTest {
 
     @Test
     public void testSerializationDeclinedNegativeBalance() {
-        doTest(DECLINED, BALANCE_NEGATIVE, "SALE", "DECLINED", "BALANCE NEGATIV");
+        doTest(DECLINED, BALANCE_NEGATIVE, "SALE", "DECLINED", "BALANCE_NEGATIVE");
     }
 
     @Test
     public void testSerializationDeclinedBalanceZero() {
-        doTest(DECLINED, BALANCE_ZERO, "SALE", "DECLINED", "BALANCE ZERO");
+        doTest(DECLINED, BALANCE_ZERO, "SALE", "DECLINED", "BALANCE_ZERO");
     }
 
     @Test
     public void testSerializationCancelledSameDayTransfer() {
-        doTest(CANCELLED, SAMEDAY_TRANSFERS, "SALE", "CANCELLED", "SAMEDAY TRANSFER");
+        doTest(CANCELLED, SAMEDAY_TRANSFERS, "SALE", "CANCELLED", "SAMEDAY_TRANSFERS");
     }
 
     private void doTest(ExternalTransferStatus status, ExternalTransferSubStatus subStatus, String expectedType, String expectedStatus,
             String expectedReason) {
         // given
         ExternalAssetOwnersReadService mockReadService = Mockito.mock(ExternalAssetOwnersReadService.class);
-        when(mockReadService.retrieveTransferData(123L)).thenReturn(createTransferData(status));
+        when(mockReadService.retrieveTransferData(123L)).thenReturn(createTransferData(status, subStatus));
+        when(mockReadService.retrieveFirstTransferByExternalId(any(ExternalId.class))).thenReturn(createTransferData(ACTIVE, null));
         Loan loan = Mockito.mock(Loan.class);
         when(loan.getCurrency()).thenReturn(new MonetaryCurrency("EUR", 2, 1));
         List<LoanCharge> loanCharges = createMockCharges();
@@ -138,7 +141,7 @@ public class InvestorBusinessEventSerializerTest {
         assertEquals("1.0", result.getPurchasePriceRatio());
         assertEquals(ASSET_OWNER_EXTERNAL_ID, result.getAssetOwnerExternalId());
         assertEquals(LOAN_EXTERNAL_ID, result.getLoanExternalId());
-        assertEquals(TRANSFER_EXTERNAL_ID, result.getTransferExternalId());
+        assertEquals(TRANSFER_EXTERNAL_ID.getValue(), result.getTransferExternalId());
         assertEquals(LOAN_ID, result.getLoanId());
         assertEquals(new BigDecimal("1108.00000"), result.getTotalOutstandingBalanceAmount());
         assertEquals(new BigDecimal("100.00000"), result.getOutstandingInterestPortion());
@@ -170,10 +173,11 @@ public class InvestorBusinessEventSerializerTest {
         when(mock.getStatus()).thenReturn(status);
         when(mock.getSubStatus()).thenReturn(subStatus);
         when(mock.getId()).thenReturn(123L);
+        when(mock.getExternalId()).thenReturn(new ExternalId("456"));
         return mock;
     }
 
-    private ExternalTransferData createTransferData(ExternalTransferStatus status) {
+    private ExternalTransferData createTransferData(ExternalTransferStatus status, ExternalTransferSubStatus subStatus) {
         ExternalTransferDataDetails details = new ExternalTransferDataDetails();
         details.setDetailsId(444L);
         details.setTotalOutstanding(new BigDecimal("1108.00000"));
@@ -186,13 +190,14 @@ public class InvestorBusinessEventSerializerTest {
         ExternalTransferData data = new ExternalTransferData();
         data.setOwner(new ExternalTransferOwnerData(ASSET_OWNER_EXTERNAL_ID));
         data.setStatus(status);
+        data.setSubStatus(subStatus);
         data.setTransferId(123L);
         data.setLoan(new ExternalTransferLoanData(LOAN_ID, LOAN_EXTERNAL_ID));
         data.setEffectiveFrom(LocalDate.of(2023, 6, 10));
         data.setEffectiveTo(LocalDate.of(9999, 12, 31));
         data.setSettlementDate(LocalDate.of(2023, 6, 11));
         data.setPurchasePriceRatio("1.0");
-        data.setTransferExternalId(TRANSFER_EXTERNAL_ID);
+        data.setTransferExternalId(TRANSFER_EXTERNAL_ID.getValue());
         data.setDetails(details);
         return data;
     }