fix(workers): clean up orphan SDK site so deleted/failed workers don't block name reuse#1004
Merged
Merged
Conversation
… worker delete Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses orphaned Microting SDK Sites/Workers left behind by failed worker creation or incomplete deletion, which can incorrectly block reusing a worker name due to the SDK’s duplicate-name check on active Sites.
Changes:
- Adds a compensating cleanup in
CreateDeviceUserto soft-delete an orphanSite(and danglingWorker) on failures after name determination, guarded by “no activeSiteWorkerlink”. - Updates the worker delete flow to invoke
core.SiteDelete(...)so deleting a worker also removes the underlying SDKSite(and cascades). - Returns a non-empty failure message key when time-registration assignment fails during creation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationAssignmentWorkerService/BackendConfigurationAssignmentWorkerService.cs |
Calls core.SiteDelete during worker delete to prevent leftover SDK Site rows from blocking name reuse. |
eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Infrastructure/Helpers/BackendConfigurationAssignmentWorkerServiceHelper.cs |
Adds create-failure cleanup for orphan SDK Site/Worker and improves the returned error message in one failure path. |
Comment on lines
+327
to
+340
| // Remove the underlying SDK Site (cascades to Worker + SiteWorker, and deregisters remotely) | ||
| // so a deleted worker does not leave an active orphan Site blocking name reuse. | ||
| if (site != null && site.MicrotingUid.HasValue) | ||
| { | ||
| try | ||
| { | ||
| await core.SiteDelete(site.MicrotingUid.Value).ConfigureAwait(false); | ||
| } | ||
| catch (Exception siteDeleteEx) | ||
| { | ||
| SentrySdk.CaptureException(siteDeleteEx); | ||
| logger.LogError(siteDeleteEx, "Delete: core.SiteDelete failed for site MicrotingUid {Uid}", site.MicrotingUid); | ||
| } | ||
| } |
Comment on lines
+878
to
+896
| // SiteCreate may throw after committing the Site row but before the SiteWorker is | ||
| // created, leaving an active orphan Site that blocks re-creating this name. Match the | ||
| // orphan by name and only remove it when it has no active SiteWorker. | ||
| var orphanSite = await sdkDbContext.Sites | ||
| .FirstOrDefaultAsync(x => x.Name == siteName | ||
| && x.WorkflowState != Constants.WorkflowStates.Removed); | ||
| if (orphanSite == null) | ||
| { | ||
| return; | ||
| } | ||
| var siteHasActiveWorkerLink = await sdkDbContext.SiteWorkers | ||
| .AnyAsync(sw => sw.SiteId == orphanSite.Id | ||
| && sw.WorkflowState != Constants.WorkflowStates.Removed); | ||
| if (siteHasActiveWorkerLink) | ||
| { | ||
| return; // fully-formed worker, not an orphan — leave it intact | ||
| } | ||
| await orphanSite.Delete(sdkDbContext); | ||
|
|
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
Creating a property worker could fail with "Medarbejderen findes allerede" ("employee already exists") for a name that has no visible worker. Root cause: a worker creation that fails partway leaves an active SDK
sitesrow with no Worker/SiteWorker (an orphan), and the create-time duplicate check (Sites.Name == FirstName+" "+LastName && WorkflowState != Removed) then permanently blocks that name. Two sources of orphans:core.SiteCreatecommits thesitesrow before creating the Worker/SiteWorker and is non-transactional with no rollback — if a later step throws, the site is orphaned. (SDK is NuGet-pinned, so the structural fix can't land here yet.)Site/Worker.Fix (plugin-side)
CreateDeviceUser: on any failure afterSiteCreate, a compensating cleanup matches the orphan by site name (available even whenSiteCreatethrows and returns no dto) and soft-deletes it only when it has no active SiteWorker — so a fully-formed worker is never deleted. A dangling Worker (matched by email, guarded the same way) is also soft-deleted. Cleanup runs only in the outer catch (not the inner time-registration catch, where the worker is already complete).Delete: after the existing teardown, callscore.SiteDelete(site.MicrotingUid)(cascades remote + Worker + SiteWorker) so deleting a worker no longer leaves an orphan.Notes / limitations
core.SiteDeleteis not null-safe for siteworker-less orphans (SDKCore.cs:3076NREs), so the create-failure path soft-deletes via the SDK dbContext directly instead.Core.SiteCreatetransactional (SDK change, future NuGet release).core, so they aren't unit-testable here; verified by compilation + code review. The pre-existing local orphan (site Fix workers page #29) was soft-deleted separately to unblock immediately.🤖 Generated with Claude Code