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


##########
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:
   Please revert this back to it's original name. It's just polluting the PR 
with unnecessary changes which makes it really hard to review.



##########
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:
   Please put this back to it's original place. It's just polluting the PR with 
unnecessary changes which makes it really hard to review.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/mapper/ClientMapper.java:
##########
@@ -0,0 +1,159 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.client.mapper;
+
+import org.apache.fineract.infrastructure.codes.data.CodeValueData;
+import org.apache.fineract.infrastructure.codes.domain.CodeValue;
+import org.apache.fineract.infrastructure.core.config.MapstructMapperConfig;
+import org.apache.fineract.infrastructure.core.data.EnumOptionData;
+import org.apache.fineract.portfolio.client.data.ClientData;
+import org.apache.fineract.portfolio.client.data.ClientTimelineData;
+import org.apache.fineract.portfolio.client.domain.Client;
+import org.apache.fineract.portfolio.client.domain.ClientEnumerations;
+import org.apache.fineract.useradministration.domain.AppUser;
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.mapstruct.Named;
+
+@Mapper(config = MapstructMapperConfig.class)
+public interface ClientMapper {
+
+    @Mapping(target = "accountNo", source = "source.accountNumber")
+    @Mapping(target = "status", source = "source", qualifiedByName = 
"clientStatusEnum")
+    @Mapping(target = "subStatus", source = "source", qualifiedByName = 
"clientSubStatusCode")
+    @Mapping(target = "officeId", source = "source.office.id")
+    @Mapping(target = "officeName", source = "source.office.name")
+    @Mapping(target = "transferToOfficeId", source = 
"source.transferToOffice.id")
+    @Mapping(target = "transferToOfficeName", source = 
"source.transferToOffice.name")
+    @Mapping(target = "externalId", source = "source.externalId")
+    @Mapping(target = "gender", source = "source", qualifiedByName = 
"clientGenderCode")
+    @Mapping(target = "imageId", source = "source.image.id")
+    @Mapping(target = "staffId", source = "source.staff.id")
+    @Mapping(target = "staffName", source = "source.staff.displayName")
+    @Mapping(target = "timeline", source = "source", qualifiedByName = 
"clientTimeloneData")
+    @Mapping(target = "savingsProductId", source = "source.savingsProductId")
+    @Mapping(target = "savingsProductName", source = "source.id")
+    @Mapping(target = "savingsAccountId", source = "source.savingsAccountId")
+    @Mapping(target = "clientType", source = "source", qualifiedByName = 
"clientTypeCode")
+    @Mapping(target = "clientClassification", source = "source", 
qualifiedByName = "clientClassificationCode")
+    @Mapping(target = "legalForm", source = "source", qualifiedByName = 
"clientLegalFormEnum")
+    @Mapping(target = "isStaff", source = "source", qualifiedByName = 
"clientIsStaff")
+    @Mapping(target = "imagePresent", ignore = true)
+    @Mapping(target = "officeOptions", ignore = true)
+    @Mapping(target = "staffOptions", ignore = true)
+    @Mapping(target = "narrations", ignore = true)
+    @Mapping(target = "savingProductOptions", ignore = true)
+    @Mapping(target = "savingAccountOptions", ignore = true)
+    @Mapping(target = "genderOptions", ignore = true)
+    @Mapping(target = "clientTypeOptions", ignore = true)
+    @Mapping(target = "clientClassificationOptions", ignore = true)
+    @Mapping(target = "clientNonPersonConstitutionOptions", ignore = true)
+    @Mapping(target = "clientNonPersonMainBusinessLineOptions", ignore = true)
+    @Mapping(target = "clientLegalFormOptions", ignore = true)
+    @Mapping(target = "familyMemberOptions", ignore = true)
+    @Mapping(target = "clientNonPersonDetails", ignore = true)
+    @Mapping(target = "address", ignore = true)
+    @Mapping(target = "isAddressEnabled", ignore = true)
+    @Mapping(target = "datatables", ignore = true)
+    @Mapping(target = "rowIndex", ignore = true)
+    @Mapping(target = "dateFormat", ignore = true)
+    @Mapping(target = "locale", ignore = true)
+    @Mapping(target = "clientTypeId", ignore = true)
+    @Mapping(target = "genderId", ignore = true)
+    @Mapping(target = "clientClassificationId", ignore = true)
+    @Mapping(target = "legalFormId", ignore = true)
+    @Mapping(target = "clientCollateralManagements", ignore = true)
+    @Mapping(target = "groups", ignore = true)
+    ClientData map(Client source);
+
+    @Named("clientTypeCode")
+    default CodeValueData clientTypeCode(Client client) {
+        final CodeValue code = client.getClientType();
+        if (code == null) {
+            return null;
+        }
+        return CodeValueData.instance(code.getId(), code.getLabel());
+    }
+
+    @Named("clientClassificationCode")
+    default CodeValueData clientClassificationCode(Client client) {
+        final CodeValue code = client.getClientClassification();
+        if (code == null) {
+            return null;
+        }
+        return CodeValueData.instance(code.getId(), code.getLabel());
+    }
+
+    @Named("clientSubStatusCode")
+    default CodeValueData clientSubStatusCode(Client client) {
+        final CodeValue code = client.getSubStatus();
+        if (code == null) {
+            return null;
+        }
+        return CodeValueData.instance(code.getId(), code.getLabel());
+    }
+
+    @Named("clientGenderCode")
+    default CodeValueData clientGenderCode(Client client) {
+        final CodeValue code = client.getGender();
+        if (code == null) {
+            return null;
+        }
+        return CodeValueData.instance(code.getId(), code.getLabel());
+    }
+
+    @Named("clientLegalFormEnum")
+    default EnumOptionData clientLegalFormEnum(Client client) {
+        return ClientEnumerations.legalForm(client.getLegalForm());
+    }
+
+    @Named("clientStatusEnum")
+    default EnumOptionData clientStatusEnum(Client client) {
+        return ClientEnumerations.status(client.getStatus());
+    }
+
+    @Named("clientTimeloneData")

Review Comment:
   Typo.



##########
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:
   Please put this back to it's original place. It's just polluting the PR with 
unnecessary changes which makes it really hard to review.



##########
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:
   Please revert this back to it's original name. It's just polluting the PR 
with unnecessary changes which makes it really hard to review.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/mapper/ClientMapper.java:
##########
@@ -0,0 +1,159 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.client.mapper;
+
+import org.apache.fineract.infrastructure.codes.data.CodeValueData;
+import org.apache.fineract.infrastructure.codes.domain.CodeValue;
+import org.apache.fineract.infrastructure.core.config.MapstructMapperConfig;
+import org.apache.fineract.infrastructure.core.data.EnumOptionData;
+import org.apache.fineract.portfolio.client.data.ClientData;
+import org.apache.fineract.portfolio.client.data.ClientTimelineData;
+import org.apache.fineract.portfolio.client.domain.Client;
+import org.apache.fineract.portfolio.client.domain.ClientEnumerations;
+import org.apache.fineract.useradministration.domain.AppUser;
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.mapstruct.Named;
+
+@Mapper(config = MapstructMapperConfig.class)
+public interface ClientMapper {
+
+    @Mapping(target = "accountNo", source = "source.accountNumber")
+    @Mapping(target = "status", source = "source", qualifiedByName = 
"clientStatusEnum")
+    @Mapping(target = "subStatus", source = "source", qualifiedByName = 
"clientSubStatusCode")
+    @Mapping(target = "officeId", source = "source.office.id")
+    @Mapping(target = "officeName", source = "source.office.name")
+    @Mapping(target = "transferToOfficeId", source = 
"source.transferToOffice.id")
+    @Mapping(target = "transferToOfficeName", source = 
"source.transferToOffice.name")
+    @Mapping(target = "externalId", source = "source.externalId")
+    @Mapping(target = "gender", source = "source", qualifiedByName = 
"clientGenderCode")
+    @Mapping(target = "imageId", source = "source.image.id")
+    @Mapping(target = "staffId", source = "source.staff.id")
+    @Mapping(target = "staffName", source = "source.staff.displayName")
+    @Mapping(target = "timeline", source = "source", qualifiedByName = 
"clientTimeloneData")
+    @Mapping(target = "savingsProductId", source = "source.savingsProductId")
+    @Mapping(target = "savingsProductName", source = "source.id")
+    @Mapping(target = "savingsAccountId", source = "source.savingsAccountId")
+    @Mapping(target = "clientType", source = "source", qualifiedByName = 
"clientTypeCode")
+    @Mapping(target = "clientClassification", source = "source", 
qualifiedByName = "clientClassificationCode")
+    @Mapping(target = "legalForm", source = "source", qualifiedByName = 
"clientLegalFormEnum")
+    @Mapping(target = "isStaff", source = "source", qualifiedByName = 
"clientIsStaff")
+    @Mapping(target = "imagePresent", ignore = true)

Review Comment:
   So why do we ignore these props if they are present in the ClientData class?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to