galovics commented on code in PR #3158:
URL: https://github.com/apache/fineract/pull/3158#discussion_r1189389540
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/api/SavingsAccountTransactionsApiResource.java:
##########
@@ -114,6 +121,53 @@ public String retrieveTemplate(@PathParam("savingsId")
final Long savingsId,
SavingsApiSetConstants.SAVINGS_TRANSACTION_RESPONSE_DATA_PARAMETERS);
}
+ @GET
+ @Consumes({ MediaType.APPLICATION_JSON })
+ @Produces({ MediaType.APPLICATION_JSON })
+ @Operation(summary = "List Savings Account Transactions", description =
"The list capability of savings account transactions can support pagination,
sorting and filtering.\n\n"
+ + "Example Requests:\n" + "\n" +
"savingsaccounts/{savingsId}/transactions\n" + "\n"
+ + "savingsaccounts/{savingsId}/transactions?offset=10&limit=50\n"
+ "\n"
+ +
"savingsaccounts/{savingsId}/transactions?orderBy=displayName&sortOrder=DESC\n"
+ "\n"
+ +
"savingsaccounts/{savingsId}/transactions?dateFormat=yyyy-MM-dd&locale=en&fromDate=2013-01-01&toDate=2013-12-01\n"
+ "\n"
+ +
"savingsaccounts/{savingsId}/transactions?fromAmount=500&toAmount=1000\n" + "\n"
+ +
"savingsaccounts/{savingsId}/transactions?transactionType=deposit")
+ @ApiResponses({
+ @ApiResponse(responseCode = "200", description = "OK", content =
@Content(schema = @Schema(implementation =
SavingsAccountTransactionsApiResourceSwagger.GetSavingsAccountTransactionsResponse.class)))
})
+ public String retrieveAll(@Context final UriInfo uriInfo,
+ @PathParam("savingsId") @Parameter(description = "savingsId")
final Long savingsId,
+ @QueryParam("externalId") @Parameter(description = "externalId")
final String externalId,
+ @QueryParam("offset") @Parameter(description = "offset") final
Integer offset,
+ @QueryParam("limit") @Parameter(description = "limit") final
Integer limit,
+ @QueryParam("orderBy") @Parameter(description = "orderBy") final
String orderBy,
+ @QueryParam("sortOrder") @Parameter(description = "sortOrder")
final String sortOrder,
+ @QueryParam("dateFormat") @Parameter(description = "dateFormat")
final String dateFormat,
+ @QueryParam("fromDate") @Parameter(description = "fromDate") final
DateParam fromDateParam,
+ @QueryParam("toDate") @Parameter(description = "toDate") final
DateParam toDateParam,
+ @QueryParam("fromAmount") @Parameter(description = "fromAmount")
@DecimalMin(value = "0", message = "must be greater than or equal to 0") final
BigDecimal fromAmount,
+ @QueryParam("toAmount") @Parameter(description = "toAmount")
@DecimalMin(value = "0", message = "must be greater than or equal to 0") final
BigDecimal toAmount,
+ @QueryParam("locale") @Parameter(description = "locale") final
String locale,
+ @QueryParam("transactionType") @Parameter(description =
"transcationType") final String transactionType) {
+
+
this.context.authenticatedUser().validateHasReadPermission(SavingsApiConstants.SAVINGS_ACCOUNT_RESOURCE_NAME);
+ final SearchParameters searchParameters =
SearchParameters.forPagination(offset, limit, orderBy, sortOrder);
+
+ LocalDate fromDate = null;
Review Comment:
Why don't we use an argument resolver for this instead? That way we wouldn't
have to duplicate this logic anywhere but just go with a plain LocalDate in the
parameter list.
##########
fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm:
##########
@@ -29369,6 +29369,242 @@ <h2>Retrieve Savings Account Transaction
Template:</h2>
</div>
</div>
+ <a id="savingsaccounts_transactions_list"
name="savingsaccounts_transactions_list" class="old-syle-anchor"> </a>
Review Comment:
This is not used anymore.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java:
##########
@@ -1267,6 +1270,99 @@ public Collection<SavingsAccountTransactionData>
retrieveAllTransactions(final L
return this.jdbcTemplate.query(sql, this.transactionsMapper, new
Object[] { savingsId, depositAccountType.getValue() }); // NOSONAR
}
+ @Override
+ public Page<SavingsAccountTransactionData> retrieveAllTransactions(final
Long savingsId, DepositAccountType depositAccountType,
+ SearchParameters searchParameters, LocalDate fromDate, LocalDate
toDate,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal fromAmount,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal toAmount, String transactionType) {
+
+ final StringBuilder sqlBuilder = new StringBuilder(200);
+ sqlBuilder.append("select " + sqlGenerator.calcFoundRows() + " ");
+ sqlBuilder.append(this.transactionsMapper.schema());
+ sqlBuilder.append(" where sa.id = ? and sa.deposit_type_enum = ? ");
+ final Object[] objectArray = new Object[12];
+ objectArray[0] = savingsId;
+ objectArray[1] = depositAccountType.getValue();
+ int arrayPos = 2;
+
+ if (StringUtils.isNotEmpty(transactionType)) {
+ Integer transcationTypeEnum = null;
+ if ("deposit".equals(transactionType.toLowerCase())) {
+ transcationTypeEnum =
SavingsAccountTransactionType.DEPOSIT.getValue();
+ } else if ("withdrawal".equals(transactionType.toLowerCase())) {
Review Comment:
magic string
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java:
##########
@@ -1267,6 +1270,99 @@ public Collection<SavingsAccountTransactionData>
retrieveAllTransactions(final L
return this.jdbcTemplate.query(sql, this.transactionsMapper, new
Object[] { savingsId, depositAccountType.getValue() }); // NOSONAR
}
+ @Override
+ public Page<SavingsAccountTransactionData> retrieveAllTransactions(final
Long savingsId, DepositAccountType depositAccountType,
+ SearchParameters searchParameters, LocalDate fromDate, LocalDate
toDate,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal fromAmount,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal toAmount, String transactionType) {
+
+ final StringBuilder sqlBuilder = new StringBuilder(200);
Review Comment:
This is a super red flag, no native queries please unless you absolutely
need them. Why don't you simply use a JPQL query with a projection for this?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformService.java:
##########
@@ -45,6 +46,10 @@ public interface SavingsAccountReadPlatformService {
Collection<SavingsAccountTransactionData> retrieveAllTransactions(Long
savingsId, DepositAccountType depositAccountType);
+ Page<SavingsAccountTransactionData> retrieveAllTransactions(Long
savingsId, DepositAccountType depositAccountType,
Review Comment:
I don't like this signature. Why don't we create a parameter object which
has all the filterable attributes instead of passing them one by one.
That way we could reduce and keep the amount of parameters at the number of
3. savingsId, accountType, searchParams
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java:
##########
@@ -1267,6 +1270,99 @@ public Collection<SavingsAccountTransactionData>
retrieveAllTransactions(final L
return this.jdbcTemplate.query(sql, this.transactionsMapper, new
Object[] { savingsId, depositAccountType.getValue() }); // NOSONAR
}
+ @Override
+ public Page<SavingsAccountTransactionData> retrieveAllTransactions(final
Long savingsId, DepositAccountType depositAccountType,
+ SearchParameters searchParameters, LocalDate fromDate, LocalDate
toDate,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal fromAmount,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal toAmount, String transactionType) {
+
+ final StringBuilder sqlBuilder = new StringBuilder(200);
+ sqlBuilder.append("select " + sqlGenerator.calcFoundRows() + " ");
+ sqlBuilder.append(this.transactionsMapper.schema());
+ sqlBuilder.append(" where sa.id = ? and sa.deposit_type_enum = ? ");
+ final Object[] objectArray = new Object[12];
Review Comment:
Nope, this won't fly.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java:
##########
@@ -1267,6 +1270,99 @@ public Collection<SavingsAccountTransactionData>
retrieveAllTransactions(final L
return this.jdbcTemplate.query(sql, this.transactionsMapper, new
Object[] { savingsId, depositAccountType.getValue() }); // NOSONAR
}
+ @Override
+ public Page<SavingsAccountTransactionData> retrieveAllTransactions(final
Long savingsId, DepositAccountType depositAccountType,
+ SearchParameters searchParameters, LocalDate fromDate, LocalDate
toDate,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal fromAmount,
Review Comment:
What's going to evaluate this validation annotation?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java:
##########
@@ -48,6 +49,7 @@
import org.apache.fineract.organisation.monetary.data.CurrencyData;
import org.apache.fineract.organisation.staff.data.StaffData;
import org.apache.fineract.organisation.staff.service.StaffReadPlatformService;
+import
org.apache.fineract.organisation.teller.exception.InvalidDateInputException;
Review Comment:
This is a teller exception, why did we import it in the savings module?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java:
##########
@@ -1267,6 +1270,99 @@ public Collection<SavingsAccountTransactionData>
retrieveAllTransactions(final L
return this.jdbcTemplate.query(sql, this.transactionsMapper, new
Object[] { savingsId, depositAccountType.getValue() }); // NOSONAR
}
+ @Override
+ public Page<SavingsAccountTransactionData> retrieveAllTransactions(final
Long savingsId, DepositAccountType depositAccountType,
+ SearchParameters searchParameters, LocalDate fromDate, LocalDate
toDate,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal fromAmount,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal toAmount, String transactionType) {
+
+ final StringBuilder sqlBuilder = new StringBuilder(200);
+ sqlBuilder.append("select " + sqlGenerator.calcFoundRows() + " ");
+ sqlBuilder.append(this.transactionsMapper.schema());
+ sqlBuilder.append(" where sa.id = ? and sa.deposit_type_enum = ? ");
+ final Object[] objectArray = new Object[12];
+ objectArray[0] = savingsId;
+ objectArray[1] = depositAccountType.getValue();
+ int arrayPos = 2;
+
+ if (StringUtils.isNotEmpty(transactionType)) {
+ Integer transcationTypeEnum = null;
+ if ("deposit".equals(transactionType.toLowerCase())) {
Review Comment:
magic string
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java:
##########
@@ -1267,6 +1270,99 @@ public Collection<SavingsAccountTransactionData>
retrieveAllTransactions(final L
return this.jdbcTemplate.query(sql, this.transactionsMapper, new
Object[] { savingsId, depositAccountType.getValue() }); // NOSONAR
}
+ @Override
+ public Page<SavingsAccountTransactionData> retrieveAllTransactions(final
Long savingsId, DepositAccountType depositAccountType,
+ SearchParameters searchParameters, LocalDate fromDate, LocalDate
toDate,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal fromAmount,
+ @DecimalMin(value = "0", message = "must be greater than or equal
to 0") BigDecimal toAmount, String transactionType) {
+
+ final StringBuilder sqlBuilder = new StringBuilder(200);
+ sqlBuilder.append("select " + sqlGenerator.calcFoundRows() + " ");
+ sqlBuilder.append(this.transactionsMapper.schema());
+ sqlBuilder.append(" where sa.id = ? and sa.deposit_type_enum = ? ");
+ final Object[] objectArray = new Object[12];
+ objectArray[0] = savingsId;
+ objectArray[1] = depositAccountType.getValue();
+ int arrayPos = 2;
+
+ if (StringUtils.isNotEmpty(transactionType)) {
+ Integer transcationTypeEnum = null;
+ if ("deposit".equals(transactionType.toLowerCase())) {
+ transcationTypeEnum =
SavingsAccountTransactionType.DEPOSIT.getValue();
+ } else if ("withdrawal".equals(transactionType.toLowerCase())) {
+ transcationTypeEnum =
SavingsAccountTransactionType.WITHDRAWAL.getValue();
+ } else {
+ transcationTypeEnum =
SavingsAccountTransactionType.INVALID.getValue();
+ }
+ sqlBuilder.append(" and ").append(" tr.transaction_type_enum = ?
");
+ objectArray[arrayPos] = transcationTypeEnum;
+ arrayPos = arrayPos + 1;
+ }
+
+ if (fromDate != null || toDate != null) {
+ if (fromDate != null && toDate != null) {
+ if (toDate.isBefore(fromDate)) {
+ throw new InvalidDateInputException(fromDate.toString(),
toDate.toString());
Review Comment:
No message, nobody will understand whats the issue without looking at the
code.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
