Add testing to contribution detail report, covering dev/core/issues#170

[eileenmcnaughton]

Overview

This PR restructures the contribution detail report to bring it under testing. It re-fixes issue 170 in a more solid tested way

Before

Report toxic & untested

After

Report less toxic & brought under testing

Technical Details

The beginPostProcessCommon function is called by unit tests / the reporttemplates.get api while postProcess is not. We remove the postProcess function & rely on the parent, moving report-specific stuff to beginPostProcessCommon

Comments

@lcdservices am hoping you can give the report a thorough review once tests pass. Can now extend this with tests for any issues found

[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).

[eileenmcnaughton]

test this please

[lcdservices]

@eileenmcnaughton I applied this and ran through a bunch of manual tests. I focused on the soft credit columns/filters, but also tested various other config options. Everything seemed to be working fine.

[lcdservices]

@eileenmcnaughton actually just stumbled on another regression. It doesn't appear to be caused by this PR, but you may want to include it.

The total amount column is no longer linked to the contribution record. The issue is here:
https://github.com/eileenmcnaughton/civicrm-core/blob/e6bab5eae9e7293dff9ecb9852f85e6aa01dad7f/CRM/Report/Form/Contribute/Detail.php#L718

The three references to civicrm_contribution_total_amount should be civicrm_contribution_total_amount_sum

If you don't want to include it in this PR let me know and I'll do a separate one.

[seamuslee001]

(CiviCRM Review Template WORD-1.1)

• (r-explain) Pass
• (r-test) Pass
• (r-code) Pass
• (r-doc) Pass
• (r-maint) Pass
• (r-run) Pass tested by Brian
• (r-user) Pass
• (r-tech) Pass

@lcdservices Brian i'm going to merge this as is as its a good improvement, if you could do a follow up on the change that would be helpful

[eileenmcnaughton]

I think we can extend the test to check for that new issue raised