diff --git a/Private/Test-ExcelRangeOverlap.ps1 b/Private/Test-ExcelRangeOverlap.ps1 new file mode 100644 index 00000000..8044b33e --- /dev/null +++ b/Private/Test-ExcelRangeOverlap.ps1 @@ -0,0 +1,17 @@ +function Test-ExcelRangeOverlap { + <# + .SYNOPSIS + Returns true when two worksheet addresses share at least one cell. + .DESCRIPTION + Takes any two objects with Start/End row and column properties (EPPlus addresses, ranges or + table addresses). Used to avoid writing overlapping tables or a table over an autofilter, + either of which Excel treats as a corrupt file (issue #1725). EPPlus has a Collide method + for this but it is not public in the version bundled here. + #> + param( + [Parameter(Mandatory = $true)]$Address1, + [Parameter(Mandatory = $true)]$Address2 + ) + return ($Address1.Start.Row -le $Address2.End.Row -and $Address1.End.Row -ge $Address2.Start.Row -and + $Address1.Start.Column -le $Address2.End.Column -and $Address1.End.Column -ge $Address2.Start.Column) +} diff --git a/Public/Add-ExcelTable.ps1 b/Public/Add-ExcelTable.ps1 index 232efaf6..5c27ef9d 100644 --- a/Public/Add-ExcelTable.ps1 +++ b/Public/Add-ExcelTable.ps1 @@ -17,8 +17,59 @@ function Add-ExcelTable { [Switch]$PassThru ) try { + #Moving an existing table onto new cells: update its range and matching filter range in place. + #When columns were added or removed, the stored column definitions must be rebuilt to match the + #new width - Excel treats a mismatch as a corrupt file. Their names come from the header row and + #must equal the header cell text exactly, so generated names are written back to empty cells. + $updateTable = { + param ($tbl, $Range) + $tableXml = $tbl.TableXml.table + $sameColumns = ($tbl.Address.Start.Column -eq $Range.Start.Column -and $tbl.Address.End.Column -eq $Range.End.Column) + #A table showing a totals row occupies one row more than its data; keep that shape when moving it. + $totalsRows = 0 + if ($tableXml.GetAttribute('totalsRowCount')) { $totalsRows = [int]$tableXml.GetAttribute('totalsRowCount') } + $newAddress = New-Object -TypeName OfficeOpenXml.ExcelAddressBase -ArgumentList $Range.Start.Row, $Range.Start.Column, ($Range.End.Row + $totalsRows), $Range.End.Column + #EPPlus caches the address on the table object and computes later changes (such as turning the + #totals row on) from that cache, so go through its Address property - internal, hence reflection - + #which updates the cache and the XML ref together. + [OfficeOpenXml.Table.ExcelTable].GetProperty('Address').SetValue($tbl, $newAddress) + #the filter range covers the header and data rows but never the totals row + if ($tableXml['autoFilter']) { $tableXml['autoFilter'].SetAttribute('ref', $Range.Address) } + if (-not $sameColumns) { + $hasHeader = $tableXml.GetAttribute('headerRowCount') -ne '0' + $tableColumns = $tableXml['tableColumns'] + $tableColumns.RemoveAll() + $width = $Range.End.Column - $Range.Start.Column + 1 + $tableColumns.SetAttribute('count', $width) + $usedNames = @{} + for ($i = 0; $i -lt $width; $i++) { + $headerCell = $Range.Worksheet.Cells[$Range.Start.Row, ($Range.Start.Column + $i)] + $columnName = "$($headerCell.Value)" + if (-not $columnName -or $usedNames[$columnName]) { + $suffix = $i + 1 + do { $columnName = "Column$suffix"; $suffix ++ } while ($usedNames[$columnName]) + if ($hasHeader) { $headerCell.Value = $columnName } + } + $usedNames[$columnName] = $true + $tableColumn = $tableXml.OwnerDocument.CreateElement('tableColumn', $tableXml.NamespaceURI) + $tableColumn.SetAttribute('id', ($i + 1)) + $tableColumn.SetAttribute('name', $columnName) + [void]$tableColumns.AppendChild($tableColumn) + } + } + Write-Verbose -Message "Re-used existing table '$($tbl.Name)', now at $($Range.Address)." + return $tbl + } if ($TableName -eq "" -or $null -eq $TableName) { - $tbl = $Range.Worksheet.Tables.Add($Range, "") + #Adding a table over cells an existing table occupies corrupts the file (issue #1725), which + #can happen when a sheet is exported to repeatedly; update the existing table instead. + $tbl = $Range.Worksheet.Tables.Where({Test-ExcelRangeOverlap -Address1 $_.Address -Address2 $Range}, 'First', 1)[0] + if ($tbl) { + $tbl = & $updateTable $tbl $Range + } + else { + $tbl = $Range.Worksheet.Tables.Add($Range, "") + } } else { if ([OfficeOpenXml.FormulaParsing.ExcelUtilities.ExcelAddressUtil]::IsValidAddress($TableName)) { @@ -36,17 +87,25 @@ function Add-ExcelTable { $ws = $Range.Worksheet #if the table exists in this worksheet, update it. if ($ws.Tables[$TableName]) { - $tbl =$ws.Tables[$TableName] - $tbl.TableXml.table.ref = $Range.Address - Write-Verbose -Message "Re-defined table '$TableName', now at $($Range.Address)." + $tbl = & $updateTable $ws.Tables[$TableName] $Range } elseif ($ws.Workbook.Worksheets.Tables.Name -contains $TableName) { Write-Warning -Message "The Table name '$TableName' is already used on a different worksheet." return } else { - $tbl = $ws.Tables.Add($Range, $TableName) - Write-Verbose -Message "Defined table '$($tbl.Name)' at $($Range.Address)" + #A second table on the same cells corrupts the file (issue #1725): take over the table already there, renaming it, rather than doubling up. + $overlapping = $ws.Tables.Where({Test-ExcelRangeOverlap -Address1 $_.Address -Address2 $Range}, 'First', 1)[0] + if ($overlapping) { + Write-Verbose -Message "Table '$($overlapping.Name)' occupied $($overlapping.Address.Address); it becomes '$TableName'." + #the Name property setter also keeps EPPlus's table name lookup in step, raw XML edits would not + $overlapping.Name = $TableName + $tbl = & $updateTable $overlapping $Range + } + else { + $tbl = $ws.Tables.Add($Range, $TableName) + Write-Verbose -Message "Defined table '$($tbl.Name)' at $($Range.Address)" + } } } #it seems that show total changes some of the others, so the sequence matters. diff --git a/Public/Export-Excel.ps1 b/Public/Export-Excel.ps1 index 5c08b208..c730e059 100644 --- a/Public/Export-Excel.ps1 +++ b/Public/Export-Excel.ps1 @@ -429,6 +429,11 @@ #Allow table to be inserted by specifying Name, or Style or both; only process autoFilter if there is no table (they clash). if ($null -ne $TableName -or $PSBoundParameters.ContainsKey('TableStyle')) { + #A worksheet autofilter left by a previous export corrupts the file when a table covers the same cells (issue #1725): the table provides its own filter, remove the old one. + if ($ws.AutoFilterAddress -and (Test-ExcelRangeOverlap -Address1 $ws.AutoFilterAddress -Address2 $ws.Cells[$dataRange])) { + $ws.Cells[$ws.AutoFilterAddress.Address].AutoFilter = $false + Write-Verbose -Message "Removed existing autofilter which overlapped the table range." + } #Already inserted Excel table if input was a DataTable if ($InputObject -isnot [System.Data.DataTable]) { Add-ExcelTable -Range $ws.Cells[$dataRange] -TableName $TableName -TableStyle $TableStyle -TableTotalSettings $TableTotalSettings @@ -436,8 +441,14 @@ } elseif ($AutoFilter) { try { - $ws.Cells[$dataRange].AutoFilter = $true - Write-Verbose -Message "Enabled autofilter. " + #An autofilter over cells covered by a table left by a previous export corrupts the file (issue #1725): leave the filtering to the table. + if ($ws.Tables.Where({Test-ExcelRangeOverlap -Address1 $_.Address -Address2 $ws.Cells[$dataRange]}, 'First', 1)[0]) { + Write-Warning -Message "Did not enable autofilter on worksheet '$WorksheetName' because a table covering the same cells already provides one." + } + else { + $ws.Cells[$dataRange].AutoFilter = $true + Write-Verbose -Message "Enabled autofilter. " + } } catch { Write-Warning -Message "Failed adding autofilter to worksheet '$WorksheetName': $_" } } diff --git a/__tests__/TableOverwrite.tests.ps1 b/__tests__/TableOverwrite.tests.ps1 new file mode 100644 index 00000000..e835fd9f --- /dev/null +++ b/__tests__/TableOverwrite.tests.ps1 @@ -0,0 +1,193 @@ +Import-Module $PSScriptRoot\..\ImportExcel.psd1 -Force + +Describe "Re-exporting tables and autofilters to an existing worksheet - Issue #1725" { + # Exporting into an existing worksheet used to stack a second table over the first (when no + # -TableName was given) or combine a table with a leftover worksheet autofilter. Excel treats + # both as a corrupt file ("We found a problem with some content...") and strips the table + # style while repairing. Each scenario below must yield exactly one table and no clashing + # worksheet autofilter. + BeforeAll { + $data = ConvertFrom-Csv -InputObject @" +Mail,List +foo.bar,Nein +"@ + $data2 = ConvertFrom-Csv -InputObject @" +Mail,List +foo.bar,Nein +baz.qux,Ja +"@ + $wsName = "Geteilte Postfächer" + } + Context "The same -TableStyle export run twice against one file" { + BeforeAll { + $path = "TestDrive:\rerun.xlsx" + $data | Export-Excel -Path $path -WorksheetName $wsName -AutoFilter -TableStyle Light2 + $data | Export-Excel -Path $path -WorksheetName $wsName -AutoFilter -TableStyle Light2 + $excel = Open-ExcelPackage -Path $path + $ws = $excel.Workbook.Worksheets[$wsName] + } + AfterAll { Close-ExcelPackage -ExcelPackage $excel -NoSave } + it "Left a single table with the requested style rather than two stacked tables " { + $ws.Tables.Count | Should -Be 1 + $ws.Tables[0].Address.Address | Should -Be 'A1:B2' + $ws.Tables[0].StyleName | Should -Be 'TableStyleLight2' + } + it "Did not add a worksheet autofilter on top of the table " { + $ws.AutoFilterAddress | Should -BeNullOrEmpty + } + } + Context "A -TableStyle export over data that grew since the previous export" { + BeforeAll { + $path = "TestDrive:\grow.xlsx" + $data | Export-Excel -Path $path -WorksheetName $wsName -TableStyle Light2 + $data2 | Export-Excel -Path $path -WorksheetName $wsName -TableStyle Light2 + $excel = Open-ExcelPackage -Path $path + $ws = $excel.Workbook.Worksheets[$wsName] + } + AfterAll { Close-ExcelPackage -ExcelPackage $excel -NoSave } + it "Stretched the existing table over the new rows instead of adding a second table " { + $ws.Tables.Count | Should -Be 1 + $ws.Tables[0].Address.Address | Should -Be 'A1:B3' + } + } + Context "A -TableStyle export into a sheet left with an autofilter by an earlier export" { + BeforeAll { + $path = "TestDrive:\afthentable.xlsx" + $data | Export-Excel -Path $path -WorksheetName $wsName -AutoFilter + $data | Export-Excel -Path $path -WorksheetName $wsName -AutoFilter -TableStyle Light2 + $excel = Open-ExcelPackage -Path $path + $ws = $excel.Workbook.Worksheets[$wsName] + } + AfterAll { Close-ExcelPackage -ExcelPackage $excel -NoSave } + it "Removed the leftover autofilter when it created the table " { + $ws.AutoFilterAddress | Should -BeNullOrEmpty + $ws.Tables.Count | Should -Be 1 + $ws.Tables[0].StyleName | Should -Be 'TableStyleLight2' + } + } + Context "An -AutoFilter export into a sheet left with a table by an earlier export" { + BeforeAll { + $path = "TestDrive:\tablethenaf.xlsx" + $data | Export-Excel -Path $path -WorksheetName $wsName -TableStyle Light2 + $data | Export-Excel -Path $path -WorksheetName $wsName -AutoFilter -WarningVariable afWarning -WarningAction SilentlyContinue + $excel = Open-ExcelPackage -Path $path + $ws = $excel.Workbook.Worksheets[$wsName] + } + AfterAll { Close-ExcelPackage -ExcelPackage $excel -NoSave } + it "Warned, and left filtering to the table instead of adding an autofilter over it " { + $afWarning | Should -Not -BeNullOrEmpty + $ws.AutoFilterAddress | Should -BeNullOrEmpty + $ws.Tables.Count | Should -Be 1 + } + } + Context "An unnamed -TableStyle export over a previously named table" { + BeforeAll { + $path = "TestDrive:\namedthenplain.xlsx" + $data | Export-Excel -Path $path -WorksheetName $wsName -TableName MailListe -TableStyle Light2 + $data2 | Export-Excel -Path $path -WorksheetName $wsName -TableStyle Light9 + $excel = Open-ExcelPackage -Path $path + $ws = $excel.Workbook.Worksheets[$wsName] + } + AfterAll { Close-ExcelPackage -ExcelPackage $excel -NoSave } + it "Re-used the named table, stretching it and applying the new style " { + $ws.Tables.Count | Should -Be 1 + $ws.Tables[0].Name | Should -Be 'MailListe' + $ws.Tables[0].Address.Address | Should -Be 'A1:B3' + $ws.Tables[0].StyleName | Should -Be 'TableStyleLight9' + } + } + Context "A named -TableName export over a previously unnamed table" { + BeforeAll { + $path = "TestDrive:\plainthennamed.xlsx" + $data | Export-Excel -Path $path -WorksheetName $wsName -TableStyle Light2 + $data | Export-Excel -Path $path -WorksheetName $wsName -TableName MailListe -TableStyle Light9 + $excel = Open-ExcelPackage -Path $path + $ws = $excel.Workbook.Worksheets[$wsName] + } + AfterAll { Close-ExcelPackage -ExcelPackage $excel -NoSave } + it "Took over the existing table, renaming it, rather than doubling up " { + $ws.Tables.Count | Should -Be 1 + $ws.Tables[0].TableXml.table.name | Should -Be 'MailListe' + $ws.Tables[0].StyleName | Should -Be 'TableStyleLight9' + } + } + Context "A -TableStyle export over data whose columns changed since the previous export" { + BeforeAll { + $wide = ConvertFrom-Csv -InputObject @" +Mail,List,Extra +foo.bar,Nein,x +"@ + $path = "TestDrive:\widen.xlsx" + $data | Export-Excel -Path $path -WorksheetName $wsName -TableStyle Light2 + $wide | Export-Excel -Path $path -WorksheetName $wsName -TableStyle Light2 + $excel = Open-ExcelPackage -Path $path + $ws = $excel.Workbook.Worksheets[$wsName] + } + AfterAll { Close-ExcelPackage -ExcelPackage $excel -NoSave } + it "Rebuilt the table's column definitions to match the new width " { + $ws.Tables.Count | Should -Be 1 + $ws.Tables[0].TableXml.table.ref | Should -Be 'A1:C2' + $ws.Tables[0].TableXml.table.autoFilter.ref | Should -Be 'A1:C2' + $ws.Tables[0].TableXml.table.tableColumns.count | Should -Be 3 + $ws.Tables[0].TableXml.table.tableColumns.tableColumn.name -join ',' | Should -Be 'Mail,List,Extra' + } + } + Context "A widening export which also turns the totals row on" { + BeforeAll { + $d2 = ConvertFrom-Csv -InputObject "Name,Amount`nAlpha,1`nBeta,2" + $w2 = ConvertFrom-Csv -InputObject "Name,Amount,Extra`nAlpha,1,x`nBeta,2,y" + $path = "TestDrive:\widentotals.xlsx" + $d2 | Export-Excel -Path $path -WorksheetName S -TableName WideTot -TableStyle Light2 + $w2 | Export-Excel -Path $path -WorksheetName S -TableName WideTot -TableStyle Light2 -TableTotalSettings @{Extra='Count';Amount='Sum'} + $excel = Open-ExcelPackage -Path $path + $tableXml = $excel.Workbook.Worksheets['S'].Tables[0].TableXml.table + } + AfterAll { Close-ExcelPackage -ExcelPackage $excel -NoSave } + it "Kept the range, filter range and column definitions consistent " { + $tableXml.ref | Should -Be 'A1:C4' + $tableXml.totalsRowCount | Should -Be '1' + $tableXml.autoFilter.ref | Should -Be 'A1:C3' + $tableXml.tableColumns.count | Should -Be 3 + } + } + Context "A re-export over a table which shows a totals row" { + BeforeAll { + $path = "TestDrive:\totals.xlsx" + $pkg = Open-ExcelPackage -Path $path -Create + $wsT = $pkg.Workbook.Worksheets.Add('S') + $wsT.Cells['A1'].Value = 'Name'; $wsT.Cells['B1'].Value = 'Amount' + $wsT.Cells['A2'].Value = 'Alpha'; $wsT.Cells['B2'].Value = 1 + Add-ExcelTable -Range $wsT.Cells['A1:B2'] -TableName TotTbl -ShowTotal + Close-ExcelPackage $pkg + $d3 = ConvertFrom-Csv -InputObject "Name,Amount`nAlpha,1`nBeta,2`nGamma,3" + $d3 | Export-Excel -Path $path -WorksheetName S -TableName TotTbl -TableStyle Light2 + $excel = Open-ExcelPackage -Path $path + $tableXml = $excel.Workbook.Worksheets['S'].Tables[0].TableXml.table + } + AfterAll { Close-ExcelPackage -ExcelPackage $excel -NoSave } + it "Kept a row for the totals below the data and excluded it from the filter range " { + $tableXml.ref | Should -Be 'A1:B5' + $tableXml.totalsRowCount | Should -Be '1' + $tableXml.autoFilter.ref | Should -Be 'A1:B4' + } + } + Context "Renaming a table in a package which stays open between exports" { + BeforeAll { + $d2 = ConvertFrom-Csv -InputObject "Name,Amount`nAlpha,1`nBeta,2" + $d3 = ConvertFrom-Csv -InputObject "Name,Amount`nAlpha,1`nBeta,2`nGamma,3" + $renameWarnings = @() + $pkg = $d2 | Export-Excel -Path "TestDrive:\rename.xlsx" -WorksheetName S -TableStyle Light1 -PassThru + $pkg = $d2 | Export-Excel -ExcelPackage $pkg -WorksheetName S -TableName Renamed -TableStyle Light1 -PassThru -WarningVariable +renameWarnings + $pkg = $d3 | Export-Excel -ExcelPackage $pkg -WorksheetName S -TableName Renamed -TableStyle Light9 -PassThru -WarningVariable +renameWarnings + $ws = $pkg.Workbook.Worksheets['S'] + } + AfterAll { Close-ExcelPackage -ExcelPackage $pkg -NoSave } + it "Found the renamed table again on the next export instead of warning " { + $renameWarnings | Should -BeNullOrEmpty + $ws.Tables.Count | Should -Be 1 + $ws.Tables[0].TableXml.table.name | Should -Be 'Renamed' + $ws.Tables[0].TableXml.table.ref | Should -Be 'A1:B4' + $ws.Tables[0].StyleName | Should -Be 'TableStyleLight9' + } + } +}