You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/08/25 23:27:26 UTC
[GitHub] [fineract] thesmallstar opened a new pull request #1279: FINERACT-854 Removed string concatenated SQL in ClientReadPlatform
thesmallstar opened a new pull request #1279:
URL: https://github.com/apache/fineract/pull/1279
Refer: https://issues.apache.org/jira/browse/FINERACT-854 and #725 #723 for background.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] thesmallstar commented on a change in pull request #1279: FINERACT-854 Removed string concatenated SQL in ClientReadPlatform
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1279:
URL: https://github.com/apache/fineract/pull/1279#discussion_r476856844
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java
##########
@@ -196,25 +197,20 @@ public ClientData retrieveTemplate(final Long officeId, final boolean staffInSel
// this.context.validateAccessRights(searchParameters.getHierarchy());
// underHierarchySearchString = searchParameters.getHierarchy() + "%";
// }
- List<Object> paramList = new ArrayList<>(Arrays.asList(underHierarchySearchString, underHierarchySearchString));
final StringBuilder sqlBuilder = new StringBuilder(200);
sqlBuilder.append("select SQL_CALC_FOUND_ROWS ");
sqlBuilder.append(this.clientMapper.schema());
- sqlBuilder.append(" where (o.hierarchy like ? or transferToOffice.hierarchy like ?) ");
+ final SQLBuilder extraCriteria = buildSqlStringFromClientCriteria(this.clientMapper.schema(), searchParameters);
+ sqlBuilder.append(" where (o.hierarchy like " + underHierarchySearchString + " or transferToOffice.hierarchy like "
+ + underHierarchySearchString + " ) ");
Review comment:
@vorburger can you please see this part?
I am not sure how to handle this or condition in our SQL builder.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] github-actions[bot] commented on pull request #1279: FINERACT-854 Removed string concatenated SQL in ClientReadPlatform
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1279:
URL: https://github.com/apache/fineract/pull/1279#issuecomment-698657777
This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] thesmallstar commented on a change in pull request #1279: FINERACT-854 Removed string concatenated SQL in ClientReadPlatform
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1279:
URL: https://github.com/apache/fineract/pull/1279#discussion_r476861121
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java
##########
@@ -196,25 +197,20 @@ public ClientData retrieveTemplate(final Long officeId, final boolean staffInSel
// this.context.validateAccessRights(searchParameters.getHierarchy());
// underHierarchySearchString = searchParameters.getHierarchy() + "%";
// }
- List<Object> paramList = new ArrayList<>(Arrays.asList(underHierarchySearchString, underHierarchySearchString));
final StringBuilder sqlBuilder = new StringBuilder(200);
sqlBuilder.append("select SQL_CALC_FOUND_ROWS ");
sqlBuilder.append(this.clientMapper.schema());
- sqlBuilder.append(" where (o.hierarchy like ? or transferToOffice.hierarchy like ?) ");
+ final SQLBuilder extraCriteria = buildSqlStringFromClientCriteria(this.clientMapper.schema(), searchParameters);
+ sqlBuilder.append(" where (o.hierarchy like " + underHierarchySearchString + " or transferToOffice.hierarchy like "
+ + underHierarchySearchString + " ) ");
if (searchParameters != null) {
if (searchParameters.isSelfUser()) {
- sqlBuilder.append(
- " and c.id in (select umap.client_id from m_selfservice_user_client_mapping as umap where umap.appuser_id = ? ) ");
- paramList.add(appUserID);
+ extraCriteria.addNonNullCriteria(
+ " c.id in (select umap.client_id from m_selfservice_user_client_mapping as umap where umap.appuser_id = ",
+ appUserID + ")");
Review comment:
(definitely wrong current implementation), How to handle nested select queries where the inner query has the parameter? The normal way should not work IMO? Any ideas?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] github-actions[bot] closed pull request #1279: FINERACT-854 Removed string concatenated SQL in ClientReadPlatform
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #1279:
URL: https://github.com/apache/fineract/pull/1279
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] thesmallstar commented on a change in pull request #1279: FINERACT-854 Removed string concatenated SQL in ClientReadPlatform
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1279:
URL: https://github.com/apache/fineract/pull/1279#discussion_r476858480
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLBuilder.java
##########
@@ -122,6 +122,18 @@ public String getSQLTemplate() {
return "";
}
+ /**
+ * Returns the added criterias, created from the {@link #addCriteria(String, Object)}, with '?' placeholders.
+ *
+ * @return added criterias
+ */
+ public String getCriteria() {
+ if (sb.length() > 0) {
+ return sb.toString();
+ }
+ return "";
+ }
+
Review comment:
I thought we would need this, initially, since the "where" was already written in a SQL query which I was not able to replace with sqlBuilder. Please look into other comments, if solved I think this would resolve (needed/to be removed) itself :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] github-actions[bot] commented on pull request #1279: FINERACT-854 Removed string concatenated SQL in ClientReadPlatform
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1279:
URL: https://github.com/apache/fineract/pull/1279#issuecomment-698657777
This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org