Skip to content

Commit 6b1dc4a

Browse files
OssianEPPlusOssianEPPlus
authored
Fix sheet deletion s608 (#1295)
1 parent 0e102ba commit 6b1dc4a

File tree

2 files changed

+175
-27
lines changed

2 files changed

+175
-27
lines changed

‎src/EPPlus/ExcelWorksheets.cs

+58-26
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,53 @@ public ExcelChartsheet AddStockChart(string Name, ExcelRangeBase CategorySerie,
256256
throw new InvalidOperationException("The worksheets collection must have at least one visible worksheet");
257257
}
258258

259+
internal int? GetLastVisibleSheetIndex()
260+
{
261+
for (int i = _worksheets.Count - 1; i > -1; i--)
262+
{
263+
if (_worksheets[i].Hidden == eWorkSheetHidden.Visible)
264+
{
265+
return i;
266+
}
267+
}
268+
throw new InvalidOperationException("The worksheets collection must have at least one visible worksheet");
269+
}
270+
271+
/// <summary>
272+
/// Get first visible index counted from input index.
273+
/// </summary>
274+
/// <param name="index">The index to start checking from</param>
275+
/// <returns></returns>
276+
/// <exception cref="InvalidOperationException"></exception>
277+
internal int? GetNextVisibleSheetIndex(int index)
278+
{
279+
if (index >= _worksheets.Count())
280+
{
281+
throw new InvalidOperationException(
282+
$"index: {index} is out of range. Number of worksheets is: " +
283+
$"{_worksheets.Count()}. Max index is: {_worksheets.Count() - 1 + _pck._worksheetAdd}");
284+
}
285+
286+
//Forward until end
287+
for (int i = index; i < _worksheets.Count; i++)
288+
{
289+
if (_worksheets[i].Hidden == eWorkSheetHidden.Visible)
290+
{
291+
return i;
292+
}
293+
}
294+
295+
//Backwards until start. We don't need to check index position again
296+
for (int i = index - 1; i > -1; i--)
297+
{
298+
if (_worksheets[i].Hidden == eWorkSheetHidden.Visible)
299+
{
300+
return i;
301+
}
302+
}
303+
throw new InvalidOperationException("The worksheets collection must have at least one visible worksheet");
304+
}
305+
259306

260307
internal string CreateWorkbookRel(string Name, int sheetID, Uri uriWorksheet, bool isChart, XmlElement sheetElement)
261308
{
@@ -441,39 +488,24 @@ public void Delete(int Index)
441488
//If not start going backwards until one isn't.
442489
if (_pck.Workbook.Worksheets.Count > 0)
443490
{
444-
var sheetIndex = 0;
491+
//ActiveTab is always 0-based
492+
int activeTabAdjustedIndex = _pck.Workbook.View.ActiveTab + _pck._worksheetAdd;
445493

446-
if (_pck.Workbook.View.ActiveTab >= _pck.Workbook.Worksheets.Count)
494+
if (Index < activeTabAdjustedIndex)
447495
{
448-
449-
for (int i = 1; i < _pck.Workbook.Worksheets.Count + 1; i++)
450-
{
451-
sheetIndex = Math.Min(_pck.Workbook.View.ActiveTab - i, _pck.Workbook.Worksheets.Count - i);
452-
if (_pck.Workbook.Worksheets[sheetIndex].Hidden == eWorkSheetHidden.Visible)
453-
{
454-
i = _pck.Workbook.Worksheets.Count;
455-
}
456-
}
457-
_pck.Workbook.View.ActiveTab = sheetIndex;
496+
//wsIndexActiveTab can impossibly be 0 since it's larger than Index with min value 0
497+
//visibility shouldn't have changed from delete, therefore
498+
_pck.Workbook.View.ActiveTab -= 1;
458499
}
459-
else
500+
else if (Index == activeTabAdjustedIndex)
460501
{
461-
for (int i = _pck.Workbook.View.ActiveTab; i < _pck.Workbook.Worksheets.Count; i++)
502+
if (activeTabAdjustedIndex >= _worksheets.Count())
462503
{
463-
if (_pck.Workbook.Worksheets[i].Hidden == eWorkSheetHidden.Visible)
464-
{
465-
_pck.Workbook.View.ActiveTab = i;
466-
return;
467-
}
504+
_pck.Workbook.View.ActiveTab = GetLastVisibleSheetIndex().Value;
468505
}
469-
470-
for (int i = _pck.Workbook.View.ActiveTab - 1; i > 0; i--)
506+
else
471507
{
472-
if (_pck.Workbook.Worksheets[i].Hidden == eWorkSheetHidden.Visible)
473-
{
474-
_pck.Workbook.View.ActiveTab = i;
475-
return;
476-
}
508+
_pck.Workbook.View.ActiveTab = GetNextVisibleSheetIndex(activeTabAdjustedIndex).Value;
477509
}
478510
}
479511
}

‎src/EPPlusTest/WorksheetsTests.cs

+117-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ Date Author Change
3737
namespace EPPlusTest
3838
{
3939
[TestClass]
40-
public class WorksheetsTests
40+
public class WorksheetsTests : TestBase
4141
{
4242
private ExcelPackage package;
4343
private ExcelWorkbook workbook;
@@ -200,5 +200,121 @@ public void CheckAddedWorksheetWithInvalidName()
200200

201201
Assert.IsNotNull(workbook.Worksheets["[NEW2]"]);
202202
}
203+
204+
[TestMethod]
205+
public void DeletingSheetMovesSelectedSheetCorrectly()
206+
{
207+
using (var package = OpenPackage("deletedSheets.xlsx", true))
208+
{
209+
package.Workbook.Worksheets.Add("VisibleSheet1");
210+
package.Workbook.Worksheets.Add("HiddenSheet1").Hidden = eWorkSheetHidden.Hidden;
211+
package.Workbook.Worksheets.Add("VisibleSheet2");
212+
package.Workbook.Worksheets.Add("HiddenSheet2").Hidden = eWorkSheetHidden.Hidden;
213+
package.Workbook.Worksheets.Add("HiddenSheet3").Hidden = eWorkSheetHidden.VeryHidden;
214+
package.Workbook.Worksheets.Add("VisibleSheet3");
215+
package.Workbook.Worksheets.Add("VisibleSheet4");
216+
package.Workbook.Worksheets.Add("HiddenSheet4").Hidden = eWorkSheetHidden.Hidden;
217+
package.Workbook.View.ActiveTab = 2;
218+
package.Workbook.Worksheets.Delete("VisibleSheet2");
219+
Assert.AreEqual(4, package.Workbook.View.ActiveTab);
220+
package.Workbook.View.ActiveTab = package.Workbook.Worksheets.GetByName("VisibleSheet4").Index;
221+
package.Workbook.Worksheets.Delete("VisibleSheet4");
222+
Assert.AreEqual(package.Workbook.Worksheets.GetByName("VisibleSheet3").Index, package.Workbook.View.ActiveTab);
223+
package.Workbook.Worksheets.Delete("HiddenSheet4");
224+
Assert.AreEqual(package.Workbook.Worksheets.GetByName("VisibleSheet3").Index, package.Workbook.View.ActiveTab);
225+
package.Workbook.Worksheets.Delete("VisibleSheet3");
226+
Assert.AreEqual(0, package.Workbook.View.ActiveTab);
227+
SaveAndCleanup(package);
228+
}
229+
}
230+
231+
[TestMethod]
232+
public void DeletingSheetBeforeSelectedSheetMovesCorrectly()
233+
{
234+
using (var package = OpenPackage("deletedSheets.xlsx", true))
235+
{
236+
package.Workbook.Worksheets.Add("VisibleSheet1");
237+
package.Workbook.Worksheets.Add("HiddenSheet1").Hidden = eWorkSheetHidden.Hidden;
238+
package.Workbook.Worksheets.Add("VisibleSheet2");
239+
package.Workbook.Worksheets.Add("HiddenSheet2").Hidden = eWorkSheetHidden.Hidden;
240+
package.Workbook.Worksheets.Add("HiddenSheet3").Hidden = eWorkSheetHidden.VeryHidden;
241+
package.Workbook.Worksheets.Add("VisibleSheet3");
242+
package.Workbook.Worksheets.Add("VisibleSheet4");
243+
package.Workbook.Worksheets.Add("HiddenSheet4").Hidden = eWorkSheetHidden.Hidden;
244+
245+
package.Workbook.View.ActiveTab = 2;
246+
247+
package.Workbook.Worksheets.Delete("VisibleSheet1");
248+
249+
Assert.AreEqual(1, package.Workbook.View.ActiveTab);
250+
251+
package.Workbook.View.ActiveTab = 4;
252+
253+
package.Workbook.Worksheets.Delete("HiddenSheet3");
254+
255+
Assert.AreEqual(package.Workbook.Worksheets.GetByName("VisibleSheet3").Index, package.Workbook.View.ActiveTab);
256+
257+
package.Workbook.Worksheets.Delete("VisibleSheet4");
258+
package.Workbook.Worksheets.Delete("VisibleSheet3");
259+
260+
Assert.AreEqual(package.Workbook.Worksheets.GetByName("VisibleSheet2").Index, package.Workbook.View.ActiveTab);
261+
262+
SaveAndCleanup(package);
263+
}
264+
}
265+
266+
[TestMethod]
267+
public void DeletedSheetMovesCorrectlyIsWorksheet1Based()
268+
{
269+
using (var package = OpenPackage("deletedSheets.xlsx", true))
270+
{
271+
package.Compatibility.IsWorksheets1Based = true;
272+
273+
package.Workbook.Worksheets.Add("VisibleSheet1");
274+
package.Workbook.Worksheets.Add("VisibleSheet2");
275+
package.Workbook.Worksheets.Add("HiddenSheet2").Hidden = eWorkSheetHidden.Hidden;
276+
package.Workbook.Worksheets.Add("HiddenSheet3").Hidden = eWorkSheetHidden.VeryHidden;
277+
package.Workbook.Worksheets.Add("VisibleSheet3");
278+
package.Workbook.Worksheets.Add("VisibleSheet4");
279+
package.Workbook.Worksheets.Add("HiddenSheet4").Hidden = eWorkSheetHidden.Hidden;
280+
281+
package.Workbook.View.ActiveTab = 5;
282+
283+
package.Workbook.Worksheets.Delete("HiddenSheet4");
284+
285+
Assert.AreEqual(5, package.Workbook.View.ActiveTab);
286+
287+
package.Workbook.Worksheets.Delete("VisibleSheet4");
288+
289+
Assert.AreEqual(4, package.Workbook.View.ActiveTab);
290+
291+
package.Workbook.View.ActiveTab = 1;
292+
293+
package.Workbook.Worksheets.Delete("VisibleSheet2");
294+
295+
Assert.AreEqual(3, package.Workbook.View.ActiveTab);
296+
297+
package.Workbook.View.ActiveTab = 0;
298+
299+
package.Workbook.Worksheets.Delete("VisibleSheet1");
300+
301+
Assert.AreEqual(2, package.Workbook.View.ActiveTab);
302+
303+
SaveAndCleanup(package);
304+
}
305+
}
306+
[TestMethod]
307+
[ExpectedException(typeof(InvalidOperationException))]
308+
public void NoVisibleSheetShouldThrow()
309+
{
310+
using (var package = new ExcelPackage("ExceptionSheet.xlsx"))
311+
{
312+
package.Workbook.Worksheets.Add("VisibleSheet1");
313+
package.Workbook.Worksheets.Add("HiddenSheet1").Hidden = eWorkSheetHidden.Hidden;
314+
package.Workbook.Worksheets.Add("HiddenSheet2").Hidden = eWorkSheetHidden.Hidden;
315+
package.Workbook.Worksheets.Delete(0);
316+
package.Save();
317+
}
318+
}
203319
}
204320
}

0 commit comments

Comments
 (0)