[PIDM-42] fix(payment-status): payment position/option status not coherent #289

dylantangredi-bvt · 2025-01-31T08:35:46Z

List of Changes

These changes are relevant in case the user pays the full amount for a payment position in presence of already paid installments/payment options:

Fixed payment option status not changing from PO_UNPAID to PO_PAID
Added detailed logs for this specific case, instead of throwing an exception that makes the db not coherent with the payment process on the node
Fixed payment position status not changing from PARTIALLY_PAID to PAID
Moved the exception handling from the /pay to the /get (activate) phase to prevent the issue

Motivation and Context

This is a hotfix that was necessary to make the db coherent with what the user experiences during checkout. Before the fix, if the user paid using a NAV corresponding to the full amount due for the payment position but one or more installment/payment options were already paid for the same payment position, then the payment option corresponding to the full amount did not change status from PO_UNPAID to PAID, causing a discrepancy in the process. This happened because the checkout ended correctly up to the receipt generation for the user payment, but in the backend a HttpStatus.CONFLICT, "Existing related payment found" was being thrown.

How Has This Been Tested?

Tested locally with Postman making the following use case:

Create debt position with 4 payment options, option 1 being for the full amount and option 2/3/4 for the 3 installments
Publish debt position
Pay payment option 2 (corresponding to one installment)
Get payment option 2 by NAV and check db status on the option (PO_PAID)
Get payment option 1 by NAV (corresponding to the full amount) -> error 409
Pay payment option 1 (corresponding to the full amount), skipping activate
Retrieve payment option by NAV and check db status on the option -> PO_PAID and debtPositionStatus -> PAID

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.

…position status not changing to PAID when paying the full amount in presence of already paid installments;

src/main/java/it/gov/pagopa/debtposition/util/DebtPositionValidation.java

…fix-payment-status # Conflicts: # src/main/java/it/gov/pagopa/debtposition/util/DebtPositionValidation.java

github-actions · 2025-01-31T09:11:44Z

src/main/java/it/gov/pagopa/debtposition/service/payments/PaymentsService.java

[google-java-format] _{reported by reviewdog 🐶}

pagopa-debt-position/src/main/java/it/gov/pagopa/debtposition/service/payments/PaymentsService.java

Lines 3 to 20 in c2e1437

import java.time.LocalDate;

import java.time.LocalDateTime;

import java.time.ZoneOffset;

import java.util.Collections;

import java.util.Comparator;

import java.util.List;

import java.util.Objects;

import java.util.Optional;

import javax.validation.Valid;

import javax.validation.constraints.NotBlank;

import javax.validation.constraints.NotNull;

import org.springframework.beans.factory.annotation.Autowired;

import org.springframework.beans.factory.annotation.Value;

import org.springframework.stereotype.Service;

import org.springframework.transaction.annotation.Transactional;

cap-ang

Great,

How about preventing these cases by blocking the activation phase with a 409 if the activation is not related to an installment (payment option with isPartialPayment true)?

… PIDM-42-fix-payment-status # Conflicts: # src/main/java/it/gov/pagopa/debtposition/util/DebtPositionValidation.java

…on to get/activate phase; adapt existing unit test;

github-actions · 2025-01-31T11:35:18Z

src/main/java/it/gov/pagopa/debtposition/service/payments/PaymentsService.java

@@ -69,6 +70,7 @@ public PaymentOption getPaymentOptionByNAV(@NotBlank String organizationFiscalCo
        // PaymentPosition used when converting PaymentOption to POWithDebtor
        DebtPositionStatus.validityCheckAndUpdate(paymentOption);
        DebtPositionStatus.expirationCheckAndUpdate(paymentOption);
+        DebtPositionStatus.checkAlreadyPaidInstallments(paymentOption, nav);

        return paymentOption;


[google-java-format] _{reported by reviewdog 🐶}

Suggested change

return paymentOption;

if (po.isEmpty()) {

throw new AppException(AppError.PAYMENT_OPTION_NOT_FOUND, organizationFiscalCode, nav);

github-actions · 2025-01-31T11:35:19Z

src/main/java/it/gov/pagopa/debtposition/service/payments/PaymentsService.java

@@ -233,7 +235,10 @@
        }


[google-java-format] _{reported by reviewdog 🐶}

Suggested change

}

pp.setLastUpdatedDate(currentDate);

// salvo l'aggiornamento del pagamento

paymentPositionRepository.saveAndFlush(pp);

return poToPay;

}

private Transfer updateTransferStatus(PaymentPosition pp, String iuv, String transferId) {

LocalDateTime currentDate = LocalDateTime.now(ZoneOffset.UTC);

long numberPOTransfers = 0;

long countReportedTransfer = 0;

Transfer reportedTransfer = null;

for (PaymentOption po : pp.getPaymentOption()) {

if (po.getIuv().equals(iuv)) {

// numero totale dei transfer per la PO

numberPOTransfers = po.getTransfer().stream().count();

// numero dei transfer della PO in stato T_REPORTED

countReportedTransfer =

po.getTransfer().stream()

.filter(t -> t.getStatus().equals(TransferStatus.T_REPORTED))

.count();

// recupero il transfer oggetto di rendicontazione

Optional<Transfer> transferToReport =

po.getTransfer().stream().filter(t -> t.getIdTransfer().equals(transferId)).findFirst();

if (transferToReport.isEmpty()) {

log.error(

"Obtained unexpected empty transfer - "

+ "[organizationFiscalCode= "

+ pp.getOrganizationFiscalCode()

+ "; "

+ "iupd= "

+ pp.getIupd()

+ "; "

+ "iuv= "

+ iuv

+ "; "

+ "idTransfer= "

+ transferId

+ "]");

throw new AppException(

AppError.TRANSFER_REPORTING_FAILED, pp.getOrganizationFiscalCode(), iuv, transferId);

}

github-actions · 2025-01-31T11:35:19Z

src/main/java/it/gov/pagopa/debtposition/service/payments/PaymentsService.java

        // aggiorno lo stato della payment position
-        if (countPaidPartialPayment > 0 && countPaidPartialPayment < numberOfPartialPayment) {
+        // PIDM-42 if paying the full amount when there is already a paid partial payment
+        // then update the payment position status to PAID
+        if (countPaidPartialPayment > 0 && countPaidPartialPayment < numberOfPartialPayment
+                && Boolean.TRUE.equals(Objects.requireNonNull(poToPay).getIsPartialPayment())) {
            pp.setStatus(DebtPositionStatus.PARTIALLY_PAID);


[google-java-format] _{reported by reviewdog 🐶}

Suggested change

// aggiorno lo stato della payment position

if (countPaidPartialPayment > 0 && countPaidPartialPayment < numberOfPartialPayment) {

// PIDM-42 if paying the full amount when there is already a paid partial payment

// then update the payment position status to PAID

if (countPaidPartialPayment > 0 && countPaidPartialPayment < numberOfPartialPayment

&& Boolean.TRUE.equals(Objects.requireNonNull(poToPay).getIsPartialPayment())) {

pp.setStatus(DebtPositionStatus.PARTIALLY_PAID);

transferToReport.get().setStatus(TransferStatus.T_REPORTED);

transferToReport.get().setLastUpdatedDate(currentDate);

countReportedTransfer++;

// aggiorno lo stato della PO

if (countReportedTransfer < numberPOTransfers) {

po.setStatus(PaymentOptionStatus.PO_PARTIALLY_REPORTED);

github-actions · 2025-01-31T11:35:19Z

src/test/java/it/gov/pagopa/debtposition/controller/PaymentsControllerTest.java

+                + auxDigit
+                + "123456IUVMULTIPLEMOCK3")


[google-java-format] _{reported by reviewdog 🐶}

Suggested change

+ auxDigit

+ "123456IUVMULTIPLEMOCK3")

+ auxDigit

+ "123456IUVMULTIPLEMOCK3")

github-actions · 2025-01-31T11:35:21Z

src/main/java/it/gov/pagopa/debtposition/service/payments/PaymentsService.java

[google-java-format] _{reported by reviewdog 🐶}

pagopa-debt-position/src/main/java/it/gov/pagopa/debtposition/service/payments/PaymentsService.java

Lines 68 to 73 in 7aac4b2

PaymentOption paymentOption = po.get();

// Update PaymentPosition instance only in memory

// PaymentPosition used when converting PaymentOption to POWithDebtor

DebtPositionStatus.validityCheckAndUpdate(paymentOption);

DebtPositionStatus.expirationCheckAndUpdate(paymentOption);

DebtPositionStatus.checkAlreadyPaidInstallments(paymentOption, nav);

github-actions · 2025-01-31T11:35:22Z

src/main/java/it/gov/pagopa/debtposition/service/payments/PaymentsService.java

[google-java-format] _{reported by reviewdog 🐶}

pagopa-debt-position/src/main/java/it/gov/pagopa/debtposition/service/payments/PaymentsService.java

Lines 244 to 245 in 7aac4b2

pp.setStatus(DebtPositionStatus.PAID);

pp.setPaymentDate(paymentOptionModel.getPaymentDate());

dylantangredi-bvt · 2025-01-31T11:45:45Z

Great,

How about preventing these cases by blocking the activation phase with a 409 if the activation is not related to an installment (payment option with isPartialPayment true)?

I assume you meant isPartialPayment false because that indicates the full amount payment (so not related to an installment). In this case it has been done with the latest commit

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

src/main/java/it/gov/pagopa/debtposition/util/DebtPositionValidation.java

+      log.warn(
+          "Potential payment state inconsistency detected || "
+              + "Organization: {} || "
+              + "IUPD: {} || "
+              + "NAV: {} || "
+              + "Position Status: {} || "
+              + "Payment Option Status: {} || "
+              + "Is Partial Payment: {} || "
+              + "Timestamp: {}",
+          ppToPay.getOrganizationFiscalCode(),
+          ppToPay.getIupd(),
+          nav,
+          ppToPay.getStatus(),
+          poToPay.getStatus(),
+          poToPay.getIsPartialPayment(),
+          LocalDateTime.now());


To fix the log injection issue, we need to sanitize the nav parameter before logging it. The best way to do this is to remove any potentially harmful characters from the nav parameter, such as new-line characters, which can be used to forge log entries. We can use a utility method to sanitize the input by replacing or removing such characters.

Create a utility method to sanitize the nav parameter by removing new-line characters and other potentially harmful characters.

Use this utility method to sanitize the nav parameter before logging it in the DebtPositionValidation class.

sonarqubecloud · 2025-01-31T11:56:16Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[PIDM-42] fix(payment-status): fix payment option status and payment …

34b4721

…position status not changing to PAID when paying the full amount in presence of already paid installments;

dylantangredi-bvt added bug Something isn't working gpd patch minor labels Jan 31, 2025

github-advanced-security bot found potential problems Jan 31, 2025

View reviewed changes

src/main/java/it/gov/pagopa/debtposition/util/DebtPositionValidation.java Fixed Show fixed Hide fixed

dylantangredi-bvt added 2 commits January 31, 2025 09:38

Merge remote-tracking branch 'refs/remotes/origin/main' into PIDM-42-…

cec4944

…fix-payment-status # Conflicts: # src/main/java/it/gov/pagopa/debtposition/util/DebtPositionValidation.java

Merge branch 'main' into PIDM-42-fix-payment-status

c2e1437

github-actions bot assigned dylantangredi-bvt Jan 31, 2025

github-actions bot added the size/small label Jan 31, 2025

github-actions bot reviewed Jan 31, 2025

View reviewed changes

cap-ang requested changes Jan 31, 2025

View reviewed changes

dylantangredi-bvt added 2 commits January 31, 2025 11:51

Merge remote-tracking branch 'origin/PIDM-42-fix-payment-status' into…

2ee80ab

… PIDM-42-fix-payment-status # Conflicts: # src/main/java/it/gov/pagopa/debtposition/util/DebtPositionValidation.java

[PIDM-42] fix(payment-status): move existing partial payment validati…

7aac4b2

…on to get/activate phase; adapt existing unit test;

github-actions bot reviewed Jan 31, 2025

View reviewed changes

dylantangredi-bvt requested a review from cap-ang January 31, 2025 11:45

dylantangredi-bvt marked this pull request as ready for review January 31, 2025 11:50

dylantangredi-bvt requested a review from a team as a code owner January 31, 2025 11:50

Apply suggestions from code review

906496f

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-advanced-security bot found potential problems Jan 31, 2025

View reviewed changes

@@ -25,2 +25,3 @@
             import org.apache.logging.log4j.util.Strings;
+            import org.apache.commons.lang3.StringUtils;
@@ -29,2 +30,9 @@
+              private static String sanitize(String input) {
+                if (input == null) {
+                  return null;
+                }
+                return StringUtils.replaceChars(input, "\n\r", "");
+              }
               private static final String LOG_BASE_PARAMS_DETAIL =
@@ -281,2 +289,3 @@
                   // log detailed information about this edge case
+                  String sanitizedNav = sanitize(nav);
                   log.warn(
@@ -292,3 +301,3 @@
                       ppToPay.getIupd(),
-                      nav,
+                      sanitizedNav,
                       ppToPay.getStatus(),

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PIDM-42] fix(payment-status): payment position/option status not coherent #289

[PIDM-42] fix(payment-status): payment position/option status not coherent #289

dylantangredi-bvt commented Jan 31, 2025 •

edited

Loading

github-actions bot Jan 31, 2025

cap-ang left a comment •

edited

Loading

github-actions bot Jan 31, 2025

github-actions bot Jan 31, 2025

github-actions bot Jan 31, 2025

github-actions bot Jan 31, 2025

github-actions bot Jan 31, 2025

github-actions bot Jan 31, 2025

dylantangredi-bvt commented Jan 31, 2025

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

sonarqubecloud bot commented Jan 31, 2025

	import java.time.LocalDate;
	import java.time.LocalDateTime;
	import java.time.ZoneOffset;
	import java.util.Collections;
	import java.util.Comparator;
	import java.util.List;
	import java.util.Objects;
	import java.util.Optional;

	import javax.validation.Valid;
	import javax.validation.constraints.NotBlank;
	import javax.validation.constraints.NotNull;

	import org.springframework.beans.factory.annotation.Autowired;
	import org.springframework.beans.factory.annotation.Value;
	import org.springframework.stereotype.Service;
	import org.springframework.transaction.annotation.Transactional;

	return paymentOption;
	if (po.isEmpty()) {
	throw new AppException(AppError.PAYMENT_OPTION_NOT_FOUND, organizationFiscalCode, nav);

-        }
+    pp.setLastUpdatedDate(currentDate);
+    // salvo l'aggiornamento del pagamento
+    paymentPositionRepository.saveAndFlush(pp);
+    return poToPay;
+  }
+  private Transfer updateTransferStatus(PaymentPosition pp, String iuv, String transferId) {
+    LocalDateTime currentDate = LocalDateTime.now(ZoneOffset.UTC);
+    long numberPOTransfers = 0;
+    long countReportedTransfer = 0;
+    Transfer reportedTransfer = null;
+    for (PaymentOption po : pp.getPaymentOption()) {
+      if (po.getIuv().equals(iuv)) {
+        // numero totale dei transfer per la PO
+        numberPOTransfers = po.getTransfer().stream().count();
+        // numero dei transfer della PO in stato T_REPORTED
+        countReportedTransfer =
+            po.getTransfer().stream()
+                .filter(t -> t.getStatus().equals(TransferStatus.T_REPORTED))
+                .count();
+        // recupero il transfer oggetto di rendicontazione
+        Optional<Transfer> transferToReport =
+            po.getTransfer().stream().filter(t -> t.getIdTransfer().equals(transferId)).findFirst();
+        if (transferToReport.isEmpty()) {
+          log.error(
+              "Obtained unexpected empty transfer - "
+                  + "[organizationFiscalCode= "
+                  + pp.getOrganizationFiscalCode()
+                  + "; "
+                  + "iupd= "
+                  + pp.getIupd()
+                  + "; "
+                  + "iuv= "
+                  + iuv
+                  + "; "
+                  + "idTransfer= "
+                  + transferId
+                  + "]");
+          throw new AppException(
+              AppError.TRANSFER_REPORTING_FAILED, pp.getOrganizationFiscalCode(), iuv, transferId);
+        }

	PaymentOption paymentOption = po.get();
	// Update PaymentPosition instance only in memory
	// PaymentPosition used when converting PaymentOption to POWithDebtor
	DebtPositionStatus.validityCheckAndUpdate(paymentOption);
	DebtPositionStatus.expirationCheckAndUpdate(paymentOption);
	DebtPositionStatus.checkAlreadyPaidInstallments(paymentOption, nav);

	pp.setStatus(DebtPositionStatus.PAID);
	pp.setPaymentDate(paymentOptionModel.getPaymentDate());

[PIDM-42] fix(payment-status): payment position/option status not coherent #289

Are you sure you want to change the base?

[PIDM-42] fix(payment-status): payment position/option status not coherent #289

Conversation

dylantangredi-bvt commented Jan 31, 2025 • edited Loading

List of Changes

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

github-actions bot Jan 31, 2025

Choose a reason for hiding this comment

cap-ang left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot Jan 31, 2025

Choose a reason for hiding this comment

github-actions bot Jan 31, 2025

Choose a reason for hiding this comment

github-actions bot Jan 31, 2025

Choose a reason for hiding this comment

github-actions bot Jan 31, 2025

Choose a reason for hiding this comment

github-actions bot Jan 31, 2025

Choose a reason for hiding this comment

github-actions bot Jan 31, 2025

Choose a reason for hiding this comment

dylantangredi-bvt commented Jan 31, 2025

sonarqubecloud bot commented Jan 31, 2025

Quality Gate passed

dylantangredi-bvt commented Jan 31, 2025 •

edited

Loading

cap-ang left a comment •

edited

Loading