[GitHub] [fineract] vishwasbabu commented on a change in pull request #574: FINERACT-609 Group Topup Loan Bug Fix.

[GitBox <gi...@apache.org>]

vishwasbabu commented on a change in pull request #574: FINERACT-609 Group Topup Loan Bug Fix.
URL: https://github.com/apache/fineract/pull/574#discussion_r293961114
 
 

 ##########
 File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/accountdetails/service/AccountDetailsReadPlatformServiceJpaRepositoryImpl.java
 ##########
 @@ -127,11 +127,18 @@ public AccountSummaryCollectionData retrieveGroupAccountDetails(final Long group
         return retrieveLoanAccountDetails(loanWhereClause, new Object[] { groupId, loanOfficerId });
     }
 
-    @Override public Collection<LoanAccountSummaryData> retrieveClientActiveLoanAccountSummary(final Long clientId) {
+    @Override
+    public Collection<LoanAccountSummaryData> retrieveClientActiveLoanAccountSummary(final Long clientId) {
         final String loanWhereClause = " where l.client_id = ? and l.loan_status_id = 300 ";
         return retrieveLoanAccountDetails(loanWhereClause, new Object[] { clientId });
     }
 
+    @Override
+    public Collection<LoanAccountSummaryData> retrieveGroupActiveLoanAccountSummary(final Long groupId) {
+        final String loanWhereClause = " where l.group_id = ? and l.loan_status_id = 300 ";
 
 Review comment:
   There are three kinds of loans in Fineract
   1) Client Loans (has a client ID and no group ID)
   2) Joint Liability Group loans (has both client and group ID)
   3) Group loans (has only group ID)
   
   With a name like "retrieveGroupActiveLoanAccountSummary", one would expect the method to return only loans of type (3). However, we are actually returning loans of both type 2 and 3. While this works as expected in this PR  (since you check if client ID is not null in the service method), it might introduce bugs in the future when someone tries to reuse this method. I would suggest returning only loans of type 3.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services