community-app icon indicating copy to clipboard operation
community-app copied to clipboard

#2225 - hide recovery payment button when written off amount is zero

Open tonic889 opened this issue 5 years ago • 7 comments

Description

per #2225

  1. hide recovery payment button in viewloandetailscontroller.js if the total_written_off amount <= 0.
  2. added a string for validation error when the user tries to enter a repayment amount greater than the total written off. this validation error will be returned by the API in a corresponding PR for fineract.

Related issues and discussion

#2225

Screenshots, if any

image

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • [X ] Validate the JS and HTML files with grunt validate to detect errors and potential problems in JavaScript code.

  • [ X] Run the tests by opening test/SpecRunner.html in the browser to make sure you didn't break anything.

  • [X ] If you have multiple commits please combine them into one commit by squashing them.

  • [ X] Read and understood the contribution guidelines at community-app/Contributing.md.

tonic889 avatar Sep 28 '20 03:09 tonic889

@tonic889 Please shorten the PR (and commit) title. Add Screenshot. Thanks for the contribution :-)

luckyman20 avatar Sep 28 '20 18:09 luckyman20

Hey @tonic889, I just tested your Pull Request on my machine, and it doesn't seem to be working. It is still accepting any amount in recovery repayment.

You stated the logic correct on the issue; even I missed that edge case. So what we need to do is that we need to validate the Transaction Amount field on whether the amount filled is (total_recovered - total_written_off >= amount_entered) or not. Please feel free to ask for any further queries.

To test, we have replicated this situation in Loan Account 99 of Staging Server. Thank you for picking up this issue; please keep contributing :-)

Since you are at it, if you want to get assigned some good to start issues, let me know.

luckyman20 avatar Sep 29 '20 18:09 luckyman20

@luckyman20 The logic for giving the validation error is actually a fineract API change. I submitted the PR for that at the same time as this PR and it was accepted earlier today (https://github.com/apache/fineract/commit/3479e44b4048ff5e4eacb65c2c4f4ed623a15caf).

The community app side changes are only for hiding the button once the written off becomes 0 or less.

tonic889 avatar Sep 30 '20 01:09 tonic889

@tonic889 I tested it again on a different server (https://demo.fineract.dev#/viewloanaccount/42) and still got the same results. Can you tell me what steps you exactly followed?

luckyman20 avatar Oct 05 '20 17:10 luckyman20

@luckyman20 I think there may be some confusion here. There two scenarios here:

  1. Hide the button when the total written off <= 0. This should work with "as-is" with my PR.
  2. When the total written off > 0, prevent the user from entering a recovery amount greater than total written off. To prevent this, I have added a validation in the fineract API (see https://github.com/apache/fineract/commit/3479e44b4048ff5e4eacb65c2c4f4ed623a15caf) which has been accepted and merged to develop branch there. Do the servers you are testing against have this commit? This scenario will not work without that commmit.

This PR merely adds a user friendly string for the new validation error that the API will return. See the screen shot attached to this PR for a sample. In this example, the account had $500 written off but user tried to enter a recovery payment of $1000 and received the message indicated in the screenshot. The Fineract API will be the one generating this error. Hope this helps.

tonic889 avatar Oct 19 '20 01:10 tonic889

@tonic889 The server I am using automatically builds after every commit, so I think we shouldn't have a problem there.

Also, "What I observed is that when I enter a recovery payment, the total written off is reduced by the recovery payment amount." - you mentioned this on the issue, but I haven't observed the same.

luckyman20 avatar Oct 19 '20 04:10 luckyman20

@tonic889 Any updates on this PR

shrunk3 avatar Mar 09 '21 05:03 shrunk3