Changes to fix PHPOffice#523 based on comments.

Made the following changes to fix PHPOffice#523 based on code-review comments. * Updating workbook attributes is now done via a loop. * Unit test no longer requires an input xlsx document.
billblume · Jun 12, 2018 · 223a368 · 223a368
1 parent 15901f5
commit 223a368
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,7 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Support to read Xlsm templates with form elements, macros, printer settings, protected elements and back compatibility drawing, and save result without losing important elements of document - [#435](https://github.com/PHPOffice/PhpSpreadsheet/issues/435)
 - Expose sheet title maximum length as `Worksheet::SHEET_TITLE_MAXIMUM_LENGTH` - [#482](https://github.com/PHPOffice/PhpSpreadsheet/issues/482)
 - Allow escape character to be set in CSV reader – [#492](https://github.com/PHPOffice/PhpSpreadsheet/issues/492)
-- Preserve workbook bookview attributes on update - [#523](https://github.com/PHPOffice/PhpSpreadsheet/issues/523)
+- Preserve Xlsx workbook attributes on update - [#523](https://github.com/PHPOffice/PhpSpreadsheet/issues/523)
 
 ### Fixed
 

diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php b/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php
@@ -12,6 +12,24 @@
 
 class Workbook extends WriterPart
 {
+    /**
+     * Constant associative array listing the bookview attributes to write.
+     * Keys are the atttribute names and values are the default value for
+     * those attributes.
+     *
+     * @var array
+     */
+    private static $bookViewAttributes = [
+        'autoFilterDateGrouping' => '1',
+        'firstSheet' => '0',
+        'minimized' => '0',
+        'showHorizontalScroll' => '1',
+        'showSheetTabs' => '1',
+        'showVerticalScroll' => '1',
+        'tabRatio' => '600',
+        'visibility' => 'visible',
+    ];
+
     /**
      * Write workbook to XML format.
      *
@@ -117,38 +135,10 @@ private function writeBookViews(XMLWriter $objWriter, Spreadsheet $spreadsheet)
         $objWriter->startElement('workbookView');
 
         $objWriter->writeAttribute('activeTab', $spreadsheet->getActiveSheetIndex());
-        $objWriter->writeAttribute(
-            'autoFilterDateGrouping',
-            $spreadsheet->getWorkbookViewAttribute('autoFilterDateGrouping', '1')
-        );
-        $objWriter->writeAttribute(
-            'firstSheet',
-            $spreadsheet->getWorkbookViewAttribute('firstSheet', '0')
-        );
-        $objWriter->writeAttribute(
-            'minimized',
-            $spreadsheet->getWorkbookViewAttribute('minimized', '0')
-        );
-        $objWriter->writeAttribute(
-            'showHorizontalScroll',
-            $spreadsheet->getWorkbookViewAttribute('showHorizontalScroll', '1')
-        );
-        $objWriter->writeAttribute(
-            'showSheetTabs',
-            $spreadsheet->getWorkbookViewAttribute('showSheetTabs', '1')
-        );
-        $objWriter->writeAttribute(
-            'showVerticalScroll',
-            $spreadsheet->getWorkbookViewAttribute('showVerticalScroll', '1')
-        );
-        $objWriter->writeAttribute(
-            'tabRatio',
-            $spreadsheet->getWorkbookViewAttribute('tabRatio', '600')
-        );
-        $objWriter->writeAttribute(
-            'visibility',
-            $spreadsheet->getWorkbookViewAttribute('visibility', 'visible')
-        );
+
+        foreach (self::$bookViewAttributes as $attribute => $defaultValue) {
+            $objWriter->writeAttribute($attribute, $spreadsheet->getWorkbookViewAttribute($attribute, $defaultValue));
+        }
 
         $objWriter->endElement();
 

diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/HiddenTabsTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/HiddenTabsTest.php
@@ -3,30 +3,64 @@
 namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
 
 use PhpOffice\PhpSpreadsheet\Settings;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
 
 class HiddenTabsTest extends TestCase
 {
+    /**
+     * Copy of PhpOffice\PhpSpreadsheet\Writer\Xlsx\Workbook::$bookViewAttributes
+     * except that the values are purposefully set to values different from the
+     * default values.
+     *
+     * @var array
+     */
+    private static $bookViewAttributes = [
+        'autoFilterDateGrouping' => '0',
+        'firstSheet' => '1',
+        'minimized' => '1',
+        'showHorizontalScroll' => '0',
+        'showSheetTabs' => '0',
+        'showVerticalScroll' => '0',
+        'tabRatio' => '601',
+        'visibility' => 'hidden',
+    ];
+
     /**
      * Test that the worksheet tabs remain hidden when reading and writing a XLSX document
      * with hidden worksheets tabs.
      */
     public function testUpdateWithHiddenTabs()
     {
-        $sourceFilename = __DIR__ . '/../../../data/Writer/XLSX/hidden_tabs.xlsx';
+        // Create a dummy workbook with two worksheets
+        $workbook = new Spreadsheet();
+        $worksheet1 = $workbook->getActiveSheet();
+        $worksheet1->setTitle('Tweedledee');
+        $worksheet1->setCellValue('A1', 1);
+        $worksheet2 = $workbook->createSheet();
+        $worksheet2->setTitle('Tweeldedum');
+        $worksheet2->setCellValue('A1', 2);
+
+        // Set the workbook bookbiews to non-default values
+        foreach (self::$bookViewAttributes as $attr => $value) {
+            $workbook->setWorkbookViewAttribute($attr, $value);
+        }
+
         Settings::setLibXmlLoaderOptions(null); // reset to default options
-        $reader = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx();
-        $excel = $reader->load($sourceFilename);
 
-        $targetFilename = tempnam(sys_get_temp_dir(), 'tst');
-        $writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($excel);
+        $targetFilename = tempnam(sys_get_temp_dir(), 'xlsx');
+        $writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($workbook);
         $writer->save($targetFilename);
 
         try {
-            $excel2 = $reader->load($targetFilename);
-            $this->assertEquals('0', $excel2->getWorkbookViewAttribute('showSheetTabs'));
+            $reader = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx();
+            $workbook2 = $reader->load($targetFilename);
+
+            foreach (self::$bookViewAttributes as $attr => $value) {
+                $this->assertEquals($value, $workbook2->getWorkbookViewAttribute($attr));
+            }
         } finally {
-            @unlink($targetFilename);
+            unlink($targetFilename);
         }
     }
 }
diff --git a/tests/data/Writer/XLSX/hidden_tabs.xlsx b/tests/data/Writer/XLSX/hidden_tabs.xlsx