Skip to content

Commit 5452427

Browse files
arianamihailescuarianamihailescu
committed
LEGLINK-956: Return proper ProblemDetails payloads from Reporting Plan APIs
Two fixes, both needed together: - Tenant/Program.cs: CustomizeProblemDetails was unconditionally overwriting ProblemDetails.Detail on every response, discarding any specific message a controller had set. Now only falls back to the generic text for a 500 (which may carry raw exception text) or a genuinely empty detail, matching the pattern Terminology and MockDmrpApi already use. - FacilityReportingPlansController: every hand-written BadRequest/ NotFound/Conflict call returned a bare string instead of ProblemDetails. Converted them all to Problem(...) via two new helpers (BadRequestProblem/NotFoundProblem), so callers get a structured type/title/status/detail/traceId body instead of raw text. Verified against a locally running instance of the service against all 5 test cases from the ticket. Claude-Session: https://claude.ai/code/session_01LVVXQjfgjXC4WwHtciJYEJ
1 parent c3ce5ef commit 5452427

3 files changed

Lines changed: 99 additions & 43 deletions

File tree

‎DotNet/DMRP/Controllers/FacilityReportingPlansController.cs‎

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public FacilityReportingPlansController(ILogger<FacilityReportingPlansController
5353
/// Get a paged list of facility reporting plans.
5454
/// </summary>
5555
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(PagedFacilityReportingPlanDto))]
56-
[ProducesResponseType(StatusCodes.Status400BadRequest)]
56+
[ProducesResponseType(StatusCodes.Status400BadRequest, Type = typeof(ProblemDetails))]
5757
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
5858
[HttpGet(Name = "GetFacilityReportingPlans")]
5959
public Task<IActionResult> GetFacilityReportingPlans(string? sortBy, SortOrder? sortOrder,
@@ -66,7 +66,7 @@ public Task<IActionResult> GetFacilityReportingPlans(string? sortBy, SortOrder?
6666
/// period and reporting state.
6767
/// </summary>
6868
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(PagedFacilityReportingPlanDto))]
69-
[ProducesResponseType(StatusCodes.Status400BadRequest)]
69+
[ProducesResponseType(StatusCodes.Status400BadRequest, Type = typeof(ProblemDetails))]
7070
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
7171
[HttpGet("search", Name = "SearchFacilityReportingPlans")]
7272
public async Task<IActionResult> SearchFacilityReportingPlans([FromQuery] FacilityReportingPlanSearchFilters filters,
@@ -81,18 +81,18 @@ public async Task<IActionResult> SearchFacilityReportingPlans([FromQuery] Facili
8181
var periodError = ValidatePeriodFilters(filters.Month, filters.Year);
8282
if (periodError is not null)
8383
{
84-
return BadRequest(periodError);
84+
return BadRequestProblem(periodError);
8585
}
8686

8787
if (sortBy is not null && !SortableColumns.Contains(sortBy))
8888
{
89-
return BadRequest($"Cannot sort by '{sortBy}'.");
89+
return BadRequestProblem($"Cannot sort by '{sortBy}'.");
9090
}
9191

9292
var pagingError = ValidatePaging(pageSize, pageNumber);
9393
if (pagingError is not null)
9494
{
95-
return BadRequest(pagingError);
95+
return BadRequestProblem(pagingError);
9696
}
9797

9898
using Activity? activity = ServiceActivitySource.Instance.StartActivity("Search Facility Reporting Plans");
@@ -109,7 +109,7 @@ public async Task<IActionResult> SearchFacilityReportingPlans([FromQuery] Facili
109109
/// reporting state.
110110
/// </summary>
111111
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List<FacilityReportingPlanModel>))]
112-
[ProducesResponseType(StatusCodes.Status400BadRequest)]
112+
[ProducesResponseType(StatusCodes.Status400BadRequest, Type = typeof(ProblemDetails))]
113113
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
114114
[HttpGet("facilities/{facilityId}")]
115115
public async Task<IActionResult> GetFacilityReportingPlansForFacility(string facilityId, int? month, int? year,
@@ -120,7 +120,7 @@ public async Task<IActionResult> GetFacilityReportingPlansForFacility(string fac
120120
var periodError = ValidatePeriodFilters(month, year);
121121
if (periodError is not null)
122122
{
123-
return BadRequest(periodError);
123+
return BadRequestProblem(periodError);
124124
}
125125

126126
using Activity? activity = ServiceActivitySource.Instance.StartActivity("Get Facility Reporting Plans For Facility");
@@ -134,7 +134,7 @@ public async Task<IActionResult> GetFacilityReportingPlansForFacility(string fac
134134
/// Gets a facility reporting plan by Id.
135135
/// </summary>
136136
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(FacilityReportingPlanModel))]
137-
[ProducesResponseType(StatusCodes.Status404NotFound)]
137+
[ProducesResponseType(StatusCodes.Status404NotFound, Type = typeof(ProblemDetails))]
138138
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
139139
[HttpGet("{id}")]
140140
public async Task<IActionResult> GetFacilityReportingPlan(string id, CancellationToken cancellationToken)
@@ -145,7 +145,7 @@ public async Task<IActionResult> GetFacilityReportingPlan(string id, Cancellatio
145145

146146
if (model == null)
147147
{
148-
return NotFound();
148+
return NotFoundProblem($"Facility reporting plan with Id: {id} not found.");
149149
}
150150

151151
return Ok(model);
@@ -155,8 +155,8 @@ public async Task<IActionResult> GetFacilityReportingPlan(string id, Cancellatio
155155
/// Creates a facility reporting plan.
156156
/// </summary>
157157
[ProducesResponseType(StatusCodes.Status201Created, Type = typeof(FacilityReportingPlanModel))]
158-
[ProducesResponseType(StatusCodes.Status400BadRequest)]
159-
[ProducesResponseType(StatusCodes.Status409Conflict)]
158+
[ProducesResponseType(StatusCodes.Status400BadRequest, Type = typeof(ProblemDetails))]
159+
[ProducesResponseType(StatusCodes.Status409Conflict, Type = typeof(ProblemDetails))]
160160
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
161161
[HttpPost]
162162
public async Task<IActionResult> CreateFacilityReportingPlan(FacilityReportingPlanRequest request, CancellationToken cancellationToken)
@@ -169,11 +169,11 @@ public async Task<IActionResult> CreateFacilityReportingPlan(FacilityReportingPl
169169
}
170170
catch (DuplicateReportingPlanException ex)
171171
{
172-
return Conflict(ex.Message);
172+
return Problem(ex.Message, statusCode: StatusCodes.Status409Conflict, title: "Conflict");
173173
}
174174
catch (ReportingPlanValidationException ex)
175175
{
176-
return BadRequest(ex.Message);
176+
return BadRequestProblem(ex.Message);
177177
}
178178
catch (Exception ex)
179179
{
@@ -190,9 +190,9 @@ public async Task<IActionResult> CreateFacilityReportingPlan(FacilityReportingPl
190190
/// Updates a facility reporting plan.
191191
/// </summary>
192192
[ProducesResponseType(StatusCodes.Status202Accepted, Type = typeof(FacilityReportingPlanModel))]
193-
[ProducesResponseType(StatusCodes.Status400BadRequest)]
194-
[ProducesResponseType(StatusCodes.Status404NotFound)]
195-
[ProducesResponseType(StatusCodes.Status409Conflict)]
193+
[ProducesResponseType(StatusCodes.Status400BadRequest, Type = typeof(ProblemDetails))]
194+
[ProducesResponseType(StatusCodes.Status404NotFound, Type = typeof(ProblemDetails))]
195+
[ProducesResponseType(StatusCodes.Status409Conflict, Type = typeof(ProblemDetails))]
196196
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
197197
[HttpPut("{id}")]
198198
public async Task<IActionResult> UpdateFacilityReportingPlan(string id, FacilityReportingPlanUpdateRequest request, CancellationToken cancellationToken)
@@ -203,12 +203,12 @@ public async Task<IActionResult> UpdateFacilityReportingPlan(string id, Facility
203203

204204
if (string.IsNullOrWhiteSpace(requestId))
205205
{
206-
return BadRequest("Id is required in the request body.");
206+
return BadRequestProblem("Id is required in the request body.");
207207
}
208208

209209
if (requestId != id)
210210
{
211-
return BadRequest("Id in the URL must match the Id in the request body.");
211+
return BadRequestProblem("Id in the URL must match the Id in the request body.");
212212
}
213213

214214
try
@@ -217,15 +217,15 @@ public async Task<IActionResult> UpdateFacilityReportingPlan(string id, Facility
217217
}
218218
catch (KeyNotFoundException ex)
219219
{
220-
return NotFound(ex.Message);
220+
return NotFoundProblem(ex.Message);
221221
}
222222
catch (DuplicateReportingPlanException ex)
223223
{
224-
return Conflict(ex.Message);
224+
return Problem(ex.Message, statusCode: StatusCodes.Status409Conflict, title: "Conflict");
225225
}
226226
catch (ReportingPlanValidationException ex)
227227
{
228-
return BadRequest(ex.Message);
228+
return BadRequestProblem(ex.Message);
229229
}
230230
catch (Exception ex)
231231
{
@@ -265,7 +265,7 @@ public async Task<IActionResult> DeleteFacilityReportingPlans(CancellationToken
265265
/// Deletes a facility reporting plan.
266266
/// </summary>
267267
[ProducesResponseType(StatusCodes.Status204NoContent)]
268-
[ProducesResponseType(StatusCodes.Status404NotFound)]
268+
[ProducesResponseType(StatusCodes.Status404NotFound, Type = typeof(ProblemDetails))]
269269
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
270270
[HttpDelete("{id}")]
271271
public async Task<IActionResult> DeleteFacilityReportingPlan(string id, CancellationToken cancellationToken)
@@ -278,7 +278,7 @@ public async Task<IActionResult> DeleteFacilityReportingPlan(string id, Cancella
278278
}
279279
catch (KeyNotFoundException ex)
280280
{
281-
return NotFound(ex.Message);
281+
return NotFoundProblem(ex.Message);
282282
}
283283
catch (Exception ex)
284284
{
@@ -293,7 +293,7 @@ public async Task<IActionResult> DeleteFacilityReportingPlan(string id, Cancella
293293
/// Deletes every reporting plan belonging to a facility.
294294
/// </summary>
295295
[ProducesResponseType(StatusCodes.Status204NoContent)]
296-
[ProducesResponseType(StatusCodes.Status400BadRequest)]
296+
[ProducesResponseType(StatusCodes.Status400BadRequest, Type = typeof(ProblemDetails))]
297297
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
298298
[HttpDelete("facilities/{facilityId}")]
299299
public async Task<IActionResult> DeleteFacilityReportingPlansForFacility(string facilityId, CancellationToken cancellationToken)
@@ -302,7 +302,7 @@ public async Task<IActionResult> DeleteFacilityReportingPlansForFacility(string
302302

303303
if (facilityId is null)
304304
{
305-
return BadRequest("FacilityId is required.");
305+
return BadRequestProblem("FacilityId is required.");
306306
}
307307

308308
try
@@ -357,6 +357,12 @@ public async Task<IActionResult> DeleteFacilityReportingPlansForFacility(string
357357
return null;
358358
}
359359

360+
private ObjectResult BadRequestProblem(string detail) =>
361+
Problem(detail, statusCode: StatusCodes.Status400BadRequest, title: "Bad Request");
362+
363+
private ObjectResult NotFoundProblem(string detail) =>
364+
Problem(detail, statusCode: StatusCodes.Status404NotFound, title: "Not Found");
365+
360366
private static string? NullIfBlank(string? value) => string.IsNullOrWhiteSpace(value) ? null : value;
361367

362368
private static FacilityReportingPlan ToEntity(FacilityReportingPlanRequest request) =>

‎DotNet/ServiceTests/IntegrationTests/DMRP/FacilityReportingPlansControllerTests.cs‎

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,18 @@ private async Task<FacilityReportingPlanModel> CreatedPlanAsync(string facilityI
110110
return (FacilityReportingPlanModel)Assert.IsType<CreatedResult>(result).Value!;
111111
}
112112

113+
private static ProblemDetails AssertProblem(IActionResult result, int status, string title)
114+
{
115+
var obj = Assert.IsType<ObjectResult>(result);
116+
Assert.Equal(status, obj.StatusCode);
117+
118+
var problem = Assert.IsType<ProblemDetails>(obj.Value);
119+
Assert.Equal(status, problem.Status);
120+
Assert.Equal(title, problem.Title);
121+
122+
return problem;
123+
}
124+
113125
[Fact]
114126
public async Task CreateFacilityReportingPlan_ThenGet_RoundTripsEveryField()
115127
{
@@ -134,14 +146,18 @@ public async Task CreateFacilityReportingPlan_ThenGet_RoundTripsEveryField()
134146
}
135147

136148
[Fact]
137-
public async Task CreateFacilityReportingPlan_DuplicatePeriod_ReturnsConflict()
149+
public async Task CreateFacilityReportingPlan_DuplicatePeriod_ReturnsConflictProblemDetails()
138150
{
139151
var request = await ValidRequestAsync();
140152

141153
await _controller.CreateFacilityReportingPlan(request, CancellationToken.None);
142-
var second = await _controller.CreateFacilityReportingPlan(request, CancellationToken.None);
154+
var secondResult = await _controller.CreateFacilityReportingPlan(request, CancellationToken.None);
143155

144-
Assert.IsType<ConflictObjectResult>(second);
156+
var problem = AssertProblem(secondResult, StatusCodes.Status409Conflict, "Conflict");
157+
Assert.Equal(
158+
$"A reporting plan already exists for facility {request.FacilityId}, measure mapping " +
159+
$"{request.MeasureMappingId} and period {request.ReportingMonth}/{request.ReportingYear}.",
160+
problem.Detail);
145161
}
146162

147163
[Fact]
@@ -152,8 +168,8 @@ public async Task CreateFacilityReportingPlan_NullMeasureMappingId_ReturnsBadReq
152168

153169
var result = await _controller.CreateFacilityReportingPlan(request, CancellationToken.None);
154170

155-
var badRequest = Assert.IsType<BadRequestObjectResult>(result);
156-
Assert.Equal("MeasureMappingId is required.", badRequest.Value);
171+
var problem = AssertProblem(result, StatusCodes.Status400BadRequest, "Bad Request");
172+
Assert.Equal("MeasureMappingId is required.", problem.Detail);
157173
}
158174

159175
[Fact]
@@ -164,7 +180,7 @@ public async Task CreateFacilityReportingPlan_UnknownMeasureMapping_ReturnsBadRe
164180

165181
var result = await _controller.CreateFacilityReportingPlan(request, CancellationToken.None);
166182

167-
Assert.IsType<BadRequestObjectResult>(result);
183+
AssertProblem(result, StatusCodes.Status400BadRequest, "Bad Request");
168184
}
169185

170186
[Fact]
@@ -176,23 +192,23 @@ public async Task CreateFacilityReportingPlan_UnknownFacility_ReturnsBadRequest(
176192

177193
var result = await _controller.CreateFacilityReportingPlan(await ValidRequestAsync(), CancellationToken.None);
178194

179-
Assert.IsType<BadRequestObjectResult>(result);
195+
AssertProblem(result, StatusCodes.Status400BadRequest, "Bad Request");
180196
}
181197

182198
[Fact]
183199
public async Task CreateFacilityReportingPlan_MonthOutOfRange_ReturnsBadRequest()
184200
{
185201
var result = await _controller.CreateFacilityReportingPlan(await ValidRequestAsync(month: 13), CancellationToken.None);
186202

187-
Assert.IsType<BadRequestObjectResult>(result);
203+
AssertProblem(result, StatusCodes.Status400BadRequest, "Bad Request");
188204
}
189205

190206
[Fact]
191207
public async Task GetFacilityReportingPlan_NotFound_Returns404()
192208
{
193209
var result = await _controller.GetFacilityReportingPlan(Guid.NewGuid().ToString(), CancellationToken.None);
194210

195-
Assert.IsType<NotFoundResult>(result);
211+
AssertProblem(result, StatusCodes.Status404NotFound, "Not Found");
196212
}
197213

198214
[Fact]
@@ -237,7 +253,7 @@ public async Task GetFacilityReportingPlansForFacility_MonthOutOfRange_ReturnsBa
237253
{
238254
var result = await _controller.GetFacilityReportingPlansForFacility(FacilityId, 0, null, null, CancellationToken.None);
239255

240-
Assert.IsType<BadRequestObjectResult>(result);
256+
AssertProblem(result, StatusCodes.Status400BadRequest, "Bad Request");
241257
}
242258

243259
[Fact]
@@ -277,7 +293,7 @@ public async Task SearchFacilityReportingPlans_UnsortableColumn_ReturnsBadReques
277293
var result = await _controller.SearchFacilityReportingPlans(new FacilityReportingPlanSearchFilters(),
278294
sortBy: "DROP TABLE", sortOrder: null, pageSize: 10, pageNumber: 1, cancellationToken: CancellationToken.None);
279295

280-
Assert.IsType<BadRequestObjectResult>(result);
296+
AssertProblem(result, StatusCodes.Status400BadRequest, "Bad Request");
281297
}
282298

283299
[Theory]
@@ -314,6 +330,29 @@ public async Task UpdateFacilityReportingPlan_ChangesTheStoredPlan()
314330
Assert.NotNull(updated.ModifyDate);
315331
}
316332

333+
[Fact]
334+
public async Task UpdateFacilityReportingPlan_MovedOntoAnotherPlansPeriod_ReturnsConflictProblemDetails()
335+
{
336+
var first = await CreatedPlanAsync(month: 5);
337+
var second = await CreatedPlanAsync(month: 6);
338+
339+
var result = await _controller.UpdateFacilityReportingPlan(second.Id!, new FacilityReportingPlanUpdateRequest
340+
{
341+
Id = second.Id,
342+
FacilityId = first.FacilityId,
343+
MeasureMappingId = first.MeasureMappingId,
344+
ReportingMonth = first.ReportingMonth,
345+
ReportingYear = first.ReportingYear,
346+
IsReporting = second.IsReporting
347+
}, CancellationToken.None);
348+
349+
var problem = AssertProblem(result, StatusCodes.Status409Conflict, "Conflict");
350+
Assert.Equal(
351+
$"A reporting plan already exists for facility {first.FacilityId}, measure mapping " +
352+
$"{first.MeasureMappingId} and period {first.ReportingMonth}/{first.ReportingYear}.",
353+
problem.Detail);
354+
}
355+
317356
[Fact]
318357
public async Task UpdateFacilityReportingPlan_MismatchedId_ReturnsBadRequest()
319358
{
@@ -322,7 +361,7 @@ public async Task UpdateFacilityReportingPlan_MismatchedId_ReturnsBadRequest()
322361
var result = await _controller.UpdateFacilityReportingPlan(created.Id!,
323362
new FacilityReportingPlanUpdateRequest { Id = Guid.NewGuid().ToString() }, CancellationToken.None);
324363

325-
Assert.IsType<BadRequestObjectResult>(result);
364+
AssertProblem(result, StatusCodes.Status400BadRequest, "Bad Request");
326365
}
327366

328367
[Fact]
@@ -333,7 +372,7 @@ public async Task UpdateFacilityReportingPlan_UnknownId_Returns404()
333372

334373
var result = await _controller.UpdateFacilityReportingPlan(unknownId, request, CancellationToken.None);
335374

336-
Assert.IsType<NotFoundObjectResult>(result);
375+
AssertProblem(result, StatusCodes.Status404NotFound, "Not Found");
337376
}
338377

339378
[Fact]
@@ -344,7 +383,7 @@ public async Task UpdateFacilityReportingPlan_MissingId_ReturnsBadRequest()
344383
var result = await _controller.UpdateFacilityReportingPlan(created.Id!,
345384
await ValidUpdateRequestAsync(id: null), CancellationToken.None);
346385

347-
Assert.IsType<BadRequestObjectResult>(result);
386+
AssertProblem(result, StatusCodes.Status400BadRequest, "Bad Request");
348387
}
349388

350389
[Fact]
@@ -361,7 +400,7 @@ public async Task UpdateFacilityReportingPlan_InvalidPlan_ReturnsBadRequestNot40
361400
ReportingYear = created.ReportingYear
362401
}, CancellationToken.None);
363402

364-
Assert.IsType<BadRequestObjectResult>(result);
403+
AssertProblem(result, StatusCodes.Status400BadRequest, "Bad Request");
365404
}
366405

367406
[Fact]
@@ -373,15 +412,15 @@ public async Task DeleteFacilityReportingPlan_ThenGet_ReturnsNotFound()
373412
Assert.IsType<NoContentResult>(deleteResult);
374413

375414
var getResult = await _controller.GetFacilityReportingPlan(created.Id!, CancellationToken.None);
376-
Assert.IsType<NotFoundResult>(getResult);
415+
AssertProblem(getResult, StatusCodes.Status404NotFound, "Not Found");
377416
}
378417

379418
[Fact]
380419
public async Task DeleteFacilityReportingPlan_NotFound_Returns404()
381420
{
382421
var result = await _controller.DeleteFacilityReportingPlan(Guid.NewGuid().ToString(), CancellationToken.None);
383422

384-
Assert.IsType<NotFoundObjectResult>(result);
423+
AssertProblem(result, StatusCodes.Status404NotFound, "Not Found");
385424
}
386425

387426
[Fact]

0 commit comments

Comments
 (0)