Skip to content

Conversation

@codesungrape
Copy link
Collaborator

Description

This PR removes the use of manually generated UUIDs in the POST /books endpoint, allowing MongoDB to handle _id generation via its native ObjectId.

Key Changes:

  • Separated Data Layer Helpers: The single insert_book_to_mongo() helper has been split into two distinct, single-responsibility functions:
  1. insert_book_to_mongo(): For the API. Performs a simple insert_one() operation which relies on MongoDB's auto-generated _id, as the API's role is strictly to create new resources.
  2. upsert_book_from_file(): A new, dedicated helper for the db-setup script. This function uses replace_one with upsert=True to correctly synchronize data from the books.json file, making the script idempotent.
  • Updated POST /books route, affected tests, test mocks and assertions for both POST and script logic
  • Updated openapi.yml to reflect use of MongoDB ObjectId for theAPI consumer

Fixes # (issue number if applicable)

Type of change

Please delete options that are not relevant.

  • [] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code refactor (improving code quality without changing functionality)
  • Performance improvement
  • Other (please describe):

How Has This Been Tested?

Automated tests, manual testing inc. verifying script idempotency, CI/CD pipeline

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My individual commit messages are descriptive and follow our commit guidelines
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Copy link

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

This PR refactors the book creation system by removing manual UUID generation and allowing MongoDB to handle ID generation using native ObjectIds. The change separates the API logic from the database setup script by creating distinct helper functions for different use cases.

  • Replaces manual UUID generation with MongoDB's native ObjectId for the POST /books endpoint
  • Splits the single insert helper into two specialized functions: one for API operations and one for file-based data synchronization
  • Updates API documentation to reflect ObjectId format instead of UUID format

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
app/routes.py Updates POST /books route to use ObjectId and handle response formatting
app/datastore/mongo_helper.py Separates insert logic into API-specific and file-sync functions
scripts/create_books.py Updates to use the new upsert function for file synchronization
openapi.yml Updates API documentation to reflect ObjectId format
tests/*.py Updates test mocks and assertions to match new ObjectId-based flow
Comments suppressed due to low confidence (3)

tests/test_mongo_helper.py:25

  • The test is calling assert_called_once() on line 24 and then assert_called_once_with() on line 25. The first assertion is redundant since the second assertion already verifies the method was called once with specific arguments.
    mock_books_collection.insert_one.assert_called_once_with(new_book)

tests/test_app.py:95

  • The test verifies that update_one was called but doesn't verify the arguments. Since the route code calls update_one with specific parameters (filter and update operation), the test should verify these arguments to ensure the correct database operation is performed.
    mock_collection.update_one.assert_called_once()

tests/test_api_security.py:96

  • Similar to the previous test, this assertion verifies that update_one was called but doesn't verify the arguments. The test should verify the filter and update parameters to ensure the correct database operation is performed.
    mock_collection.update_one.assert_called_once()

@codesungrape codesungrape merged commit 291e3f3 into main Jul 29, 2025
2 checks passed
@codesungrape codesungrape deleted the Remove-redundant-UUID-in-POST-endpoint branch July 29, 2025 16:15
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.

3 participants