You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "josehernandezfintecheandomx (via GitHub)" <gi...@apache.org> on 2023/07/18 07:07:08 UTC
[GitHub] [fineract] josehernandezfintecheandomx opened a new pull request, #3311: FINERACT-1754: Fix Search API
josehernandezfintecheandomx opened a new pull request, #3311:
URL: https://github.com/apache/fineract/pull/3311
## Description
Fix the Internal Server Error in the Search by Resource API when we use more than one `resource`
- clients
- clientIdentifiers
- groups
- savings
- shares
- loans
It includes the Integration Tests for the Search API too
<img width="2045" alt="Screenshot 2023-07-18 at 0 51 02" src="https://github.com/apache/fineract/assets/44206706/2fc9fa47-f773-4882-a4f6-562bd75c5e18">
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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:
Please remove string concatenation by + operator... use the `append` method since you are using StringBuilder!
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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 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);
}
if (searchConditions.isGroupSearch()) {
- sql.append(groupMatchSql).append(union);
+ sql.append("(" + groupMatchSql + ")").append(union);
Review Comment:
Please remove string concatenation by + operator... use the `append` method since you are using StringBuilder!
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "josehernandezfintecheandomx (via GitHub)" <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1269554348
##########
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:
There will be not a problem, because in the SQL both values are now Strings values in the Loan will be null and in the Savings will be the Deposit Type (if exists) as `String` too. We get this because we are using the `concat` function. Even in the POJO `SearchData` this attribute is `String` type
` private final String subEntityType;
`
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy merged pull request #3311: FINERACT-1754: Fix Search API
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #3311:
URL: https://github.com/apache/fineract/pull/3311
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #3311: FINERACT-1754: Fix Search API
Posted by "josehernandezfintecheandomx (via GitHub)" <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #3311:
URL: https://github.com/apache/fineract/pull/3311#discussion_r1266704713
##########
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:
Done
##########
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:
No, none of the others filters by id and there is an error to compare bigint ans¡d string
##########
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:
Done
##########
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:
Done
##########
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:
If we leave the original SQL we get an error when we use more than one resource why? because the column subEntityType (as you have viewed) in almost all the SQL uses null (the null is considered as string type by the RDBMS) and in the Savings SQL filter It has a value for the Savings attribute deposit_type_enum (int type) and that difference to the RDBMS generates tan error, so It is not possible to manage after run the SQL in the mapper
##########
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:
No, none of the others filters by id and there is an error to compare bigint ans¡d string
##########
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);
}
if (searchConditions.isGroupSearch()) {
- sql.append(groupMatchSql).append(union);
+ sql.append("(" + groupMatchSql + ")").append(union);
Review Comment:
Done
##########
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:
The answer is No, the group id filter is not used more for the next reasons;
- None of the others filters use the filter by id (bigint)
- The RDBMS sends an error about to compare the id (bigint) against query (string)
##########
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:
done
##########
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:
Done
##########
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:
The reason to have this is because the RDBMS can not have in the same column strings (due the nulls) and integer for the Savings for the column deposit types
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org