ptuomola commented on a change in pull request #1286:
URL: https://github.com/apache/fineract/pull/1286#discussion_r477940195
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/organisation/staff/api/StaffApiResource.java
##########
@@ -118,7 +118,6 @@ public StaffApiResource(final PlatformSecurityContext
context, final StaffReadPl
@ApiResponse(responseCode = "200", description = "OK", content =
@Content(array = @ArraySchema(schema = @Schema(implementation =
StaffApiResourceSwagger.GetStaffResponse.class)))),
@ApiResponse(responseCode = "200", description = "GET
https://DomainName/api/v1/staff?status={ACTIVE|INACTIVE|ALL}", content =
@Content(schema = @Schema(implementation =
StaffApiResourceSwagger.GetStaffResponse.class))) })
public String retrieveStaff(@Context final UriInfo uriInfo,
- @QueryParam("sqlSearch") @Parameter(description = "sqlSearch")
final String sqlSearch,
@QueryParam("officeId") @Parameter(description = "officeId") final
Long officeId,
Review comment:
Are we sure that no client is calling this - i.e. will this change make
an impact to one of the UIs?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/organisation/staff/service/StaffReadPlatformServiceImpl.java
##########
@@ -204,49 +215,38 @@ public StaffData retrieveStaff(final Long staffId) {
}
@Override
- public Collection<StaffData> retrieveAllStaff(final String sqlSearch,
final Long officeId, final boolean loanOfficersOnly,
- final String status) {
- final String extraCriteria = getStaffCriteria(sqlSearch, officeId,
loanOfficersOnly, status);
- return retrieveAllStaff(extraCriteria, officeId);
+ public Collection<StaffData> retrieveAllStaff(final Long officeId, final
boolean loanOfficersOnly, final String status) {
+ final SQLBuilder extraCriteria = getStaffCriteria(officeId,
loanOfficersOnly, status);
+ return retrieveAllStaff(extraCriteria);
}
- private Collection<StaffData> retrieveAllStaff(final String extraCriteria,
Long officeId) {
+ private Collection<StaffData> retrieveAllStaff(final SQLBuilder
extraCriteria) {
final StaffMapper rm = new StaffMapper();
String sql = "select " + rm.schema();
- final String hierarchy =
this.context.authenticatedUser().getOffice().getHierarchy() + "%";
- if (StringUtils.isNotBlank(extraCriteria)) {
Review comment:
I think it would have actually been good to leave looking up hierarchy
and adding the condition for it here. That way, regardless of how you call
retrieveAllStaff, you can't retrieve things you are not allowed to see.
Otherwise we are relying on the caller to add the condition for hierarchy -
which they may forget...
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/organisation/staff/service/StaffReadPlatformServiceImpl.java
##########
@@ -159,7 +159,18 @@ public StaffData mapRow(final ResultSet rs,
@SuppressWarnings("unused") final in
@Override
public Collection<StaffData> retrieveAllLoanOfficersInOfficeById(final
Long officeId) {
- return retrieveAllStaff(" office_id = ? and is_loan_officer=1 and
o.hierarchy like ?", officeId);
+ final StaffMapper rm = new StaffMapper();
+ String sql = "select " + rm.schema();
+ SQLBuilder extraCriteria = new SQLBuilder();
+ extraCriteria.addCriteria(" office_id = ", officeId);
+ extraCriteria.addCriteria(" is_loan_officer= = ", 1);
+ final String hierarchy =
this.context.authenticatedUser().getOffice().getHierarchy() + "%";
+ extraCriteria.addCriteria(" o.hierarchy like ", hierarchy);
+
+ sql += " " + extraCriteria.getSQLTemplate();
+ sql = sql + " order by s.lastname ";
+
+ return this.jdbcTemplate.query(sql, rm, extraCriteria.getArguments());
Review comment:
Would we not want to do this "magic" of adding the condition for
hierarchy and then executing the JDBC template only in one place? I.e. would it
not be better to keep this code still in retrieveAllStaff and just call
retrieveAllStaff with the right SQLBuilder from here? Or is there a reason why
we want to duplicate the code here?
----------------------------------------------------------------
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:
[email protected]
