Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch… #2852

Conversation

josehernandezfintecheandomx
Copy link
Contributor

Description

Fix on API level which will remove the Edit option of Goodwill Credit, Payout Refund and Merchant Issued Refund transactions

FINERACT-1847

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.

@adamsaghy
Copy link
Contributor

@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)

@josehernandezfintecheandomx josehernandezfintecheandomx changed the title Removing Edit function from Goodwill Credit or Payout Refund or Merch… FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch… Jan 5, 2023
@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_transactions_control branch 3 times, most recently from 802fc0a to 8c4edf6 Compare January 6, 2023 05:21
@josehernandezfintecheandomx
Copy link
Contributor Author

@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

@galovics
Copy link
Contributor

galovics commented Jan 6, 2023

@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)

@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_transactions_control branch 11 times, most recently from 45f45e9 to 981e176 Compare January 7, 2023 22:18
Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Also please remove the "dead code" which was added (CommandWrapperBuilder.java)

Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test cases:
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.

Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josehernandezfintecheandomx Testing whether adjusting

  • merchant issued refund
  • chargeback

is not enabled is still missing. Would you please add the test cases for these two as well?

@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_transactions_control branch 5 times, most recently from 41dcf53 to 685d176 Compare January 17, 2023 19:33
Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please extract the bugfix of "updateClient" logic into a new, separate PR!

…Refund or Merchant Issued Refund transactions on API level
Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adamsaghy adamsaghy merged commit 6d3cac8 into apache:develop Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants