Fixes #4063, allow csv renderer to handle arbitrarily nested data structures...

Fixes #4063, allow csv renderer to handle arbitrarily nested data structures at least a little bit gracefully.

core/DataTable/Renderer/Csv.php

@@ -202,36 +202,11 @@ class Csv extends Renderer
                 }
             }
         }
+
         $csv = array();
+        $allColumns = array();
         foreach ($table->getRows() as $row) {
-            $csvRow = array();
-
-            $columns = $row->getColumns();
-            foreach ($columns as $name => $value) {
-                //goals => array( 'idgoal=1' =>array(..), 'idgoal=2' => array(..))
-                if (is_array($value)) {
-                    foreach ($value as $key => $subValues) {
-                        if (is_array($subValues)) {
-                            foreach ($subValues as $subKey => $subValue) {
-                                if ($this->translateColumnNames) {
-                                    $subName = $name != 'goals' ? $name . ' ' . $key
-                                        : Piwik_Translate('Goals_GoalX', $key);
-                                    $columnName = $this->translateColumnName($subKey)
-                                        . ' (' . $subName . ')';
-                                } else {
-                                    // goals_idgoal=1
-                                    $columnName = $name . "_" . $key . "_" . $subKey;
-                                }
-                                $allColumns[$columnName] = true;
-                                $csvRow[$columnName] = $subValue;
-                            }
-                        }
-                    }
-                } else {
-                    $allColumns[$name] = true;
-                    $csvRow[$name] = $value;
-                }
-            }
+            $csvRow = $this->flattenColumnArray($row->getColumns());
 
             if ($this->exportMetadata) {
                 $metadata = $row->getMetadata();
@@ -246,7 +221,6 @@ class Csv extends Renderer
                         $name = 'metadata_' . $name;
                     }
 
-                    $allColumns[$name] = true;
                     $csvRow[$name] = $value;
                 }
             }
@@ -260,6 +234,10 @@ class Csv extends Renderer
                 }
             }
 
+            foreach ($csvRow as $name => $value) {
+                $allColumns[$name] = true;
+            }
+
             $csv[] = $csvRow;
         }
 
@@ -378,4 +356,47 @@ class Csv extends Renderer
         @header('Content-Disposition: attachment; filename="' . $fileName . '"');
         Piwik::overrideCacheControlHeaders();
     }
-}
+
+    /**
+     * Flattens an array of column values so they can be outputted as CSV (which does not support
+     * nested structures).
+     */
+    private function flattenColumnArray($columns, &$csvRow = array(), $csvColumnNameTemplate = '%s')
+    {
+        foreach ($columns as $name => $value) {
+            $csvName = sprintf($csvColumnNameTemplate, $this->getCsvColumnName($name));
+
+            if (is_array($value)) {
+                // if we're translating column names and this is an array of arrays, the column name
+                // format becomes a bit more complicated. also in this case, we assume $value is not
+                // nested beyond 2 levels (ie, array(0 => array(0 => 1, 1 => 2)), but not array(
+                // 0 => array(0 => array(), 1 => array())) )
+                if ($this->translateColumnNames
+                    && is_array(reset($value))
+                ) {
+                    foreach ($value as $level1Key => $level1Value) {
+                        $inner = $name == 'goals' ? Piwik_Translate('Goals_GoalX', $level1Key) : $name . ' ' . $level1Key;
+                        $columnNameTemplate = '%s (' . $inner . ')';
+
+                        $this->flattenColumnArray($level1Value, $csvRow, $columnNameTemplate);
+                    }
+                } else {
+                    $this->flattenColumnArray($value, $csvRow, $csvName . '_%s');
+                }
+            } else {
+                $csvRow[$csvName] = $value;
+            }
+        }
+
+        return $csvRow;
+    }
+
+    private function getCsvColumnName($name)
+    {
+        if ($this->translateColumnNames) {
+            return $this->translateColumnName($name);
+        } else {
+            return $name;
+        }
+    }
+}
\ No newline at end of file

tests/PHPUnit/Integration/CsvExportTest.php

@@ -44,6 +44,14 @@ class Test_Piwik_Integration_CsvExport extends IntegrationTestCase
                                     'otherRequestParameters' => $deExtraParam,
                                     'language'               => 'de',
                                     'testSuffix'             => '_xp1_inner1_trans-de')),
+
+            // TODO: we cannot currently test the csv output of Live.getLastVisitsDetails. The API method includes
+            //       random output. Normally we remove this when it's in XML output, but it's harder to do that w/
+            //       CSV output. For now, we use compareOutput => false to just check that no errors result.
+            array('Live.getLastVisitsDetails', array('idSite'        => $idSite,
+                                                     'date'          => $dateTime,
+                                                     'format'        => 'csv',
+                                                     'compareOutput' => false))
         );
     }
 

tests/PHPUnit/IntegrationTestCase.php

@@ -710,7 +710,7 @@ abstract class IntegrationTestCase extends PHPUnit_Framework_TestCase
         }
     }
 
-    protected function _testApiUrl($testName, $apiId, $requestUrl)
+    protected function _testApiUrl($testName, $apiId, $requestUrl, $compareOutput = true)
     {
         $isTestLogImportReverseChronological = strpos($testName, 'ImportedInRandomOrderTest') === false;
         $isLiveMustDeleteDates = (strpos($requestUrl, 'Live.getLastVisits') !== false
@@ -743,6 +743,10 @@ abstract class IntegrationTestCase extends PHPUnit_Framework_TestCase
 
         file_put_contents($processedFilePath, $response);
 
+        if (!$compareOutput) {
+            return;
+        }
+
         $expected = $this->loadExpectedFile($expectedFilePath);
         if (empty($expected)) {
             print("The expected file is not found at '$expectedFilePath'. The Processed response was:");
@@ -1014,7 +1018,8 @@ abstract class IntegrationTestCase extends PHPUnit_Framework_TestCase
             isset($params['fileExtension']) ? $params['fileExtension'] : false);
 
         foreach ($requestUrls as $apiId => $requestUrl) {
-            $this->_testApiUrl($testName . $testSuffix, $apiId, $requestUrl);
+            $this->_testApiUrl($testName . $testSuffix, $apiId, $requestUrl,
+                isset($params['compareOutput']) ? $params['compareOutput'] : true);
         }
 
         // change the language back to en