Skip to content

Commit 4c2f6e1

Browse files
authored
Merge pull request #1603 from microting/fix/export-shift-index-out-of-range
fix(export): don't crash on shift slot ids > 289 (out-of-range Options index)
2 parents 57ccd6b + b509cb6 commit 4c2f6e1

3 files changed

Lines changed: 145 additions & 5 deletions

File tree

eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DagsoversigtWorksheetExportTests.cs

Lines changed: 103 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,10 +263,111 @@ await SeedSiteWithTwoDays(siteUid: 9902, employeeNo: "2",
263263
AssertRowDateAndEmployee(rowsByIndex[3], workbookPart, date15, "2");
264264
}
265265

266+
// ------------------------------------------------------------------
267+
// 5. Regression: a cross-midnight / out-of-range shift slot id (> 289)
268+
// must not crash the export. Production bug: Stop1Id = 313 (= 02:00
269+
// next day) made GetShiftTime index past the 288-entry plr.Options
270+
// array and throw IndexOutOfRange. The all-workers path was the one
271+
// that crashed in production, so both overloads are covered.
272+
// ------------------------------------------------------------------
273+
274+
[Test]
275+
public async Task Export_WithCrossMidnightShiftSlotId_DoesNotThrow()
276+
{
277+
// Start1Id = 265 -> (265-1)*5 = 1320 min -> 22:00.
278+
// Stop1Id = 313 -> (313-1)*5 = 1560 min -> 26:00 (= 02:00 next day),
279+
// the > 289 case that used to overflow plr.Options and throw.
280+
// Pause1Id = 295 -> (295-1)*5 = 1470 min -> 24:30; Pause always goes
281+
// through the crashing 2-arg GetShiftTime path (actualStamp
282+
// is always null for pause), so it exercises the fix too.
283+
await SeedSiteAndPlanRegistration(
284+
siteUid: 9810,
285+
employeeNo: "1",
286+
date: new DateTime(2026, 5, 15),
287+
useOneMinuteIntervals: false,
288+
start1Id: 265, stop1Id: 313, pause1Id: 295);
289+
290+
// --- Single-worker overload ---
291+
var singleResult = await _service.GenerateExcelDashboard(
292+
new TimePlanningWorkingHoursRequestModel
293+
{
294+
SiteId = 9810,
295+
DateFrom = new DateTime(2026, 5, 15),
296+
DateTo = new DateTime(2026, 5, 15),
297+
});
298+
299+
Assert.That(singleResult.Success, Is.True, singleResult.Message);
300+
Assert.That(singleResult.Model, Is.Not.Null);
301+
Assert.That(singleResult.Model!.Length, Is.GreaterThan(0));
302+
303+
// Confirm not just "no throw" but correct arithmetic output: the
304+
// Shift1 Stop cell for slot 313 renders "26:00" on the Dashboard sheet.
305+
var (_, shift1Stop) = ReadDashboardShift1Cells(singleResult.Model!);
306+
Assert.That(shift1Stop, Is.EqualTo("26:00"),
307+
"Out-of-range slot 313 must render arithmetically as 26:00, not throw");
308+
309+
// Release the single-worker file handle before invoking the all-workers
310+
// overload. Both exports write to /tmp/results/{yyyyMMdd_HHmmss}_.xlsx and
311+
// return a still-open FileStream; calling them back-to-back inside the same
312+
// second would otherwise collide on the identical filename and fail with
313+
// an IOException unrelated to the slot-id regression under test.
314+
await singleResult.Model!.DisposeAsync();
315+
316+
// --- All-workers overload (the path that crashed in production) ---
317+
var allResult = await _service.GenerateExcelDashboard(
318+
new TimePlanningWorkingHoursReportForAllWorkersRequestModel
319+
{
320+
DateFrom = new DateTime(2026, 5, 15),
321+
DateTo = new DateTime(2026, 5, 15),
322+
});
323+
324+
Assert.That(allResult.Success, Is.True, allResult.Message);
325+
Assert.That(allResult.Model, Is.Not.Null);
326+
Assert.That(allResult.Model!.Length, Is.GreaterThan(0));
327+
328+
// The all-workers workbook has no "Dashboard" sheet; the positional
329+
// FillDataRow layout lives on the per-site sheet, named after the site
330+
// ("Site 9810"). Same 0-indexed columns: 7=Shift1Start, 8=Shift1Stop.
331+
var (_, allShift1Stop) = ReadDashboardShift1Cells(allResult.Model!, "Site 9810");
332+
Assert.That(allShift1Stop, Is.EqualTo("26:00"),
333+
"All-workers path (the one that crashed in production) must also render slot 313 as 26:00");
334+
335+
await allResult.Model!.DisposeAsync();
336+
}
337+
266338
// ------------------------------------------------------------------
267339
// Helpers
268340
// ------------------------------------------------------------------
269341

342+
/// <summary>
343+
/// Opens the xlsx stream and returns the (Shift1Start, Shift1Stop) cell text
344+
/// for the first populated data row of the positional "Dashboard" sheet.
345+
/// Column layout from FillDataRow (0-indexed): 7=Shift1Start, 8=Shift1Stop.
346+
/// </summary>
347+
private static (string Start, string Stop) ReadDashboardShift1Cells(Stream xlsx, string sheetName = "Dashboard")
348+
{
349+
xlsx.Position = 0;
350+
using var doc = SpreadsheetDocument.Open(xlsx, false);
351+
var workbookPart = doc.WorkbookPart!;
352+
var dashboardSheet = workbookPart.Workbook.Descendants<Sheet>()
353+
.First(s => s.Name == sheetName);
354+
var dashboardPart = (WorksheetPart)workbookPart.GetPartById(dashboardSheet.Id!);
355+
var rows = dashboardPart.Worksheet.Descendants<Row>().ToList();
356+
foreach (var row in rows.Where(r => r.RowIndex == null || r.RowIndex! > 1U))
357+
{
358+
var cells = row.Elements<Cell>().ToList();
359+
if (cells.Count < 9) continue;
360+
var shift1Start = CellText(cells[7], workbookPart);
361+
var shift1Stop = CellText(cells[8], workbookPart);
362+
if (!string.IsNullOrEmpty(shift1Start) || !string.IsNullOrEmpty(shift1Stop))
363+
{
364+
return (shift1Start, shift1Stop);
365+
}
366+
}
367+
return ("", "");
368+
}
369+
370+
270371
private static void AssertRowDateAndEmployee(Row row, WorkbookPart wb, double expectedOaDate, string expectedEmployeeNo)
271372
{
272373
var employeeCell = row.Elements<Cell>().Single(c =>
@@ -297,7 +398,7 @@ private static string CellText(Cell c, WorkbookPart wb)
297398
/// </summary>
298399
private async Task SeedSiteAndPlanRegistration(
299400
int siteUid, string employeeNo, DateTime date, bool useOneMinuteIntervals,
300-
int start1Id, int stop1Id)
401+
int start1Id, int stop1Id, int pause1Id = 0)
301402
{
302403
var core = await GetCore();
303404
var sdkDb = core.DbContextHelper.GetDbContext();
@@ -355,7 +456,7 @@ private async Task SeedSiteAndPlanRegistration(
355456
Date = date,
356457
Start1Id = start1Id,
357458
Stop1Id = stop1Id,
358-
Pause1Id = 0,
459+
Pause1Id = pause1Id,
359460
PlanText = "",
360461
CommentOffice = "",
361462
CommentOfficeAll = "",

eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PlanRegistrationHelperTests.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,40 @@ public void GetShiftTime_FlagOnWithActualStamp_ReturnsHHmm()
836836
Assert.That(flagOff, Is.EqualTo("08:00"), "Flag-off must use legacy 5-min Options[96] = \"08:00\"");
837837
}
838838

839+
/// <summary>
840+
/// Production regression (Excel export crash): the 2-arg
841+
/// <c>GetShiftTime(plr, shift)</c> must compute the 5-minute time-of-day
842+
/// arithmetically rather than indexing the fixed 288-entry
843+
/// <c>plr.Options</c> list. Slot ids &gt;= 290 (cross-midnight / night
844+
/// shifts, or mis-encoded device values) previously threw
845+
/// IndexOutOfRange and aborted the whole export
846+
/// (FillDataRow → GetShiftTime). Slot <c>s</c> encodes <c>(s-1)*5</c>
847+
/// minutes; the don't-wrap convention keeps 289 → "24:00" and extends
848+
/// past midnight: 290 → "24:05", 313 → "26:00".
849+
/// </summary>
850+
[TestCase(1, "00:00")]
851+
[TestCase(91, "07:30")]
852+
[TestCase(288, "23:55")]
853+
[TestCase(289, "24:00")]
854+
[TestCase(290, "24:05")] // crash case before the fix
855+
[TestCase(313, "26:00")] // 02:00 next-day cross-midnight
856+
[TestCase(0, "")]
857+
[TestCase(null, "")]
858+
public void GetShiftTime_2Arg_ComputesTimeAndHandlesOutOfRangeSlots(int? shift, string expected)
859+
{
860+
var plr = new PlanRegistration();
861+
// PlanRegistration's parameterless constructor populates Options with 288
862+
// 5-minute strings "00:00".."23:55"; the fix no longer indexes them.
863+
864+
var service = (TimePlanningWorkingHoursService)
865+
System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject(
866+
typeof(TimePlanningWorkingHoursService));
867+
868+
var result = service.GetShiftTime(plr, shift);
869+
870+
Assert.That(result, Is.EqualTo(expected), $"Slot {shift} must map to {expected}");
871+
}
872+
839873
/// <summary>
840874
/// Phase 4 contract: the Excel dashboard export
841875
/// (<c>GenerateExcelDashboard</c>) emits <c>HH:mm</c> string cells

eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2784,11 +2784,16 @@ private Cell CreateWeekNumberCell(DateTime dateValue)
27842784

27852785
internal string GetShiftTime(PlanRegistration plr, int? shift)
27862786
{
2787-
if (shift == 289)
2787+
if (shift is null or <= 0)
27882788
{
2789-
return "24:00";
2789+
return "";
27902790
}
2791-
return shift > 0 ? plr.Options[(int)shift - 1] : "";
2791+
// A shift slot id encodes a 5-minute time-of-day: slot s -> (s-1)*5 minutes.
2792+
// Computed arithmetically instead of indexing the fixed 288-entry plr.Options,
2793+
// so cross-midnight / out-of-range slot ids (>= 290) don't overflow:
2794+
// 288 -> 23:55, 289 -> 24:00, 290 -> 24:05, 313 -> 26:00 (don't-wrap convention).
2795+
var minutes = (shift.Value - 1) * 5;
2796+
return $"{minutes / 60:00}:{minutes % 60:00}";
27922797
}
27932798

27942799
/// <summary>

0 commit comments

Comments
 (0)