Conversation
…pport - Added LOCATION_INPUT_FEATURE.md documentation detailing the new location input system. - Updated StorageLocations.cshtml and StorageLocations.cshtml.cs to integrate location search, geolocation, and manual entry. - Created locations-search.css for styling the new location input UI components. - Developed locations-geocoding.js for handling location search, geolocation, and manual coordinate input. - Introduced scripts for checking StorageLens status and cleaning, building, and running Docker containers. - Added migration for soft delete functionality in StorageLocations.
…ting transactions
- Repository pattern: ILocationRepository, IScanJobRepository, IFileInventoryRepository,
IDuplicateRepository, IWorkflowRepository with EF Core implementations. All 5 data-
bearing services now take their repository interface instead of DbContext directly.
Program.cs files register repos as Scoped. Tests updated to construct via repository.
- Saga pattern: IWorkflowCompensator/WorkflowCompensator issues best-effort DELETE calls
to FileInventory and Duplicates services when a workflow fails, undoing side-effects of
completed steps. Added checkpoint/resume: already-completed steps are skipped on retry.
- New endpoints: DELETE api/files/by-scan-job/{scanJobId}, DELETE api/duplicates/by-scan-job/{scanJobId},
POST api/orchestrator/workflows/{id}/compensate.
- Added FileInventory upstream to OrchestratorUpstreamOptions.
- Fixed SetWorkflowStatus ordering so downstream update failures don't leave the workflow
in Completed before transitioning to Failed.
- Added WorkflowSagaTests.cs with 4 new tests. Fixed pre-existing StorageLocationDto
constructor calls in WorkflowOrchestratorServiceTests.
- Updated docs/CHANGELOG.md.
Agent-Logs-Url: https://github.com/DeeDee1103/StorageLens/sessions/d461ea0f-fcf6-4f0e-95b2-d97a1be74510
Co-authored-by: DeeDee1103 <61173060+DeeDee1103@users.noreply.github.com>
Agent-Logs-Url: https://github.com/DeeDee1103/StorageLens/sessions/d461ea0f-fcf6-4f0e-95b2-d97a1be74510 Co-authored-by: DeeDee1103 <61173060+DeeDee1103@users.noreply.github.com>
feat: Repository abstraction layer + Saga pattern with compensating transactions
There was a problem hiding this comment.
Pull request overview
This PR expands StorageLens’ Storage Locations experience with multi-method coordinate acquisition (search/geolocation/manual), and strengthens backend consistency by introducing repository abstractions plus saga-style step compensation in the orchestrator.
Changes:
- Added location search + geocoding UI (Nominatim), updated validation to require coordinates, and improved related UI/UX across pages (sidebar modes, map empty states, delete confirmations).
- Introduced repository abstractions across microservices to isolate EF Core data access from service logic; updated DI and tests accordingly.
- Implemented saga checkpoint/resume and best-effort compensating transactions (cleanup endpoints + downstream DELETE APIs) to improve recoverability after workflow failures.
Reviewed changes
Copilot reviewed 66 out of 67 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/StorageLens.Services.ScanJobs.Tests/ScanJobsServiceTests.cs | Updates tests to construct ScanJobsService via repository abstraction. |
| tests/StorageLens.Services.Orchestrator.Tests/WorkflowSagaTests.cs | Adds saga tests for resume/checkpoint behavior and compensation calls. |
| tests/StorageLens.Services.Orchestrator.Tests/WorkflowOrchestratorServiceTests.cs | Updates orchestrator tests for repository + compensator wiring and updated DTO shape. |
| tests/StorageLens.Services.Orchestrator.Tests/OrchestratorControllerTests.cs | Updates controller tests for new compensator dependency. |
| tests/StorageLens.Services.Locations.Tests/LocationServiceTests.cs | Updates tests for repository usage and new soft-delete visibility behavior. |
| tests/StorageLens.Services.Locations.Tests/LocationsControllerTests.cs | Updates controller tests for updated request/DTO fields. |
| tests/StorageLens.Services.FileInventory.Tests/FileInventoryServiceTests.cs | Updates tests to construct FileInventoryService via repository abstraction. |
| tests/StorageLens.Services.Duplicates.Tests/DuplicatesServiceTests.cs | Updates tests to construct DuplicatesService via repository abstraction. |
| tests/StorageLens.Integration.Tests/Orchestrator/OrchestratorWebFactory.cs | Updates integration stubs for extended StorageLocationDto parameters. |
| tests/StorageLens.Integration.Tests/Locations/LocationsIntegrationTests.cs | Adds integration coverage for soft-delete hiding locations from UI APIs. |
| tests/StorageLens.Integration.Tests/Infrastructure/TestDataBuilders.cs | Extends test builders for site/status/coordinates fields. |
| src/StorageLens.Web/wwwroot/js/site.js | Adds persisted sidebar mode (expanded/compact/hidden) with ARIA updates. |
| src/StorageLens.Web/wwwroot/js/locations-geocoding.js | Adds Nominatim search + geolocation + manual coordinate application logic. |
| src/StorageLens.Web/wwwroot/css/site.css | Adds sidebar transitions/modes and delete dialog styling; adjusts some UI spacing. |
| src/StorageLens.Web/wwwroot/css/locations-search.css | Adds styling for location search dropdown, statuses, advanced coords panel, dark mode. |
| src/StorageLens.Web/Pages/StorageLocations.cshtml | Updates Storage Locations UI to include search/geolocation/manual coordinate workflows. |
| src/StorageLens.Web/Pages/StorageLocations.cshtml.cs | Updates add-location validation: coordinates required, SiteLocation optional with fallback. |
| src/StorageLens.Web/Pages/Shared/_Layout.cshtml | Includes new locations-search.css and adds sidebar toggle buttons. |
| src/StorageLens.Web/Pages/ScanJobs.cshtml.cs | Adds ScanJobs delete handler calling ScanJobs API client. |
| src/StorageLens.Web/Pages/ScanJobs.cshtml | Adds delete-job UI with confirmation overlay and JS wiring. |
| src/StorageLens.Web/Pages/GlobalView.cshtml | Improves empty states when Leaflet/Chart.js assets are unavailable; better map no-data messaging. |
| src/StorageLens.Web/Pages/FileInventory.cshtml.cs | Adds LocationsApiClient usage + richer location snapshots for UI grouping. |
| src/StorageLens.Web/Pages/FileInventory.cshtml | Uses server-provided location snapshots; enhances action button labeling. |
| src/StorageLens.Web/Clients/ScanJobsApiClient.cs | Adds DeleteAsync wrapper with service-unavailable handling. |
| src/StorageLens.Shared.Infrastructure/StartupDatabaseInitializer.cs | Adds relational migrate fallback that reconciles missing columns for legacy schemas. |
| src/StorageLens.Services.ScanJobs/Services/ScanJobsService.cs | Refactors service to depend on IScanJobRepository; adds delete support. |
| src/StorageLens.Services.ScanJobs/Repositories/ScanJobRepository.cs | New EF-backed repository implementing scan job CRUD + fast-path deletes/updates. |
| src/StorageLens.Services.ScanJobs/Repositories/IScanJobRepository.cs | New repository interface for ScanJobs EF operations. |
| src/StorageLens.Services.ScanJobs/Program.cs | Registers scan job repository and updated service in DI. |
| src/StorageLens.Services.ScanJobs/Interfaces/IScanJobsService.cs | Adds DeleteAsync to service interface. |
| src/StorageLens.Services.ScanJobs/Controllers/ScanJobsController.cs | Adds DELETE endpoint for scan jobs. |
| src/StorageLens.Services.Orchestrator/Services/WorkflowOrchestratorService.cs | Refactors to use repository + compensator; adds step checkpoint/resume and compensation on failure. |
| src/StorageLens.Services.Orchestrator/Sagas/WorkflowCompensator.cs | Implements best-effort compensation via downstream DELETE calls. |
| src/StorageLens.Services.Orchestrator/Sagas/IWorkflowCompensator.cs | Defines compensator abstraction. |
| src/StorageLens.Services.Orchestrator/Repositories/WorkflowRepository.cs | New EF-backed repository for orchestrator workflows/steps/errors. |
| src/StorageLens.Services.Orchestrator/Repositories/IWorkflowRepository.cs | New repository interface for orchestrator EF operations. |
| src/StorageLens.Services.Orchestrator/Program.cs | Registers repository + compensator; adds FileInventory upstream client and health check. |
| src/StorageLens.Services.Orchestrator/Controllers/OrchestratorController.cs | Adds operator-triggered workflow compensation endpoint and new DI dependency. |
| src/StorageLens.Services.Orchestrator/Clients/OrchestratorUpstreamOptions.cs | Adds FileInventory upstream base URL option. |
| src/StorageLens.Services.Locations/Services/LocationService.cs | Refactors to use ILocationRepository and implements soft delete semantics. |
| src/StorageLens.Services.Locations/Repositories/LocationRepository.cs | New EF-backed repository with visibility filtering + soft delete. |
| src/StorageLens.Services.Locations/Repositories/ILocationRepository.cs | New repository interface for Locations EF operations. |
| src/StorageLens.Services.Locations/Program.cs | Registers location repository and updated service in DI. |
| src/StorageLens.Services.Locations/Models/StorageLocation.cs | Adds IsVisibleInUI flag for soft delete. |
| src/StorageLens.Services.Locations/Migrations/LocationsDbContextModelSnapshot.cs | Updates snapshot to include visibility and related indexes/columns. |
| src/StorageLens.Services.Locations/Migrations/20260326233000_AddLocationVisibilitySoftDelete.cs | Adds migration for IsVisibleInUI + index. |
| src/StorageLens.Services.Locations/Data/LocationsDbContext.cs | Configures IsVisibleInUI defaults and adds index. |
| src/StorageLens.Services.FileInventory/Services/FileInventoryService.cs | Refactors to use IFileInventoryRepository; adds delete-by-scan-job for compensation. |
| src/StorageLens.Services.FileInventory/Repositories/IFileInventoryRepository.cs | New repository interface for FileInventory EF operations. |
| src/StorageLens.Services.FileInventory/Repositories/FileInventoryRepository.cs | New EF-backed repository implementing prior service queries/updates. |
| src/StorageLens.Services.FileInventory/Program.cs | Registers FileInventory repository and updated service in DI. |
| src/StorageLens.Services.FileInventory/Interfaces/IFileInventoryService.cs | Adds DeleteByScanJobAsync for compensation. |
| src/StorageLens.Services.FileInventory/Controllers/FilesController.cs | Adds DELETE by scan job endpoint for compensator use. |
| src/StorageLens.Services.Duplicates/Services/DuplicatesService.cs | Refactors to use IDuplicateRepository; adds delete-by-scan-job for compensation. |
| src/StorageLens.Services.Duplicates/Repositories/IDuplicateRepository.cs | New repository interface + aggregate record. |
| src/StorageLens.Services.Duplicates/Repositories/DuplicateRepository.cs | New EF-backed repository for duplicates queries and replace semantics. |
| src/StorageLens.Services.Duplicates/Program.cs | Registers duplicate repository and updated service in DI. |
| src/StorageLens.Services.Duplicates/Interfaces/IDuplicatesService.cs | Adds DeleteByScanJobAsync for compensation. |
| src/StorageLens.Services.Duplicates/Controllers/DuplicatesController.cs | Adds DELETE by scan job endpoint for compensator use. |
| scripts/clean-build-run.ps1 | Adds scripted docker compose clean/build/up flow with logging. |
| scripts/clean-build-pull-run.cmd | Adds Windows cmd variant of clean/build/pull/up flow with logs. |
| scripts/check-storagelens-status.ps1 | Adds helper to dump current docker ps status for StorageLens containers. |
| IMPLEMENTATION_SUMMARY.md | Adds implementation summary documentation for location input feature. |
| docs/implementation/LOCATION_INPUT_FEATURE.md | Adds detailed implementation/testing guide for multi-method location input. |
| docs/CHANGELOG.md | Documents repository abstraction + saga/compensation additions. |
| DEBUG_DD_WORD_FILES_LOCATION.md | Adds debugging guide for missing-location-on-map scenario. |
| .gitignore | Ignores runtime-* logs/artifacts generated by new scripts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const results = await response.json(); | ||
|
|
||
| if (!results || results.length === 0) { | ||
| dropdown.innerHTML = '<div class="location-dropdown-item" disabled style="color: var(--sl-muted);">No locations found for "' + query + '"</div>'; | ||
| dropdown.hidden = false; | ||
| if (statusEl) statusEl.innerHTML = '<span class="location-status-error">No locations found. Try a different search term.</span>'; | ||
| return; |
There was a problem hiding this comment.
The "no locations found" message injects the raw query string into innerHTML without escaping. Since query comes directly from user input, this is an XSS vector (e.g., a crafted string with HTML). Escape/sanitize the query (or use textContent) when rendering this message and any other UI that interpolates user-provided search text.
| const latInput = document.getElementById(`${formType === 'classic' ? 'classicLatitude' : 'InteractiveLatitude'}`); | ||
| const lonInput = document.getElementById(`${formType === 'classic' ? 'classicLongitude' : 'InteractiveLongitude'}`); | ||
| const siteLocationInput = document.getElementById(`${formType === 'classic' ? 'classicSiteLocation' : 'InteractiveSiteLocation'}`); | ||
| const searchInput = document.getElementById(`${formType === 'classic' ? 'classicLocationSearch' : 'InteractiveLocationSearch'}`); | ||
| const statusEl = document.getElementById(`${formType}LocationStatus`); | ||
|
|
||
| // Hidden inputs for actual form submission | ||
| const hiddenLat = document.querySelector(`input[name="Latitude"]`); | ||
| const hiddenLon = document.querySelector(`input[name="Longitude"]`); | ||
| const hiddenSiteLocation = document.querySelector(`input[name="SiteLocation"]`); | ||
|
|
||
| if (hiddenLat && hiddenLon) { | ||
| hiddenLat.value = lat; | ||
| hiddenLon.value = lon; | ||
| } |
There was a problem hiding this comment.
selectLocation updates hidden form fields via document.querySelector('input[name="Latitude"]') / Longitude / SiteLocation, which always returns the first matching input on the page. Since StorageLocations.cshtml renders both classic and interactive hidden inputs with the same name, selecting a location in the interactive form will write coordinates into the classic form fields (and vice versa). Scope the lookup to the active form (or use the already-resolved element IDs like classicLatitude / InteractiveLatitude) so each mode updates its own hidden inputs reliably.
| item.innerHTML = ` | ||
| <div class="location-result"> | ||
| <span class="location-result-icon">📍</span> | ||
| <div class="location-result-details"> | ||
| <div class="location-result-name">${this.escapeHtml(result.name)}</div> | ||
| <div class="location-result-country">${this.escapeHtml(result.address?.country || '')}</div> |
There was a problem hiding this comment.
Nominatim search responses typically provide display_name rather than a name field. Using result.name for the dropdown label / selected value can produce blank labels and an empty SiteLocation when name is missing. Prefer result.display_name (or fallback: result.name ?? result.display_name) consistently when rendering and when populating SiteLocation / search input.
| button.dark-mode .location-dropdown { | ||
| background: #1a2a42; | ||
| border-color: rgba(108, 140, 196, 0.34); | ||
| box-shadow: 0 4px 12px rgba(0, 0, 0, 0.45); | ||
| } |
There was a problem hiding this comment.
Dark-mode styling for the dropdown is keyed off button.dark-mode, but the app applies the dark-mode class to body (see site.js). As written, the dropdown background/border won't switch in dark mode. Update the selector to target body.dark-mode .location-dropdown (consistent with the rest of the file).
| var groupedByLocation = files | ||
| .GroupBy(file => string.IsNullOrWhiteSpace(file.LocationName) ? "Unassigned" : file.LocationName) | ||
| .ToDictionary(group => group.Key, StringComparer.OrdinalIgnoreCase); | ||
|
|
There was a problem hiding this comment.
GroupBy(...).ToDictionary(group => group.Key, StringComparer.OrdinalIgnoreCase) can throw if there are two LocationName values that differ only by case (e.g., "NAS" vs "nas"). GroupBy is currently case-sensitive, but the dictionary is case-insensitive, so duplicate keys will crash the page. Use a case-insensitive GroupBy/normalization before building the dictionary (or avoid the dictionary entirely).
| [HttpPost("workflows/{id:guid}/compensate")] | ||
| public async Task<IActionResult> Compensate(Guid id, CancellationToken cancellationToken) | ||
| { | ||
| var workflow = await workflowService.GetWorkflowAsync(id, cancellationToken); | ||
| if (workflow is null) | ||
| { | ||
| return NotFound(); | ||
| } | ||
|
|
||
| var instance = new Models.WorkflowInstance | ||
| { | ||
| Id = workflow.Id, | ||
| ScanJobId = workflow.ScanJobId, | ||
| CorrelationId = workflow.CorrelationId, | ||
| StorageLocationId = workflow.StorageLocationId, | ||
| RootPath = workflow.RootPath, | ||
| Status = workflow.Status, | ||
| CreatedAtUtc = workflow.CreatedAtUtc, | ||
| Steps = workflow.Steps | ||
| .Select(s => new Models.WorkflowStepExecution | ||
| { | ||
| Id = Guid.NewGuid(), | ||
| WorkflowInstanceId = workflow.Id, | ||
| StepName = s.StepName, | ||
| Status = s.Status, | ||
| StartedAtUtc = s.StartedAtUtc, | ||
| CompletedAtUtc = s.CompletedAtUtc, | ||
| Notes = s.Notes | ||
| }) | ||
| .ToList() | ||
| }; | ||
|
|
||
| await compensator.CompensateAsync(instance, cancellationToken); | ||
| return Accepted(); | ||
| } |
There was a problem hiding this comment.
The operator-triggered compensation endpoint can be called for any workflow returned by GetWorkflowAsync, including Running/Completed workflows. Since compensation issues DELETEs to downstream services, this allows wiping data for healthy workflows. Consider restricting compensation to failed/cancelled workflows (and returning 409/400 otherwise) and tightening authorization (e.g., role/policy) given the destructive nature of this operation.
| // ── Geolocation Handler ────────────────────────────────────── | ||
| const initGeolocationButtons = () => { | ||
| const classicBtn = document.getElementById('classicGeoBtn'); | ||
| const interactiveBtn = document.getElementById('interactiveGeoBtn'); | ||
|
|
||
| const handleGeolocation = (latInputId, lngInputId, statusContainerId) => { | ||
| const latInput = document.getElementById(latInputId); | ||
| const lngInput = document.getElementById(lngInputId); | ||
| const statusContainer = document.getElementById(statusContainerId); | ||
|
|
||
| if (!latInput || !lngInput) return; | ||
|
|
||
| if (!navigator.geolocation) { | ||
| showGeoError(statusContainer, 'Geolocation is not supported by this browser.'); | ||
| return; | ||
| } | ||
|
|
||
| showGeoLoading(statusContainer, 'Requesting location...'); | ||
|
|
||
| navigator.geolocation.getCurrentPosition( | ||
| (position) => { | ||
| const lat = position.coords.latitude.toFixed(6); | ||
| const lng = position.coords.longitude.toFixed(6); | ||
| const accuracy = Math.round(position.coords.accuracy); | ||
|
|
||
| latInput.value = lat; | ||
| lngInput.value = lng; | ||
|
|
||
| showGeoSuccess(statusContainer, `Location acquired: ${lat}, ${lng} (±${accuracy}m)`); | ||
| }, | ||
| (error) => { | ||
| let message = 'Unable to retrieve your location.'; | ||
| if (error.code === error.PERMISSION_DENIED) { | ||
| message = 'Location permission denied. Enable it in browser settings to use this feature.'; | ||
| } else if (error.code === error.POSITION_UNAVAILABLE) { | ||
| message = 'Your device location is unavailable.'; | ||
| } else if (error.code === error.TIMEOUT) { | ||
| message = 'Location request timed out. Please try again.'; | ||
| } | ||
| showGeoError(statusContainer, message); | ||
| }, | ||
| { timeout: 10000, enableHighAccuracy: false } | ||
| ); | ||
| }; | ||
|
|
||
| if (classicBtn) { | ||
| classicBtn.addEventListener('click', (e) => { | ||
| e.preventDefault(); | ||
| handleGeolocation('classicLatitude', 'classicLongitude', 'classicGeoStatus'); | ||
| }); | ||
| } | ||
|
|
||
| if (interactiveBtn) { | ||
| interactiveBtn.addEventListener('click', (e) => { | ||
| e.preventDefault(); | ||
| handleGeolocation('InteractiveLatitude', 'InteractiveLongitude', 'interactiveGeoStatus'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
This page includes an inline geolocation implementation that attaches click handlers to classicGeoBtn / interactiveGeoBtn, while locations-geocoding.js also attaches handlers to the same buttons. This will cause duplicate geolocation requests and competing updates to the hidden coordinate inputs. Additionally, the inline handler writes status to classicGeoStatus / interactiveGeoStatus, but those elements are not present in the markup, so status updates never render. Consolidate geolocation into a single implementation (preferably locations-geocoding.js) and remove/fix the unused status IDs to avoid conflicting behavior.
| private static async Task EnsureRelationalColumnsForCurrentModelAsync( | ||
| DbContext dbContext, | ||
| ILogger logger, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| if (!dbContext.Database.ProviderName?.Contains("SqlServer", StringComparison.OrdinalIgnoreCase) == true) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The provider guard if (!dbContext.Database.ProviderName?.Contains("SqlServer", ...) == true) behaves incorrectly if ProviderName is null (it will proceed instead of returning). Use a clearer null-safe check like if (dbContext.Database.ProviderName?.Contains(... ) != true) return; to avoid accidentally running SQL Server-specific DDL against an unknown provider.
| private static async Task MigrateRelationalWithFallbackAsync( | ||
| DbContext dbContext, | ||
| ILogger logger, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| try | ||
| { | ||
| await dbContext.Database.MigrateAsync(cancellationToken); | ||
| } | ||
| catch (Exception ex) when (LooksLikeLegacyEnsureCreatedDatabase(ex)) | ||
| { | ||
| logger.LogWarning( | ||
| ex, | ||
| "Detected legacy relational schema state. Reconciling missing model columns before continuing."); | ||
| } | ||
|
|
||
| await EnsureRelationalColumnsForCurrentModelAsync(dbContext, logger, cancellationToken); | ||
| } |
There was a problem hiding this comment.
When MigrateAsync fails due to a legacy EnsureCreated-style schema, the code logs and then only adds missing columns. It never retries migrations afterward, so any schema changes beyond columns (indexes, constraints, new tables, etc.) may remain unapplied while the service continues startup. Consider retrying MigrateAsync after reconciling columns (or explicitly documenting/handling which migration operations are supported by this fallback).
This pull request delivers a major enhancement to the StorageLens Storage Locations feature, focusing on a user-friendly, multi-method location input system and robust backend improvements. The new system enables users to add storage locations via search, browser geolocation, or manual entry, with significant UI/UX, validation, and accessibility upgrades. Additionally, the backend introduces a repository abstraction layer for all microservices and implements the Saga pattern with compensating transactions to ensure data consistency and recoverability. Comprehensive debugging documentation and implementation summaries have been added for maintainability and support.
Frontend & User Experience Improvements:
locations-search.cssandlocations-geocoding.jsfor styling and geocoding logic, and updated form layouts and validation to require coordinates from any method, making latitude/longitude knowledge unnecessary for users.Backend & Architecture Enhancements:
ILocationRepository,IScanJobRepository, etc.), updated service constructors, and registration in dependency injection.Testing, Documentation & Debugging Support:
LOCATION_INPUT_FEATURE.md, a detailed implementation summary, and a new debugging guide (DEBUG_DD_WORD_FILES_LOCATION.md) for diagnosing missing locations on the map. [1] [2]These changes collectively resolve the "DD Word Files" scenario, enhance maintainability, and provide a robust, user-friendly experience for managing storage locations.
References:
[1] [2] [3]