(dev/core/40) Show List of Contributions On Recurring Contribution View

[MiyaNoctem]

Overview

Currently, when viewing a recurring contribution from contact's detailed view (on Contributions tab), the only information shown is its basic information. It would be great if we could also see the contributions related to that contribution on that same view.

Before

Clicking on a recurring contribution's View action only shows it's basic information on the opened modal dialog.

After

Opening modal dialog to view a recurring contribution now show a table with a paginated list of associated contributions.

Technical Details

Implemented by loading a new block via ajax

Comments

Issue created in gitlab issue tracker.
https://lab.civicrm.org/dev/core/issues/40

[seamuslee001]

Jenkins re test this please

[seamuslee001]

ping @eileenmcnaughton @mattwire @monishdeb @JoeMurray I quite like this idea and its a nice improvement IMO thoughts?

[eileenmcnaughton]

I think the UI improvement is good & it makes sense on view & edit views.

In terms of implementation I'm less convinced. I think the way we have been going with the payments block & some other areas is to have a re-usable block that can show up in various places (often via ajax). This seems very much coded into a specific page.

[mattwire]

Does this automatically create a pager if there are more than 50 contributions?

[MiyaNoctem]

@mattwire ,

It does create pager controls, that was the idea behind using this controller to show the list of related contributions. I've updated screenshot to show the pager controls working.

[mattwire]

I like it as a UI improvement.

@MiyaNoctem What does this do on an "Edit" recurring contribution view?
@eileenmcnaughton Can you point to the "payments block" ajax code?

I do think a reuseable block would be better if it was possible to do. For example, you could then do things like rearrange the contribution tab/create a new recur contribution tab so contributions are grouped by recurring contribution.
Though I note that this PR is already reusing the search task so possibly most of the way there.

[eileenmcnaughton]

hmm - I think reusing search task puts us in the situation of re-using hard to use / read / test/ maintain code :-( I think the search & sort functionality comes from the use of the datatables + ajax not the selector code per se.

I'm trying to find a really good example. Somewhere between the way the transaction block shows when you expand the contribution (civicrm/payment?transaction=view)

Although I hate the way it uses the same form for view as edit & just does something completely different on the same form!

  if ($this->_view == 'transaction' && ($this->_action & CRM_Core_Action::BROWSE)) {
      $title = $this->assignPaymentInfoBlock();
      CRM_Utils_System::setTitle($title);
      return;
    }

And the way the relationshipSelector stuff works - although I hate the face it doesn't have it's own class & just uses a function in the ajax class.

The activity block is also rendered via ajax (sadly the unit test was retrofitted onto it.

   <item>
     <path>civicrm/ajax/contactactivity</path>
     <page_callback>CRM_Activity_Page_AJAX::getContactActivity</page_callback>
     <access_arguments>access CiviCRM</access_arguments>
  </item>

[mattwire]

I wonder if there could be a halfway compromise, whereby the "search task" is loaded via an ajax function and could easily be replaced by improved/well-tested code in future?

[eileenmcnaughton]

probably best to get @colemanw input.

I did get a fairly generic approach mostly working in an extension - but some gaps there still from memory

https://github.com/eileenmcnaughton/org.wikimedia.omnimail/blob/master/CRM/Omnimail/Page/MailingsView.php
https://github.com/eileenmcnaughton/org.wikimedia.omnimail/blob/master/templates/CRM/Omnimail/Page/MailingsView.tpl

[JoeMurray]

I like the direction of the UI enhanvement but want it to be standardized in terms of design pattern with other places like payments when contributions on contact summary page contributions tab have the triangle clicked to open the view of them. I too think we should be trying to reuse implementation pattern of an includable file. I suppose we should concern ourselves with possibility of a pager when there are > 50 contributions, but I'd be willing to accept a poor job on that.

[MiyaNoctem]

@mattwire , on editing a recurring contribution, there would be no change. This is only shown on view action of a recurring contribution.

[MiyaNoctem]

@eileenmcnaughton, @mattwire,

I've re-written how the related contributions are loaded and used something similar to what Eileen did for Omnimail, basically load the contributions with the API, then process them for showing by adding the payment expansion control and actions (among other fixes) and finally assigning them to the template for them to be dealt with by Smarty.

I've updated the screenshot to show how it works now.

@JoeMurray, you will also find the expanding triangle and the paging working.

Let me know what you guys think!

CC @colemanw

[MiyaNoctem]

Jenkins re test this please

[eileenmcnaughton]

This looks good - I'm still a bit concerned in general about putting new visual blocks in their own class / tpl & my preference is that they be loaded by ajax - but I defer to 'whatever @colemanw says' on this question.

Other than than that I agree this is an improvement

[guanhuan]

Thanks @eileenmcnaughton. @colemanw any thoughts?

[eileenmcnaughton]

Just a note - I don't know if this is the best way but this is what I wound up using recently to load a visual block via ajax elsewhere

<div id="contact-summary-relationship-block" class="crm-clear crm-inline-block-content">

  <div class = 'crm-summary-row crm-label'>My special block</div>

</div>
<script>
  {literal}
  CRM.loadPage(
    CRM.url(
      'civicrm/relationshipblock/view',
      {"snippet" : 1, "contact_id" : {/literal}{$contactID}{literal}},
      'back'
    ),
    {
      target : '#contact-summary-relationship-block',
      dialog : false
    }
  );
{/literal}
</script>

[colemanw]

@eileenmcnaughton why ajax rather than a smarty include?

[eileenmcnaughton]

@colemanw if you say a smarty include is better you are the decider on that. My main concern is that it be re-usable & ideally should be a one-line to add it to a form php class & a one-line to add it to a tpl. It seems inevitable to me that this block will be re-used & wind up on both edit & view & should be written with that understanding

I do think bringing in by ajax can reduce initial page-load times - esp for things that are going to appear further down the page

[colemanw]

I do think bringing in by ajax can reduce initial page-load times - esp for things that are going to appear further down the page

Interesting, my concern was the opposite - building a page by ajax can be quite slow because it involves bootstrapping the CMS and Civi once for every request. Were you thinking it would hamper the initial page load because this block involves slow queries?

[eileenmcnaughton]

@colemanw yeah - that was my perspective - there are a number of civi forms that are pretty slow to load because they do so much before they render.

[MiyaNoctem]

@eileenmcnaughton, @colemanw,

So the decision here is to do this using ajax calls? Can you please confirm?

[eileenmcnaughton]

@MiyaNoctem I would say @colemanw is on the fence so you don't have to go that way if you don't want to - but please move the php code out to it's own class & the tpl wrapper block to it's own file - so these can be included in other classes (or it could be exposed via ajax later if we wanted)

[MiyaNoctem]

@eileenmcnaughton, I've created a new page with it's own class and template to show the list of contributions and I'm loading it using an ajax call as you suggested. Thanks for the example by the way, it worked like a charm. Let me know what you think!

[eileenmcnaughton]

Thanks @MiyaNoctem I'll take a look!

[eileenmcnaughton]

@MiyaNoctem I think the code looks good - it's really readable which is fantastic!

I hit one issue on testing - [COMMENT UPDATED & THIS REMOVED as rebuilding menu fixed it]

[eileenmcnaughton]

Actually I realise after reviewing the other PR that I was not getting the right view as I needed to do more cache-clearing. I now see

[eileenmcnaughton]

I'm going to squash and merge this - I generally prefer to get you to squash commits because it is a little better in the merge history. However, I found this conflicts with your other similar PR (the block on the membership view) and by getting this merged now I remove a block on that one.