CRM-21148 - Refactor two near-identical date functions into one

[eileenmcnaughton]

Overview

At some point, a method was copy-pasted, then modified. This is a partial / reviewer's cut of #10941 which reconciles the differences and reduces code duplication.

Before

No functional change, code tidy up

After

No functional change, code tidy up

Technical Details

This is a partial of #10941 but it skips the conversion from string to integer which had been queried & also the changes in calling functions, which I think is not required for a 'no-change' outcome.

In fact relativeToAbsolute only returns a date-only string or one of the default time strings ('000000' for 'from' or '235959' for 'to'). However, it is unreliable - e.g this.week does not append 235959 when it should so we should continue to ignore the time returned from this function

The original commit changed the way this was called by a bunch of reports. I have not carried that over as it should not be necessary now to call these functions differently to achieve the same result

Comments

@MegaphoneJon Please check this - if you are happy with it I will merge & you can evaluate whether there are additional changes in your PR that you wish to follow up with

---

• CRM-21148: Refactor "getFromTo()" functions

[MegaphoneJon]

@eileenmcnaughton I've reviewed this, and unfortunately this approach won't work.
When the function signature has an optional default value:

public function getFromTo($relative, $from, $to, $fromTime = NULL, $toTime = '235959') {

There's a difference between these two invocations:

getFromTo(1, 2, 3, 4);
getFromTo(1, 2, 3, 4, NULL);

In the first case, $toTime will be '235959'; in the other, it will be NULL.

We discussed this some on the other PR, but the old CRM_Core_Form::getFromTo had these lines which gave the same result for the two commands above:

if (empty($toTime)) {
  $toTime = '235959';
}

We either need to a) add that into CRM_Utils_Date::getFromTo or modify the reports so that they don't pass NULL.

Note that this is a separate issue from converting $fromTime and $toTime to int. We could split the difference between our PRs by leaving them as strings, but still modify the existing reports not to pass NULL.

Anyway, you can replicate the bug I found by running the Activity Summary report with an Activity Date filter set to "today", then consulting the Developer tab.

With master or my PR, you should see this line in the WHERE clauses:

activity_date_time >= 20180411000000 ) AND ( activity_date_time <= 20180411235959 )

With this PR, it's:

activity_date_time >= 20180411000000 ) AND ( activity_date_time <= 20180411000000 )

[eileenmcnaughton]

@MegaphoneJon how about now - I re-instated the oddity in that function & soft deprecated the function (I figure it makes more sense to update the calls to bypass that function when we touch them).

[MegaphoneJon]

@eileenmcnaughton I've just retested this and feel good about this new cut. The test failure seems unrelated, so I'll give this a merge-on-pass. Thanks for all of your help getting this to the finish line.

[seamuslee001]

Jenkins re test this please

[eileenmcnaughton]

I'm going to merge this now rather than wait for unrelated test failure - we should get Paypal onto guzzle so we can prevent those fails