Skip to content

Expose explicit conflict when same ingredient+unit appears with different amounts#1063

Merged
dgee2 merged 1 commit into
mainfrom
dgee2/issue-980-expose-explicit-duplicate-conflicts-only-ee5ff0
May 23, 2026
Merged

Expose explicit conflict when same ingredient+unit appears with different amounts#1063
dgee2 merged 1 commit into
mainfrom
dgee2/issue-980-expose-explicit-duplicate-conflicts-only-ee5ff0

Conversation

@dgee2

@dgee2 dgee2 commented May 23, 2026

Copy link
Copy Markdown
Owner

Why

When a recipe request contained the same ingredient+unit combination more than once with different amounts, the API would crash with HTTP 500. NormalizeRecipeIngredients used Distinct() (structural equality on all three fields: ingredient name, unit, and amount), so exact duplicates were silently absorbed, but entries that differed only in amount both passed through. Both then entered UpsertRecipeIngredientsAsync, where each attempted to insert a row with the same composite primary key (RecipeId, IngredientId, UnitId), causing an unhandled DbUpdateException and a 500 response.

This is a business-significant conflict: a recipe listing the same ingredient and unit twice with contradictory amounts is logically invalid, and the user must resolve the ambiguity before the request can succeed.

What changed

RecipeService.NormalizeRecipeIngredients now performs conflict detection after the existing Distinct() deduplication pass. Any (IngredientName, UnitName) group that still has more than one entry (meaning the same ingredient+unit with different amounts) triggers a BusinessValidationException that names all conflicting pairs. BusinessValidationExceptionHandler converts this to HTTP 422, which both recipe endpoints already advertise in their OpenAPI contract.

The ordering matters: exact duplicates are still silently absorbed first (behaviour from #977), then the conflict check runs over what remains. No repository, schema, or OpenAPI contract changes are required.

Tests

  • Unit tests (4 new): verify that CreateRecipeAsync and UpdateRecipeAsync throw BusinessValidationException and make no repository write calls when conflicting amounts are present; verify that all conflicting pairs are reported in the message; verify that mixed exact-duplicate + conflicting-amount input is still detected as a conflict.
  • Integration tests (2 new): verify that POST /recipe and PUT /recipe/{id} return HTTP 422 with a problem detail when the same ingredient+unit appears with different amounts.

Fixes: #980

…cipe

When a recipe request contains the same ingredient+unit combination more than
once with different amounts, the behavior was previously undefined: exact
duplicates were silently absorbed by Distinct(), but conflicting-amount pairs
both survived into UpsertRecipeIngredientsAsync where the composite PK
(RecipeId, IngredientId, UnitId) would cause an unhandled DbUpdateException
and an HTTP 500.

This is a business-significant duplicate: the same ingredient+unit appearing
with two different amounts in one request is contradictory and the user must
resolve it. The fix adds explicit conflict detection in
RecipeService.NormalizeRecipeIngredients: after Distinct() removes exact
duplicates, any remaining (IngredientName, UnitName) group with more than one
entry throws a BusinessValidationException with all conflicting pairs named.

BusinessValidationExceptionHandler returns HTTP 422. Both recipe endpoints
already declare ProducesProblem(422) in the OpenAPI contract, so no contract
or frontend changes are required.

Closes #980

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 23, 2026 21:04
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR prevents an HTTP 500 when a recipe request includes the same ingredient+unit more than once with different amounts by detecting that business conflict in RecipeService and returning a client-visible validation error (HTTP 422) instead.

Changes:

  • Add conflict detection in RecipeService.NormalizeRecipeIngredients after exact-duplicate deduplication, throwing BusinessValidationException for ingredient+unit pairs that appear with multiple amounts.
  • Add unit test coverage to ensure conflicts throw and repository writes are not invoked.
  • Add integration tests to verify POST /api/recipe and PUT /api/recipe/{id} return HTTP 422 with problem details for conflicting ingredient amounts.

Reviewed changes

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

File Description
backend/MenuApi/Services/RecipeService.cs Adds ingredient+unit conflict detection after exact-duplicate removal and throws BusinessValidationException to prevent DB constraint crashes.
backend/MenuApi.Tests/Services/RecipeServiceTests.cs Adds unit tests for conflicting ingredient amount detection and verifies no repository writes occur on conflict.
backend/MenuApi.Integration.Tests/RecipeWithIngredientsIntegrationTests.cs Adds integration tests asserting HTTP 422 problem details for create/update requests with conflicting ingredient amounts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dgee2 dgee2 merged commit 54e2ad8 into main May 23, 2026
14 checks passed
@dgee2 dgee2 deleted the dgee2/issue-980-expose-explicit-duplicate-conflicts-only-ee5ff0 branch May 23, 2026 21:11
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.

Expose explicit duplicate conflicts only where silent handling is incorrect

2 participants