Skip to content

Conversation

@piyush-jaiswal
Copy link
Owner

@piyush-jaiswal piyush-jaiswal commented Oct 2, 2025

Summary by CodeRabbit

  • New Features

    • Clearer API responses: Updating categories or subcategories to an existing name now returns 409 Conflict with a specific message.
  • Bug Fixes

    • Consistent conflict handling on updates, aligning with create behavior while preserving existing linkage conflict responses.
  • Tests

    • Added tests covering duplicate-name update conflicts for categories and subcategories.
    • Streamlined auth handling in tests: creation calls unauthenticated; update/delete provide headers inline.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

Walkthrough

Adds specific IntegrityError handling for unique name violations during Category and Subcategory updates, returning HTTP 409 with targeted messages. Aligns PUT behavior with existing POST logic. Updates tests to inline auth header creation and adds new tests asserting duplicate-name update conflicts for categories and subcategories.

Changes

Cohort / File(s) Summary
Category endpoints
app/migrated_routes/category.py
In PUT, detect name unique-constraint violations and return 409 with a duplicate-name message; existing linkage UniqueViolation handling retained. Mirrors POST duplicate-name behavior.
Subcategory endpoints
app/migrated_routes/subcategory.py
In PUT, add early handling for name unique-constraint UniqueViolation, returning 409 with a specific message before other constraint checks.
Tests — category
tests/test_category.py
Inline auth headers on update/delete requests; creation calls no longer pass headers. Add test_update_category_duplicate_name to assert unique-name conflict on update and database state.
Tests — subcategory
tests/test_subcategory.py
Inline auth headers for update/delete; creation without headers. Add test_update_subcategory_duplicate_name to validate unique-name update conflict and resulting DB contents.
Tests — product
tests/test_product.py
Creation calls remove headers; update/delete pass inline auth headers. No logic changes beyond header handling.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant API as Category PUT /categories/{id}
  participant DB as Database

  C->>API: PUT { name: "NewName", ... }
  API->>DB: Update category
  DB-->>API: IntegrityError (UniqueViolation on name)
  alt Name unique constraint violated
    API-->>C: 409 Conflict (Category name already exists)
  else Other constraint violations
    API-->>C: 409 Conflict (existing linkage constraint handling)
  end
Loading
sequenceDiagram
  autonumber
  participant C as Client
  participant API as Subcategory PUT /subcategories/{id}
  participant DB as Database

  C->>API: PUT { name: "NewSubName", ... }
  API->>DB: Update subcategory
  DB-->>API: IntegrityError (UniqueViolation)
  alt Name unique constraint violated
    API-->>C: 409 Conflict (Subcategory name already exists)
  else Other constraint violations
    API-->>C: 409 Conflict (per existing checks)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

Hop hop! I guard the names so neat,
No twin titles in our data street.
409 drums a gentle thrum—
“Pick a name, just not that one!”
Categories, subs all stand in line;
Bugs hop out, constraints align. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title directly reflects the primary change—adding duplicate name detection on PUT update endpoints—and is concise and focused on the main feature without extraneous details.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/detect-duplicate-name-in-put

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Oct 2, 2025

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e0c5fc0 and 686bfcc.

📒 Files selected for processing (5)
  • app/migrated_routes/category.py (1 hunks)
  • app/migrated_routes/subcategory.py (1 hunks)
  • tests/test_category.py (2 hunks)
  • tests/test_product.py (2 hunks)
  • tests/test_subcategory.py (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/test_product.py (1)
tests/conftest.py (3)
  • create_product (87-98)
  • client (14-21)
  • create_authenticated_headers (43-55)
tests/test_subcategory.py (2)
tests/conftest.py (3)
  • create_subcategory (72-83)
  • client (14-21)
  • create_authenticated_headers (43-55)
app/migrated_routes/subcategory.py (1)
  • put (161-244)
tests/test_category.py (2)
tests/conftest.py (3)
  • create_category (59-68)
  • client (14-21)
  • create_authenticated_headers (43-55)
app/migrated_routes/category.py (1)
  • put (153-221)
🔇 Additional comments (8)
app/migrated_routes/category.py (1)

207-212: LGTM! Duplicate name handling aligns with POST.

The duplicate name detection for PUT is correctly implemented and mirrors the existing POST behavior. The check is appropriately placed before the linkage constraint check, ensuring specific error messages for each violation type.

tests/test_product.py (2)

73-82: LGTM! Header handling refactor improves clarity.

The change separates creation (using fixture default authentication) from mutation (explicit inline authentication), making the authentication requirements clearer at the call site.


92-99: LGTM! Consistent with update test refactoring.

The DELETE test follows the same header handling pattern as the update test, maintaining consistency across the test suite.

app/migrated_routes/subcategory.py (1)

225-230: LGTM! Mirrors category implementation.

The duplicate name handling for subcategory PUT is correctly implemented and consistent with both the category PUT logic and the subcategory POST logic.

tests/test_subcategory.py (2)

77-86: LGTM! Header handling consistent with product tests.

The refactoring aligns with the pattern established in test_product.py, separating creation authentication (implicit) from mutation authentication (explicit).


115-122: LGTM! Delete test follows consistent pattern.

The header handling refactor is consistent with the update test and aligns with the broader test suite changes.

tests/test_category.py (2)

77-86: LGTM! Header handling aligns with test suite pattern.

The refactoring is consistent with the changes in test_product.py and test_subcategory.py.


116-123: LGTM! Delete test follows suite-wide pattern.

The header handling refactor maintains consistency with the update test and other test files in the PR.

@piyush-jaiswal piyush-jaiswal merged commit 1697146 into master Oct 2, 2025
3 checks passed
@piyush-jaiswal piyush-jaiswal deleted the feature/detect-duplicate-name-in-put branch October 2, 2025 12: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.

2 participants