[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API

2023-07-20 Thread via GitHub


adamsaghy commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1269542879


##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -90,7 +90,7 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 + " order by l.id desc";
 
 final String savingMatchSql = " (select 'SAVING' as entityType, s.id 
as entityId, sp.name as entityName, s.external_id as entityExternalId, 
s.account_no as entityAccountNo "
-+ " , coalesce(c.id,g.id) as parentId, 
coalesce(c.display_name,g.display_name) as parentName, null as entityMobileNo, 
s.status_enum as entityStatusEnum, s.deposit_type_enum as subEntityType, CASE 
WHEN g.id is null THEN 'client' ELSE 'group' END as parentType "
++ " , coalesce(c.id,g.id) as parentId, 
coalesce(c.display_name, g.display_name) as parentName, null as entityMobileNo, 
s.status_enum as entityStatusEnum, concat(s.deposit_type_enum, '') as 
subEntityType, CASE WHEN g.id is null THEN 'client' ELSE 'group' END as 
parentType "

Review Comment:
   how does solve this problem by changing this to not be null only one place? 
what happens if you search in SAVINGS accounts and LOAN accounts? 



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



[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API

2023-07-18 Thread via GitHub


adamsaghy commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1266820857


##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -108,33 +108,33 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 
 final String groupMatchSql = " (select CASE WHEN g.level_id=1 THEN 
'CENTER' ELSE 'GROUP' END as entityType, g.id as entityId, g.display_name as 
entityName, g.external_id as entityExternalId, g.account_no as entityAccountNo, 
"
 + " g.office_id as parentId, o.name as parentName, null as 
entityMobileNo, g.status_enum as entityStatusEnum, null as subEntityType, null 
as parentType "
-+ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search or g.id like :search )) "

Review Comment:
   Not answered...



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



[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API

2023-07-18 Thread via GitHub


adamsaghy commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1266819189


##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -90,7 +90,7 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 + " order by l.id desc";
 
 final String savingMatchSql = " (select 'SAVING' as entityType, s.id 
as entityId, sp.name as entityName, s.external_id as entityExternalId, 
s.account_no as entityAccountNo "
-+ " , coalesce(c.id,g.id) as parentId, 
coalesce(c.display_name,g.display_name) as parentName, null as entityMobileNo, 
s.status_enum as entityStatusEnum, s.deposit_type_enum as subEntityType, CASE 
WHEN g.id is null THEN 'client' ELSE 'group' END as parentType "
++ " , coalesce(c.id,g.id) as parentId, 
coalesce(c.display_name, g.display_name) as parentName, null as entityMobileNo, 
s.status_enum as entityStatusEnum, concat(s.deposit_type_enum, '') as 
subEntityType, CASE WHEN g.id is null THEN 'client' ELSE 'group' END as 
parentType "

Review Comment:
   Nothing changed...



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



[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API

2023-07-18 Thread via GitHub


adamsaghy commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1266639823


##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -90,7 +90,7 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 + " order by l.id desc";
 
 final String savingMatchSql = " (select 'SAVING' as entityType, s.id 
as entityId, sp.name as entityName, s.external_id as entityExternalId, 
s.account_no as entityAccountNo "
-+ " , coalesce(c.id,g.id) as parentId, 
coalesce(c.display_name,g.display_name) as parentName, null as entityMobileNo, 
s.status_enum as entityStatusEnum, s.deposit_type_enum as subEntityType, CASE 
WHEN g.id is null THEN 'client' ELSE 'group' END as parentType "
++ " , coalesce(c.id,g.id) as parentId, 
coalesce(c.display_name, g.display_name) as parentName, null as entityMobileNo, 
s.status_enum as entityStatusEnum, concat(s.deposit_type_enum, '') as 
subEntityType, CASE WHEN g.id is null THEN 'client' ELSE 'group' END as 
parentType "

Review Comment:
   Every other places the `subEntityType` is null, i dont think thats the place 
to fix this. Please rather have a condition in the mapper if you dont wanna see 
`DepositType.INVALID`



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



[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API

2023-07-18 Thread via GitHub


adamsaghy commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1266639823


##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -90,7 +90,7 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 + " order by l.id desc";
 
 final String savingMatchSql = " (select 'SAVING' as entityType, s.id 
as entityId, sp.name as entityName, s.external_id as entityExternalId, 
s.account_no as entityAccountNo "
-+ " , coalesce(c.id,g.id) as parentId, 
coalesce(c.display_name,g.display_name) as parentName, null as entityMobileNo, 
s.status_enum as entityStatusEnum, s.deposit_type_enum as subEntityType, CASE 
WHEN g.id is null THEN 'client' ELSE 'group' END as parentType "
++ " , coalesce(c.id,g.id) as parentId, 
coalesce(c.display_name, g.display_name) as parentName, null as entityMobileNo, 
s.status_enum as entityStatusEnum, concat(s.deposit_type_enum, '') as 
subEntityType, CASE WHEN g.id is null THEN 'client' ELSE 'group' END as 
parentType "

Review Comment:
   Every other places the `subEntityType` is null, i dont think thats the place 
to fix this. Please rather have a condition in the mapper to avoid NPE issues.



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



[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API

2023-07-18 Thread via GitHub


adamsaghy commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1266636681


##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -108,33 +108,33 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 
 final String groupMatchSql = " (select CASE WHEN g.level_id=1 THEN 
'CENTER' ELSE 'GROUP' END as entityType, g.id as entityId, g.display_name as 
entityName, g.external_id as entityExternalId, g.account_no as entityAccountNo, 
"
 + " g.office_id as parentId, o.name as parentName, null as 
entityMobileNo, g.status_enum as entityStatusEnum, null as subEntityType, null 
as parentType "
-+ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search or g.id like :search )) "
++ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search )) "
 + " order by g.id desc";
 
 final StringBuilder sql = new StringBuilder();
 
 if (searchConditions.isClientSearch()) {
-sql.append(clientMatchSql).append(union);
+sql.append("(" + clientMatchSql + ")").append(union);
 }
 
 if (searchConditions.isLoanSeach()) {
-sql.append(loanMatchSql).append(union);
+sql.append("(" + loanMatchSql + ")").append(union);
 }
 
 if (searchConditions.isSavingSeach()) {
-sql.append(savingMatchSql).append(union);
+sql.append("(" + savingMatchSql + ")").append(union);
 }
 
 if (searchConditions.isShareSeach()) {
-sql.append(shareMatchSql).append(union);
+sql.append("(" + shareMatchSql + ")").append(union);

Review Comment:
   Please remove string concatenation by + operator... use the `append` method 
since you are using StringBuilder!



##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -108,33 +108,33 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 
 final String groupMatchSql = " (select CASE WHEN g.level_id=1 THEN 
'CENTER' ELSE 'GROUP' END as entityType, g.id as entityId, g.display_name as 
entityName, g.external_id as entityExternalId, g.account_no as entityAccountNo, 
"
 + " g.office_id as parentId, o.name as parentName, null as 
entityMobileNo, g.status_enum as entityStatusEnum, null as subEntityType, null 
as parentType "
-+ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search or g.id like :search )) "
++ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search )) "
 + " order by g.id desc";
 
 final StringBuilder sql = new StringBuilder();
 
 if (searchConditions.isClientSearch()) {
-sql.append(clientMatchSql).append(union);
+sql.append("(" + clientMatchSql + ")").append(union);
 }
 
 if (searchConditions.isLoanSeach()) {
-sql.append(loanMatchSql).append(union);
+sql.append("(" + loanMatchSql + ")").append(union);
 }
 
 if (searchConditions.isSavingSeach()) {
-sql.append(savingMatchSql).append(union);
+sql.append("(" + savingMatchSql + ")").append(union);
 }
 
 if (searchConditions.isShareSeach()) {
-sql.append(shareMatchSql).append(union);
+sql.append("(" + shareMatchSql + ")").append(union);
 }
 
 if (searchConditions.isClientIdentifierSearch()) {
-sql.append(clientIdentifierMatchSql).append(union);
+sql.append("(" + clientIdentifierMatchSql + ")").append(union);

Review Comment:
   Please remove string concatenation by + operator... use the `append` method 
since you are using StringBuilder!



##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -108,33 +108,33 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 
 final String groupMatchSql = " (select CASE WHEN g.level_id=1 THEN 
'CENTER' ELSE 'GROUP' END as entityType, g.id as entityId, g.display_name as 
entityName, g.external_id as entityExternalId, g.account_no as entityAccountNo, 
"
 + " g.office_id as parentId, o.name as parentName, null as 
entityMobileNo, g.status_enum as entityStatusEnum, null as 

[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API

2023-07-18 Thread via GitHub


adamsaghy commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1266636421


##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -108,33 +108,33 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 
 final String groupMatchSql = " (select CASE WHEN g.level_id=1 THEN 
'CENTER' ELSE 'GROUP' END as entityType, g.id as entityId, g.display_name as 
entityName, g.external_id as entityExternalId, g.account_no as entityAccountNo, 
"
 + " g.office_id as parentId, o.name as parentName, null as 
entityMobileNo, g.status_enum as entityStatusEnum, null as subEntityType, null 
as parentType "
-+ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search or g.id like :search )) "
++ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search )) "
 + " order by g.id desc";
 
 final StringBuilder sql = new StringBuilder();
 
 if (searchConditions.isClientSearch()) {
-sql.append(clientMatchSql).append(union);
+sql.append("(" + clientMatchSql + ")").append(union);

Review Comment:
   Please remove string concatenation by + operator... use the `append` method 
since you are using StringBuilder!



##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -108,33 +108,33 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 
 final String groupMatchSql = " (select CASE WHEN g.level_id=1 THEN 
'CENTER' ELSE 'GROUP' END as entityType, g.id as entityId, g.display_name as 
entityName, g.external_id as entityExternalId, g.account_no as entityAccountNo, 
"
 + " g.office_id as parentId, o.name as parentName, null as 
entityMobileNo, g.status_enum as entityStatusEnum, null as subEntityType, null 
as parentType "
-+ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search or g.id like :search )) "
++ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search )) "
 + " order by g.id desc";
 
 final StringBuilder sql = new StringBuilder();
 
 if (searchConditions.isClientSearch()) {
-sql.append(clientMatchSql).append(union);
+sql.append("(" + clientMatchSql + ")").append(union);
 }
 
 if (searchConditions.isLoanSeach()) {
-sql.append(loanMatchSql).append(union);
+sql.append("(" + loanMatchSql + ")").append(union);

Review Comment:
   Please remove string concatenation by + operator... use the `append` method 
since you are using StringBuilder!



##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -108,33 +108,33 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 
 final String groupMatchSql = " (select CASE WHEN g.level_id=1 THEN 
'CENTER' ELSE 'GROUP' END as entityType, g.id as entityId, g.display_name as 
entityName, g.external_id as entityExternalId, g.account_no as entityAccountNo, 
"
 + " g.office_id as parentId, o.name as parentName, null as 
entityMobileNo, g.status_enum as entityStatusEnum, null as subEntityType, null 
as parentType "
-+ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search or g.id like :search )) "
++ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search )) "
 + " order by g.id desc";
 
 final StringBuilder sql = new StringBuilder();
 
 if (searchConditions.isClientSearch()) {
-sql.append(clientMatchSql).append(union);
+sql.append("(" + clientMatchSql + ")").append(union);
 }
 
 if (searchConditions.isLoanSeach()) {
-sql.append(loanMatchSql).append(union);
+sql.append("(" + loanMatchSql + ")").append(union);
 }
 
 if (searchConditions.isSavingSeach()) {
-sql.append(savingMatchSql).append(union);
+sql.append("(" + savingMatchSql + ")").append(union);

Review Comment:
   

[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API

2023-07-18 Thread via GitHub


adamsaghy commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1266635037


##
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadPlatformServiceImpl.java:
##
@@ -108,33 +108,33 @@ public String searchSchema(final SearchConditions 
searchConditions) {
 
 final String groupMatchSql = " (select CASE WHEN g.level_id=1 THEN 
'CENTER' ELSE 'GROUP' END as entityType, g.id as entityId, g.display_name as 
entityName, g.external_id as entityExternalId, g.account_no as entityAccountNo, 
"
 + " g.office_id as parentId, o.name as parentName, null as 
entityMobileNo, g.status_enum as entityStatusEnum, null as subEntityType, null 
as parentType "
-+ " from m_group g join m_office o on o.id = g.office_id where 
o.hierarchy like :hierarchy and (g.account_no like :search or g.display_name 
like :search or g.external_id like :search or g.id like :search )) "

Review Comment:
   filtering by group id is not needed anymore?



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