FINERACT-1926: Asset transfer to external owner COB business step#3207
Conversation
adamsaghy
force-pushed
the
FINERACT-1926/cob_job
branch
4 times, most recently
from
6d17ccc to
d610fb5
Compare
ruchiD
left a comment
ruchiD
left a comment
There was a problem hiding this comment.
LGTM from functional/use cases perspective.
Is adding this business step to cob job a separate change. (liquibase script to add step to cob, integrating/configuring this step with fineract-provider)
taskain7
left a comment
taskain7
left a comment
There was a problem hiding this comment.
please check my comments
There was a problem hiding this comment.
this is also the first element of the list
There was a problem hiding this comment.
I think we should also check if the loan is in active status.
Or is the positive totalOutstanding implicitly means that the loan is active?
There was a problem hiding this comment.
it means that, yeah...
There was a problem hiding this comment.
We use this multiple times, it would be a good idea to extract it to a global constant
There was a problem hiding this comment.
could you write some unit tests for this, to cover the use-cases?
There was a problem hiding this comment.
I understand the purpose of this, but don't you think it'd be better to catch this when submitting a transfer?
There was a problem hiding this comment.
Let's not let entities past through the transactional boundary (i.e. the service is annotated with Transactional so anything that's coming out of that will be detached) and could cause maintenance issues in the future (just like for the majority of fineract today).
There was a problem hiding this comment.
Same as above for the read service on boundaries.
There was a problem hiding this comment.
Same as above for the read service on boundaries.
There was a problem hiding this comment.
Bonus: here as well since the parameter is an entity.
There was a problem hiding this comment.
Let's simply annotate the class instead.
adamsaghy
force-pushed
the
FINERACT-1926/cob_job
branch
from
d610fb5 to
2c02690
Compare
It will not be added by default. |
4 participants
Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated Apache Fineract JIRA ticket.
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.