diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DayLockHelperTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DayLockHelperTests.cs index 4159fcbd..ae47b6af 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DayLockHelperTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DayLockHelperTests.cs @@ -169,16 +169,27 @@ public void IsLocked_IgnoresTimeOfDay() Is.True, "the boundary day is locked for its whole length"); } + /// + /// UtcNow, not Now, on BOTH sides, because CanReconcile reads UtcNow. The + /// shipped container has no TZ set so the two agree there; on a dev machine + /// they diverge, and then DateTime.Now here breaks in either direction: + /// - UTC+N, just after local midnight (Now.Date is a day AHEAD of + /// UtcNow.Date): the "yesterday is reconcilable" assertion fails, because + /// Now.Date.AddDays(-1) IS UtcNow.Date, i.e. still today in UTC. + /// - UTC-N, late in the local evening (Now.Date is a day BEHIND): the + /// "today is not reconcilable" assertion fails instead, because Now.Date + /// is already yesterday in UTC and so genuinely reconcilable. + /// [Test] public void CanReconcile_TodayAndFuture_False_Past_True() { Assert.Multiple(() => { - Assert.That(DayLockHelper.CanReconcile(DateTime.Now.Date), Is.False, + Assert.That(DayLockHelper.CanReconcile(DateTime.UtcNow.Date), Is.False, "today must stay open so time can still be registered"); - Assert.That(DayLockHelper.CanReconcile(DateTime.Now.Date.AddDays(1)), Is.False); - Assert.That(DayLockHelper.CanReconcile(DateTime.Now.Date.AddDays(-1)), Is.True); - Assert.That(DayLockHelper.CanReconcile(DateTime.Now.Date.AddHours(23)), Is.False, + Assert.That(DayLockHelper.CanReconcile(DateTime.UtcNow.Date.AddDays(1)), Is.False); + Assert.That(DayLockHelper.CanReconcile(DateTime.UtcNow.Date.AddDays(-1)), Is.True); + Assert.That(DayLockHelper.CanReconcile(DateTime.UtcNow.Date.AddHours(23)), Is.False, "a time-of-day on today is still today"); }); } diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ReconcileServiceTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ReconcileServiceTests.cs index 58e5d2ec..161c541e 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ReconcileServiceTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ReconcileServiceTests.cs @@ -384,10 +384,28 @@ public async Task Reconcile_APastDay_SetsFlagAndTimestamp() }); } + /// + /// UtcNow, deliberately: this date is fed to CanReconcile, which compares + /// against UtcNow.Date, and it sits exactly ON that boundary -- so a local + /// clock an offset away from UTC flips the outcome. + /// + /// The fixture's other DateTime.Now dates stay as they are, including the + /// many that DO reach CanReconcile via _service.Reconcile. Every date that + /// reaches it is -3 or older, or +3 forward, and the largest real UTC offset + /// is 14 hours, so no offset can move one across the boundary. The file's -1 + /// dates never reach CanReconcile: they are open days and window bounds, + /// tested by IsLocked against a boundary the fixture stored on the same + /// clock. Only a date sitting exactly ON the boundary can be flipped, and + /// this test holds the only one. + /// + /// Reconcile_AFutureDay_IsRejected below moved too, even though +3 days is + /// safe at any offset, so the pair of tests naming this one rule reads on + /// one clock rather than two. + /// [Test] public async Task Reconcile_Today_IsRejected() { - var row = await SeedPlain(901, DateTime.Now.Date); + var row = await SeedPlain(901, DateTime.UtcNow.Date); var result = await _service.Reconcile(row.Id); @@ -399,7 +417,7 @@ public async Task Reconcile_Today_IsRejected() [Test] public async Task Reconcile_AFutureDay_IsRejected() { - var row = await SeedPlain(902, DateTime.Now.Date.AddDays(3)); + var row = await SeedPlain(902, DateTime.UtcNow.Date.AddDays(3)); var result = await _service.Reconcile(row.Id); @@ -876,6 +894,38 @@ public async Task UpdateWorkingHour_Kiosk_AnOlderReconciledDay_ReturnsDayIsRecon Assert.That(result.Message, Is.EqualTo("DayIsReconciled")); } + /// + /// The lock's unknown case must REFUSE, not permit. The kiosk overload takes + /// an int? site id with no null guard of its own, so before the fix a null + /// sailed straight past the guard (RefuseIfNotWritableAsync's null answer + /// means "writable, proceed") and then died on a `sdkSiteId!.Value` + /// dereference further down. Now it answers with a message and writes + /// nothing. + /// + /// Note what the null case is NOT: with no site id the lock is never + /// evaluated at all, so relaxing the refusal would not "let a locked day + /// through" -- it would crash on that dereference instead, which is how the + /// bug presented. The seeded boundary is here to make the scenario realistic + /// (a real kiosk posting into a frozen period), not because the assertion + /// depends on the day being locked. + /// + [Test] + public async Task UpdateWorkingHour_Kiosk_WithNoSiteId_IsRefused_AndWritesNothing() + { + var token = await SeedKioskDeviceAsync(); + await SeedReconciledBoundaryAsync(934, DateTime.Now.Date.AddDays(-3)); + var lockedDate = DateTime.Now.Date.AddDays(-4); + + var result = await BuildWorkingHoursService().UpdateWorkingHour( + null, new TimePlanningWorkingHoursUpdateModel { Date = lockedDate }, token); + + Assert.That(result.Success, Is.False); + Assert.That(result.Message, Is.EqualTo("SiteNotFound")); + Assert.That(await TimePlanningPnDbContext!.PlanRegistrations + .AnyAsync(x => x.Date == lockedDate), Is.False, + "an unresolvable site must not create a row on a locked day"); + } + [Test] public async Task UpdateWorkingHour_Personal_TheBoundaryDay_ReturnsDayIsReconciled() { diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursImportRemovedRowTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursImportRemovedRowTests.cs index 9e8a483b..5a633bb4 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursImportRemovedRowTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursImportRemovedRowTests.cs @@ -48,6 +48,11 @@ public async Task SetUpTest() var localizationService = Substitute.For(); localizationService.GetString(Arg.Any()).Returns(x => x[0]?.ToString()); + // The format overload echoes its arguments, so a test can assert what a + // message NAMES (here: how many locked days the import left alone), not + // just which key it used. + localizationService.GetString(Arg.Any(), Arg.Any()) + .Returns(x => x[0] + "|" + string.Join("|", (object[])x[1])); _coreService = Substitute.For(); var core = await GetCore(); @@ -114,6 +119,8 @@ public async Task Import_SkipsRemovedRow_AndDoesNotCrashOnRemovedActivePair() // Post-fix: no crash, and the ACTIVE row is the import target. Assert.That(result.Success, Is.True, result.Message); + Assert.That(result.Message, Is.EqualTo("Imported"), + "an import that skipped nothing says only that it imported"); var reloadedActive = await TimePlanningPnDbContext.PlanRegistrations .AsNoTracking().FirstAsync(x => x.Id == activeId); @@ -142,6 +149,11 @@ public async Task Import_SkipsRemovedRow_AndDoesNotCrashOnRemovedActivePair() /// day (it drops dates before now minus one day, which by time of day also /// drops yesterday), so no boundary that satisfies I2 is reachable. The /// lock itself does not check I2; this pins the skip as defense in depth. + /// + /// It also pins that the skip is REPORTED. A silent skip is what makes + /// someone re-import a corrected timesheet over a reconciled month forever: + /// they are told "Imported", they see the old numbers, and nothing in the + /// result says which days did not move. /// [Test] public async Task Import_SkipsALockedDay_AndImportsTheRest() @@ -178,6 +190,10 @@ public async Task Import_SkipsALockedDay_AndImportsTheRest() var result = await _service.Import(FormFile(xlsx)); Assert.That(result.Success, Is.True, result.Message); + // The mock echoes "key|arg", so this asserts both the message and the + // count it names -- one locked day, not "some". + Assert.That(result.Message, Is.EqualTo("Imported ImportLockedDaysSkipped|1"), + "the result must say how many days the lock left unchanged"); var lockedAfter = await TimePlanningPnDbContext.PlanRegistrations .AsNoTracking().FirstAsync(x => x.Id == boundary.Id); @@ -192,6 +208,50 @@ public async Task Import_SkipsALockedDay_AndImportsTheRest() Assert.That(openRow.PlanText, Is.EqualTo("IMPORTED-OPEN"), "the open day is still imported"); } + /// + /// A sheet whose name DOES match a worker, but whose worker has no + /// MicrotingUid, is skipped whole AND reported. Skipping it is the lock + /// requirement -- an unreadable boundary makes IsLocked false for every row, + /// so importing it would import the sheet with no lock check at all. But a + /// silent skip here would be its own bug: the name matched, so the user + /// believes that worker's sheet went in, and a plain success would leave + /// them re-importing a file that never lands. + /// + /// Distinct from a sheet matching NO worker, which stays deliberately + /// silent -- such a tab may not be about a worker at all. + /// + [Test] + public async Task Import_ASheetWhoseWorkerHasNoMicrotingUid_IsSkippedAndReported() + { + const string siteName = "ImportNoUidSite"; + var importDate = DateTime.Now.AddDays(5).Date; + + var core = await _coreService.GetCore(); + var sdkDbContext = core.DbContextHelper.GetDbContext(); + // Name matches the worksheet; MicrotingUid deliberately absent. Cleared + // AFTER Create and then re-read, so the arrange cannot quietly test the + // wrong thing if Create ever starts back-filling a uid of its own. + var site = new SdkSite { Name = siteName, MicrotingUid = null }; + await site.Create(sdkDbContext); + site.MicrotingUid = null; + await sdkDbContext.SaveChangesAsync(); + Assert.That((await sdkDbContext.Sites.AsNoTracking().FirstAsync(x => x.Id == site.Id)) + .MicrotingUid, Is.Null, "arrange: the site must really have no MicrotingUid"); + + var xlsx = BuildWorkbook(siteName, + (importDate.ToString("dd.MM.yyyy"), "8", "SHOULD-NOT-LAND")); + + var result = await _service.Import(FormFile(xlsx)); + + Assert.That(result.Success, Is.True, result.Message); + Assert.That(result.Message, Is.EqualTo("Imported ImportUnresolvableSheetsSkipped|1"), + "the result must name the sheet it could not import"); + + Assert.That(await TimePlanningPnDbContext.PlanRegistrations + .AnyAsync(x => x.Date == importDate), Is.False, + "a sheet that cannot be lock-checked must not be imported at all"); + } + private static IFormFile FormFile(byte[] xlsx) { var file = Substitute.For(); diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/DayLockHelper.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/DayLockHelper.cs index b4cf9f8e..493d0af0 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/DayLockHelper.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/DayLockHelper.cs @@ -131,10 +131,29 @@ public static async Task LockedMessageKeyAsync( /// registered. This is also what makes the forward flex cascades unable to /// reach a locked day -- see the design doc before relaxing it. /// - /// DateTime.Now, not UtcNow: PlanRegistration.Date is a local midnight, and - /// the existing mobile guard compares the same way. + /// DateTime.UtcNow, not Now. PlanRegistration.Date is a calendar-day LABEL + /// with the time zeroed, not an instant in any timezone, so there is no + /// local midnight for a local clock to be "consistent" with. Writers do not + /// even agree on how they build that label -- most zero a constructed date, + /// but TimePlanningFlexService derives one from DateTime.Now -- which is a + /// reason to pin THIS comparison to one clock, not to follow theirs. + /// + /// Direction, stated as what it is rather than as a law: at a POSITIVE + /// offset (CET/CEST, where this product runs) UtcNow is the conservative + /// choice -- late in the local day it briefly declines to freeze a day that + /// is still "today" in UTC, and for a rule whose whole purpose is "this day + /// can no longer be written", refusing too much beats allowing too much. At + /// a NEGATIVE offset the same expression is the PERMISSIVE one: it would + /// accept the local today and break I2 from the worker's point of view. + /// That case does not arise here, and the answer if it ever does is an + /// explicit business timezone, not a switch back to the server's clock. + /// + /// This repo's Dockerfile sets no TZ and installs no tzdata, and the + /// mcr.microsoft.com/dotnet/aspnet base image it runs on defaults to UTC, so + /// the container this plugin ships in compares the same way before and after + /// this change; it only makes dev machines behave like the container. /// - public static bool CanReconcile(DateTime date) => date.Date < DateTime.Now.Date; + public static bool CanReconcile(DateTime date) => date.Date < DateTime.UtcNow.Date; /// /// What counts as a boundary row, in one place: Reconciled and not diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/GoogleSheetHelper.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/GoogleSheetHelper.cs index 702aef7a..146c8301 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/GoogleSheetHelper.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/GoogleSheetHelper.cs @@ -355,8 +355,14 @@ public static async Task PullEverythingFromGoogleSheet(Core core, TimePlanningPn // Decided before the row is even loaded, so a locked row is // never tracked and no later save in this loop can flush it. - if (site.MicrotingUid is { } lockSiteUid - && DayLockHelper.IsLocked(lockedThroughBySite, lockSiteUid, dateValue)) + // + // `!` deliberately: sitesByKey filters MicrotingUid != null + // (the Where above), and `workers` is built only from + // sitesByKey, so a null cannot reach this line. Written as a + // null-tolerant check it would not be defensive -- it would + // silently skip the lock check for a value that cannot + // occur, which is the one outcome this rule must never have. + if (DayLockHelper.IsLocked(lockedThroughBySite, (int)site.MicrotingUid!, dateValue)) { lockedDaysSkipped++; continue; diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PlanRegistrationHelper.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PlanRegistrationHelper.cs index 111462e5..1ad05b94 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PlanRegistrationHelper.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PlanRegistrationHelper.cs @@ -867,13 +867,17 @@ await dbContext.PlanRegistrations.AsNoTracking() // stays tracked and the next save fails on it anyway. catch (Exception e) when (e is not DayLockedException) { - logger.LogError( - $"Could not parse PlanText for planning with id: {planRegistration.Id} the PlanText was: {planRegistration.PlanText}"); - SentrySdk.CaptureMessage( - $"Could not parse PlanText for planning with id: {planRegistration.Id} the PlanText was: {planRegistration.PlanText}"); - //SentrySdk.CaptureException(e); - logger.LogError(e.Message); - logger.LogTrace(e.StackTrace); + // The try above ends in planRegistration.Update calls, so + // this catches DbUpdateException, DbUpdateConcurrencyException, + // MySqlException and friends -- not just PlanText parsing. + // It used to guess "Could not parse PlanText" twice (log + + // Sentry message) while the actual type and stack went to + // Trace, which most hosts do not emit. Report the exception + // ONCE, on each channel, and let it name itself. + SentrySdk.CaptureException(e); + logger.LogError(e, + "UpdatePlanRegistrationsInPeriod failed for PlanRegistration {PlanRegistrationId} (PlanText: {PlanText})", + planRegistration.Id, planRegistration.PlanText); } } @@ -1557,10 +1561,26 @@ await dbContext.PlanRegistrations.AsNoTracking() await planRegistration.Update(dbContext).ConfigureAwait(false); } } - catch (Exception) + // Same shape as the catch in UpdatePlanRegistrationsInPeriod, and + // fixed the same way. The try above ends in a planRegistration.Update, + // so this catches DbUpdateException, DbUpdateConcurrencyException, + // MySqlException and friends, which the old "Could not parse + // PlanText" message mislabelled while losing the type and stack. + // Capture the exception itself, and only that. + // + // The DayLockedException filter matches the other site: a lock + // refusal is routine, not an incident, and must not be reported. + // It is a control-flow change, but not an outcome change -- the + // rejected entry stays tracked either way, so the sole caller's own + // Update (TimePlanningWorkingHoursService.UpdatePlanning) rethrows + // it whether or not this catch swallowed it. + // + // This method takes no ILogger, so Sentry carries the detail. (The + // sibling UpdatePlanRegistrationsInPeriod in this class does take + // one and logs there.) + catch (Exception e) when (e is not DayLockedException) { - SentrySdk.CaptureMessage( - $"Could not parse PlanText for planning with id: {planRegistration.Id} the PlanText was: {planRegistration.PlanText}"); + SentrySdk.CaptureException(e); } // } return planRegistration; diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.Designer.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.Designer.cs index c137ec3c..7ce74cc3 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.Designer.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.Designer.cs @@ -356,5 +356,29 @@ internal static string SuccessfullyUnlockedDay { return ResourceManager.GetString("SuccessfullyUnlockedDay", resourceCulture); } } + + internal static string SiteNotFound { + get { + return ResourceManager.GetString("SiteNotFound", resourceCulture); + } + } + + internal static string Imported { + get { + return ResourceManager.GetString("Imported", resourceCulture); + } + } + + internal static string ImportLockedDaysSkipped { + get { + return ResourceManager.GetString("ImportLockedDaysSkipped", resourceCulture); + } + } + + internal static string ImportUnresolvableSheetsSkipped { + get { + return ResourceManager.GetString("ImportUnresolvableSheetsSkipped", resourceCulture); + } + } } } diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.da.resx b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.da.resx index 0af50400..a2907d06 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.da.resx +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.da.resx @@ -225,4 +225,16 @@ Dagen er låst op + + Medarbejder ikke fundet. + + + Importeret. + + + {0} låst(e) dag(e) blev ikke ændret. + + + Ark ikke importeret, fordi medarbejderen ikke er fuldt oprettet: {0}. + \ No newline at end of file diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.resx b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.resx index ebaf692a..ee849fc8 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.resx +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.resx @@ -225,4 +225,16 @@ Day unlocked + + Worker not found. + + + Imported. + + + Locked days left unchanged: {0}. + + + Sheets not imported because the worker is not fully registered: {0}. + \ No newline at end of file diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs index bc12b167..0eee56dc 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs @@ -2369,11 +2369,17 @@ private static void AddLockedPlaceholderDays( /// /// Sets Reconciled and ReconciledAt together and saves. I1: flag and - /// timestamp always change together. DateTime.Now, not UtcNow: the - /// tooltip renders this verbatim as "Afstemt kl. ", and UTC - /// would read 1-2 hours off in Danish time. Consistent with the - /// CanReconcile comparison. Can throw DayLockedException; callers decide - /// how to handle that race. + /// timestamp always change together. Can throw DayLockedException; callers + /// decide how to handle that race. + /// + /// ReconciledAt is an AUDIT/DISPLAY instant -- the tooltip renders it + /// verbatim as "Afstemt kl. " -- never an input to a lock + /// comparison, so it is deliberately NOT covered by + /// DayLockHelper.CanReconcile's UtcNow rule. DateTime.Now is left here + /// unchanged: this repo's Dockerfile sets no TZ, so the shipped container + /// resolves it to UTC anyway and the stored instant is the same either way. + /// Which clock the tooltip ought to render is a presentation question and + /// is tracked separately. /// private async Task SetReconciledAsync(PlanRegistration planning, bool reconciled) { diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs index ed5cee6d..c5d78afb 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs @@ -1147,23 +1147,62 @@ await pushNotificationService.SendToSiteAsync( if (dbAssignedSite.UseGoogleSheetAsDefault) return new OperationResult(true, localizationService.GetString("AssignedSiteUpdatedSuccessfuly")); + // NO DAY-LOCK GUARD IS NEEDED HERE, and this is not an oversight. + // Everything below only reads, creates or writes rows with + // Date >= today (UTC midnight). A locked day is always strictly in the + // past: PlanRegistration.Reconciled has exactly one writer + // (TimePlanningPlanningService.SetReconciledAsync), and the only two + // callers that set it TRUE -- Reconcile and ReconcileThrough -- both + // gate on DayLockHelper.CanReconcile, which requires date < UtcNow.Date. + // (Unreconcile is the third caller and only ever sets it false, which + // removes a boundary.) So no boundary can be today or later, whichever + // order days were reconciled in. Both sides compare against UtcNow.Date, + // which makes the two ranges disjoint: no write from here can reach a + // locked row. + var midnight = new DateTime(DateTime.UtcNow.Year, DateTime.UtcNow.Month, DateTime.UtcNow.Day, 0, 0, 0); + var planRegistrationsFromTodayAndForward = await dbContext.PlanRegistrations + .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) + .Where(x => x.SdkSitId == siteId) + .Where(x => x.Date >= midnight) + .OrderBy(x => x.Date) + .ToListAsync(); + + if (planRegistrationsFromTodayAndForward.Count == 0) { - var midnight = new DateTime(DateTime.UtcNow.Year, DateTime.UtcNow.Month, DateTime.UtcNow.Day, 0, 0, 0); - var planRegistrationsFromTodayAndForward = await dbContext.PlanRegistrations - .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) - .Where(x => x.SdkSitId == siteId) - .Where(x => x.Date >= midnight) - .OrderBy(x => x.Date) - .ToListAsync(); + // create new plannings for 30 days as of today and forward + for (int i = 0; i < 30; i++) + { + var newPlanRegistration = new PlanRegistration + { + Date = midnight.AddDays(i), + SdkSitId = siteId, + CreatedByUserId = userService.UserId, + UpdatedByUserId = userService.UserId + }; - if (planRegistrationsFromTodayAndForward.Count == 0) + await newPlanRegistration.Create(dbContext); + } + } else + { + if (planRegistrationsFromTodayAndForward.Count < 30) { - // create new plannings for 30 days as of today and forward + // we need to fill all the gaps from today and forward with a new planning + var datesInPeriod = planRegistrationsFromTodayAndForward.Select(x => x.Date).ToList(); + var missingDates = new List(); for (int i = 0; i < 30; i++) + { + var date = midnight.AddDays(i); + if (!datesInPeriod.Contains(date)) + { + missingDates.Add(date); + } + } + + foreach (var missingDate in missingDates) { var newPlanRegistration = new PlanRegistration { - Date = midnight.AddDays(i), + Date = missingDate, SdkSitId = siteId, CreatedByUserId = userService.UserId, UpdatedByUserId = userService.UserId @@ -1171,207 +1210,178 @@ await pushNotificationService.SendToSiteAsync( await newPlanRegistration.Create(dbContext); } - } else + } + } + + planRegistrationsFromTodayAndForward = await dbContext.PlanRegistrations + .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) + .Where(x => x.SdkSitId == siteId) + .Where(x => x.Date >= midnight) + .OrderBy(x => x.Date) + .ToListAsync(); + + foreach (var planRegistration in planRegistrationsFromTodayAndForward) + { + var dayOfWeek = planRegistration.Date.DayOfWeek; + switch (dayOfWeek) { - if (planRegistrationsFromTodayAndForward.Count < 30) - { - // we need to fill all the gaps from today and forward with a new planning - var datesInPeriod = planRegistrationsFromTodayAndForward.Select(x => x.Date).ToList(); - var missingDates = new List(); - for (int i = 0; i < 30; i++) + case DayOfWeek.Monday: + planRegistration.PlanHours = dbAssignedSite.MondayPlanHours != 0 ? (double)dbAssignedSite.MondayPlanHours / 60 : 0; + if (!dbAssignedSite.UseOnlyPlanHours) { - var date = midnight.AddDays(i); - if (!datesInPeriod.Contains(date)) - { - missingDates.Add(date); - } + planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartMonday ?? 0; + planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndMonday ?? 0; + planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakMonday ?? 0; + planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartMonday2NdShift ?? 0; + planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndMonday2NdShift ?? 0; + planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakMonday2NdShift ?? 0; + planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartMonday3RdShift ?? 0; + planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndMonday3RdShift ?? 0; + planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakMonday3RdShift ?? 0; + planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartMonday4ThShift ?? 0; + planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndMonday4ThShift ?? 0; + planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakMonday4ThShift ?? 0; + planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartMonday5ThShift ?? 0; + planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndMonday5ThShift ?? 0; + planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakMonday5ThShift ?? 0; } - foreach (var missingDate in missingDates) + break; + case DayOfWeek.Tuesday: + planRegistration.PlanHours = dbAssignedSite.TuesdayPlanHours != 0 ? (double)dbAssignedSite.TuesdayPlanHours / 60 : 0; + if (!dbAssignedSite.UseOnlyPlanHours) { - var newPlanRegistration = new PlanRegistration - { - Date = missingDate, - SdkSitId = siteId, - CreatedByUserId = userService.UserId, - UpdatedByUserId = userService.UserId - }; - - await newPlanRegistration.Create(dbContext); + planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartTuesday ?? 0; + planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndTuesday ?? 0; + planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakTuesday ?? 0; + planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartTuesday2NdShift ?? 0; + planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndTuesday2NdShift ?? 0; + planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakTuesday2NdShift ?? 0; + planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartTuesday3RdShift ?? 0; + planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndTuesday3RdShift ?? 0; + planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakTuesday3RdShift ?? 0; + planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartTuesday4ThShift ?? 0; + planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndTuesday4ThShift ?? 0; + planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakTuesday4ThShift ?? 0; + planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartTuesday5ThShift ?? 0; + planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndTuesday5ThShift ?? 0; + planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakTuesday5ThShift ?? 0; } - } - } - - planRegistrationsFromTodayAndForward = await dbContext.PlanRegistrations - .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) - .Where(x => x.SdkSitId == siteId) - .Where(x => x.Date >= midnight) - .OrderBy(x => x.Date) - .ToListAsync(); - foreach (var planRegistration in planRegistrationsFromTodayAndForward) - { - var dayOfWeek = planRegistration.Date.DayOfWeek; - switch (dayOfWeek) - { - case DayOfWeek.Monday: - planRegistration.PlanHours = dbAssignedSite.MondayPlanHours != 0 ? (double)dbAssignedSite.MondayPlanHours / 60 : 0; - if (!dbAssignedSite.UseOnlyPlanHours) - { - planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartMonday ?? 0; - planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndMonday ?? 0; - planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakMonday ?? 0; - planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartMonday2NdShift ?? 0; - planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndMonday2NdShift ?? 0; - planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakMonday2NdShift ?? 0; - planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartMonday3RdShift ?? 0; - planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndMonday3RdShift ?? 0; - planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakMonday3RdShift ?? 0; - planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartMonday4ThShift ?? 0; - planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndMonday4ThShift ?? 0; - planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakMonday4ThShift ?? 0; - planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartMonday5ThShift ?? 0; - planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndMonday5ThShift ?? 0; - planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakMonday5ThShift ?? 0; - } - - break; - case DayOfWeek.Tuesday: - planRegistration.PlanHours = dbAssignedSite.TuesdayPlanHours != 0 ? (double)dbAssignedSite.TuesdayPlanHours / 60 : 0; - if (!dbAssignedSite.UseOnlyPlanHours) - { - planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartTuesday ?? 0; - planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndTuesday ?? 0; - planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakTuesday ?? 0; - planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartTuesday2NdShift ?? 0; - planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndTuesday2NdShift ?? 0; - planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakTuesday2NdShift ?? 0; - planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartTuesday3RdShift ?? 0; - planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndTuesday3RdShift ?? 0; - planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakTuesday3RdShift ?? 0; - planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartTuesday4ThShift ?? 0; - planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndTuesday4ThShift ?? 0; - planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakTuesday4ThShift ?? 0; - planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartTuesday5ThShift ?? 0; - planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndTuesday5ThShift ?? 0; - planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakTuesday5ThShift ?? 0; - } - - break; - case DayOfWeek.Wednesday: - planRegistration.PlanHours = dbAssignedSite.WednesdayPlanHours != 0 ? (double)dbAssignedSite.WednesdayPlanHours / 60 : 0; - if (!dbAssignedSite.UseOnlyPlanHours) - { - planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartWednesday ?? 0; - planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndWednesday ?? 0; - planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakWednesday ?? 0; - planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartWednesday2NdShift ?? 0; - planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndWednesday2NdShift ?? 0; - planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakWednesday2NdShift ?? 0; - planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartWednesday3RdShift ?? 0; - planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndWednesday3RdShift ?? 0; - planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakWednesday3RdShift ?? 0; - planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartWednesday4ThShift ?? 0; - planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndWednesday4ThShift ?? 0; - planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakWednesday4ThShift ?? 0; - planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartWednesday5ThShift ?? 0; - planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndWednesday5ThShift ?? 0; - planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakWednesday5ThShift ?? 0; - } - - break; - case DayOfWeek.Thursday: - planRegistration.PlanHours = dbAssignedSite.ThursdayPlanHours != 0 ? (double)dbAssignedSite.ThursdayPlanHours / 60 : 0; - if (!dbAssignedSite.UseOnlyPlanHours) - { - planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartThursday ?? 0; - planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndThursday ?? 0; - planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakThursday ?? 0; - planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartThursday2NdShift ?? 0; - planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndThursday2NdShift ?? 0; - planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakThursday2NdShift ?? 0; - planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartThursday3RdShift ?? 0; - planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndThursday3RdShift ?? 0; - planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakThursday3RdShift ?? 0; - planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartThursday4ThShift ?? 0; - planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndThursday4ThShift ?? 0; - planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakThursday4ThShift ?? 0; - planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartThursday5ThShift ?? 0; - planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndThursday5ThShift ?? 0; - planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakThursday5ThShift ?? 0; - } + break; + case DayOfWeek.Wednesday: + planRegistration.PlanHours = dbAssignedSite.WednesdayPlanHours != 0 ? (double)dbAssignedSite.WednesdayPlanHours / 60 : 0; + if (!dbAssignedSite.UseOnlyPlanHours) + { + planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartWednesday ?? 0; + planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndWednesday ?? 0; + planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakWednesday ?? 0; + planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartWednesday2NdShift ?? 0; + planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndWednesday2NdShift ?? 0; + planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakWednesday2NdShift ?? 0; + planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartWednesday3RdShift ?? 0; + planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndWednesday3RdShift ?? 0; + planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakWednesday3RdShift ?? 0; + planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartWednesday4ThShift ?? 0; + planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndWednesday4ThShift ?? 0; + planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakWednesday4ThShift ?? 0; + planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartWednesday5ThShift ?? 0; + planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndWednesday5ThShift ?? 0; + planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakWednesday5ThShift ?? 0; + } - break; - case DayOfWeek.Friday: - planRegistration.PlanHours = dbAssignedSite.FridayPlanHours != 0 ? (double)dbAssignedSite.FridayPlanHours / 60 : 0; - if (!dbAssignedSite.UseOnlyPlanHours) - { - planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartFriday ?? 0; - planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndFriday ?? 0; - planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakFriday ?? 0; - planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartFriday2NdShift ?? 0; - planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndFriday2NdShift ?? 0; - planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakFriday2NdShift ?? 0; - planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartFriday3RdShift ?? 0; - planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndFriday3RdShift ?? 0; - planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakFriday3RdShift ?? 0; - planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartFriday4ThShift ?? 0; - planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndFriday4ThShift ?? 0; - planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakFriday4ThShift ?? 0; - planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartFriday5ThShift ?? 0; - planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndFriday5ThShift ?? 0; - planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakFriday5ThShift ?? 0; - } + break; + case DayOfWeek.Thursday: + planRegistration.PlanHours = dbAssignedSite.ThursdayPlanHours != 0 ? (double)dbAssignedSite.ThursdayPlanHours / 60 : 0; + if (!dbAssignedSite.UseOnlyPlanHours) + { + planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartThursday ?? 0; + planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndThursday ?? 0; + planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakThursday ?? 0; + planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartThursday2NdShift ?? 0; + planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndThursday2NdShift ?? 0; + planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakThursday2NdShift ?? 0; + planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartThursday3RdShift ?? 0; + planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndThursday3RdShift ?? 0; + planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakThursday3RdShift ?? 0; + planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartThursday4ThShift ?? 0; + planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndThursday4ThShift ?? 0; + planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakThursday4ThShift ?? 0; + planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartThursday5ThShift ?? 0; + planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndThursday5ThShift ?? 0; + planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakThursday5ThShift ?? 0; + } - break; - case DayOfWeek.Saturday: - planRegistration.PlanHours = dbAssignedSite.SaturdayPlanHours != 0 ? (double)dbAssignedSite.SaturdayPlanHours / 60 : 0; - if (!dbAssignedSite.UseOnlyPlanHours) - { - planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartSaturday ?? 0; - planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndSaturday ?? 0; - planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakSaturday ?? 0; - planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartSaturday2NdShift ?? 0; - planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndSaturday2NdShift ?? 0; - planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakSaturday2NdShift ?? 0; - planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartSaturday3RdShift ?? 0; - planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndSaturday3RdShift ?? 0; - planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakSaturday3RdShift ?? 0; - planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartSaturday4ThShift ?? 0; - planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndSaturday4ThShift ?? 0; - planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakSaturday4ThShift ?? 0; - planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartSaturday5ThShift ?? 0; - planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndSaturday5ThShift ?? 0; - planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakSaturday5ThShift ?? 0; - } + break; + case DayOfWeek.Friday: + planRegistration.PlanHours = dbAssignedSite.FridayPlanHours != 0 ? (double)dbAssignedSite.FridayPlanHours / 60 : 0; + if (!dbAssignedSite.UseOnlyPlanHours) + { + planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartFriday ?? 0; + planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndFriday ?? 0; + planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakFriday ?? 0; + planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartFriday2NdShift ?? 0; + planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndFriday2NdShift ?? 0; + planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakFriday2NdShift ?? 0; + planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartFriday3RdShift ?? 0; + planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndFriday3RdShift ?? 0; + planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakFriday3RdShift ?? 0; + planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartFriday4ThShift ?? 0; + planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndFriday4ThShift ?? 0; + planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakFriday4ThShift ?? 0; + planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartFriday5ThShift ?? 0; + planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndFriday5ThShift ?? 0; + planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakFriday5ThShift ?? 0; + } - break; - case DayOfWeek.Sunday: - planRegistration.PlanHours = dbAssignedSite.SundayPlanHours != 0 ? (double)dbAssignedSite.SundayPlanHours / 60 : 0; - if (!dbAssignedSite.UseOnlyPlanHours) - { - planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartSunday ?? 0; - planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndSunday ?? 0; - planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakSunday ?? 0; - planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartSunday2NdShift ?? 0; - planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndSunday2NdShift ?? 0; - planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakSunday2NdShift ?? 0; - planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartSunday3RdShift ?? 0; - planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndSunday3RdShift ?? 0; - planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakSunday3RdShift ?? 0; - planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartSunday4ThShift ?? 0; - planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndSunday4ThShift ?? 0; - planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakSunday4ThShift ?? 0; - planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartSunday5ThShift ?? 0; - planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndSunday5ThShift ?? 0; - planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakSunday5ThShift ?? 0; - } + break; + case DayOfWeek.Saturday: + planRegistration.PlanHours = dbAssignedSite.SaturdayPlanHours != 0 ? (double)dbAssignedSite.SaturdayPlanHours / 60 : 0; + if (!dbAssignedSite.UseOnlyPlanHours) + { + planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartSaturday ?? 0; + planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndSaturday ?? 0; + planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakSaturday ?? 0; + planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartSaturday2NdShift ?? 0; + planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndSaturday2NdShift ?? 0; + planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakSaturday2NdShift ?? 0; + planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartSaturday3RdShift ?? 0; + planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndSaturday3RdShift ?? 0; + planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakSaturday3RdShift ?? 0; + planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartSaturday4ThShift ?? 0; + planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndSaturday4ThShift ?? 0; + planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakSaturday4ThShift ?? 0; + planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartSaturday5ThShift ?? 0; + planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndSaturday5ThShift ?? 0; + planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakSaturday5ThShift ?? 0; + } - break; - } + break; + case DayOfWeek.Sunday: + planRegistration.PlanHours = dbAssignedSite.SundayPlanHours != 0 ? (double)dbAssignedSite.SundayPlanHours / 60 : 0; + if (!dbAssignedSite.UseOnlyPlanHours) + { + planRegistration.PlannedStartOfShift1 = dbAssignedSite.StartSunday ?? 0; + planRegistration.PlannedEndOfShift1 = dbAssignedSite.EndSunday ?? 0; + planRegistration.PlannedBreakOfShift1 = dbAssignedSite.BreakSunday ?? 0; + planRegistration.PlannedStartOfShift2 = dbAssignedSite.StartSunday2NdShift ?? 0; + planRegistration.PlannedEndOfShift2 = dbAssignedSite.EndSunday2NdShift ?? 0; + planRegistration.PlannedBreakOfShift2 = dbAssignedSite.BreakSunday2NdShift ?? 0; + planRegistration.PlannedStartOfShift3 = dbAssignedSite.StartSunday3RdShift ?? 0; + planRegistration.PlannedEndOfShift3 = dbAssignedSite.EndSunday3RdShift ?? 0; + planRegistration.PlannedBreakOfShift3 = dbAssignedSite.BreakSunday3RdShift ?? 0; + planRegistration.PlannedStartOfShift4 = dbAssignedSite.StartSunday4ThShift ?? 0; + planRegistration.PlannedEndOfShift4 = dbAssignedSite.EndSunday4ThShift ?? 0; + planRegistration.PlannedBreakOfShift4 = dbAssignedSite.BreakSunday4ThShift ?? 0; + planRegistration.PlannedStartOfShift5 = dbAssignedSite.StartSunday5ThShift ?? 0; + planRegistration.PlannedEndOfShift5 = dbAssignedSite.EndSunday5ThShift ?? 0; + planRegistration.PlannedBreakOfShift5 = dbAssignedSite.BreakSunday5ThShift ?? 0; + } - await planRegistration.Update(dbContext); + break; } + + await planRegistration.Update(dbContext); } return new OperationResult(true, localizationService.GetString("AssignedSiteUpdatedSuccessfuly")); diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs index d882c906..57e1cf97 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs @@ -1365,19 +1365,36 @@ private static void ApplyPunchClockFlexChainDecimal( } /// - /// The day-lock guard for both UpdateWorkingHour overloads: a failure to - /// return when the day is locked, else null. Neither overload has a - /// try/catch, so this answers with a message instead of letting the - /// interceptor throw. The message follows the planning service's rule, the - /// row's own Reconciled flag, so mobile says what web says for the same day + /// The write guard for both UpdateWorkingHour overloads: a failure to + /// return when this (site, day) may NOT be written, else null. Neither + /// overload has a try/catch, so this answers with a message instead of + /// letting the interceptor throw. + /// + /// Named for what it answers, not for one of its reasons: it refuses on two + /// distinct grounds -- the day is locked, or the site could not be resolved + /// at all -- and only the first is a day-lock outcome. (It is also not the + /// same method as TimePlanningPlanningService.CheckDayLockAsync, which + /// takes a loaded PlanRegistration; an earlier audit flagged the shared + /// name as a trap.) + /// + /// Locked case: the message follows the planning service's rule, the row's + /// own Reconciled flag, so mobile says what web says for the same day /// (spec §11.3). The row may not be loaded yet (or may not exist), so the /// flag costs one cheap query, and only when the day is locked. + /// + /// Null site id REFUSES. null is this method's "writable, proceed" answer, + /// so returning it for an unresolvable site would waive the freeze for + /// exactly the caller whose day cannot be evaluated -- and for a rule whose + /// purpose is "this day cannot be written", the unknown case must refuse, + /// not permit. It costs nothing: the kiosk overload dereferences the same + /// id with `!` a few hundred lines later, so a null that got past here + /// ended as a raw InvalidOperationException instead of a message. /// - private async Task CheckDayLockAsync(int? sdkSitId, DateTime date) + private async Task RefuseIfNotWritableAsync(int? sdkSitId, DateTime date) { if (sdkSitId is not { } siteId) { - return null; + return new OperationResult(false, localizationService.GetString("SiteNotFound")); } var lockedThrough = await DayLockHelper.LockedThroughAsync(dbContext, siteId); @@ -1451,9 +1468,9 @@ public async Task UpdateWorkingHour(TimePlanningWorkingHoursUpd localizationService.GetString("EditingNotAllowedForWorker")); } - if (await CheckDayLockAsync(sdkSite.MicrotingUid, model.Date) is { } dayLocked) + if (await RefuseIfNotWritableAsync(sdkSite.MicrotingUid, model.Date) is { } refusal) { - return dayLocked; + return refusal; } var todayAtMidnight = model.Date; @@ -2086,9 +2103,9 @@ public async Task UpdateWorkingHour(int? sdkSiteId, TimePlannin // Before both branches below (each repeats its own assigned-site lookup), // and after the token check so an unknown device learns nothing about // the lock. - if (await CheckDayLockAsync(sdkSiteId, model.Date) is { } dayLocked) + if (await RefuseIfNotWritableAsync(sdkSiteId, model.Date) is { } refusal) { - return dayLocked; + return refusal; } registrationDevice.OsVersion = model.OsVersion; @@ -3993,6 +4010,17 @@ await OneMinuteModeTimeline.BuildAsync(dbContext, assignedSiteForCache) public async Task Import(IFormFile file) { + // Method scope, so the finally below and the success exit at the bottom + // can both read them. The two counters are incremented at DIFFERENT + // levels, so an audit has to look in both places: + // unresolvableSheetsSkipped -- the sheet loop, at the third of its + // three `continue`s (the first two are deliberately silent; each + // says why). + // lockedDaysSkipped -- the nested ROW loop, at its locked-day + // `continue`. This is the counter the deleted Console.WriteLine + // used to report. + var lockedDaysSkipped = 0; + var unresolvableSheetsSkipped = 0; try { // Get core @@ -4017,20 +4045,38 @@ public async Task Import(IFormFile file) return new OperationResult(false, localizationService.GetString("FileFormatError")); } - // Observability only: the locked-day skip below is silent otherwise. - var lockedDaysSkipped = 0; foreach (Sheet sheet in sheets) { + // A malformed sheet element, not user data. Silent. if (sheet.Name?.Value == null || sheet.Id?.Value == null) { continue; } var site = await sdkContext.Sites.FirstOrDefaultAsync(x => x.Name.Replace(" ", "").ToLower() == sheet.Name.Value.Replace(" ", "").ToLower()); + + // DELIBERATELY SILENT, and not the same case as the one + // below. A sheet whose name matches no worker may not be + // about a worker at all -- a cover tab, instructions, a + // summary -- so reporting every one would cry wolf on + // ordinary workbooks. (Pre-existing behaviour; if it is + // ever revisited, it needs its own decision about which + // unmatched names are worth naming.) if (site == null) { continue; } + // REPORTED, because the name DID match a worker: this is + // a real data problem the user has to hear about, not an + // unrelated tab. Skip the whole sheet -- a null boundary + // makes IsLocked false for EVERY row, which would import + // it with no lock check at all. + if (site.MicrotingUid is not { } importSiteUid) + { + unresolvableSheetsSkipped++; + continue; + } + // ONE timeline per sheet (= per site), built BEFORE the row // loop and never per row. The update leg below may only clear // a row's seconds columns once it knows the row ran in @@ -4038,15 +4084,14 @@ public async Task Import(IFormFile file) var importAssignedSite = await dbContext.AssignedSites .AsNoTracking() .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) - .FirstOrDefaultAsync(x => x.SiteId == site.MicrotingUid); + .FirstOrDefaultAsync(x => x.SiteId == importSiteUid); var importTimeline = await OneMinuteModeTimeline.BuildAsync(dbContext, importAssignedSite); // A bulk import skips locked days (frozen means frozen) // rather than failing the whole file. Once per sheet // (= per site), never per row. - var importLockedThrough = site.MicrotingUid is { } importSiteUid - ? await DayLockHelper.LockedThroughAsync(dbContext, importSiteUid) - : null; + var importLockedThrough = + await DayLockHelper.LockedThroughAsync(dbContext, importSiteUid); var worksheetPart = (WorksheetPart)workbookPart.GetPartById(sheet.Id.Value); var sheetData = worksheetPart.Worksheet.Elements().First(); @@ -4110,12 +4155,12 @@ public async Task Import(IFormFile file) var preTimePlanning = await dbContext.PlanRegistrations.AsNoTracking() .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) - .Where(x => x.Date < dateValue && x.SdkSitId == (int)site.MicrotingUid!) + .Where(x => x.Date < dateValue && x.SdkSitId == importSiteUid) .OrderByDescending(x => x.Date) .FirstOrDefaultAsync(); var planRegistration = await dbContext.PlanRegistrations.FirstOrDefaultAsync(x => - x.Date == dateValue && x.SdkSitId == site.MicrotingUid + x.Date == dateValue && x.SdkSitId == importSiteUid && x.WorkflowState != Constants.WorkflowStates.Removed); if (planRegistration == null) @@ -4125,7 +4170,7 @@ public async Task Import(IFormFile file) Date = dateValue, PlanText = planText, PlanHours = parsedPlanHours, - SdkSitId = (int)site.MicrotingUid!, + SdkSitId = importSiteUid, CreatedByUserId = userService.UserId, UpdatedByUserId = userService.UserId, NettoHours = 0, @@ -4196,8 +4241,6 @@ public async Task Import(IFormFile file) } } } - - Console.WriteLine($"[Import] summary: skipped {lockedDaysSkipped} locked day(s)."); } } } @@ -4207,7 +4250,56 @@ public async Task Import(IFormFile file) logger.LogError(ex.Message); return new OperationResult(false, ex.Message); } - return new OperationResult(true, "Imported"); + finally + { + // IN A FINALLY, so it runs on EVERY exit from this method: the + // success path below, the catch above, and the two malformed-file + // early returns. Placed after the catch's `return` it would have + // been skipped exactly when it matters most -- a file that skipped + // 40 locked days and then threw on sheet 9 would lose both counts. + // + // This is the channel ops actually has: it works whatever the UI + // does with the response, and it replaces the Console.WriteLine + // this PR removed. It logs even when both counts are zero, so + // "skipped nothing" stays distinguishable from "never got here". + // "ended", not "finished": on the catch path it did not finish. + logger.LogInformation( + "Import ended: {LockedDaysSkipped} locked day(s) skipped, {UnresolvableSheetsSkipped} sheet(s) skipped for a worker with no MicrotingUid.", + lockedDaysSkipped, unresolvableSheetsSkipped); + } + + // A sheet whose name matched a Site row that carries no MicrotingUid is + // a data problem someone has to fix, not routine bookkeeping, so it also + // raises a Sentry warning -- the same treatment GoogleSheetHelper gives + // its own sheet problems. Only when non-zero: a zero is worth nothing + // there. Only on the success path, too: a run that threw has already + // reported that exception to Sentry, and the finally above carries the + // counts, so repeating them here would be a second ticket for one run. + if (unresolvableSheetsSkipped > 0) + { + SentrySdk.CaptureMessage( + $"Import: {unresolvableSheetsSkipped} sheet(s) named a worker with no MicrotingUid and were skipped.", + SentryLevel.Warning); + } + + // Then the user-facing message: one success exit, naming each skip with + // its reason -- the rule ReconcileThrough already follows. A bare + // "Imported" over a reconciled month reads as "your corrections are in" + // while the locked rows still hold their old numbers, so the user + // re-imports the same file forever. Each part is a whole sentence, so + // they join in any language without having to agree grammatically. + var summary = new List { localizationService.GetString("Imported") }; + if (lockedDaysSkipped > 0) + { + summary.Add(localizationService.GetString( + "ImportLockedDaysSkipped", lockedDaysSkipped)); + } + if (unresolvableSheetsSkipped > 0) + { + summary.Add(localizationService.GetString( + "ImportUnresolvableSheetsSkipped", unresolvableSheetsSkipped)); + } + return new OperationResult(true, string.Join(" ", summary)); } private string GetCellValue(WorkbookPart workbookPart, Row row, int columnIndex) diff --git a/eform-client/src/app/plugins/modules/time-planning-pn/modules/working-hours/components/working-hours-actions/working-hours-upload-modal/working-hours-upload-modal.component.ts b/eform-client/src/app/plugins/modules/time-planning-pn/modules/working-hours/components/working-hours-actions/working-hours-upload-modal/working-hours-upload-modal.component.ts index b6ca73ba..960746bf 100644 --- a/eform-client/src/app/plugins/modules/time-planning-pn/modules/working-hours/components/working-hours-actions/working-hours-upload-modal/working-hours-upload-modal.component.ts +++ b/eform-client/src/app/plugins/modules/time-planning-pn/modules/working-hours/components/working-hours-actions/working-hours-upload-modal/working-hours-upload-modal.component.ts @@ -41,10 +41,37 @@ export class WorkingHoursUploadModalComponent implements OnInit { // this.workingHoursFileUploader.onBuildItemForm = (item, form) => { // //form.append('templateId', this.selectedTemplate.id); // }; - this.workingHoursFileUploader.onSuccessItem = () => { + // HTTP 200 DOES NOT MEAN THE IMPORT SUCCEEDED. The controller returns an + // OperationResult as a 200 body whatever its Success flag says, so + // ng2-file-upload routes server-reported FAILURES -- a malformed workbook, + // or an exception part way through the file -- to onSuccessItem, not to + // onErrorItem. Branch on the body, never on the transport. + // + // On success the body also reports what the import actually did: how many + // days it could not write because they are reconciled, and how many sheets + // belong to a worker with no id. Those rows are silently NOT imported, so a + // hardcoded "uploaded successfully" would tell someone their corrections + // went in when they did not, and they would re-upload the same file forever. + this.workingHoursFileUploader.onSuccessItem = (item, response) => { this.workingHoursFileUploader.clearQueue(); + const result = this.parseOperationResult(response); + if (result && !result.success) { + // Report the server's own diagnosis and stop: no success toast, and the + // dialog is not dismissed as done -- same shape as onErrorItem below. + this.toastrService.error( + result.message ?? + this.translateService.instant('Error while uploading file') + ); + return; + } + // Green even when the message carries skips ("Imported. Locked days left + // unchanged: 3."). A warning toast would read better, but the only signal + // here is a localized string, and sniffing it for skip text is fragile. + // Doing it properly means the endpoint returning counts as data -- + // OperationDataResult with a typed model, the way ReconcileThrough does. this.toastrService.success( - this.translateService.instant('File has been uploaded successfully') + result?.message ?? + this.translateService.instant('File has been uploaded successfully') ); this.hideZipModal(true); }; @@ -61,9 +88,41 @@ export class WorkingHoursUploadModalComponent implements OnInit { }; } + /** + * The server's OperationResult, parsed ONCE so the caller can branch on + * success and reuse the message, or null when the body is not a recognisable + * OperationResult. Defensive on purpose: ng2-file-upload hands back the raw + * response text, and a proxy or an error page can put anything in it. A null + * result means "cannot tell", which the caller treats as the old + * success-with-translated-fallback behaviour. + */ + private parseOperationResult( + response: string + ): { success: boolean; message: string | null } | null { + try { + const body = JSON.parse(response); + if (!body || typeof body.success !== 'boolean') { + return null; + } + return { + success: body.success, + message: + typeof body.message === 'string' && body.message ? body.message : null, + }; + } catch { + return null; + } + } + uploadTemplateZIP() { this.workingHoursFileUploader.queue[0].upload(); this.dialogRef.close(true); + // DO NOT merge this with the toast in onSuccessItem. The two say different + // things at different times and both are wanted: this one fires the moment + // the upload STARTS and is honest that processing takes a while, while + // onSuccessItem fires when the server has finished and reports what the + // import actually did. Collapsing them would either lose the "this takes a + // while" warning or make the dialog block until the import completes. this.toastrService.success( this.translateService.instant('File has been uploaded successfully, processing file can take a while, depending on the number of records') );