Convert CiviCampaign Dashboard to SearchKit

[colemanw]

Overview

Replaces the 3-tab CiviCampaign dashboard with an Afform containing 3 tabs of SearchKit displays.

Before

Smarty+datatable.js-based dashboard tabs have lots of fun technical debt we could work on...

After

Or we could just replace the whole thing with an Afform and shed 2000+ lines of ancient code.

Comments

This depends on making Afform a required extension.

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[aydun]

Great to see some legacy code being deleted!

Overall, very nice but just a few minor bits:

1. On the test build site, the main menu still shows 'Dashboard' with a submenu of 'Surveys', 'Petitions', 'Campaigns' as well as the new 'Campaign Dashboard'
2. Comparing old and new, the old lets you search campaigns by start/end date and 'is active?'
3. Adding a survey does not work well as a pop-up. The first page is ok but the second provides blank text boxes for Contact Info and Questions, but it should have dropdowns. It also does not click through to the Results configuration screen. Opening Add Survey in a new tab works as expected.
4. From the new dashboard, go to Surveys, edit a survey: on that page, the breadcrumb points to the old link: http://core-27271-vkw.test-1.civicrm.org:8001/civicrm/campaign?reset=1&subPage=survey - which does go to the new dashboard but the default 'Campaigns' tab.
5. On Petitions, the Signatures menu has 'Form Link' which should be 'Sign'
6. As noted previously, breadcrumbs need work.

The 'Hide Pager if One Page' option is enabled for all of these search tables but is not working - but that is also not working on master.

[larssandergreen]

Re the Hide pager thing, I think we decided we needed to show the Page Size option when the pager was hidden, but I think we should hide it. I think that's what you mean, @aydun. Can't see why someone would need to adjust the page size when everything is on one page already (I've many times made the page size larger, but never smaller).

I wonder if we could lose the ID column for campaigns? I don't really see what purpose it serves to show that to the user (anyone technical enough to need the id for some reason can get it without it being shown explicitly). I think we should never show the ID as a general rule.

[aydun]

Hide pager: I don't see any benefit to showing the page size option if results are on one page anyway. I thought that should already be hidden by the setting, but if not then let's change it.

For these results, since the number of results is shown in the tab title we don't need it in the footer as well, but that's very minor.

ID's: that's a broader question so let's discuss that it chat.

[colemanw]

I've rebased this with the improvement from #27553 so that the tab titles would be translated.
This conversion has been a slog, but we're in the home stretch! I think all the blockers may have been resolved. To recap @aydun's list with @larssandergreen's additions:

1. This can be fixed by running regen after this PR is merged.
2. I thought the date filters were almost useless since they were exact-match only. And I left out the active filter because the search display sorts by active so you already see the active ones on top.
3. Fixed.
4. As we discussed earlier, the browser history doesn't work with these tabs but I don't think it's a blocker.
5. Fixed
6. Here's a fix: Afform - Switch to user-oriented breadcrumbs, move admin link to hover button #27618
7. Hide pager: fixed via SearchKit - When hiding pager, also hide page count #27616
8. Id column: I went ahead and removed it from the Campaigns tab. It already wasn't in the other two so at least now we're consistent.
9. "since the number of results is shown in the tab title we don't need it in the footer as well" - agreed. Removed.

[aydun]

Test this please

[aydun]

4. For a new survey, it feels a bit odd having the initial page as a pop-up and the subsequent pages not. But it works as is.

Let's get this merged. It works as is and we can always tweak it later.

[colemanw]

Wohoo!

[jensschuppe]

This removes public Core code on BAO classes that has not even been deprecated … just saying 😞

[colemanw]

@jensschuppe which BAO function were you using?

Edit: oh I see. Following the link to systopia/de.systopia.campaign#114 the Campaign Manager extension was using the count methods.

[jensschuppe]

Those three:

• CRM_Campaign_BAO_Campaign::getCampaignCount()
• CRM_Campaign_BAO_Survey::getSurveyCount()
• CRM_Campaign_BAO_Petition::getPetitionCount()
  … easily replaceable with API calls, sure, but still …

[colemanw]

Hmm... well I guess there are 2 ways to look at it:

1. BAO methods are not considered to be public APIs, and we've been telling extension developers "Don't use BAO functions, use the API" for at least 10 years now. And this type of situation is exactly the reason for saying that.
2. We do have an established pattern of deprecating code before removing it, and this refactor skipped that step.

@jensschuppe if you would like to do a PR against 5.67 that reverts the removal of the functions and adds noisy deprecation, I'd be happy to merge it.

[jensschuppe]

I think you are right, but that's just the ideal. Maybe let's continue this discussion over at https://chat.civicrm.org/civicrm/pl/31s333m6y7be8pakafb89aqary.

Re: removed Campaign methods: I think we'll rather adjust the extension, as that's the same effort as reverting in this case. Thanks a lot for the offer to revert, though.