[GitHub] [fineract] adamsaghy commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy commented on code in PR #2852: URL: https://github.com/apache/fineract/pull/2852#discussion_r1073524648 ## fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientWritePlatformServiceJpaRepositoryImpl.java: ## @@ -430,7 +430,8 @@ public CommandProcessingResult updateClient(final Long clientId, final JsonComma } final ExternalId externalId = externalIdFactory.createFromCommand(command, ClientApiConstants.externalIdParamName); -if (command.isChangeInStringParameterNamed(ClientApiConstants.externalIdParamName, externalId.getValue())) { +if (command.isChangeInStringParameterNamed(ClientApiConstants.externalIdParamName, Review Comment: Please raise a separate PR for fixing the client external id updating bug! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy commented on code in PR #2852: URL: https://github.com/apache/fineract/pull/2852#discussion_r1065583905 ## integration-tests/src/test/java/org/apache/fineract/integrationtests/ClientLoanCreditBalanceRefundandRepaymentTypeIntegrationTest.java: ## @@ -438,4 +439,73 @@ public void newGoodwillCreditCreatesCorrectJournalEntriesForCashAccountingTest() } +@Test Review Comment: Please add test cases to cover the adjusting of the goodwill credit, payout refund and merchant issued refund and chargeback is not possible and they throw the proper error. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy commented on code in PR #2852: URL: https://github.com/apache/fineract/pull/2852#discussion_r1064756703 ## integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanTransactionChargebackTest.java: ## @@ -114,7 +116,7 @@ public void applyLoanTransactionChargeback() { // Try to reverse a Loan Transaction charge back PostLoansLoanIdTransactionsResponse reverseTransactionResponse = loanTransactionHelper.reverseLoanTransaction(loanId, -chargebackTransactionId, operationDate, responseSpecErr503); +chargebackTransactionId, operationDate, responseSpecErr403); Review Comment: So reversing or adjusting a chargeback is not possible, right? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy commented on code in PR #2852: URL: https://github.com/apache/fineract/pull/2852#discussion_r1064749265 ## fineract-provider/src/main/java/org/apache/fineract/commands/service/CommandWrapperBuilder.java: ## @@ -913,6 +913,15 @@ public CommandWrapperBuilder adjustTransaction(final Long loanId, final Long tra return this; } +public CommandWrapperBuilder undoTransaction(final Long loanId, final Long transactionId) { Review Comment: Dead code? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy commented on code in PR #2852: URL: https://github.com/apache/fineract/pull/2852#discussion_r1063438155 ## integration-tests/src/test/java/org/apache/fineract/integrationtests/ClientLoanCreditBalanceRefundandRepaymentTypeIntegrationTest.java: ## @@ -438,4 +439,76 @@ public void newGoodwillCreditCreatesCorrectJournalEntriesForCashAccountingTest() } +@Test +public void tryToUndoGoodWillCreditTransactionTest() { Review Comment: Please amend this test cases to tests for testing whether: - these transactions can be undone - these transactions cannot be adjusted -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy commented on code in PR #2852: URL: https://github.com/apache/fineract/pull/2852#discussion_r1063436939 ## integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java: ## @@ -1830,7 +1825,7 @@ public void shouldReturnOkStatusForBatchGoodwillCreditReversal() { * @see AdjustTransactionCommandStrategy */ @Test -public void shouldReturnOkStatusOnSuccessfulTransactionMerchantIssuedAndPayoutRefundReversal() { +public void shouldReturnNOkStatusOnUnSuccessfulTransactionMerchantIssuedAndPayoutRefundReversal() { Review Comment: Undo merchant issues refund is supported. Please revert this test case. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy commented on code in PR #2852: URL: https://github.com/apache/fineract/pull/2852#discussion_r1063436619 ## integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java: ## @@ -1808,19 +1806,16 @@ public void shouldReturnOkStatusForBatchGoodwillCreditReversal() { final List responses = BatchHelper.postBatchRequestsWithoutEnclosingTransaction(this.requestSpec, this.responseSpec, BatchHelper.toJsonString(batchRequests)); +Assertions.assertEquals(HttpStatus.SC_INTERNAL_SERVER_ERROR, (long) responses.get(4).getStatusCode(), Review Comment: Undo goodwill credit is supported. Please revert this test case. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy commented on code in PR #2852: URL: https://github.com/apache/fineract/pull/2852#discussion_r1063435445 ## fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java: ## @@ -1063,9 +1063,10 @@ public CommandProcessingResult adjustLoanTransaction(final Long loanId, final Lo transactionId); } -if (transactionToAdjust.isChargeback()) { -throw new PlatformServiceUnavailableException("error.msg.loan.transaction.update.not.allowed", "Loan transaction:" -+ transactionId + " update not allowed as loan transaction is charge back and is linked to other transaction", +if (transactionToAdjust.isChargeback() || transactionToAdjust.isGoodwillCredit() || transactionToAdjust.isPayoutRefund() Review Comment: This is not correct. We should let the undo, but the adjusting. Please amend this logic whether it is just undo (transactionAmount=0) or undo and create new (transactionAmount>0) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org