chore: port still-relevant changes from PR #83 (close #83)#98
Merged
Conversation
Collapses the two near-duplicate `app.Use(...)` blocks for `/` redirect into a single `app.UseRootRedirect(devTarget, prodTarget)` call. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The interface had a single field (IsDevelopment) and a single concrete implementation. A record value is enough; drop the interface and the NSubstitute mock in tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Both auth paths registered the two API clients separately. A small local AddApiClient helper folds the four blocks into four calls, keeping the test-auth/prod differences (base address shape, auth handler) explicit at the call site. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nd into Common.ps1 deploy.ps1, update.ps1, and teardown.ps1 each defined their own copy of the same output helpers (with three different sets of glyphs). Move them into scripts\Common.ps1 and dot-source from each consumer. Glyphs unified on ASCII (+, x, !) since the helper file lives without BOM-anchored unicode. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Most of the file duplicated README, docs/development/docker-setup.md, and docs/deployment/getting-started.md. Strip down to: env table, config-file load order (the genuinely unique content), connection-string snippets, and a short troubleshooting list. Link out to the canonical docs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mcr.microsoft.com/mssql/server has no ARM64 image, so docker-compose up silently fails on Apple Silicon and Pi. Make the limitation explicit in README and docker-setup.md instead of leaving it implicit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Test Results2 tests 2 ✅ 6s ⏱️ Results for commit 62ed6a6. ♻️ This comment has been updated with latest results. |
SummarySummary
CoverageAHKFlowApp.API - 63.3%
AHKFlowApp.Application - 98.1%
AHKFlowApp.Domain - 100%
AHKFlowApp.Infrastructure - 100%
AHKFlowApp.TestUtilities - 57.5%
AHKFlowApp.UI.Blazor - 76%
Per-assembly thresholds: Domain line≥85% br≥70% · Application line≥85% br≥45% · Infrastructure line≥70% br≥50% · API line≥57% br≥50% · UI.Blazor line≥65% br≥28% |
Contributor
There was a problem hiding this comment.
Pull request overview
Ports a subset of still-relevant refactors and documentation/script cleanups from the previously opened PR #83, aligning backend environment handling, middleware setup, frontend HTTP client wiring, and PowerShell helper reuse.
Changes:
- Replace
IDevEnvironment/DevEnvironmentwith a simpleAppEnvironmentrecord and update consumers/tests. - Deduplicate root redirect middleware in the API via a
UseRootRedirectextension. - Deduplicate Blazor
AddHttpClientsetup via a shared local helper and centralize PowerShell output/prereq helpers inscripts/Common.ps1, plus docs reductions/clarifications.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/AHKFlowApp.Application.Tests/Hotstrings/SeedHotstringsCommandHandlerTests.cs | Updates tests to use AppEnvironment instead of an IDevEnvironment mock. |
| src/Frontend/AHKFlowApp.UI.Blazor/Program.cs | Introduces AddApiClient helper to DRY up HttpClient + resilience configuration across auth modes. |
| src/Backend/AHKFlowApp.Application/Commands/Dev/SeedHotstringsCommand.cs | Switches handler dependency from IDevEnvironment to AppEnvironment. |
| src/Backend/AHKFlowApp.Application/AppEnvironment.cs | Adds the new AppEnvironment record type. |
| src/Backend/AHKFlowApp.Application/Abstractions/IDevEnvironment.cs | Removes the no-longer-used IDevEnvironment abstraction. |
| src/Backend/AHKFlowApp.API/Program.cs | Registers AppEnvironment and replaces duplicated “/” redirect middleware with UseRootRedirect. |
| src/Backend/AHKFlowApp.API/Extensions/ApiExtensions.cs | Adds UseRootRedirect extension method for consistent root-path redirection. |
| src/Backend/AHKFlowApp.API/DevEnvironment.cs | Removes obsolete DevEnvironment implementation. |
| scripts/update.ps1 | Dot-sources Common.ps1 instead of duplicating output helpers. |
| scripts/teardown.ps1 | Dot-sources Common.ps1 instead of duplicating output helpers. |
| scripts/deploy.ps1 | Dot-sources Common.ps1 instead of duplicating output/prereq helpers. |
| scripts/Common.ps1 | New shared script helper module for consistent output/prereq checks. |
| docs/environments.md | Condenses environment documentation and points to canonical docs. |
| docs/development/docker-setup.md | Documents x64/amd64-only constraint due to SQL Server image limitations. |
| README.md | Adds the same x64/amd64-only note for Docker Compose setup. |
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.
Summary
Ports the changes from #83 that are still relevant given main has independently re-implemented most of the same plan. Six small, focused commits on top of main.
What's ported
refactor(api): collapse the two duplicateapp.Use(/)redirect blocks into aUseRootRedirectextension.refactor(api): collapseIDevEnvironment(single field, single concrete impl) into a publicAppEnvironmentrecord.refactor(blazor): dedupe the fourAddHttpClient<>blocks across the test-auth and prod paths via a localAddApiClienthelper.refactor(scripts): extract theWrite-Step/Write-Success/Write-Warn/Write-Fail/Confirm-Commandhelpers (duplicated acrossdeploy.ps1,update.ps1,teardown.ps1with three different glyph sets) intoscripts\Common.ps1. Glyphs unified on ASCII to avoid BOM-anchored unicode.docs: compressdocs/environments.mdfrom 280 lines to ~50 — the rest duplicated README, docker-setup, and getting-started.docs: classify ARM64 / Raspberry Pi as unsupported for the bundled stack (no SQL Server ARM64 image).What's skipped from #83 (already on main)
create-github-issues.ps1move (commit66fb99a)#Requires -Version 5.1across scriptsdocs/scripts/removal.NET 10SDK preflight indeploy.ps1(commit4319410)deploy-frontend.yml(commit19b4fbd)d5d0ea9)What's skipped from #83 (no longer applies)
AHKFlowApp.Domain.Tests— main added realHotstringentity tests there since refactor: simplify codebase and standardize scripts #83 was opened (commit8ae9a9f).Program.csintoConfigureObservability/Services/Middleware— current 250-line linear bootstrap is readable enough;UseRootRedirectalready removed the duplication that was the strongest argument for splitting.Test plan
dotnet build AHKFlowApp.slnx --configuration Release— 13 projects, 0 errors, 0 warningsdotnet test AHKFlowApp.slnx --configuration Release— 155 tests, 0 faileddotnet format --verify-no-changesCommon.ps1,deploy.ps1,update.ps1,teardown.ps1Common.ps1smoke-test —. Common.ps1; Write-Step ...; Write-Success ...; Write-Warn ...produces expected outputdocker compose up --buildand verifyhttp://localhost:5600/redirects to/health,http://localhost:5601loads as Local Userdeploy.ps1 -SkipPrereqCheckin dry-run to confirm output helpers still workAfter merge: close #83 with a comment pointing to this PR; delete
feature/plan-c-script-standardization.