galovics commented on code in PR #2783:
URL: https://github.com/apache/fineract/pull/2783#discussion_r1045521379


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java:
##########
@@ -370,77 +305,141 @@ public Collection<ClientData> 
retrieveActiveClientMembersOfGroup(final Long grou
                 hierarchySearchString, groupId, 
ClientStatus.ACTIVE.getValue());
     }
 
-    private static final class ClientMembersOfGroupMapper implements 
RowMapper<ClientData> {
+    private String buildSqlStringFromClientCriteria(String schemaSql, final 
SearchParameters searchParameters, List<Object> paramList) {
+
+        String sqlSearch = searchParameters.getSqlSearch();
+        final Long officeId = searchParameters.getOfficeId();
+        final String externalId = searchParameters.getExternalId();
+        final String displayName = searchParameters.getName();
+        final String firstname = searchParameters.getFirstname();
+        final String lastname = searchParameters.getLastname();
+        final String status = searchParameters.getStatus();
+
+        String extraCriteria = "";
+        if (sqlSearch != null) {
+            sqlSearch = sqlSearch.replaceAll(" display_name ", " 
c.display_name ");
+            sqlSearch = sqlSearch.replaceAll("display_name ", "c.display_name 
");
+            extraCriteria = " and (" + sqlSearch + ")";
+            this.columnValidator.validateSqlInjection(schemaSql, sqlSearch);
+        }
+
+        if (officeId != null) {
+            extraCriteria += " and c.office_id = ? ";
+            paramList.add(officeId);
+        }
+
+        if (externalId != null) {
+            paramList.add(externalId);
+            extraCriteria += " and c.external_id like ? ";
+        }
+
+        if (displayName != null) {
+            // extraCriteria += " and concatcoalesce(c.firstname, ''),
+            // if(c.firstname > '',' ', '') , coalesce(c.lastname, '')) like "
+            paramList.add("%" + displayName + "%");
+            extraCriteria += " and c.display_name like ? ";
+        }
+
+        if (status != null) {
+            ClientStatus clientStatus = ClientStatus.fromString(status);
+            extraCriteria += " and c.status_enum = " + 
clientStatus.getValue().toString() + " ";
+        }
+
+        if (firstname != null) {
+            paramList.add(firstname);
+            extraCriteria += " and c.firstname like ? ";
+        }
+
+        if (lastname != null) {
+            paramList.add(lastname);
+            extraCriteria += " and c.lastname like ? ";
+        }
+
+        if (searchParameters.isScopedByOfficeHierarchy()) {
+            paramList.add(searchParameters.getHierarchy() + "%");
+            extraCriteria += " and o.hierarchy like ? ";
+        }
+
+        if (searchParameters.isOrphansOnly()) {
+            extraCriteria += " and c.id NOT IN (select client_id from 
m_group_client) ";
+        }
+
+        if (StringUtils.isNotBlank(extraCriteria)) {
+            extraCriteria = extraCriteria.substring(4);
+        }
+        return extraCriteria;
+    }
+
+    private static final class ClientToDataMapper implements 
RowMapper<ClientData> {
 
         private final String schema;
 
-        ClientMembersOfGroupMapper() {
-            final StringBuilder sqlBuilder = new StringBuilder(200);
+        ClientToDataMapper() {
+            final StringBuilder builder = new StringBuilder(400);

Review Comment:
   The variable was renamed. Please undo it.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java:
##########
@@ -370,77 +305,141 @@ public Collection<ClientData> 
retrieveActiveClientMembersOfGroup(final Long grou
                 hierarchySearchString, groupId, 
ClientStatus.ACTIVE.getValue());
     }
 
-    private static final class ClientMembersOfGroupMapper implements 
RowMapper<ClientData> {
+    private String buildSqlStringFromClientCriteria(String schemaSql, final 
SearchParameters searchParameters, List<Object> paramList) {

Review Comment:
   I never said this was renamed. The method was reordered within the class. 
Please put it back.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java:
##########
@@ -732,6 +718,20 @@ public ClientData mapRow(final ResultSet rs, final int 
rowNum) throws SQLExcepti
         }
     }
 
+    @Override
+    public Collection<ClientData> retrieveActiveClientMembersOfCenter(final 
Long centerId) {

Review Comment:
   It's been reordered, please undo it.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java:
##########
@@ -370,77 +305,141 @@ public Collection<ClientData> 
retrieveActiveClientMembersOfGroup(final Long grou
                 hierarchySearchString, groupId, 
ClientStatus.ACTIVE.getValue());
     }
 
-    private static final class ClientMembersOfGroupMapper implements 
RowMapper<ClientData> {
+    private String buildSqlStringFromClientCriteria(String schemaSql, final 
SearchParameters searchParameters, List<Object> paramList) {
+
+        String sqlSearch = searchParameters.getSqlSearch();
+        final Long officeId = searchParameters.getOfficeId();
+        final String externalId = searchParameters.getExternalId();
+        final String displayName = searchParameters.getName();
+        final String firstname = searchParameters.getFirstname();
+        final String lastname = searchParameters.getLastname();
+        final String status = searchParameters.getStatus();
+
+        String extraCriteria = "";
+        if (sqlSearch != null) {
+            sqlSearch = sqlSearch.replaceAll(" display_name ", " 
c.display_name ");
+            sqlSearch = sqlSearch.replaceAll("display_name ", "c.display_name 
");
+            extraCriteria = " and (" + sqlSearch + ")";
+            this.columnValidator.validateSqlInjection(schemaSql, sqlSearch);
+        }
+
+        if (officeId != null) {
+            extraCriteria += " and c.office_id = ? ";
+            paramList.add(officeId);
+        }
+
+        if (externalId != null) {
+            paramList.add(externalId);
+            extraCriteria += " and c.external_id like ? ";
+        }
+
+        if (displayName != null) {
+            // extraCriteria += " and concatcoalesce(c.firstname, ''),
+            // if(c.firstname > '',' ', '') , coalesce(c.lastname, '')) like "
+            paramList.add("%" + displayName + "%");
+            extraCriteria += " and c.display_name like ? ";
+        }
+
+        if (status != null) {
+            ClientStatus clientStatus = ClientStatus.fromString(status);
+            extraCriteria += " and c.status_enum = " + 
clientStatus.getValue().toString() + " ";
+        }
+
+        if (firstname != null) {
+            paramList.add(firstname);
+            extraCriteria += " and c.firstname like ? ";
+        }
+
+        if (lastname != null) {
+            paramList.add(lastname);
+            extraCriteria += " and c.lastname like ? ";
+        }
+
+        if (searchParameters.isScopedByOfficeHierarchy()) {
+            paramList.add(searchParameters.getHierarchy() + "%");
+            extraCriteria += " and o.hierarchy like ? ";
+        }
+
+        if (searchParameters.isOrphansOnly()) {
+            extraCriteria += " and c.id NOT IN (select client_id from 
m_group_client) ";
+        }
+
+        if (StringUtils.isNotBlank(extraCriteria)) {
+            extraCriteria = extraCriteria.substring(4);
+        }
+        return extraCriteria;
+    }
+
+    private static final class ClientToDataMapper implements 
RowMapper<ClientData> {
 
         private final String schema;
 
-        ClientMembersOfGroupMapper() {
-            final StringBuilder sqlBuilder = new StringBuilder(200);
+        ClientToDataMapper() {
+            final StringBuilder builder = new StringBuilder(400);
 
-            sqlBuilder.append(
+            builder.append(
                     "c.id as id, c.account_no as accountNo, c.external_id as 
externalId, c.status_enum as statusEnum,c.sub_status as subStatus, ");
-            sqlBuilder.append(
+            builder.append(
                     "cvSubStatus.code_value as 
subStatusValue,cvSubStatus.code_description as subStatusDesc,c.office_id as 
officeId, o.name as officeName, ");
-            sqlBuilder.append("c.transfer_to_office_id as transferToOfficeId, 
transferToOffice.name as transferToOfficeName, ");
-            sqlBuilder.append("c.firstname as firstname, c.middlename as 
middlename, c.lastname as lastname, ");
-            sqlBuilder.append("c.fullname as fullname, c.display_name as 
displayName, ");
-            sqlBuilder.append("c.mobile_no as mobileNo, ");
-            sqlBuilder.append("c.is_staff as isStaff, ");
-            sqlBuilder.append("c.email_address as emailAddress, ");
-            sqlBuilder.append("c.date_of_birth as dateOfBirth, ");
-            sqlBuilder.append("c.gender_cv_id as genderId, ");
-            sqlBuilder.append("cv.code_value as genderValue, ");
-            sqlBuilder.append("c.client_type_cv_id as clienttypeId, ");
-            sqlBuilder.append("cvclienttype.code_value as clienttypeValue, ");
-            sqlBuilder.append("c.client_classification_cv_id as 
classificationId, ");
-            sqlBuilder.append("cvclassification.code_value as 
classificationValue, ");
-            sqlBuilder.append("c.legal_form_enum as legalFormEnum, ");
-            sqlBuilder.append("c.activation_date as activationDate, c.image_id 
as imageId, ");
-            sqlBuilder.append("c.staff_id as staffId, s.display_name as 
staffName,");
-            sqlBuilder.append("c.default_savings_product as savingsProductId, 
sp.name as savingsProductName, ");
-            sqlBuilder.append("c.default_savings_account as savingsAccountId, 
");
-
-            sqlBuilder.append("c.submittedon_date as submittedOnDate, ");
-            sqlBuilder.append("sbu.username as submittedByUsername, ");
-            sqlBuilder.append("sbu.firstname as submittedByFirstname, ");
-            sqlBuilder.append("sbu.lastname as submittedByLastname, ");
+            builder.append("c.transfer_to_office_id as transferToOfficeId, 
transferToOffice.name as transferToOfficeName, ");
+            builder.append("c.firstname as firstname, c.middlename as 
middlename, c.lastname as lastname, ");
+            builder.append("c.fullname as fullname, c.display_name as 
displayName, ");
+            builder.append("c.mobile_no as mobileNo, ");
+            builder.append("c.is_staff as isStaff, ");
+            builder.append("c.email_address as emailAddress, ");
+            builder.append("c.date_of_birth as dateOfBirth, ");
+            builder.append("c.gender_cv_id as genderId, ");
+            builder.append("cv.code_value as genderValue, ");
+            builder.append("c.client_type_cv_id as clienttypeId, ");
+            builder.append("cvclienttype.code_value as clienttypeValue, ");
+            builder.append("c.client_classification_cv_id as classificationId, 
");
+            builder.append("cvclassification.code_value as 
classificationValue, ");
+            builder.append("c.legal_form_enum as legalFormEnum, ");
 
-            sqlBuilder.append("c.closedon_date as closedOnDate, ");
-            sqlBuilder.append("clu.username as closedByUsername, ");
-            sqlBuilder.append("clu.firstname as closedByFirstname, ");
-            sqlBuilder.append("clu.lastname as closedByLastname, ");
+            builder.append("c.submittedon_date as submittedOnDate, ");
+            builder.append("sbu.username as submittedByUsername, ");
+            builder.append("sbu.firstname as submittedByFirstname, ");
+            builder.append("sbu.lastname as submittedByLastname, ");
 
-            sqlBuilder.append("acu.username as activatedByUsername, ");
-            sqlBuilder.append("acu.firstname as activatedByFirstname, ");
-            sqlBuilder.append("acu.lastname as activatedByLastname, ");
+            builder.append("c.closedon_date as closedOnDate, ");
+            builder.append("clu.username as closedByUsername, ");
+            builder.append("clu.firstname as closedByFirstname, ");
+            builder.append("clu.lastname as closedByLastname, ");
 
-            sqlBuilder.append("cnp.constitution_cv_id as constitutionId, ");
-            sqlBuilder.append("cvConstitution.code_value as constitutionValue, 
");
-            sqlBuilder.append("cnp.incorp_no as incorpNo, ");
-            sqlBuilder.append("cnp.incorp_validity_till as incorpValidityTill, 
");
-            sqlBuilder.append("cnp.main_business_line_cv_id as 
mainBusinessLineId, ");
-            sqlBuilder.append("cvMainBusinessLine.code_value as 
mainBusinessLineValue, ");
-            sqlBuilder.append("cnp.remarks as remarks ");
+            // builder.append("c.submittedon as submittedOnDate, ");

Review Comment:
   In terms of what the diff shows, this wasn't part of the query. If this is 
due to reordering again, please undo it and I can check it properly.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java:
##########
@@ -545,90 +543,77 @@ public ClientData mapRow(final ResultSet rs, final int 
rowNum) throws SQLExcepti
         }
     }
 
-    @Override
-    public Collection<ClientData> retrieveActiveClientMembersOfCenter(final 
Long centerId) {
-
-        final AppUser currentUser = this.context.authenticatedUser();
-        final String hierarchy = currentUser.getOffice().getHierarchy();
-        final String hierarchySearchString = hierarchy + "%";
-
-        final String sql = "select " + this.membersOfGroupMapper.schema()
-                + " left join m_group g on pgc.group_id=g.id where o.hierarchy 
like ? and g.parent_id = ? and c.status_enum = ? group by c.id";
-
-        return this.jdbcTemplate.query(sql, this.membersOfGroupMapper, // 
NOSONAR
-                hierarchySearchString, centerId, 
ClientStatus.ACTIVE.getValue());
-    }
-
-    private static final class ClientMapper implements RowMapper<ClientData> {
+    private static final class ClientMembersOfGroupMapper implements 
RowMapper<ClientData> {
 
         private final String schema;
 
-        ClientMapper() {
-            final StringBuilder builder = new StringBuilder(400);
+        ClientMembersOfGroupMapper() {
+            final StringBuilder sqlBuilder = new StringBuilder(200);

Review Comment:
   The variable was renamed. Please undo it.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java:
##########
@@ -370,77 +305,141 @@ public Collection<ClientData> 
retrieveActiveClientMembersOfGroup(final Long grou
                 hierarchySearchString, groupId, 
ClientStatus.ACTIVE.getValue());
     }
 
-    private static final class ClientMembersOfGroupMapper implements 
RowMapper<ClientData> {
+    private String buildSqlStringFromClientCriteria(String schemaSql, final 
SearchParameters searchParameters, List<Object> paramList) {
+
+        String sqlSearch = searchParameters.getSqlSearch();
+        final Long officeId = searchParameters.getOfficeId();
+        final String externalId = searchParameters.getExternalId();
+        final String displayName = searchParameters.getName();
+        final String firstname = searchParameters.getFirstname();
+        final String lastname = searchParameters.getLastname();
+        final String status = searchParameters.getStatus();
+
+        String extraCriteria = "";
+        if (sqlSearch != null) {
+            sqlSearch = sqlSearch.replaceAll(" display_name ", " 
c.display_name ");
+            sqlSearch = sqlSearch.replaceAll("display_name ", "c.display_name 
");
+            extraCriteria = " and (" + sqlSearch + ")";
+            this.columnValidator.validateSqlInjection(schemaSql, sqlSearch);
+        }
+
+        if (officeId != null) {
+            extraCriteria += " and c.office_id = ? ";
+            paramList.add(officeId);
+        }
+
+        if (externalId != null) {
+            paramList.add(externalId);
+            extraCriteria += " and c.external_id like ? ";
+        }
+
+        if (displayName != null) {
+            // extraCriteria += " and concatcoalesce(c.firstname, ''),
+            // if(c.firstname > '',' ', '') , coalesce(c.lastname, '')) like "
+            paramList.add("%" + displayName + "%");
+            extraCriteria += " and c.display_name like ? ";
+        }
+
+        if (status != null) {
+            ClientStatus clientStatus = ClientStatus.fromString(status);
+            extraCriteria += " and c.status_enum = " + 
clientStatus.getValue().toString() + " ";
+        }
+
+        if (firstname != null) {
+            paramList.add(firstname);
+            extraCriteria += " and c.firstname like ? ";
+        }
+
+        if (lastname != null) {
+            paramList.add(lastname);
+            extraCriteria += " and c.lastname like ? ";
+        }
+
+        if (searchParameters.isScopedByOfficeHierarchy()) {
+            paramList.add(searchParameters.getHierarchy() + "%");
+            extraCriteria += " and o.hierarchy like ? ";
+        }
+
+        if (searchParameters.isOrphansOnly()) {
+            extraCriteria += " and c.id NOT IN (select client_id from 
m_group_client) ";
+        }
+
+        if (StringUtils.isNotBlank(extraCriteria)) {
+            extraCriteria = extraCriteria.substring(4);
+        }
+        return extraCriteria;
+    }
+
+    private static final class ClientToDataMapper implements 
RowMapper<ClientData> {
 
         private final String schema;
 
-        ClientMembersOfGroupMapper() {
-            final StringBuilder sqlBuilder = new StringBuilder(200);
+        ClientToDataMapper() {
+            final StringBuilder builder = new StringBuilder(400);
 
-            sqlBuilder.append(
+            builder.append(
                     "c.id as id, c.account_no as accountNo, c.external_id as 
externalId, c.status_enum as statusEnum,c.sub_status as subStatus, ");
-            sqlBuilder.append(
+            builder.append(
                     "cvSubStatus.code_value as 
subStatusValue,cvSubStatus.code_description as subStatusDesc,c.office_id as 
officeId, o.name as officeName, ");
-            sqlBuilder.append("c.transfer_to_office_id as transferToOfficeId, 
transferToOffice.name as transferToOfficeName, ");
-            sqlBuilder.append("c.firstname as firstname, c.middlename as 
middlename, c.lastname as lastname, ");
-            sqlBuilder.append("c.fullname as fullname, c.display_name as 
displayName, ");
-            sqlBuilder.append("c.mobile_no as mobileNo, ");
-            sqlBuilder.append("c.is_staff as isStaff, ");
-            sqlBuilder.append("c.email_address as emailAddress, ");
-            sqlBuilder.append("c.date_of_birth as dateOfBirth, ");
-            sqlBuilder.append("c.gender_cv_id as genderId, ");
-            sqlBuilder.append("cv.code_value as genderValue, ");
-            sqlBuilder.append("c.client_type_cv_id as clienttypeId, ");
-            sqlBuilder.append("cvclienttype.code_value as clienttypeValue, ");
-            sqlBuilder.append("c.client_classification_cv_id as 
classificationId, ");
-            sqlBuilder.append("cvclassification.code_value as 
classificationValue, ");
-            sqlBuilder.append("c.legal_form_enum as legalFormEnum, ");
-            sqlBuilder.append("c.activation_date as activationDate, c.image_id 
as imageId, ");
-            sqlBuilder.append("c.staff_id as staffId, s.display_name as 
staffName,");
-            sqlBuilder.append("c.default_savings_product as savingsProductId, 
sp.name as savingsProductName, ");
-            sqlBuilder.append("c.default_savings_account as savingsAccountId, 
");
-
-            sqlBuilder.append("c.submittedon_date as submittedOnDate, ");
-            sqlBuilder.append("sbu.username as submittedByUsername, ");
-            sqlBuilder.append("sbu.firstname as submittedByFirstname, ");
-            sqlBuilder.append("sbu.lastname as submittedByLastname, ");
+            builder.append("c.transfer_to_office_id as transferToOfficeId, 
transferToOffice.name as transferToOfficeName, ");
+            builder.append("c.firstname as firstname, c.middlename as 
middlename, c.lastname as lastname, ");
+            builder.append("c.fullname as fullname, c.display_name as 
displayName, ");
+            builder.append("c.mobile_no as mobileNo, ");
+            builder.append("c.is_staff as isStaff, ");
+            builder.append("c.email_address as emailAddress, ");
+            builder.append("c.date_of_birth as dateOfBirth, ");
+            builder.append("c.gender_cv_id as genderId, ");
+            builder.append("cv.code_value as genderValue, ");
+            builder.append("c.client_type_cv_id as clienttypeId, ");
+            builder.append("cvclienttype.code_value as clienttypeValue, ");
+            builder.append("c.client_classification_cv_id as classificationId, 
");
+            builder.append("cvclassification.code_value as 
classificationValue, ");
+            builder.append("c.legal_form_enum as legalFormEnum, ");
 
-            sqlBuilder.append("c.closedon_date as closedOnDate, ");
-            sqlBuilder.append("clu.username as closedByUsername, ");
-            sqlBuilder.append("clu.firstname as closedByFirstname, ");
-            sqlBuilder.append("clu.lastname as closedByLastname, ");
+            builder.append("c.submittedon_date as submittedOnDate, ");
+            builder.append("sbu.username as submittedByUsername, ");
+            builder.append("sbu.firstname as submittedByFirstname, ");
+            builder.append("sbu.lastname as submittedByLastname, ");
 
-            sqlBuilder.append("acu.username as activatedByUsername, ");
-            sqlBuilder.append("acu.firstname as activatedByFirstname, ");
-            sqlBuilder.append("acu.lastname as activatedByLastname, ");
+            builder.append("c.closedon_date as closedOnDate, ");
+            builder.append("clu.username as closedByUsername, ");
+            builder.append("clu.firstname as closedByFirstname, ");
+            builder.append("clu.lastname as closedByLastname, ");
 
-            sqlBuilder.append("cnp.constitution_cv_id as constitutionId, ");
-            sqlBuilder.append("cvConstitution.code_value as constitutionValue, 
");
-            sqlBuilder.append("cnp.incorp_no as incorpNo, ");
-            sqlBuilder.append("cnp.incorp_validity_till as incorpValidityTill, 
");
-            sqlBuilder.append("cnp.main_business_line_cv_id as 
mainBusinessLineId, ");
-            sqlBuilder.append("cvMainBusinessLine.code_value as 
mainBusinessLineValue, ");
-            sqlBuilder.append("cnp.remarks as remarks ");
+            // builder.append("c.submittedon as submittedOnDate, ");
+            builder.append("acu.username as activatedByUsername, ");
+            builder.append("acu.firstname as activatedByFirstname, ");
+            builder.append("acu.lastname as activatedByLastname, ");
 
-            sqlBuilder.append("from m_client c ");
-            sqlBuilder.append("join m_office o on o.id = c.office_id ");
-            sqlBuilder.append("left join m_client_non_person cnp on 
cnp.client_id = c.id ");
-            sqlBuilder.append("join m_group_client pgc on pgc.client_id = c.id 
");

Review Comment:
   In terms of what the diff shows, this isn't there If this is due to 
reordering again, please undo it and I can check it properly.



-- 
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-unsubscr...@fineract.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to