Fix issues with updating Conditional Formatting when inserting/deleting rows/columns

MarkBaker · MarkBaker · commit 6faf828db7d8 · 2022-03-17T11:37:15.000+01:00
- Update existing ranges, expanding if necessary, rather than trying to clone for each individual cell
 - Update conditions, so that cell references and formulae are maintained correctly

Note that absolute as well as relative cell references should be updated in conditions
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 ### Fixed
 
+- Update Conditional Formatting ranges and rule conditions when inserting/deleting rows/columns [Issue #2678](https://github.com/PHPOffice/PhpSpreadsheet/issues/2678) [PR #2689](https://github.com/PHPOffice/PhpSpreadsheet/pull/2689)
 - Allow `INDIRECT()` to accept row/column ranges as well as cell ranges [PR #2687](https://github.com/PHPOffice/PhpSpreadsheet/pull/2687)
 - Fix bug when deleting cells with hyperlinks, where the hyperlink was then being "inherited" by whatever cell moved to that cell address.
 - Fix bug in Conditional Formatting in the Xls Writer that resulted in a broken file when there were multiple conditional ranges in a worksheet.
diff --git a/src/PhpSpreadsheet/CellReferenceHelper.php b/src/PhpSpreadsheet/CellReferenceHelper.php
@@ -43,6 +43,11 @@ public function __construct(string $beforeCellAddress = 'A1', int $numberOfColum
         $this->beforeRow = (int) $beforeRow;
     }
 
+    public function beforeCellAddress(): string
+    {
+        return $this->beforeCellAddress;
+    }
+
     public function refreshRequired(string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): bool
     {
         return $this->beforeCellAddress !== $beforeCellAddress ||
diff --git a/src/PhpSpreadsheet/ReferenceHelper.php b/src/PhpSpreadsheet/ReferenceHelper.php
@@ -5,6 +5,7 @@
 use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
 use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
 use PhpOffice\PhpSpreadsheet\Cell\DataType;
+use PhpOffice\PhpSpreadsheet\Style\Conditional;
 use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter;
 use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
 
@@ -198,6 +199,45 @@ protected function adjustHyperlinks($worksheet, $numberOfColumns, $numberOfRows)
         }
     }
 
+    /**
+     * Update conditional formatting styles when inserting/deleting rows/columns.
+     *
+     * @param Worksheet $worksheet The worksheet that we're editing
+     * @param int $numberOfColumns Number of columns to insert/delete (negative values indicate deletion)
+     * @param int $numberOfRows Number of rows to insert/delete (negative values indicate deletion)
+     */
+    protected function adjustConditionalFormatting($worksheet, $numberOfColumns, $numberOfRows): void
+    {
+        $aStyles = $worksheet->getConditionalStylesCollection();
+        ($numberOfColumns > 0 || $numberOfRows > 0)
+            ? uksort($aStyles, [self::class, 'cellReverseSort'])
+            : uksort($aStyles, [self::class, 'cellSort']);
+
+        foreach ($aStyles as $cellAddress => $cfRules) {
+            $worksheet->removeConditionalStyles($cellAddress);
+            $newReference = $this->updateCellReference($cellAddress);
+
+            foreach ($cfRules as &$cfRule) {
+                /** @var Conditional $cfRule */
+                $conditions = $cfRule->getConditions();
+                foreach ($conditions as &$condition) {
+                    if (is_string($condition)) {
+                        $condition = $this->updateFormulaReferences(
+                            $condition,
+                            $this->cellReferenceHelper->beforeCellAddress(),
+                            $numberOfColumns,
+                            $numberOfRows,
+                            $worksheet->getTitle(),
+                            true
+                        );
+                    }
+                }
+                $cfRule->setConditions($conditions);
+            }
+            $worksheet->setConditionalStyles($newReference, $cfRules);
+        }
+    }
+
     /**
      * Update data validations when inserting/deleting rows/columns.
      *
@@ -442,6 +482,9 @@ function ($coordinate) use ($allCoordinates) {
         // Update worksheet: hyperlinks
         $this->adjustHyperlinks($worksheet, $numberOfColumns, $numberOfRows);
 
+        // Update worksheet: conditional formatting styles
+        $this->adjustConditionalFormatting($worksheet, $numberOfColumns, $numberOfRows);
+
         // Update worksheet: data validations
         $this->adjustDataValidations($worksheet, $numberOfColumns, $numberOfRows);
 
@@ -505,8 +548,14 @@ function ($coordinate) use ($allCoordinates) {
      *
      * @return string Updated formula
      */
-    public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1', $numberOfColumns = 0, $numberOfRows = 0, $worksheetName = '')
-    {
+    public function updateFormulaReferences(
+        $formula = '',
+        $beforeCellAddress = 'A1',
+        $numberOfColumns = 0,
+        $numberOfRows = 0,
+        $worksheetName = '',
+        bool $includeAbsoluteReferences = false
+    ) {
         if (
             $this->cellReferenceHelper === null ||
             $this->cellReferenceHelper->refreshRequired($beforeCellAddress, $numberOfColumns, $numberOfRows)
@@ -528,8 +577,8 @@ public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1'
                     foreach ($matches as $match) {
                         $fromString = ($match[2] > '') ? $match[2] . '!' : '';
                         $fromString .= $match[3] . ':' . $match[4];
-                        $modified3 = substr($this->updateCellReference('$A' . $match[3]), 2);
-                        $modified4 = substr($this->updateCellReference('$A' . $match[4]), 2);
+                        $modified3 = substr($this->updateCellReference('$A' . $match[3], $includeAbsoluteReferences), 2);
+                        $modified4 = substr($this->updateCellReference('$A' . $match[4], $includeAbsoluteReferences), 2);
 
                         if ($match[3] . ':' . $match[4] !== $modified3 . ':' . $modified4) {
                             if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
@@ -553,8 +602,8 @@ public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1'
                     foreach ($matches as $match) {
                         $fromString = ($match[2] > '') ? $match[2] . '!' : '';
                         $fromString .= $match[3] . ':' . $match[4];
-                        $modified3 = substr($this->updateCellReference($match[3] . '$1'), 0, -2);
-                        $modified4 = substr($this->updateCellReference($match[4] . '$1'), 0, -2);
+                        $modified3 = substr($this->updateCellReference($match[3] . '$1', $includeAbsoluteReferences), 0, -2);
+                        $modified4 = substr($this->updateCellReference($match[4] . '$1', $includeAbsoluteReferences), 0, -2);
 
                         if ($match[3] . ':' . $match[4] !== $modified3 . ':' . $modified4) {
                             if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
@@ -578,8 +627,8 @@ public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1'
                     foreach ($matches as $match) {
                         $fromString = ($match[2] > '') ? $match[2] . '!' : '';
                         $fromString .= $match[3] . ':' . $match[4];
-                        $modified3 = $this->updateCellReference($match[3]);
-                        $modified4 = $this->updateCellReference($match[4]);
+                        $modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences);
+                        $modified4 = $this->updateCellReference($match[4], $includeAbsoluteReferences);
 
                         if ($match[3] . $match[4] !== $modified3 . $modified4) {
                             if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
@@ -606,7 +655,7 @@ public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1'
                         $fromString = ($match[2] > '') ? $match[2] . '!' : '';
                         $fromString .= $match[3];
 
-                        $modified3 = $this->updateCellReference($match[3]);
+                        $modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences);
                         if ($match[3] !== $modified3) {
                             if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
                                 $toString = ($match[2] > '') ? $match[2] . '!' : '';
@@ -786,18 +835,18 @@ private function updateRowRangesAllWorksheets(string $formula, int $numberOfRows
      *
      * @return string Updated cell range
      */
-    private function updateCellReference($cellReference = 'A1')
+    private function updateCellReference($cellReference = 'A1', bool $includeAbsoluteReferences = false)
     {
         // Is it in another worksheet? Will not have to update anything.
         if (strpos($cellReference, '!') !== false) {
             return $cellReference;
         // Is it a range or a single cell?
         } elseif (!Coordinate::coordinateIsRange($cellReference)) {
             // Single cell
-            return $this->cellReferenceHelper->updateCellReference($cellReference);
+            return $this->cellReferenceHelper->updateCellReference($cellReference, $includeAbsoluteReferences);
         } elseif (Coordinate::coordinateIsRange($cellReference)) {
             // Range
-            return $this->updateCellRange($cellReference);
+            return $this->updateCellRange($cellReference, $includeAbsoluteReferences);
         }
 
         // Return original
@@ -839,7 +888,7 @@ public function updateNamedFormulas(Spreadsheet $spreadsheet, $oldName = '', $ne
      *
      * @return string Updated cell range
      */
-    private function updateCellRange(string $cellRange = 'A1:A1'): string
+    private function updateCellRange(string $cellRange = 'A1:A1', bool $includeAbsoluteReferences = false): string
     {
         if (!Coordinate::coordinateIsRange($cellRange)) {
             throw new Exception('Only cell ranges may be passed to this method.');
@@ -853,14 +902,14 @@ private function updateCellRange(string $cellRange = 'A1:A1'): string
             for ($j = 0; $j < $jc; ++$j) {
                 if (ctype_alpha($range[$i][$j])) {
                     $range[$i][$j] = Coordinate::coordinateFromString(
-                        $this->cellReferenceHelper->updateCellReference($range[$i][$j] . '1')
+                        $this->cellReferenceHelper->updateCellReference($range[$i][$j] . '1', $includeAbsoluteReferences)
                     )[0];
                 } elseif (ctype_digit($range[$i][$j])) {
                     $range[$i][$j] = Coordinate::coordinateFromString(
-                        $this->cellReferenceHelper->updateCellReference('A' . $range[$i][$j])
+                        $this->cellReferenceHelper->updateCellReference('A' . $range[$i][$j], $includeAbsoluteReferences)
                     )[1];
                 } else {
-                    $range[$i][$j] = $this->cellReferenceHelper->updateCellReference($range[$i][$j]);
+                    $range[$i][$j] = $this->cellReferenceHelper->updateCellReference($range[$i][$j], $includeAbsoluteReferences);
                 }
             }
         }
@@ -985,17 +1034,8 @@ private function duplicateStylesByColumn(Worksheet $worksheet, int $beforeColumn
             $coordinate = $beforeColumnName . $i;
             if ($worksheet->cellExists($coordinate)) {
                 $xfIndex = $worksheet->getCell($coordinate)->getXfIndex();
-                $conditionalStyles = $worksheet->conditionalStylesExists($coordinate) ?
-                    $worksheet->getConditionalStyles($coordinate) : false;
                 for ($j = $beforeColumn; $j <= $beforeColumn - 1 + $numberOfColumns; ++$j) {
                     $worksheet->getCellByColumnAndRow($j, $i)->setXfIndex($xfIndex);
-                    if ($conditionalStyles) {
-                        $cloned = [];
-                        foreach ($conditionalStyles as $conditionalStyle) {
-                            $cloned[] = clone $conditionalStyle;
-                        }
-                        $worksheet->setConditionalStyles(Coordinate::stringFromColumnIndex($j) . $i, $cloned);
-                    }
                 }
             }
         }
@@ -1009,17 +1049,8 @@ private function duplicateStylesByRow(Worksheet $worksheet, int $beforeColumn, i
             $coordinate = Coordinate::stringFromColumnIndex($i) . ($beforeRow - 1);
             if ($worksheet->cellExists($coordinate)) {
                 $xfIndex = $worksheet->getCell($coordinate)->getXfIndex();
-                $conditionalStyles = $worksheet->conditionalStylesExists($coordinate) ?
-                    $worksheet->getConditionalStyles($coordinate) : false;
                 for ($j = $beforeRow; $j <= $beforeRow - 1 + $numberOfRows; ++$j) {
                     $worksheet->getCell(Coordinate::stringFromColumnIndex($i) . $j)->setXfIndex($xfIndex);
-                    if ($conditionalStyles) {
-                        $cloned = [];
-                        foreach ($conditionalStyles as $conditionalStyle) {
-                            $cloned[] = clone $conditionalStyle;
-                        }
-                        $worksheet->setConditionalStyles(Coordinate::stringFromColumnIndex($i) . $j, $cloned);
-                    }
                 }
             }
         }
diff --git a/tests/PhpSpreadsheetTests/ReferenceHelperTest.php b/tests/PhpSpreadsheetTests/ReferenceHelperTest.php
@@ -7,6 +7,7 @@
 use PhpOffice\PhpSpreadsheet\Comment;
 use PhpOffice\PhpSpreadsheet\ReferenceHelper;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\Wizard;
 use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
 use PHPUnit\Framework\TestCase;
 
@@ -296,4 +297,42 @@ function (Hyperlink $value) {
 
         self::assertSame(['A3' => 'https://phpspreadsheet.readthedocs.io/en/latest/'], $hyperlinks);
     }
+
+    public function testInsertRowsWithConditionalFormatting(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->fromArray([[1, 2, 3, 4], [3, 4, 5, 6], [5, 6, 7, 8], [7, 8, 9, 10], [9, 10, 11, 12]], null, 'C3', true);
+        $sheet->getCell('H5')->setValue(5);
+
+        $cellRange = 'C3:F7';
+        $conditionalStyles = [];
+        $wizardFactory = new Wizard($cellRange);
+        /** @var Wizard\CellValue $cellWizard */
+        $cellWizard = $wizardFactory->newRule(Wizard::CELL_VALUE);
+
+        $cellWizard->equals('$H$5', Wizard::VALUE_TYPE_CELL);
+        $conditionalStyles[] = $cellWizard->getConditional();
+
+        $cellWizard->greaterThan('$H$5', Wizard::VALUE_TYPE_CELL);
+        $conditionalStyles[] = $cellWizard->getConditional();
+
+        $cellWizard->lessThan('$H$5', Wizard::VALUE_TYPE_CELL);
+        $conditionalStyles[] = $cellWizard->getConditional();
+
+        $spreadsheet->getActiveSheet()
+            ->getStyle($cellWizard->getCellRange())
+            ->setConditionalStyles($conditionalStyles);
+        $sheet->insertNewRowBefore(4, 2);
+
+        $styles = $sheet->getConditionalStylesCollection();
+        // verify that the conditional range has been updated
+        self::assertSame('C3:F9', array_keys($styles)[0]);
+        // verify that the conditions have been updated
+        foreach ($styles as $style) {
+            foreach ($style as $conditions) {
+                self::assertSame('$H$7', $conditions->getConditions()[0]);
+            }
+        }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,11 @@ public function __construct(string $beforeCellAddress = 'A1', int $numberOfColum`
`43`	`43`	`$this->beforeRow = (int) $beforeRow;`
`44`	`44`	`}`
`45`	`45`
	`46`	`+ public function beforeCellAddress(): string`
	`47`	`+ {`
	`48`	`+ return $this->beforeCellAddress;`
	`49`	`+ }`
	`50`	`+`
`46`	`51`	`public function refreshRequired(string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): bool`
`47`	`52`	`{`
`48`	`53`	`return $this->beforeCellAddress !== $beforeCellAddress \|\|`