fix: push the Google Sheet headers when a worker is created - #1281
Merged
Merged
Conversation
A worker created with time registration enabled got an AssignedSite but no columns in the PlanTimer sheet: CreateDeviceUser never pushed the headers, while UpdateDeviceUser has always pushed right after creating the same row. The import matches columns by name, so such a worker was never imported at all -- their columns only appeared once somebody edited them again or saved the TimePlanning settings page. CreateDeviceUser now pushes too. The call is guarded: it reaches a third-party API and reads a plugin setting a tenant may not have, and neither may cost us the worker. PushToGoogleSheet gets the same fix as the timeplanning plugin's copy: headers are matched ignoring case, whitespace and dash style rather than by exact text, a site missing half a pair gets a complete new pair and a reported column to clean up, only the appended cells are written, and appends never land in the date columns the import skips. PlanTimerSheetColumns is a hand-kept copy of that plugin's file, which is itself a copy of the scheduled import's. Consolidating the three into Microting.TimePlanningBase needs a base release and is left for later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
New unit tests currently contradict the ParseHours implementation and the Google Sheet rename-write path can still throw on an empty header row, which can break updates in valid scenarios.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR fixes a time-registration edge case in the BackendConfiguration plugin where newly created workers could get an AssignedSite but still have no corresponding - timer / - tekst columns in the TimePlanning Google Sheet, preventing them from being imported (imports match columns by name).
Changes:
- Extend
CreateDeviceUserto push PlanTimer sheet headers immediately after creating the assignment (errors are caught/logged so worker creation is not blocked). - Rework
PushToGoogleSheetheader handling to plan/apply only needed header appends (normalized matching, avoid full-row rewrites) and report inconsistencies via log + Sentry. - Add
PlanTimerSheetColumnshelper plus new unit tests covering header mapping and append planning.
File summaries
| File | Description |
|---|---|
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationAssignmentWorkerService/BackendConfigurationAssignmentWorkerService.cs | Passes logger through to helper so CreateDeviceUser can safely log sheet-push failures. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Infrastructure/Helpers/PlanTimerSheetColumns.cs | Introduces normalized header mapping + append planning for PlanTimer columns. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Infrastructure/Helpers/GoogleSheetHelper.cs | Uses PlanTimerSheetColumns to append only missing headers and logs/sends Sentry warnings for layout problems. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Infrastructure/Helpers/BackendConfigurationAssignmentWorkerServiceHelper.cs | Adds optional ILogger and pushes sheet headers on worker creation (guarded try/catch). |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn.Test/PlanTimerSheetColumnsTests.cs | Adds focused unit coverage for mapping, normalization, and append planning behavior. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+151
to
154
| var columnLetter = PlanTimerSheetColumns.ColumnLetter(existingHeaders.Count - 1); | ||
| var updateHeaderRequest = | ||
| service.Spreadsheets.Values.Update(updateRequest, googleSheetId, $"{sheetName}!A1:{columnLetter}1"); | ||
| updateHeaderRequest.ValueInputOption = |
Comment on lines
+217
to
+221
| if (!double.TryParse(text.Replace(",", "."), NumberStyles.AllowDecimalPoint, | ||
| NumberFormatInfo.InvariantInfo, out var hours) || !double.IsFinite(hours)) | ||
| { | ||
| return null; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A worker created in BackendConfiguration with time registration enabled gets an
AssignedSite, but no columns in the TimePlanning Google Sheet.CreateDeviceUsercreates the assignment atBackendConfigurationAssignmentWorkerServiceHelper.cs:1843and stops there.UpdateDeviceUserdoes the very same thing at line 1403 and then pushes the headers on line 1404 (and again at 1222 for the "assignment already exists" branch). Nothing else syncs headers — there is no scheduled job for it in any repo.Because the import matches columns by name, a worker with no column is never imported at all. Their columns appear only once somebody edits them again, or saves the TimePlanning settings page. After #1300 the scheduled import does log it ("site … imports from the sheet but no column names it"), but only at
infolevel.Fix
CreateDeviceUsernow pushes the headers, right after the assignment is created, inside theTimeRegistrationEnabledbranch. The call is wrapped in try/catch with a log and a Sentry event: it reaches a third-party API and reads a plugin setting a tenant may not have (PushToGoogleSheetdoes.Single(...)onPluginConfigurationValuesbefore its env-var check), and neither may cost us the worker.CreateDeviceUsergained an optionalILogger? logger = nullparameter, so its existing callers — several inBackendConfiguration.Pn.Integration.Test— keep compiling.PushToGoogleSheetgets the same fix as the timeplanning plugin's copy (#1714), which is where tenant 1063's split pairs came from:Tests
PlanTimerSheetColumnsTests(new, pure unit) covers the mapping and the append planning. This repo runs the whole unit-test project, so no allowlist entry is needed.Verified locally:
dotnet buildofBackendConfiguration.Pn,BackendConfiguration.Pn.TestandBackendConfiguration.Pn.Integration.Test. Tests run in CI.Note
PlanTimerSheetColumns.cshere is a hand-kept copy of the same file in the timeplanning plugin, which is itself a copy of the scheduled import's. The three must agree or the same sheet is read and written differently through each path. Consolidating them intoMicroting.TimePlanningBaseis the real fix and needs a base release.🤖 Generated with Claude Code