Fixes cycle time calculation when the start date is defined in the future

5 files changed, 28 insertions, 20 deletions

diff --git a/ChangeLog b/ChangeLog
index 12c91269..18526e4e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -25,12 +25,10 @@ Improvements:
 * Added Ajax loading icon for submit buttons
 * Added support for HTTP header "X-Forwarded-Proto: https"
 
-Security issues:
-
-* Access allowed to any tasks from the shared public board by changing the URL parameters
-
 Bug fixes:
 
+* Fixes cycle time calculation when the start date is defined in the future
+* Access allowed to any tasks from the shared public board by changing the URL parameters
 * Fix invalid user filter for API procedure createLdapUser()
 * Ambiguous column name with very old version of Sqlite
 
diff --git a/app/Analytic/AverageLeadCycleTimeAnalytic.php b/app/Analytic/AverageLeadCycleTimeAnalytic.php
index 5a7a3c0f..62c83559 100644
--- a/app/Analytic/AverageLeadCycleTimeAnalytic.php
+++ b/app/Analytic/AverageLeadCycleTimeAnalytic.php
@@ -85,14 +85,15 @@ class AverageLeadCycleTimeAnalytic extends Base
      */
     private function calculateCycleTime(array &$task)
     {
-        if (empty($task['date_started'])) {
-            return 0;
-        }
+        $end = (int) $task['date_completed'] ?: time();
+        $start = (int) $task['date_started'];
 
-        $end = $task['date_completed'] ?: time();
-        $start = $task['date_started'];
+        // Start date can be in the future when defined with the Gantt chart
+        if ($start > 0 && $end > $start) {
+            return $end - $start;
+        }
 
-        return $end - $start;
+        return 0;
     }
 
     /**
diff --git a/app/Model/ProjectDailyStats.php b/app/Model/ProjectDailyStats.php
index 957ad51d..974f5813 100644
--- a/app/Model/ProjectDailyStats.php
+++ b/app/Model/ProjectDailyStats.php
@@ -56,12 +56,19 @@ class ProjectDailyStats extends Base
      */
     public function getRawMetrics($project_id, $from, $to)
     {
-        return $this->db->table(self::TABLE)
+        $metrics = $this->db->table(self::TABLE)
             ->columns('day', 'avg_lead_time', 'avg_cycle_time')
             ->eq('project_id', $project_id)
             ->gte('day', $from)
             ->lte('day', $to)
             ->asc('day')
             ->findAll();
+
+        foreach ($metrics as &$metric) {
+            $metric['avg_lead_time'] = (int) $metric['avg_lead_time'];
+            $metric['avg_cycle_time'] = (int) $metric['avg_cycle_time'];
+        }
+
+        return $metrics;
     }
 }
diff --git a/tests/units/Analytic/AverageLeadCycleTimeAnalyticTest.php b/tests/units/Analytic/AverageLeadCycleTimeAnalyticTest.php
index b8faec6c..b87e9573 100644
--- a/tests/units/Analytic/AverageLeadCycleTimeAnalyticTest.php
+++ b/tests/units/Analytic/AverageLeadCycleTimeAnalyticTest.php
@@ -23,6 +23,7 @@ class AverageLeadCycleTimeAnalyticTest extends Base
         $this->assertEquals(2, $taskCreationModel->create(array('project_id' => 1, 'title' => 'test')));
         $this->assertEquals(3, $taskCreationModel->create(array('project_id' => 1, 'title' => 'test')));
         $this->assertEquals(4, $taskCreationModel->create(array('project_id' => 1, 'title' => 'test')));
+        $this->assertEquals(5, $taskCreationModel->create(array('project_id' => 1, 'title' => 'test')));
 
         // LT=3600 CT=1800
         $this->container['db']->table(Task::TABLE)->eq('id', 1)->update(array('date_completed' => $now + 3600, 'date_started' => $now + 1800));
@@ -36,21 +37,22 @@ class AverageLeadCycleTimeAnalyticTest extends Base
         // LT=2*3600 CT=0
         $this->container['db']->table(Task::TABLE)->eq('id', 4)->update(array('date_completed' => $now + 2 * 3600));
 
+        // CT=0
+        $this->container['db']->table(Task::TABLE)->eq('id', 5)->update(array('date_started' => $now + 900));
+
         $stats = $averageLeadCycleTimeAnalytic->build(1);
 
-        $this->assertEquals(4, $stats['count']);
+        $this->assertEquals(5, $stats['count']);
         $this->assertEquals(3600 + 1800 + 3600 + 2*3600, $stats['total_lead_time'], '', 5);
         $this->assertEquals(1800 + 900, $stats['total_cycle_time'], '', 5);
-        $this->assertEquals((3600 + 1800 + 3600 + 2*3600) / 4, $stats['avg_lead_time'], '', 5);
-        $this->assertEquals((1800 + 900) / 4, $stats['avg_cycle_time'], '', 5);
+        $this->assertEquals((3600 + 1800 + 3600 + 2*3600) / 5, $stats['avg_lead_time'], '', 5);
+        $this->assertEquals((1800 + 900) / 5, $stats['avg_cycle_time'], '', 5);
     }
 
     public function testBuildWithNoTasks()
     {
-        $taskCreationModel = new TaskCreation($this->container);
         $projectModel = new Project($this->container);
         $averageLeadCycleTimeAnalytic = new AverageLeadCycleTimeAnalytic($this->container);
-        $now = time();
 
         $this->assertEquals(1, $projectModel->create(array('name' => 'test1')));
         $this->assertEquals(2, $projectModel->create(array('name' => 'test1')));
diff --git a/tests/units/Model/ProjectDailyStatsTest.php b/tests/units/Model/ProjectDailyStatsTest.php
index c3b20cb9..60afc4c3 100644
--- a/tests/units/Model/ProjectDailyStatsTest.php
+++ b/tests/units/Model/ProjectDailyStatsTest.php
@@ -45,10 +45,10 @@ class ProjectDailyStatsTest extends Base
         $this->assertEquals($expected[0]['day'], $metrics[0]['day']);
         $this->assertEquals($expected[1]['day'], $metrics[1]['day']);
 
-        $this->assertEquals($expected[0]['avg_lead_time'], $metrics[0]['avg_lead_time'], '', 2);
-        $this->assertEquals($expected[1]['avg_lead_time'], $metrics[1]['avg_lead_time'], '', 2);
+        $this->assertSame($expected[0]['avg_lead_time'], $metrics[0]['avg_lead_time'], '', 2);
+        $this->assertSame($expected[1]['avg_lead_time'], $metrics[1]['avg_lead_time'], '', 2);
 
-        $this->assertEquals($expected[0]['avg_cycle_time'], $metrics[0]['avg_cycle_time'], '', 2);
-        $this->assertEquals($expected[1]['avg_cycle_time'], $metrics[1]['avg_cycle_time'], '', 2);
+        $this->assertSame($expected[0]['avg_cycle_time'], $metrics[0]['avg_cycle_time'], '', 2);
+        $this->assertSame($expected[1]['avg_cycle_time'], $metrics[1]['avg_cycle_time'], '', 2);
     }
 }