compucorp / uk.co.compucorp.civicrm.pivotreport

CiviCRM Pivot table reporting solution
Other
8 stars 13 forks source link

GS-123: Fix getPaginatedResults Page Number Logic #110

Closed vinuvarshith closed 6 years ago

vinuvarshith commented 6 years ago

Overview

In a site with a lot of cases, a good number of them were missing on the pivot reports.

Issue

On looking into the code, getPaginatedResults() checks for previous index name and if its the same it increments the page number and if the index is different it resets page number to 0. However in certain cases there are indexes which are not sequential but have the same index name. In such cases the page number is wrongly set to 0 and the old page entities are missed.

Resolution

Maintain an array of index names and page numbers instead of a single index name.

tunbola commented 6 years ago

@vinuvarshith, Solution looks good. Can you make the following changes to PR Prefix the PR title with the ticket number e.g GS-123: Fix getPaginatedResults Page Number Logic and also the commit messages. This is the standard in CiviHR but I don't know if its followed for this project but nonetheless I think its a good one.

guanhuan commented 6 years ago

Thanks @tunbola, @vinuvarshith it is the same process on the client project side. Please note it for going forward. Also please repoint this to develop following the git workflow.

vinuvarshith commented 6 years ago

Created new PR against develop branchhttps://github.com/compucorp/uk.co.compucorp.civicrm.pivotreport/pull/111 Closing this