Skip to content

Fix issue #1364: Removing last row incorrect behavior #1365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 26, 2020
Merged

Fix issue #1364: Removing last row incorrect behavior #1365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
255d1c0
Fix issue 1364: Removing last row incorrect behavior
TimGa Feb 17, 2020
412c263
Update changelog
TimGa Feb 17, 2020
19a26c3
Fix code style according to PSR-2
TimGa Feb 18, 2020
2513912
Merge branch 'master' into fix-issue-1364
TimGa Mar 11, 2020
4e2a9b2
Update CHANGELOG.md
PowerKiKi Apr 26, 2020
e7de132
Merge branch 'master' into fix-issue-1364
PowerKiKi Apr 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 16 additions & 11 deletions src/PhpSpreadsheet/Worksheet/Worksheet.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2114,20 +2114,25 @@ public function insertNewColumnBeforeByIndex($beforeColumnIndex, $pNumCols = 1)
*/
public function removeRow($pRow, $pNumRows = 1)
{
if ($pRow >= 1) {
for ($r = 0; $r < $pNumRows; ++$r) {
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;
}
}

$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 {
throw new Exception('Rows to be deleted should at least start from row 1.');
$objReferenceHelper = ReferenceHelper::getInstance();
$objReferenceHelper->insertNewBefore('A' . ($pRow + $pNumRows), 0, -$pNumRows, $this);
for ($r = 0; $r < $removedRowsCounter; ++$r) {
$this->getCellCollection()->removeRow($highestRow);
--$highestRow;
}

return $this;
Expand Down
132 changes: 132 additions & 0 deletions tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,4 +272,136 @@ public function testRemoveColumn(
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());
}
}