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


##########
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:
   I don't know why this was left here as a comment. Also as I wrote above, 
please move back everything to its original place.



##########
gradle.properties:
##########
@@ -16,7 +16,7 @@
 # specific language governing permissions and limitations
 # under the License.
 #
-org.gradle.jvmargs=-Xmx2g --add-exports 
jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports 
jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports 
jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-exports 
jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-exports 
jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED 
--add-exports=java.naming/com.sun.jndi.ldap=ALL-UNNAMED 
--add-opens=java.base/java.lang=ALL-UNNAMED 
--add-opens=java.base/java.lang.invoke=ALL-UNNAMED 
--add-opens=java.base/java.io=ALL-UNNAMED 
--add-opens=java.base/java.security=ALL-UNNAMED 
--add-opens=java.base/java.util=ALL-UNNAMED 
--add-opens=java.management/javax.management=ALL-UNNAMED 
--add-opens=java.naming/javax.naming=ALL-UNNAMED 
--add-opens=java.rmi/sun.rmi.transport=null
+org.gradle.jvmargs=-Xmx4g --add-exports 
jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports 
jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports 
jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-exports 
jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-exports 
jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED 
--add-exports=java.naming/com.sun.jndi.ldap=ALL-UNNAMED 
--add-opens=java.base/java.lang=ALL-UNNAMED 
--add-opens=java.base/java.lang.invoke=ALL-UNNAMED 
--add-opens=java.base/java.io=ALL-UNNAMED 
--add-opens=java.base/java.security=ALL-UNNAMED 
--add-opens=java.base/java.util=ALL-UNNAMED 
--add-opens=java.management/javax.management=ALL-UNNAMED 
--add-opens=java.naming/javax.naming=ALL-UNNAMED 
--add-opens=java.rmi/sun.rmi.transport=null

Review Comment:
   Please revert this.



##########
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:
   This has been deleted. I know it wasn't used in the select but this is an 
inner join which implicitly does filtering on the dataset. Move it back pls. 
Also as I wrote above, please move back everything to its original place.



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