Wire domain service composition and enforce required talk IDs - #5
Merged
Conversation
- Move test project from src/ to tst/ folder - Move exception types into Exceptions/ folder - Move ITalkCatalogRepository into Interfaces/ folder - Move repository and service into Services/ folder - Rename PresentationFamilyReference -> PresentationFamily in Entities/ - Move ProposalCopyItem, RelatedContentItem into Entities/ - Rename TalkRecord -> Talk in Entities/ - Move TalkCatalog into Entities/ - Move PublicPresentationReference into Entities/ - Extract FileSystemTalkCatalogRepository and YAML DTOs into new TalkFolio.Data.YamlFile project - Extract repository tests into new TalkFolio.Data.YamlFile.Tests project - Add TalkCatalogService thin client wrapper in Services/ - Remove YamlDotNet and Extensions.Options from domain project (no longer needed) - Update solution file with new project structure - Update all namespaces to match new folder layout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename TalkFolio.Data.YamlFile repository and options types - Remove FileSystem and Yaml prefixes from data-layer DTOs - Rename repository log helper to a simpler form - Update API configuration binding and tests for the new names - Keep domain and data project boundaries intact Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move YAML serialization models into a Serialization folder - Keep repository implementation and options alongside functional data-layer code - Update repository to use the new serialization namespace Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Register TalkCatalogService in API DI and route /talks through the domain service - Fail fast when a talk file omits Id by throwing MissingTalkIdException - Add repository log event and regression test for missing Id - Update schema and ADR docs to clarify duplicate key is Title + PresentationFamily.Variant only Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The YAML repository implementation and its tests reference domain exceptions without importing the TalkFolio namespace, which will cause compilation failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors talk catalog access so the API composes a domain-level TalkCatalogService instead of injecting the YAML repository directly, while enforcing a required Id invariant during YAML loads (with a dedicated MissingTalkIdException) and updating tests and documentation to match the refined domain model and uniqueness rules.
Changes:
- Wire
TalkCatalogServiceinto API composition and update/talksto load through the domain service. - Enforce fail-fast validation for missing talk IDs during YAML catalog load (exception + error log) and add regression coverage.
- Move/rename domain model types and configuration keys (e.g.,
TalkRecord->Talk,TalkCatalogRepository:*->TalkCatalog:*), and update docs to clarify duplicate detection keys.
File summaries
| File | Description |
|---|---|
| tst/TalkFolio.Tests/TalksEndpoint_GetTalks_Should.cs | Updates API config key and response model type to Talk. |
| tst/TalkFolio.Tests/TalkFolio.Tests.csproj | Fixes project references after moving test projects under tst/. |
| tst/TalkFolio.Tests/TalkCatalogService_Should.cs | Adds unit test for new domain service (naming should align to conventions). |
| tst/TalkFolio.Data.YamlFile.Tests/TalkFolio.Data.YamlFile.Tests.csproj | Introduces dedicated test project for YAML repository. |
| tst/TalkFolio.Data.YamlFile.Tests/TalkCatalogRepository_LoadAsync_Should.cs | Expands repository test coverage, including missing-Id regression. |
| TalkFolio.slnx | Updates solution structure to include src/ and tst/ projects. |
| src/TalkFolio/TalkFolio.csproj | Removes YAML/options dependencies from core domain project. |
| src/TalkFolio/Services/TalkCatalogService.cs | Adds domain service wrapper over ITalkCatalogRepository. |
| src/TalkFolio/Interfaces/ITalkCatalogRepository.cs | Moves repository interface into TalkFolio.Interfaces and updates doc intent. |
| src/TalkFolio/Exceptions/TalkCatalogLoadException.cs | Adds domain base exception for catalog load failures. |
| src/TalkFolio/Exceptions/MissingTalkIdException.cs | Adds explicit exception for missing required talk Id. |
| src/TalkFolio/Exceptions/MalformedTalkYamlException.cs | Adds explicit exception for malformed YAML payloads. |
| src/TalkFolio/Exceptions/DuplicateTalkTitleVariantException.cs | Adds explicit exception for duplicate (Title, Variant) keys. |
| src/TalkFolio/Exceptions/DuplicateTalkIdException.cs | Adds explicit exception for duplicate talk Id keys. |
| src/TalkFolio/Entities/TalkCatalog.cs | Updates catalog entity to hold Talk instead of TalkRecord. |
| src/TalkFolio/Entities/Talk.cs | Renames canonical talk record type and updates presentation family type. |
| src/TalkFolio/Entities/RelatedContentItem.cs | Moves entity namespace to TalkFolio.Entities. |
| src/TalkFolio/Entities/PublicPresentationReference.cs | Moves entity namespace to TalkFolio.Entities. |
| src/TalkFolio/Entities/ProposalCopyItem.cs | Moves entity namespace to TalkFolio.Entities. |
| src/TalkFolio/Entities/PresentationFamily.cs | Renames and moves presentation family entity to TalkFolio.Entities. |
| src/TalkFolio.Data.YamlFile/TalkFolio.Data.YamlFile.csproj | Adds new YAML repository project with dependencies. |
| src/TalkFolio.Data.YamlFile/TalkCatalogRepositoryLog.cs | Renames log helper and adds missing-Id error logging. |
| src/TalkFolio.Data.YamlFile/TalkCatalogRepository.cs | Enforces required Id, updates mapping to domain entities, updates namespaces. |
| src/TalkFolio.Data.YamlFile/TalkCatalogOptions.cs | Renames options type and broadens doc wording. |
| src/TalkFolio.Data.YamlFile/Serialization/TalkRecord.cs | Renames/relocates YAML projection types for deserialization. |
| src/TalkFolio.Data.YamlFile/Serialization/RelatedContentItem.cs | Renames/relocates YAML projection type. |
| src/TalkFolio.Data.YamlFile/Serialization/PublicPresentationReference.cs | Renames/relocates YAML projection type. |
| src/TalkFolio.Data.YamlFile/Serialization/ProposalCopyItem.cs | Renames/relocates YAML projection type. |
| src/TalkFolio.Data.YamlFile/Serialization/PresentationFamilyReference.cs | Renames/relocates YAML projection type. |
| src/TalkFolio.Api/TalkFolio.Api.csproj | Adds reference to YAML repository project for composition. |
| src/TalkFolio.Api/Program.cs | Wires options, repository, and domain service; updates /talks to use service. |
| docs/TalkSchema.md | Updates schema examples/notes and documents duplicate-key behavior. |
| docs/ADRs.md | Clarifies uniqueness key and catalog-load exception expectations. |
| .vscode/settings.json | Adds terminal auto-approve entry for dotnet build. |
| .github/instructions/csharp.instructions.md | Removes unused alias example from guidance doc. |
Review details
Suppressed comments (2)
src/TalkFolio.Data.YamlFile/TalkCatalogRepository.cs:5
- This file references domain exceptions (e.g., MissingTalkIdException, MalformedTalkYamlException, DuplicateTalkIdException) but does not import the TalkFolio namespace, so it will not compile.
tst/TalkFolio.Data.YamlFile.Tests/TalkCatalogRepository_LoadAsync_Should.cs:5 - This test file asserts on domain exceptions (e.g., MalformedTalkYamlException, DuplicateTalkIdException, MissingTalkIdException) but does not import the TalkFolio namespace, so it will not compile.
- Files reviewed: 31/35 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are compile-blocking issues in the new/updated tests and YAML repository code (missing namespace imports and incorrect NSubstitute return type setup).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
src/TalkFolio.Data.YamlFile/TalkCatalogRepository.cs:5
- TalkCatalogRepository references domain exceptions (e.g., MissingTalkIdException, DuplicateTalkIdException) that live in the TalkFolio namespace, but this file does not import that namespace, so it will not compile.
tst/TalkFolio.Data.YamlFile.Tests/TalkCatalogRepository_LoadAsync_Should.cs:7 - This test file uses domain exceptions (e.g., MalformedTalkYamlException, MissingTalkIdException, DuplicateTalkIdException) from the TalkFolio namespace, but does not import it, so the test project will not compile.
tst/TalkFolio.Tests/TalkCatalogService_Should.cs:8 - Test file name should match the test class name per repository test conventions. Consider renaming this file to TalkCatalogService_LoadAsync_Should.cs to match the class it contains.
- Files reviewed: 31/35 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+13
to
+16
| var repository = Substitute.For<ITalkCatalogRepository>(); | ||
| var expected = new TalkCatalog([]); | ||
| repository.LoadAsync(Arg.Any<CancellationToken>()).Returns(expected); | ||
| var target = new TalkCatalogService(repository); |
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\n- register TalkCatalogService in API composition and have /talks use the domain service instead of injecting the repository directly\n- add fail-fast validation for missing Talk Id values by introducing MissingTalkIdException\n- add logging for missing required Id failures in the YAML repository\n- add regression test for missing Id behavior\n- update schema/ADR docs to clarify duplicate-talk uniqueness is Title + PresentationFamily.Variant only (and does not include PresentationFamily.Name)\n\n## Validation\n- dotnet test --nologo\n