Merge pull request #11887 from eileenmcnaughton/date

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

CRM/Report/Form.php

@@ -2056,6 +2056,8 @@ public function dateClause(
   /**
    * Get values for from and to for date ranges.
    *
+   * @deprecated
+   *
    * @param bool $relative
    * @param string $from
    * @param string $to
@@ -2066,19 +2068,12 @@ public function dateClause(
    */
   public function getFromTo($relative, $from, $to, $fromTime = NULL, $toTime = NULL) {
     if (empty($toTime)) {
+      // odd legacy behaviour to treat NULL as 'end of the day'
+      // recommend updating reports to call CRM_Utils_Date::getFromTo
+      //directly (default on the function is the actual default there).
       $toTime = '235959';
     }
-    //FIX ME not working for relative
-    if ($relative) {
-      list($term, $unit) = CRM_Utils_System::explode('.', $relative, 2);
-      $dateRange = CRM_Utils_Date::relativeToAbsolute($term, $unit);
-      $from = substr($dateRange['from'], 0, 8);
-      //Take only Date Part, Sometime Time part is also present in 'to'
-      $to = substr($dateRange['to'], 0, 8);
-    }
-    $from = CRM_Utils_Date::processDate($from, $fromTime);
-    $to = CRM_Utils_Date::processDate($to, $toTime);
-    return array($from, $to);
+    return CRM_Utils_Date::getFromTo($relative, $from, $to, $fromTime, $toTime);
   }
 
   /**

CRM/Utils/Date.php

@@ -791,26 +791,34 @@ public static function getRange($startDate, $endDate) {
    * Get start date and end from
    * the given relative term and unit
    *
-   * @param date $relative
-   *   Eg: term.unit.
+   * @param string $relative Relative format in the format term.unit.
+   *   Eg: previous.day
    *
-   * @param $from
-   * @param $to
+   * @param string $from
+   * @param string $to
+   * @param string $fromTime
+   * @param string $toTime
    *
    * @return array
    *   start date, end date
    */
-  public static function getFromTo($relative, $from, $to) {
+  public static function getFromTo($relative, $from, $to, $fromTime = NULL, $toTime = '235959') {
     if ($relative) {
-      list($term, $unit) = explode('.', $relative);
+      list($term, $unit) = explode('.', $relative, 2);
       $dateRange = self::relativeToAbsolute($term, $unit);
-      $from = $dateRange['from'];
-      //Take only Date Part, Sometime Time part is also present in 'to'
+      $from = substr($dateRange['from'], 0, 8);
       $to = substr($dateRange['to'], 0, 8);
+      // @todo fix relativeToAbsolute & add tests
+      // relativeToAbsolute returns 8 char date strings
+      // or 14 char date + time strings.
+      // We should use those. However, it turns out to be unreliable.
+      // e.g. this.week does NOT return 235959 for 'from'
+      // so our defaults are more reliable.
+      // Currently relativeToAbsolute only supports 'whole' days so that is ok
     }
 
-    $from = self::processDate($from);
-    $to = self::processDate($to, '235959');
+    $from = self::processDate($from, $fromTime);
+    $to = self::processDate($to, $toTime);
 
     return array($from, $to);
   }

tests/phpunit/CRM/Report/FormTest.php

@@ -0,0 +1,90 @@
+<?php
+
+/*
+  +--------------------------------------------------------------------+
+  | CiviCRM version 4.7                                                |
+  +--------------------------------------------------------------------+
+  | Copyright CiviCRM LLC (c) 2004-2017                                |
+  +--------------------------------------------------------------------+
+  | This file is a part of CiviCRM.                                    |
+  |                                                                    |
+  | CiviCRM is free software; you can copy, modify, and distribute it  |
+  | under the terms of the GNU Affero General Public License           |
+  | Version 3, 19 November 2007 and the CiviCRM Licensing Exception.   |
+  |                                                                    |
+  | CiviCRM is distributed in the hope that it will be useful, but     |
+  | WITHOUT ANY WARRANTY; without even the implied warranty of         |
+  | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.               |
+  | See the GNU Affero General Public License for more details.        |
+  |                                                                    |
+  | You should have received a copy of the GNU Affero General Public   |
+  | License and the CiviCRM Licensing Exception along                  |
+  | with this program; if not, contact CiviCRM LLC                     |
+  | at info[AT]civicrm[DOT]org. If you have questions about the        |
+  | GNU Affero General Public License or the licensing of CiviCRM,     |
+  | see the CiviCRM license FAQ at http://civicrm.org/licensing        |
+  +--------------------------------------------------------------------+
+ */
+
+/**
+ *  File for the FormTest class
+ *
+ *  (PHP 5)
+ *
+ * @author Jon Goldberg <jon@megaphonetech.com>
+ */
+
+/**
+ *  Test CRM_Report_Form functions.
+ *
+ * @package   CiviCRM
+ * @group headless
+ */
+class CRM_Report_FormTest extends CiviUnitTestCase {
+
+  public function setUp() {
+    // There are only unit tests here at present, we can skip database loading.
+    return TRUE;
+  }
+
+  public function tearDown() {
+    // There are only unit tests here at present, we can skip database loading.
+    return TRUE;
+  }
+
+  public function fromToData() {
+    $cases = array();
+    // Absolute dates
+    $cases[] = array('20170901000000', '20170913235959', 0, '09/01/2017', '09/13/2017');
+    // "Today" relative date filter
+    $date = new DateTime();
+    $expectedFrom = $date->format('Ymd') . '000000';
+    $expectedTo = $date->format('Ymd') . '235959';
+    $cases[] = array($expectedFrom, $expectedTo, 'this.day', '', '');
+    // "yesterday" relative date filter
+    $date = new DateTime();
+    $date->sub(new DateInterval('P1D'));
+    $expectedFrom = $date->format('Ymd') . '000000';
+    $expectedTo = $date->format('Ymd') . '235959';
+    $cases[] = array($expectedFrom, $expectedTo, 'previous.day', '', '');
+    return $cases;
+  }
+
+  /**
+   * Test that getFromTo returns the correct dates.
+   *
+   * @dataProvider fromToData
+   * @param $expectedFrom
+   * @param $expectedTo
+   * @param $relative
+   * @param $from
+   * @param $to
+   */
+  public function testGetFromTo($expectedFrom, $expectedTo, $relative, $from, $to) {
+    $obj = new CRM_Report_Form();
+    list($calculatedFrom, $calculatedTo) = $obj->getFromTo($relative, $from, $to);
+    $this->assertEquals($expectedFrom, $calculatedFrom);
+    $this->assertEquals($expectedTo, $calculatedTo);
+  }
+
+}

tests/phpunit/CRM/Utils/DateTest.php

@@ -0,0 +1,90 @@
+<?php
+
+/*
+  +--------------------------------------------------------------------+
+  | CiviCRM version 4.7                                                |
+  +--------------------------------------------------------------------+
+  | Copyright CiviCRM LLC (c) 2004-2017                                |
+  +--------------------------------------------------------------------+
+  | This file is a part of CiviCRM.                                    |
+  |                                                                    |
+  | CiviCRM is free software; you can copy, modify, and distribute it  |
+  | under the terms of the GNU Affero General Public License           |
+  | Version 3, 19 November 2007 and the CiviCRM Licensing Exception.   |
+  |                                                                    |
+  | CiviCRM is distributed in the hope that it will be useful, but     |
+  | WITHOUT ANY WARRANTY; without even the implied warranty of         |
+  | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.               |
+  | See the GNU Affero General Public License for more details.        |
+  |                                                                    |
+  | You should have received a copy of the GNU Affero General Public   |
+  | License and the CiviCRM Licensing Exception along                  |
+  | with this program; if not, contact CiviCRM LLC                     |
+  | at info[AT]civicrm[DOT]org. If you have questions about the        |
+  | GNU Affero General Public License or the licensing of CiviCRM,     |
+  | see the CiviCRM license FAQ at http://civicrm.org/licensing        |
+  +--------------------------------------------------------------------+
+ */
+
+/**
+ *  File for the DateTest class
+ *
+ *  (PHP 5)
+ *
+ * @author Jon Goldberg <jon@megaphonetech.com>
+ */
+
+/**
+ *  Test CRM_Utils_Date functions.
+ *
+ * @package   CiviCRM
+ * @group headless
+ */
+class CRM_Utils_DateTest extends CiviUnitTestCase {
+
+  public function setUp() {
+    // There are only unit tests here at present, we can skip database loading.
+    return TRUE;
+  }
+
+  public function tearDown() {
+    // There are only unit tests here at present, we can skip database loading.
+    return TRUE;
+  }
+
+  public function fromToData() {
+    $cases = array();
+    // Absolute dates
+    $cases[] = array('20170901000000', '20170913235959', 0, '09/01/2017', '09/13/2017');
+    // "Today" relative date filter
+    $date = new DateTime();
+    $expectedFrom = $date->format('Ymd') . '000000';
+    $expectedTo = $date->format('Ymd') . '235959';
+    $cases[] = array($expectedFrom, $expectedTo, 'this.day', '', '');
+    // "yesterday" relative date filter
+    $date = new DateTime();
+    $date->sub(new DateInterval('P1D'));
+    $expectedFrom = $date->format('Ymd') . '000000';
+    $expectedTo = $date->format('Ymd') . '235959';
+    $cases[] = array($expectedFrom, $expectedTo, 'previous.day', '', '');
+    return $cases;
+  }
+
+  /**
+   * Test that getFromTo returns the correct dates.
+   *
+   * @dataProvider fromToData
+   * @param $expectedFrom
+   * @param $expectedTo
+   * @param $relative
+   * @param $from
+   * @param $to
+   */
+  public function testGetFromTo($expectedFrom, $expectedTo, $relative, $from, $to) {
+    $obj = new CRM_Utils_Date();
+    list($calculatedFrom, $calculatedTo) = $obj->getFromTo($relative, $from, $to);
+    $this->assertEquals($expectedFrom, $calculatedFrom);
+    $this->assertEquals($expectedTo, $calculatedTo);
+  }
+
+}