[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
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
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
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
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
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
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
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
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