Convert case activity links to 'actionLinks'

[mattwire]

Overview

Convert the case activity links to use the standard "actionLinks()" method instead of hardcoding html and making it impossible to override cleanly.

Before

Really hard to override/edit case activity links. Fixed format.

After

Overrideable in standard way via hook. Formatted for display via the standard "formatLinks()" function.

Technical Details

Introduces the concept of "linkButtons" which are currently implemented in a custom way for each form that uses them. This allows the output of actionLinks() to be used to generate the standard set of "extra" links that appear when a table of entities is shown but also show them on the "detail" View/Edit form. So for cases "Move to Case" and "Copy to Case" are now also available on the "View activity" form.

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[agh1]

@mattwire it looks like all the PRs you were waiting for have been merged. If you can do whatever rebase and style cleanup you need I'd be happy to look at it in Brooklyn next week. I've been in case-land recently, and the links hook is my favorite hook.

[mattwire]

@agh1 Thanks - now rebased and ready for review

[agh1]

This PR moves a sizable function and heavily reworks it, so @eileenmcnaughton recommended that I do the move as a separate first step. See #14512.

Overall, this is a good change, but it has a couple of issues.

• General standards
  • (r-explain) PASS
  • (r-user) PASS: the change for the affected pages makes it more consistent with most other parts of CiviCRM.
  • (r-doc) PASS
  • (r-run) ISSUE: See the inline comments about broken links on the Activity Details report and wrong icons for Move To Case, Copy To Case, and Restore.
• Developer standards
  • (r-tech) PASS: The change preserves compatibility with existing callers/code/downstream.
  • (r-code) PASS: However, while you're fixing things, see the comment about {assign} instead of {capture}{/capture}
  • (r-maint) PASS: It would be nice to have test coverage for getting the right action links in the right situations (permissions, available cases to reassign to, etc.), but that logic is a little outside the scope of this change, which does a lot to make the code more consistent.
  • (r-test) PASS: The test results are all-clear.

[agh1]

A couple of things about this:

1. if a non-case activity follows a case activity the $caseId never gets set back to null: the API finds no case ID, throws an exception, and the catch never does anything. The result is that this creates broken links that are in the case activity style but for other activities.

2. This is going to run 50 separate queries against the same table you've already queried. Instead, get the case_id as part of the main query by adding it to the big $this->_columns array and making it no_display and required (like source_record_id).

[mattwire]

Right, makes sense I think. I'll need to come back to this to look at that :-)

[mattwire]

@agh1 That turned into a bit of an effort... but I've now done it! Could you test the reporting side again and confirm if you are happy?

[mattwire]

I've dropped the troublesome report changes from this PR and put them in #15135 so we can get this merged. If @agh1 is happy with this now? As the only issue before was the reports.

[jitendrapurohit]

I've tested this and confirm that links are loaded correctly on the case manage screen and view activity page. As the comments from @agh1 is resolved now, this is ok to be merged.

[eileenmcnaughton]

I've added merge-ready based on @jitendrapurohit review - I just want to give @agh1 a last chance to weigh in / comment

[eileenmcnaughton]

erm @jitendrapurohit there are changes since your review - I guess you may need to recheck?

[mattwire]

@eileenmcnaughton @jitendrapurohit Only changes were comments in formButtons.tpl per comment. Annoying that there doesn't seem to be an easy way to show a comparison!

[eileenmcnaughton]

Actually force-pushed is a link - https://github.com/civicrm/civicrm-core/compare/b5c5dd71cd00d10b722349e0adf9d02f712e608f..7bf3b68c5243fccac0a381858d4d2bf8e2c8350a - but since there is a rebase in there it's not readable. I'll take your word on no substantive change