dev/membership#27 Update outdated membership statuses in preProcess rather than submit

[eileenmcnaughton]

Overview

Moves the updating of any expired memberships from after the form is submitted to when it is loaded - - resulting in an accurate status showing on the form and also simplified code

Also addresses an unreleased regression found in the process

Before

fixMembershipBeforeRenew is called (in some flows) in the postProcess. If it is out of date the out-of-date status is displayed when the form loads

After

fixMembershipBeforeRenew is always called in preProcess - resulting in an accurate status showing on the form and also simplified code

Technical Details

Per https://lab.civicrm.org/dev/membership/-/issues/27 the function fixMembershipBeforeRenew is agreed to be a useful part of the Membership renewal form flow

(ie we should set to expired before renewing if that is the correct pre-renewal status). As discussed on that
issue this moves that handling to the pre-process function.

In the process I hit what appears to be an unreleased regression affecting fixMembershipBeforeRenew borking
when changeDate = NULL from 2cb6497#diff-f43c8498e32f5b2d68ab27bcd243ca36L1136

The regression only affects master & is fixed in this line https://github.com/civicrm/civicrm-core/pull/18621/files#diff-f43c8498e32f5b2d68ab27bcd243ca36L1184

I also moved formatting of end_date to the tpl layer

Comments

I did look at calling 'Membership.create' with skipStatusCalc = FALSE but it needs to be tweaked to support that without passing in dates (which I could have done but I felt it was better to clean up separately)

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

@seamuslee001 any chance you can check this - the regression I found wasn't in this flow but when I tried to call the function with the second param = NULL it resulted in a fatal error & it's called with NULL in the second one down of these...

[mattwire]

Not blocking but we should check this doesn't change any messaging sent to users around membership expiry/renewal (or improves the situation).

[eileenmcnaughton]

@mattwire I don't think it does change anything - the same method still runs - just a bit earlier and it doesn't trigger any emails. We simplified the UI message a while back because there was an issue with changes made by hooks not being picked up & not wanting to reload a bunch of stuff on the off chance so it's pretty simple now...

/**
   * Set the renewal notification status message.
   */
  public function setRenewalMessage() {
    $statusMsg = ts('%1 membership for %2 has been renewed.', [1 => $this->membershipTypeName, 2 => $this->_memberDisplayName]);

    if ($this->isMailSent) {
      $statusMsg .= ' ' . ts('A renewal confirmation and receipt has been sent to %1.', [1 => $this->_contributorEmail]);
    }
    CRM_Core_Session::setStatus($statusMsg, ts('Complete'), 'success');
  }

[seamuslee001]

Change looks fine to me merging