You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2023/01/02 21:39:40 UTC
[GitHub] [fineract] josehernandezfintecheandomx opened a new pull request, #2852: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
josehernandezfintecheandomx opened a new pull request, #2852:
URL: https://github.com/apache/fineract/pull/2852
Fix on API level which will remove the Edit option of `Goodwill Credit`, `Payout Refund` and `Merchant Issued Refund` transactions
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@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…
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2852:
URL: https://github.com/apache/fineract/pull/2852#discussion_r1073648909
##########
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:
Done
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@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…
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
Posted by GitBox <gi...@apache.org>.
galovics commented on PR #2852:
URL: https://github.com/apache/fineract/pull/2852#issuecomment-1373577096
@josehernandezfintecheandomx test is still failing
```
Test applyLoanTransactionChargeback() FAILED
java.lang.AssertionError: 1 expectation failed.
Expected status code <403> but was <503>.
at org.apache.fineract.integrationtests.LoanTransactionChargebackTest.applyLoanTransactionChargeback(LoanTransactionChargebackTest.java:120)
```
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@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…
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2852:
URL: https://github.com/apache/fineract/pull/2852#discussion_r1067432126
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/ClientLoanCreditBalanceRefundandRepaymentTypeIntegrationTest.java:
##########
@@ -438,4 +439,73 @@ public void newGoodwillCreditCreatesCorrectJournalEntriesForCashAccountingTest()
}
+ @Test
Review Comment:
The IT were added in the class `ClientLoanCreditBalanceRefundandRepaymentTypeIntegrationTest`
<img width="878" alt="Screenshot 2023-01-11 at 14 32 40" src="https://user-images.githubusercontent.com/44206706/211911802-4cb232fa-e388-410c-bbef-b101fa8e5575.png">
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2852:
URL: https://github.com/apache/fineract/pull/2852#discussion_r1064771698
##########
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:
Done!
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@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…
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on PR #2852:
URL: https://github.com/apache/fineract/pull/2852#issuecomment-1373173004
> @josehernandezfintecheandomx Please double check the tests. It is failing with this:
>
> ```
> java.lang.NullPointerException: Cannot invoke "com.google.gson.JsonElement.getAsJsonArray()" because the return value of "com.google.gson.JsonObject.get(String)" is null
> [2138](https://github.com/apache/fineract/actions/runs/3825276813/jobs/6508131758#step:11:2139)
> at org.apache.fineract.integrationtests.BatchApiTest.shouldReturnOkStatusForBatchGoodwillCreditReversal(BatchApiTest.java:1815)
> ```
Done! @adamsaghy I needed to update this test because the reversal or edit for these transactions are not allowed more
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on pull request #2852: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on PR #2852:
URL: https://github.com/apache/fineract/pull/2852#issuecomment-1372241467
@josehernandezfintecheandomx Please double check the tests. It is failing with this:
```
java.lang.NullPointerException: Cannot invoke "com.google.gson.JsonElement.getAsJsonArray()" because the return value of "com.google.gson.JsonObject.get(String)" is null
[2138](https://github.com/apache/fineract/actions/runs/3825276813/jobs/6508131758#step:11:2139)
at org.apache.fineract.integrationtests.BatchApiTest.shouldReturnOkStatusForBatchGoodwillCreditReversal(BatchApiTest.java:1815)
```
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@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…
Posted by GitBox <gi...@apache.org>.
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<BatchResponse> 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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@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…
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy merged pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
Posted by GitBox <gi...@apache.org>.
adamsaghy merged PR #2852:
URL: https://github.com/apache/fineract/pull/2852
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@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…
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2852:
URL: https://github.com/apache/fineract/pull/2852#discussion_r1064775626
##########
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:
Yes, I needed to change the HTTP code due I sync the Exception used in this case to `InvalidLoanTransactionTypeException`
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@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…
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2852:
URL: https://github.com/apache/fineract/pull/2852#discussion_r1064625539
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/ClientLoanCreditBalanceRefundandRepaymentTypeIntegrationTest.java:
##########
@@ -438,4 +439,76 @@ public void newGoodwillCreditCreatesCorrectJournalEntriesForCashAccountingTest()
}
+ @Test
+ public void tryToUndoGoodWillCreditTransactionTest() {
Review Comment:
Done
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org