[GitHub] [fineract] fynmanoj commented on a diff in pull request #2310: FINERACT-1609: Added Pagination to reports

[GitBox <gi...@apache.org>]

fynmanoj commented on code in PR #2310:
URL: https://github.com/apache/fineract/pull/2310#discussion_r863021056


##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/common/GlobalConfigurationHelper.java:
##########
@@ -99,8 +99,8 @@ public static void verifyAllDefaultGlobalConfigurations(final RequestSpecificati
         ArrayList<HashMap> actualGlobalConfigurations = getAllGlobalConfigurations(requestSpec, responseSpec);
 
         // There are currently 37 global configurations.
-        Assertions.assertEquals(38, expectedGlobalConfigurations.size());
-        Assertions.assertEquals(38, actualGlobalConfigurations.size());
+        Assertions.assertEquals(39, expectedGlobalConfigurations.size());
+        Assertions.assertEquals(39, actualGlobalConfigurations.size());

Review Comment:
   this also requires updating the Map for teardown. in `getAllDefaultGlobalConfigurations` method of `GlobalConfigurationHelper`



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/data/GenericResultsetData.java:
##########
@@ -41,6 +46,14 @@ public List<ResultsetRowData> getData() {
         return this.data;
     }
 
+    public void setReportSize(Integer totalReportSize) {

Review Comment:
   Let the setter name be the same as the variable. Easier to read code.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadReportingServiceImpl.java:
##########
@@ -162,9 +180,25 @@ public GenericResultsetData retrieveGenericResultset(final String name, final St
         final long startTime = System.currentTimeMillis();
         LOG.info("STARTING REPORT: {}   Type: {}", name, type);
 
-        final String sql = getSQLtoRun(name, type, queryParams, isSelfServiceUserReport);
-
-        final GenericResultsetData result = this.genericDataService.fillGenericResultSet(sql);
+        String sql = getSQLtoRun(name, type, queryParams, isSelfServiceUserReport);
+        final GenericResultsetData result;
+        Boolean paginationAllowed = BooleanUtil.parseBoolean(queryParams.get(ReportingConstants.paginationAllowed));
+
+        if (paginationAllowed) {
+            final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
+            final DataValidatorBuilder baseDataValidator = new DataValidatorBuilder(dataValidationErrors);
+            baseDataValidator.reset().parameter(pageNo).value(queryParams.get(pageNo)).notNull().throwValidationErrors();
+            Integer pageContent = this.configurationDomainService.pageContentLimit();
+            Page<GenericResultsetData> reportData = this.paginationHelper.fetchPage(this.jdbcTemplate, sql, null, new ReportMapper(sql));
+            Integer pageNo = Integer.parseInt(queryParams.get(ReportingConstants.pageNo));
+            pageNo = pageNo * pageContent;
+            sql = sql + " order by " + queryParams.get(paginationOrderBy) + " limit " + pageContent + " offset " + pageNo;

Review Comment:
   validate `paginationOrderBy`.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/ConfigurationDomainServiceJpa.java:
##########
@@ -220,6 +220,16 @@ public int getRoundingMode() {
         return defaultValue;
     }
 
+    @Override
+    public Integer pageContentLimit() {
+        final String propertyName = "page-content-limit";

Review Comment:
   Use more descriptive config name here . sample :"reports-pagination-number-of-items-per-page"



-- 
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