diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e65a1a8..391115a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Fix ROUNDUP and ROUNDDOWN for floating-point rounding error [#1404](https://github.com/PHPOffice/PhpSpreadsheet/pull/1404) - Fix loading styles from vmlDrawings when containing whitespace [#1347](https://github.com/PHPOffice/PhpSpreadsheet/issues/1347) +- Fix incorrect behavior when removing last row [#1365](https://github.com/PHPOffice/PhpSpreadsheet/pull/1365) ## [1.11.0] - 2020-03-02 diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index f3a5b4da..9a3f9647 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -2114,22 +2114,27 @@ class Worksheet implements IComparable */ public function removeRow($pRow, $pNumRows = 1) { - if ($pRow >= 1) { - for ($r = 0; $r < $pNumRows; ++$r) { - $this->getCellCollection()->removeRow($pRow + $r); - } - - $highestRow = $this->getHighestDataRow(); - $objReferenceHelper = ReferenceHelper::getInstance(); - $objReferenceHelper->insertNewBefore('A' . ($pRow + $pNumRows), 0, -$pNumRows, $this); - for ($r = 0; $r < $pNumRows; ++$r) { - $this->getCellCollection()->removeRow($highestRow); - --$highestRow; - } - } else { + if ($pRow < 1) { throw new Exception('Rows to be deleted should at least start from row 1.'); } + $highestRow = $this->getHighestDataRow(); + $removedRowsCounter = 0; + + for ($r = 0; $r < $pNumRows; ++$r) { + if ($pRow + $r <= $highestRow) { + $this->getCellCollection()->removeRow($pRow + $r); + ++$removedRowsCounter; + } + } + + $objReferenceHelper = ReferenceHelper::getInstance(); + $objReferenceHelper->insertNewBefore('A' . ($pRow + $pNumRows), 0, -$pNumRows, $this); + for ($r = 0; $r < $removedRowsCounter; ++$r) { + $this->getCellCollection()->removeRow($highestRow); + --$highestRow; + } + return $this; } diff --git a/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php b/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php index e1b4738b..d1e19df4 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php @@ -272,4 +272,136 @@ class WorksheetTest extends TestCase self::assertSame($expectedHighestColumn, $worksheet->getHighestColumn()); self::assertSame($expectedData, $worksheet->toArray()); } + + public function removeRowsProvider() + { + return [ + 'Remove all rows except first one' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ['A4', 'B4', 'C4'], + ], + 2, + 3, + [ + ['A1', 'B1', 'C1'], + ], + 1, + ], + 'Remove all rows except last one' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ['A4', 'B4', 'C4'], + ], + 1, + 3, + [ + ['A4', 'B4', 'C4'], + ], + 1, + ], + 'Remove last row' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ['A4', 'B4', 'C4'], + ], + 4, + 1, + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ], + 3, + ], + 'Remove first row' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ['A4', 'B4', 'C4'], + ], + 1, + 1, + [ + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ['A4', 'B4', 'C4'], + ], + 3, + ], + 'Remove all rows except first and last' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ['A4', 'B4', 'C4'], + ], + 2, + 2, + [ + ['A1', 'B1', 'C1'], + ['A4', 'B4', 'C4'], + ], + 2, + ], + 'Remove non existing rows' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ['A4', 'B4', 'C4'], + ], + 2, + 10, + [ + ['A1', 'B1', 'C1'], + ], + 1, + ], + 'Remove only non existing rows' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ['A4', 'B4', 'C4'], + ], + 5, + 10, + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ['A3', 'B3', 'C3'], + ['A4', 'B4', 'C4'], + ], + 4, + ], + ]; + } + + /** + * @dataProvider removeRowsProvider + */ + public function testRemoveRows( + array $initialData, + int $rowToRemove, + int $rowsQtyToRemove, + array $expectedData, + int $expectedHighestRow + ) { + $workbook = new Spreadsheet(); + $worksheet = $workbook->getActiveSheet(); + $worksheet->fromArray($initialData); + + $worksheet->removeRow($rowToRemove, $rowsQtyToRemove); + + self::assertSame($expectedData, $worksheet->toArray()); + self::assertSame($expectedHighestRow, $worksheet->getHighestRow()); + } }