From b7c025a183f5f774842ad5f6f5617778781897c0 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 22 Sep 2022 01:10:53 +0200 Subject: [PATCH 1/3] Correct update to named ranges and formulae when inserting/deleting columns/rows --- src/PhpSpreadsheet/ReferenceHelper.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/PhpSpreadsheet/ReferenceHelper.php b/src/PhpSpreadsheet/ReferenceHelper.php index 822b754ada..7ae7f1def5 100644 --- a/src/PhpSpreadsheet/ReferenceHelper.php +++ b/src/PhpSpreadsheet/ReferenceHelper.php @@ -539,8 +539,17 @@ function ($coordinate) use ($cellCollection) { // Update workbook: define names if (count($worksheet->getParent()->getDefinedNames()) > 0) { foreach ($worksheet->getParent()->getDefinedNames() as $definedName) { - if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { - $definedName->setValue($this->updateCellReference($definedName->getValue())); + if ($definedName->isFormula() === false) { + $asFormula = ($definedName->getValue()[0] === '=') ? '=' : ''; + if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { + $definedName->setValue($asFormula . $this->updateCellReference(ltrim($definedName->getValue(), '='))); + } + } else { + $formula = $definedName->getValue(); + if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { + $formula = $this->updateFormulaReferences($formula, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle()); + $definedName->setValue($formula); + } } } } From abcbac6f9a9e8dba989cecb40e294231a7ed4b06 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 22 Sep 2022 01:10:53 +0200 Subject: [PATCH 2/3] Correct update to named ranges and formulae when inserting/deleting columns/rows --- src/PhpSpreadsheet/ReferenceHelper.php | 31 ++++- .../ReferenceHelperTest.php | 109 ++++++++++++++++++ 2 files changed, 135 insertions(+), 5 deletions(-) diff --git a/src/PhpSpreadsheet/ReferenceHelper.php b/src/PhpSpreadsheet/ReferenceHelper.php index 822b754ada..16323ed5e4 100644 --- a/src/PhpSpreadsheet/ReferenceHelper.php +++ b/src/PhpSpreadsheet/ReferenceHelper.php @@ -538,11 +538,7 @@ function ($coordinate) use ($cellCollection) { // Update workbook: define names if (count($worksheet->getParent()->getDefinedNames()) > 0) { - foreach ($worksheet->getParent()->getDefinedNames() as $definedName) { - if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { - $definedName->setValue($this->updateCellReference($definedName->getValue())); - } - } + $this->updateDefinedNames($worksheet, $beforeCellAddress, $numberOfColumns, $numberOfRows); } // Garbage collect @@ -1163,4 +1159,29 @@ final public function __clone() { throw new Exception('Cloning a Singleton is not allowed!'); } + + public function updateDefinedNames(Worksheet $worksheet, string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): void + { + foreach ($worksheet->getParent()->getDefinedNames() as $definedName) { + if ($definedName->isFormula() === false) { + $cellAddress = $definedName->getValue(); + $asFormula = ($cellAddress[0] === '='); + if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { + if ($asFormula === true) { + $formula = $definedName->getValue(); + $formula = $this->updateFormulaReferences($formula, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle()); + $definedName->setValue($formula); + } else { + $definedName->setValue($asFormula . $this->updateCellReference(ltrim($cellAddress, '='))); + } + } + } else { + $formula = $definedName->getValue(); + if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { + $formula = $this->updateFormulaReferences($formula, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle()); + $definedName->setValue($formula); + } + } + } + } } diff --git a/tests/PhpSpreadsheetTests/ReferenceHelperTest.php b/tests/PhpSpreadsheetTests/ReferenceHelperTest.php index fbd60155e8..ac686a298e 100644 --- a/tests/PhpSpreadsheetTests/ReferenceHelperTest.php +++ b/tests/PhpSpreadsheetTests/ReferenceHelperTest.php @@ -2,9 +2,12 @@ namespace PhpOffice\PhpSpreadsheetTests; +use PhpOffice\PhpSpreadsheet\Calculation\Calculation; use PhpOffice\PhpSpreadsheet\Cell\DataType; use PhpOffice\PhpSpreadsheet\Cell\Hyperlink; use PhpOffice\PhpSpreadsheet\Comment; +use PhpOffice\PhpSpreadsheet\NamedFormula; +use PhpOffice\PhpSpreadsheet\NamedRange; use PhpOffice\PhpSpreadsheet\ReferenceHelper; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\Wizard; @@ -538,4 +541,110 @@ public function testDeleteColumnsWithPrintArea(): void $printArea = $sheet->getPageSetup()->getPrintArea(); self::assertSame('A1:H10', $printArea); } + + public function testInsertRowsWithDefinedNames(): void + { + $spreadsheet = $this->buildDefinedNamesTestWorkbook(); + /** @var Worksheet $dataSheet */ + $dataSheet = $spreadsheet->getSheetByName('Data'); + /** @var Worksheet $totalsSheet */ + $totalsSheet = $spreadsheet->getSheetByName('Totals'); + + $dataSheet->insertNewRowBefore(4, 2); + Calculation::getInstance($spreadsheet)->flushInstance(); + + /** @var NamedRange $firstColumn */ + $firstColumn = $spreadsheet->getNamedRange('FirstColumn'); + /** @var NamedRange $secondColumn */ + $secondColumn = $spreadsheet->getNamedRange('SecondColumn'); + + self::assertSame('=Data!$A$2:$A8', $firstColumn->getRange()); + self::assertSame('=Data!B$2:B8', $secondColumn->getRange()); + self::assertSame(30, $totalsSheet->getCell('A20')->getCalculatedValue()); + self::assertSame(25, $totalsSheet->getCell('B20')->getCalculatedValue()); + self::assertSame(750, $totalsSheet->getCell('D20')->getCalculatedValue()); + } + + public function testInsertColumnsWithDefinedNames(): void + { + $spreadsheet = $this->buildDefinedNamesTestWorkbook(); + /** @var Worksheet $dataSheet */ + $dataSheet = $spreadsheet->getSheetByName('Data'); + /** @var Worksheet $totalsSheet */ + $totalsSheet = $spreadsheet->getSheetByName('Totals'); + + $dataSheet->insertNewColumnBefore('B', 2); + Calculation::getInstance($spreadsheet)->flushInstance(); + + /** @var NamedRange $firstColumn */ + $firstColumn = $spreadsheet->getNamedRange('FirstColumn'); + /** @var NamedRange $secondColumn */ + $secondColumn = $spreadsheet->getNamedRange('SecondColumn'); + + self::assertSame('=Data!$A$2:$A6', $firstColumn->getRange()); + self::assertSame('=Data!D$2:D6', $secondColumn->getRange()); + self::assertSame(30, $totalsSheet->getCell('A20')->getCalculatedValue()); + self::assertSame(25, $totalsSheet->getCell('B20')->getCalculatedValue()); + self::assertSame(750, $totalsSheet->getCell('D20')->getCalculatedValue()); + } + + public function testDeleteRowsWithDefinedNames(): void + { + $spreadsheet = $this->buildDefinedNamesTestWorkbook(); + /** @var Worksheet $dataSheet */ + $dataSheet = $spreadsheet->getSheetByName('Data'); + /** @var Worksheet $totalsSheet */ + $totalsSheet = $spreadsheet->getSheetByName('Totals'); + + $dataSheet->removeRow(3, 2); + Calculation::getInstance($spreadsheet)->flushInstance(); + + /** @var NamedRange $firstColumn */ + $firstColumn = $spreadsheet->getNamedRange('FirstColumn'); + /** @var NamedRange $secondColumn */ + $secondColumn = $spreadsheet->getNamedRange('SecondColumn'); + + self::assertSame('=Data!$A$2:$A4', $firstColumn->getRange()); + self::assertSame('=Data!B$2:B4', $secondColumn->getRange()); + self::assertSame(20, $totalsSheet->getCell('A20')->getCalculatedValue()); + self::assertSame(17, $totalsSheet->getCell('B20')->getCalculatedValue()); + self::assertSame(340, $totalsSheet->getCell('D20')->getCalculatedValue()); + } + + private function buildDefinedNamesTestWorkbook(): Spreadsheet + { + $spreadsheet = new Spreadsheet(); + $dataSheet = $spreadsheet->getActiveSheet(); + $dataSheet->setTitle('Data'); + + $totalsSheet = $spreadsheet->addSheet(new Worksheet()); + $totalsSheet->setTitle('Totals'); + + $spreadsheet->setActiveSheetIndexByName('Data'); + + $dataSheet->fromArray([['Column 1', 'Column 2'], [2, 1], [4, 3], [6, 5], [8, 7], [10, 9]], null, 'A1', true); + + $spreadsheet->addNamedRange( + new NamedRange('FirstColumn', $spreadsheet->getActiveSheet(), '=Data!$A$2:$A6') + ); + $spreadsheet->addNamedFormula( + new NamedFormula('FirstTotal', $spreadsheet->getActiveSheet(), '=SUM(FirstColumn)') + ); + $totalsSheet->setCellValue('A20', '=FirstTotal'); + + $spreadsheet->addNamedRange( + new NamedRange('SecondColumn', $spreadsheet->getActiveSheet(), '=Data!B$2:B6') + ); + $spreadsheet->addNamedFormula( + new NamedFormula('SecondTotal', $spreadsheet->getActiveSheet(), '=SUM(SecondColumn)') + ); + $totalsSheet->setCellValue('B20', '=SecondTotal'); + + $spreadsheet->addNamedFormula( + new NamedFormula('ProductTotal', $spreadsheet->getActiveSheet(), '=FirstTotal*SecondTotal') + ); + $totalsSheet->setCellValue('D20', '=ProductTotal'); + + return $spreadsheet; + } } From d682f2f95e2c1b9a6c663a8fb2a64fe6ea35e257 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Fri, 23 Sep 2022 14:03:38 +0200 Subject: [PATCH 3/3] Correct update to named ranges and formulae when inserting/deleting columns/rows --- CHANGELOG.md | 1 + src/PhpSpreadsheet/DefinedName.php | 2 +- src/PhpSpreadsheet/ReferenceHelper.php | 64 +++++++++++++--------- src/PhpSpreadsheet/Worksheet/Worksheet.php | 2 +- 4 files changed, 40 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 225ea2af8f..0f731ba47d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Fix update to defined names when inserting/deleting rows/columns [Issue #3076](https://github.com/PHPOffice/PhpSpreadsheet/issues/3076) [PR #3077](https://github.com/PHPOffice/PhpSpreadsheet/pull/3077) - Fix DataValidation sqRef when inserting/deleting rows/columns [Issue #3056](https://github.com/PHPOffice/PhpSpreadsheet/issues/3056) [PR #3074](https://github.com/PHPOffice/PhpSpreadsheet/pull/3074) - Named ranges not usable as anchors in OFFSET function [Issue #3013](https://github.com/PHPOffice/PhpSpreadsheet/issues/3013) - Fully flatten an array [Issue #2955](https://github.com/PHPOffice/PhpSpreadsheet/issues/2955) [PR #2956](https://github.com/PHPOffice/PhpSpreadsheet/pull/2956) diff --git a/src/PhpSpreadsheet/DefinedName.php b/src/PhpSpreadsheet/DefinedName.php index 3b874b435b..464fa8e346 100644 --- a/src/PhpSpreadsheet/DefinedName.php +++ b/src/PhpSpreadsheet/DefinedName.php @@ -150,7 +150,7 @@ public function setName(string $name): self // New title $newTitle = $this->name; - ReferenceHelper::getInstance()->updateNamedFormulas($this->worksheet->getParent(), $oldTitle, $newTitle); + ReferenceHelper::getInstance()->updateNamedFormulae($this->worksheet->getParent(), $oldTitle, $newTitle); } return $this; diff --git a/src/PhpSpreadsheet/ReferenceHelper.php b/src/PhpSpreadsheet/ReferenceHelper.php index 16323ed5e4..1d2ab6a0b0 100644 --- a/src/PhpSpreadsheet/ReferenceHelper.php +++ b/src/PhpSpreadsheet/ReferenceHelper.php @@ -862,13 +862,13 @@ private function updateCellReference($cellReference = 'A1', bool $includeAbsolut } /** - * Update named formulas (i.e. containing worksheet references / named ranges). + * Update named formulae (i.e. containing worksheet references / named ranges). * * @param Spreadsheet $spreadsheet Object to update * @param string $oldName Old name (name to replace) * @param string $newName New name */ - public function updateNamedFormulas(Spreadsheet $spreadsheet, $oldName = '', $newName = ''): void + public function updateNamedFormulae(Spreadsheet $spreadsheet, $oldName = '', $newName = ''): void { if ($oldName == '') { return; @@ -889,6 +889,41 @@ public function updateNamedFormulas(Spreadsheet $spreadsheet, $oldName = '', $ne } } + private function updateDefinedNames(Worksheet $worksheet, string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): void + { + foreach ($worksheet->getParent()->getDefinedNames() as $definedName) { + if ($definedName->isFormula() === false) { + $this->updateNamedRange($definedName, $worksheet, $beforeCellAddress, $numberOfColumns, $numberOfRows); + } else { + $this->updateNamedFormula($definedName, $worksheet, $beforeCellAddress, $numberOfColumns, $numberOfRows); + } + } + } + + private function updateNamedRange(DefinedName $definedName, Worksheet $worksheet, string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): void + { + $cellAddress = $definedName->getValue(); + $asFormula = ($cellAddress[0] === '='); + if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { + if ($asFormula === true) { + $formula = $definedName->getValue(); + $formula = $this->updateFormulaReferences($formula, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle()); + $definedName->setValue($formula); + } else { + $definedName->setValue($asFormula . $this->updateCellReference(ltrim($cellAddress, '='))); + } + } + } + + private function updateNamedFormula(DefinedName $definedName, Worksheet $worksheet, string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): void + { + if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { + $formula = $definedName->getValue(); + $formula = $this->updateFormulaReferences($formula, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle()); + $definedName->setValue($formula); + } + } + /** * Update cell range. * @@ -1159,29 +1194,4 @@ final public function __clone() { throw new Exception('Cloning a Singleton is not allowed!'); } - - public function updateDefinedNames(Worksheet $worksheet, string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): void - { - foreach ($worksheet->getParent()->getDefinedNames() as $definedName) { - if ($definedName->isFormula() === false) { - $cellAddress = $definedName->getValue(); - $asFormula = ($cellAddress[0] === '='); - if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { - if ($asFormula === true) { - $formula = $definedName->getValue(); - $formula = $this->updateFormulaReferences($formula, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle()); - $definedName->setValue($formula); - } else { - $definedName->setValue($asFormula . $this->updateCellReference(ltrim($cellAddress, '='))); - } - } - } else { - $formula = $definedName->getValue(); - if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { - $formula = $this->updateFormulaReferences($formula, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle()); - $definedName->setValue($formula); - } - } - } - } } diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 5532793260..8235ccf140 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -923,7 +923,7 @@ public function setTitle($title, $updateFormulaCellReferences = true, $validate $this->parent->getCalculationEngine() ->renameCalculationCacheForWorksheet($oldTitle, $newTitle); if ($updateFormulaCellReferences) { - ReferenceHelper::getInstance()->updateNamedFormulas($this->parent, $oldTitle, $newTitle); + ReferenceHelper::getInstance()->updateNamedFormulae($this->parent, $oldTitle, $newTitle); } }