Skip to content
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

Intra-RC regression: User dashboard: show most recent contributions, not earliest #13903

Merged
merged 2 commits into from
Mar 27, 2019

Conversation

agh1
Copy link
Contributor

@agh1 agh1 commented Mar 27, 2019

This is the same as #13901, just for the 5.12 RC.

Overview

A recent change in how contributions are listed in the User Dashboard has caused a regression where it displays the 12 contributions with the lowest IDs (generally the earliest) rather than the most recent ones.

Before

A test user makes 15 contributions, one on each day in March. The first 12 appear.

Screenshot_2019-03-27 Dashboard - demo example com grantdetailreport(1)

After

Same scenario: 15 contributions, one on each day in March. The most recent 12 appear.

Screenshot_2019-03-27 Dashboard - demo example com grantdetailreport

Technical Details

The listing was switched to use the API in #13584. There isn't a good way in APIv3 to list the most recent things starting with the oldest. In this case, I had to sort by most recent and then reverse the array.

Comments

I might wonder whether the header should say "Your Most Recent Contribution(s)", but that's a bit outside scope, and the user dash appears to have always just been the dozen most recent contributions.

@civibot
Copy link

civibot bot commented Mar 27, 2019

(Standard links)

@eileenmcnaughton
Copy link
Contributor

thanks @agh1 looks good to me. It took me a while to understand your sort comments - but I got there.

@agh1
Copy link
Contributor Author

agh1 commented Mar 27, 2019

I just updated the test that was failing on #13901: the CRM_Contact_Page_View_UserDashBoardTest::testDashboardContentContributionsWithInvoicingEnabled test (whether intentionally or not) was checking that the first contribution created appeared first in the listing when all three contributions were set with the same date. I updated it so that the contributions have different dates, and the first one created is second chronologically, so the test now confirms that the second one listed is ID 1.

@eileenmcnaughton
Copy link
Contributor

Those tests were written prior to switching over to ensure the switch didn't cause regressions so it will have been testing 'what was happening at the time it was written' - obviously having the dates the same was a pretty limited data set & your change sounds like an improvement

@eileenmcnaughton eileenmcnaughton merged commit 816197a into civicrm:5.12 Mar 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants