AdminUI - Replace Contact Summary Activities tab

[colemanw]

Overview

3rd core contact summary tab to be replaced by SearchKit!
Unlike #27610 and #27701 this one is added to the optional Admin UI extension instead of directly to core, since it's more complex and more heavily used. This will give a trial period with some time to test the new version & update any customizations they've made to the old version of the tab.

Before

After

Comments

A couple differences between the before & after UI:

1. The (rather obscure) 'preserve_activity_tab_filter' system setting is now ignored. Filters are always remembered on this tab. Instead of via the setting, the behavior can be disabled by toggling "Remember Filters" in the Afform (thanks to SearchKit - Optionally remember filter values when user revisits sear… #27737 it's now a general feature for every search form not just the activity tab).
2. To create a new activity, instead of picking an activity type from a dropdown and then getting a popup, you get a popup and then pick the activity type from a dropdown. Fixed via SearchKit - New getLinks API action for advanced link handling #27973, see updated screenshot above.

Note: The civicaseShowCaseActivities system setting is respected by this new display. Case activities will be shown if that setting is enabled. However, once this code gets migrated from AdminUI to core, we should probably look at phasing out that setting since it can be achieved simply by editing the SavedSearch criteria.

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

There is a conflict now in Summary.php as a result of merging the relationships tab.

1. Note that not all activities are available through the 'Add Activity' pop-up. So for example 'Send en Email' and 'Print/Merge Document' cannot be added.
2. Go to a contact, look at activities, go to Contributions tab, add a Contribution:
   a. The Activities tab header does not update with the new activities count
   b. Click to the Activities tab: does not show the new Contribution activity. Page refresh shows the new activity.
3. (Minor) - the Add Activity type selector does not include icons, but that is a limitation of that old form.

Great to see another tab being replaced!

[francescbassas]

I can't see +Add Activity button:

Also I have some doubts, in terms of usability, about removing the exclude activity type on the first view. From the perspective of the previous design, in general, I use more often exclude mailing activity type. With this proposal, I need to know that I have to modify the "is one of" selector and also make 1 click more than with the previous design.

[demeritcowboy]

It looks nice. A couple notes:

• I don't see any activities on the tab if I'm logged in as the demo user (not admin). TypeError: apiResults is undefined
• I don't see file-on-case in the row actions? It normally only shows if you have at least one case, but I do have at least one case.
• There's an existing setting to allow viewing case activities on the activity tab. It's not working, but even if it was I expect it would get the url wrong since case activities have different urls, and using the regular edit url can lead to broken activities.
• I haven't tested this yet but the edit/delete row actions should be dependent on the activity type, and sometimes permissions (e.g. inbound email). That might be ok though if the form preprocess already checks it (which it should, hopefully?).

[colemanw]

@aydun Note that not all activities are available through the 'Add Activity' pop-up.

Fixed. I had to add an ugly hack to copy the list of activities from the toolbar menu at the top of the page. Don't look at the code. But at least it's self-contained and we can justify it with the thousands of lines of bad code this will get rid of.

@aydun does not show the new Contribution activity

From what I can tell, that's a pre-existing condition. But ok, fixed.

@francescbassas I can't see +Add Activity button
@demeritcowboy I don't see any activities on the tab if I'm logged in as the demo user

Those were both caused by the same permission-related bug in the API. Fixed.

@francescbassas I have some doubts, in terms of usability, about removing the exclude activity type on the first view

Well, personally I think the previous design was quite silly to put two mutually-exclusive filters on the same form (if you fill out both they will contradict each other). But whatever, if the path of least resistance to getting this merged is to add it, ok, I've added it.

@demeritcowboy I don't see file-on-case in the row actions?

Hmm, ok that's a todo.

@demeritcowboy There's an existing setting to allow viewing case activities on the activity tab

I guess that setting will be irrelevant now. The new way to change the behavior would be to locally override the saved search and remove the where clause that excludes cases.

@demeritcowboy I expect it would get the url wrong since case activities have different urls, and using the regular edit url can lead to broken activities.

Is that even true for updates? Maybe we can just fix the form.

@demeritcowboy I haven't tested this yet but the edit/delete row actions should be dependent on the activity type...

Again, I'm not sure that's true for updates, but if so we ought to fix it in the form, as per-activity-type dynamic urls would be crazy hard.

[demeritcowboy]

Those were both caused by the same permission-related bug in the API. Fixed.

Cool.

There's an existing setting to allow viewing case activities on the activity tab

I guess that setting will be irrelevant now. The new way to change the behavior would be to locally override the saved search and remove the where clause that excludes cases.

Yeah ok. I see this tab in particular suffering from the message-template upgrade style nightmare, having to merge in changes from core if you've overridden it, but I still don't have a good idea what to do about that.

Another general thing maybe better discussed elsewhere, is that there are existing unit tests for this feature which obviously didn't catch this here. We maybe need a strategy for what to do with existing unit tests on various features on tabs, and also for admin-ui although I doubt there are many for those.

I expect it would get the url wrong since case activities have different urls, and using the regular edit url can lead to broken activities.

Is that even true for updates? Maybe we can just fix the form.

I'm not sure what you mean by "even for updates", but yes? I will grant that since activity revisions are gone it at least doesn't have that problem. But as one simple example edit an Open Case activity with the regular url. First, the fields are not the right set of fields. Second, the sections are not the right sections, e.g. there's supposed to be a "send a copy" section, and I definitely don't want a "repeat activity" section on open case. Third, if you edit the activity date it doesn't do all the right things. I also get a "We could not find a contact id" error popup on save which doesn't at first glance affect the save, but is obviously wrong.

So I'm not sure what you mean by "fix the form". It's a different form.

I haven't tested this yet but the edit/delete row actions should be dependent on the activity type...

Again, I'm not sure that's true for updates, but if so we ought to fix it in the form, as per-activity-type dynamic urls would be crazy hard.

It's true for some case activity types, and at least for regular inbound and outbound email. I did test now.

Also contribution and event registration activities used to link to their entities instead. They're now linking to the regular activity url which is ... weird.

Is there a way for extensions to change these links per-row? (Core or 3rd party extensions.) Or is that what you mean by crazy hard - that these have to be part of the search display definition?

[colemanw]

@demeritcowboy thanks for summing up this crummy situation. IMO it's a bad design if the form to edit a thing requires a lot of information about the thing to be passed through the URL. I've seen it in other places too, and fixed it where I could. For example the Manage Case Form was requiring both an id and a cid when the latter can be looked up by the form.

When you only ever link to a form from one place, then it seems easy enough to generate fat links that carry a lot of calculated data. But as soon as you start needing to link from other places, performing that same logic all over the place gets messy. IMO the ideal edit link carries nothing but the id, and the form calculates the rest.

SearchKit is most efficient with thin (id-only) links. It can deal with fatter links and will add the necessary SELECTs to the query, but only if it can all be expressed with tokens. For example, the Relationship edit form, for some reason, requires 3 bits of data to be passed in: id, cid & rtype. That probably represents a design flaw in the form, but SearchKit copes with it because it knows what to do with tokens.

But what you're describing requires the links be completely different depending on the activity type, and some of the forms aren't even activity forms! That's not tokenizeable. Bleh.
Well maybe it is...

What if we made the tokenized link look like this:

• civicrm/activity/[activity_type_id:name]?reset=1&action=add
• civicrm/activity/[activity_type_id:name]?reset=1&action=update&id=[id]
• civicrm/activity/[activity_type_id:name]?reset=1&action=delete&id=[id]

SearchKit would handle that just fine. And because of the way paths work in CiviCRM, that makes civicrm/activity a fall-back if a specialized form doesn't exist for that activity type. That just requires us to rearrange (or add aliases for) the existing paths.

[francescbassas]

Thanks @colemanw

I can see the "+ Add Activity" button

First version:

Current version:

Regarding the inclusion and exclusion filters, I share with you that the first version avoids the confusion of using the inclusion and exclusion filters at the same time. But instead, the current version maintains the practicality of being able to directly enter the activities to be excluded. Perhaps some warning could be added if both filters are filled in the style: "Only one of the exclude or include filters should be used."

For me, I have no resistance about the first option, just doubts. In any case, if we keep the current option, we would add labels to the fields (include, exclude, date and status). Otherwise, once the value is entered, the context of which filter is applied is lost.

[larssandergreen]

Few UI thoughts:

The display needs a default sort order of date DESC.

Hamburger menu links should just say View, Edit and Delete (instead of View Activity, Update Activity, Delete Activity). Do we need the menu though? Just three links as we have currently would be more usable. Might be my personal bugbear, but the red highlighting of delete makes it stand out more than other options, when it is the least used and should be less prominent.

Clicking a contact in Added by / With / Assigned opens in a new tab, which I don't think we want.

Not sure entirely what's going on, but clicking around I'm sometimes getting two Add Activity buttons and sometimes the date field is quite small.

If you select "Choose Date Range" the UI gets a bit weird as it jumps to another line, not sure what could be done about that:

We don't really need times here. Do we have an option to do dates only with these?
I think there should be some space between the fields and the Add Activity button, it's weird how they touch.
Also noting that we still have the problem where the end date is not inclusive (i.e. if you set Oct 5 as the end of your range, the search does not include activities on Oct 5).

[colemanw]

Thanks @larssandergreen for testing.
I've fixed the sort order and the double buttons. The rest feels like fine-tuning of SearchKit in general and maybe not a blocker.
However, what is a blocker is the activity links :(
Marking as a draft until that can get solved somehow.

[colemanw]

I've added the "Remember Filters" feature to Afform, which will replace the functionality of the 'preserve_activity_tab_filter' setting.

[colemanw]

@aydun @larssandergreen @francescbassas @demeritcowboy I've reworked this PR in tandem with #27973. When that one is merged this one will be fully-functional. I've also moved this into the Admin UI extension to give it a softer launch. I've updated the description, screenshots and removed draft status. It's ready!

[francescbassas]

I've tested it above and it looks fine. I don't feel like I'm missing anything from the original interface. Just a little thing. When a contribution is generated, just before the number of activities is updated, a string "true" appears.

[colemanw]

Thanks @francescbassas - you must have very quick fingers to have captured that screenshot!
I found the issue & fixed with f34b768. I believe this is merge-ready now.

[eileenmcnaughton]

I'm going to merge this based on the discussion / review / testing that has already gone into it

I think the issue is how we communicate around this one as per mentioned above this is a tab some users have customised

[aydun]

I'm slowly catching up... Looks good @colemanw !