LEGLINK-697: Create Separate DMRP C# Project & Module Structure - #1776
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdded a complete DMRP .NET 8 service with persistence, CRUD APIs, SDK integration, tests, container configuration, and Azure DevOps deployment pipelines. ChangesDMRP service
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant DMRPController
participant DMRPManager
participant DmrpDbContext
Client->>DMRPController: Submit CRUD request
DMRPController->>DMRPManager: Invoke operation
DMRPManager->>DmrpDbContext: Persist or retrieve entity
DmrpDbContext-->>DMRPManager: Return entity or result
DMRPManager-->>DMRPController: Return operation result
DMRPController-->>Client: Return HTTP response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
DotNet/DMRP/Program.cs (1)
127-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicate
FromLogContextenricher.
.Enrich.FromLogContext()is called on line 132 and again on line 135. One call is sufficient.♻️ Proposed fix
.Enrich.WithSpan() .Enrich.With<ActivityEnricher>() - .Enrich.FromLogContext() .CreateLogger();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNet/DMRP/Program.cs` around lines 127 - 136, Remove the duplicate Enrich.FromLogContext() call from the LoggerConfiguration chain, keeping one invocation while preserving the existing logger enrichers and filters.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Azure_Pipelines/azure-pipelines.dmrp.cd.yaml`:
- Around line 87-99: Update the Docker@2 task labeled “Build & Push DMRP Docker
Image” to remove the unconditional always() condition and require successful
completion of the preceding validation and tag-generation tasks before building
or pushing either image tag.
In `@DotNet/DMRP/Business/Managers/FacilityReportingPlanManager.cs`:
- Around line 36-40: Preserve cancellation by adding a filtered
OperationCanceledException catch before each general Exception handler in
CreateAsync, UpdateAsync, and DeleteAsync of FacilityReportingPlanManager.cs and
MeasureMappingManager.cs, rethrowing the cancellation unchanged; retain the
existing ApplicationException wrapping for other exceptions at all six sites.
- Line 53: Sanitize caller-provided id values before every affected ILogger call
in FacilityReportingPlanManager.cs at lines 53-53 and 77-77, and
MeasureMappingManager.cs at lines 53-53 and 77-77. Apply the same established
identifier-sanitization approach consistently at all four logging sites before
passing id to LogError.
- Around line 50-60: Update the update flow in FacilityReportingPlanManager.cs
at lines 50-60 by copying the permitted fields from the facilityReportingPlan
request model onto existing before calling Update and SaveChangesAsync. Apply
the same change in MeasureMappingManager.cs at lines 50-60, copying permitted
measureMapping fields onto existing; preserve the existing not-found and save
behavior.
In `@DotNet/DMRP/Business/Queries/FacilityReportingPlanQueries.cs`:
- Around line 38-63: Bound pageSize before pagination in both PagedSearchAsync
implementations: DotNet/DMRP/Business/Queries/FacilityReportingPlanQueries.cs
lines 38-63 and DotNet/DMRP/Business/Queries/MeasureMappingQueries.cs lines
38-63. Reject invalid values or normalize pageSize consistently before using it
in Skip, Take, and PaginationMetadata.TotalPages, while preserving the existing
pageNumber normalization behavior.
In `@DotNet/DMRP/Controllers/FacilityReportingPlansController.cs`:
- Around line 40-53: Clamp pageSize to the supported range before invoking
PagedSearchAsync in GetFacilityReportingPlans within
DotNet/DMRP/Controllers/FacilityReportingPlansController.cs#L40-L53. Apply the
same pageSize bounds in the corresponding handler in
DotNet/DMRP/Controllers/MeasureMappingsController.cs#L40-L53, preserving the
existing pageNumber normalization and query flow.
- Around line 91-117: Map validated request fields onto the entities before
manager calls: in DotNet/DMRP/Controllers/FacilityReportingPlansController.cs
lines 91-117 and DotNet/DMRP/Controllers/MeasureMappingsController.cs lines
91-117, update CreateFacilityReportingPlan and the corresponding MeasureMappings
POST handler to create from the mapped request and return the created
representation; in DotNet/DMRP/Controllers/FacilityReportingPlansController.cs
lines 127-153 and DotNet/DMRP/Controllers/MeasureMappingsController.cs lines
127-153, reject null requests with 400 and pass mapped entities with the
requested ID to UpdateAsync instead of new ID-only entities.
In `@DotNet/DMRP/Dockerfile`:
- Line 3: Update the Dockerfile’s base image reference to an explicit version
tag or digest for reproducible builds, and in the final stage after `FROM base
AS final`, add `USER 1001` unless the pinned runtime image already declares a
non-root user.
In `@DotNet/DMRP/packages.lock.json`:
- Around line 947-950: Update the centralized package versions in
Directory.Packages.props for SQLitePCLRaw.lib.e_sqlite3, AutoMapper, and
Microsoft.EntityFrameworkCore.Sqlite to patched releases, then regenerate or
update every affected packages.lock.json so no vulnerable versions remain.
In `@DotNet/DMRP/Program.cs`:
- Line 92: Update the client-facing ProblemDetails.Detail message in the
error-handling flow to correct “occured” to “occurred” and “assistence” to
“assistance,” preserving the rest of the message unchanged.
In `@DotNet/LinkSdk/Clients/DmrpServiceClient.cs`:
- Around line 11-21: The DmrpServiceClient currently creates its own HTTP client
instead of using the SDK’s configured factory. Update DmrpServiceClient and its
LinkApiClientBase integration to accept and use the compatible request-header
abstraction, then update ServiceCollectionExtensions registration to use
AddHttpClient<IDmrpServiceClient, DmrpServiceClient>() and configure the
required request headers through the typed client.
In `@DotNet/LinkSdk/Clients/Interfaces/IDmrpServiceClient.cs`:
- Around line 12-18: The DMRP search methods currently discard their typed paged
payloads. Update SearchMeasureMappingsAsync and
SearchFacilityReportingPlansAsync in
DotNet/LinkSdk/Clients/Interfaces/IDmrpServiceClient.cs (lines 12-18) and the
corresponding implementations in DotNet/LinkSdk/Clients/DmrpServiceClient.cs
(lines 50-57 and 84-91) to return LinkApiResponse<PagedMeasureMappingDto> and
LinkApiResponse<PagedFacilityReportingPlanDto>, respectively, using reusable
paged SDK response types while preserving the existing request and pagination
behavior.
In `@DotNet/Shared/Application/Models/Configs/ServiceRegistry.cs`:
- Around line 106-115: Add focused XUnit tests in ServiceRegistryTests for both
DmrpServiceApiUrl and PublicDmrpServiceApiUrl, covering null or empty service
URLs, URLs without a trailing slash, and URLs with trailing slashes. Assert
missing values produce the expected null result, non-trailing URLs append
“/api”, and trailing slashes are normalized before appending “/api”.
---
Nitpick comments:
In `@DotNet/DMRP/Program.cs`:
- Around line 127-136: Remove the duplicate Enrich.FromLogContext() call from
the LoggerConfiguration chain, keeping one invocation while preserving the
existing logger enrichers and filters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9eb71ce8-c60d-4e18-8d95-44a1a5d3c268
📒 Files selected for processing (49)
Azure_Pipelines/_deploy_all_services.ymlAzure_Pipelines/azure-pipelines.dmrp.cd.yamlAzure_Pipelines/deploy_tags_all.ymlDotNet/DMRP/Business/Managers/FacilityReportingPlanManager.csDotNet/DMRP/Business/Managers/MeasureMappingManager.csDotNet/DMRP/Business/Queries/FacilityReportingPlanQueries.csDotNet/DMRP/Business/Queries/MeasureMappingQueries.csDotNet/DMRP/Config/DmrpConstants.csDotNet/DMRP/Controllers/FacilityReportingPlansController.csDotNet/DMRP/Controllers/MeasureMappingsController.csDotNet/DMRP/DMRP.csprojDotNet/DMRP/Data/Entities/FacilityReportingPlan.csDotNet/DMRP/Data/Entities/MeasureMapping.csDotNet/DMRP/Data/Repository/DmrpDbContext.csDotNet/DMRP/Data/Repository/FacilityReportingPlanRepository.csDotNet/DMRP/Data/Repository/Mappings/FacilityReportingPlanConfigMap.csDotNet/DMRP/Data/Repository/Mappings/MeasureMappingConfigMap.csDotNet/DMRP/Data/Repository/MeasureMappingRepository.csDotNet/DMRP/DockerfileDotNet/DMRP/Migrations/20260803165942_InitialCreate.Designer.csDotNet/DMRP/Migrations/20260803165942_InitialCreate.csDotNet/DMRP/Migrations/DmrpDbContextModelSnapshot.csDotNet/DMRP/Models/PagedFacilityReportingPlanDto.csDotNet/DMRP/Models/PagedMeasureMappingDto.csDotNet/DMRP/Program.csDotNet/DMRP/Properties/launchSettings.jsonDotNet/DMRP/Services/DatabaseHealthCheck.csDotNet/DMRP/appsettings.Development.jsonDotNet/DMRP/appsettings.Docker.jsonDotNet/DMRP/appsettings.jsonDotNet/DMRP/packages.lock.jsonDotNet/LinkSdk/Clients/DmrpServiceClient.csDotNet/LinkSdk/Clients/Interfaces/IDmrpServiceClient.csDotNet/LinkSdk/DependencyInjection/ServiceCollectionExtensions.csDotNet/ServiceTests/IntegrationTests/DMRP/DmrpIntegrationTestFixture.csDotNet/ServiceTests/IntegrationTests/DMRP/FacilityReportingPlansControllerTests.csDotNet/ServiceTests/IntegrationTests/DMRP/MeasureMappingsControllerTests.csDotNet/ServiceTests/IntegrationTests/IntegrationTestCollection.csDotNet/ServiceTests/ServiceTests.csprojDotNet/ServiceTests/UnitTests/DMRP/FacilityReportingPlanManagerTests.csDotNet/ServiceTests/UnitTests/DMRP/FacilityReportingPlanQueriesTests.csDotNet/ServiceTests/UnitTests/DMRP/MeasureMappingManagerTests.csDotNet/ServiceTests/UnitTests/DMRP/MeasureMappingQueriesTests.csDotNet/ServiceTests/packages.lock.jsonDotNet/Shared/Application/Models/Configs/ServiceRegistry.csDotNet/Shared/Application/Models/Integration/DMRP/DMRPApiModels.csapp-config.yamldocker-compose.ymllink-cloud.sln
DMRP functionality will be integrated into Tenant rather than existing as a separate standalone service.
Such requests would have already been rejected during model binding.
This replaces the manual registration of repositories, managers, and queries, which could eventually drift from `AddDmrpModule`'s implementation.
johnbritton
approved these changes
Aug 6, 2026
This was referenced Aug 7, 2026
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
3 participants
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.
🛠️ Description of Changes
Created/scaffolded the DMRP module.
🧪 Testing Performed
Ran unit/integration tests locally. Confirmed healthy startup in Compose stack.
🧑🔬 Unit Testing
📓 Documentation Updated
Added setting to
app-config.yaml.