-
-
Notifications
You must be signed in to change notification settings - Fork 825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(dev/core#1252) Fix paging on contribution summary #15528
Conversation
(Standard links)
|
02d0f41
to
9b81f33
Compare
9b81f33
to
50f2c45
Compare
@VangelisP it looks like we will do another 5.18 drop early next week - if you are able to check out if this resolves the issue without other issues we might be able to get it into that drop |
I will start reviewing it asap |
Hello @eileenmcnaughton , looks like it's working properly and we got the pager back without any side-effects (that i could see). Tried it in Civi 5.16.2 & 5.18.3. |
Reproduced the issue , did an r-run with the patch and pagination works. I would like to see this in 5.19 (RC) I also agree it makes sense to add to 5.18 if another release is being readied |
Merging based on @kcristiano review. @eileenmcnaughton Do you want to do a backport for 5.18? |
OK - I've created a 5.19 port - #15558 - we shouldn't really have merged this to master once we decided to put in 5.19 but I doubt it will matter |
5.18 #15559 |
@eileenmcnaughton Sorry, I read 5.19 in the comments and my mind gave up noticing that it was still on master branch |
Overview
@VangelisP I haven't put this through it's paces but I think something like this would be the right way to fix the paging bug & also improve the report
Before
Paging sometimes not available - FGB dependent
After
Technical Details
Per https://lab.civicrm.org/dev/core/issues/1252#note_24325
Comments
https://lab.civicrm.org/dev/core/issues/1252
@yashodha you might have an interest in this - also maybe @kcristiano