Skip to content

feat(reporting-api): add PUT review endpoint and wire ReportStatus.Reviewed#303

Merged
willvelida merged 3 commits intomainfrom
feat/reporting-api-review-persistence
Apr 20, 2026
Merged

feat(reporting-api): add PUT review endpoint and wire ReportStatus.Reviewed#303
willvelida merged 3 commits intomainfrom
feat/reporting-api-review-persistence

Conversation

@willvelida
Copy link
Copy Markdown
Owner

Summary

Adds durable persistence for report review results in Reporting.Api (Phase 2 of the ReportReviewerService fail-closed fix, Issue #269). After this change, the PUT /api/reports/{jobId}/review endpoint transitions reports from GeneratedReviewed status with review metadata persisted to blob storage.

Changes

Model

  • Added 4 nullable review fields to ReportMetadata: ReviewedAt, ReviewApproved, ReviewConcerns, ReviewValidatedSummary
  • All fields use [JsonPropertyName] with camelCase convention, nullable for backward compatibility

Service Layer

  • Added UpdateReviewResultAsync to IBlobStorageService interface
  • Implemented in BlobStorageService following the existing UpdateJobStatusAsync pattern (get → guard status → mutate → dual-write)
  • Status guard rejects non-"generated" reports with InvalidOperationException

Endpoint

  • Created PUT /api/reports/{jobId}/review in ReviewEndpoints.cs
  • Returns 204 No Content on success, 404 Not Found, 409 Conflict, 400 Bad Request
  • Uses ChatApiAgent authorization policy (same as all other endpoints)
  • SubmitReviewRequest POCO defined in same file (matches GenerateReportRequest pattern)

Telemetry

  • Added ReportsReviewed counter to ReportingTelemetry (auto-exported to Application Insights via existing OpenTelemetry pipeline)

Tests

  • 4 unit tests in ReviewEndpointsShould.cs (handler logic in isolation)
  • 4 default-value tests in ReportMetadataShould.cs for new fields
  • 6 contract tests in SubmitReviewTests.cs (HTTP pipeline via WebApplicationFactory)
  • 1 authorization test added to AuthorizationPolicyShould.cs

Validation

  • 209 tests pass (0 failures)
  • 70.2% line coverage (threshold: 60%)
  • All new code paths covered: SubmitReviewRequest 100%, ReportMetadata 100%, ReportingTelemetry 100%

Context

…viewed

- add 4 nullable review fields to ReportMetadata model

- add UpdateReviewResultAsync to IBlobStorageService and BlobStorageService

- create PUT /api/reports/{jobId}/review endpoint (204 No Content)

- add ReportsReviewed OTel counter to ReportingTelemetry

- add unit tests and contract tests for review endpoint

Signed-off-by: Will Velida <willvelida@hotmail.co.uk>
agent: github-copilot
model: Claude Opus 4.6
contribution: code-generation
Copilot AI review requested due to automatic review settings April 20, 2026 04:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds durable persistence of report review results to Biotrackr.Reporting.Api by introducing a new PUT /api/reports/{jobId}/review endpoint that transitions reports from generatedreviewed and writes review metadata to blob storage, plus telemetry and tests to support the new flow.

Changes:

  • Extended ReportMetadata with nullable review fields and wired ReportStatus.Reviewed.
  • Added IBlobStorageService.UpdateReviewResultAsync + BlobStorageService implementation to persist review results.
  • Introduced ReviewEndpoints PUT handler, a ReportsReviewed metric, and unit/contract/auth tests.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api/Telemetry/ReportingTelemetry.cs Adds ReportsReviewed counter for review submissions.
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api/Services/IBlobStorageService.cs Extends blob service contract with UpdateReviewResultAsync.
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api/Services/BlobStorageService.cs Implements review persistence + status transition to Reviewed.
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api/Program.cs Wires MapReviewEndpoints() into the app pipeline.
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api/Models/ReportMetadata.cs Adds persisted review fields (ReviewedAt, approval, concerns, validated summary).
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api/Endpoints/ReviewEndpoints.cs New PUT endpoint handler for submitting review results.
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api.UnitTests/Models/ReportMetadataShould.cs Adds default/null tests for new metadata fields.
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api.UnitTests/Endpoints/ReviewEndpointsShould.cs Unit-tests handler responses for success/not found/conflict/bad request.
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api.IntegrationTests/Contract/SubmitReviewTests.cs Contract tests validating routing and status-code mapping for the new endpoint.
src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api.IntegrationTests/Contract/AuthorizationPolicyShould.cs Verifies the new endpoint is protected by the expected authorization policy.

Comment thread src/Biotrackr.Reporting.Api/Biotrackr.Reporting.Api/Endpoints/ReviewEndpoints.cs Outdated
- add null-coalesce for Concerns and ValidatedSummary in endpoint handler

- use KeyNotFoundException for job-not-found instead of message matching

- add ArgumentNullException.ThrowIfNull guards in BlobStorageService

Signed-off-by: Will Velida <willvelida@hotmail.co.uk>
agent: github-copilot
model: Claude Opus 4.6
contribution: code-generation
- block background Task.Run via UpdateJobStatusAsync mock to ensure semaphore is held when second call executes

Signed-off-by: Will Velida <willvelida@hotmail.co.uk>
agent: github-copilot
model: Claude Opus 4.6
contribution: code-generation
@github-actions
Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Health
Biotrackr.Reporting.Api 70% 46%
Summary 70% (493 / 700) 46% (102 / 221)

Minimum allowed line rate is 60%

@willvelida willvelida merged commit 8a6267e into main Apr 20, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants