-
Notifications
You must be signed in to change notification settings - Fork 1
Complete mongo db integration for books resource (get) #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Complete mongo db integration for books resource (get) #5
Conversation
There was a problem hiding this 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 completes the MongoDB integration for the GET /books endpoint, transitioning from an in-memory datastore to a full database-backed implementation. The change introduces proper service layer architecture, comprehensive error handling, and maintains 100% test coverage.
Key changes include:
- Service Layer Introduction: New
book_service.pymodule withfetch_active_books()andformat_books_for_api()functions to handle database operations and data formatting - Database Integration: Added
find_books()helper function inmongo_helper.pyto wrap MongoDB query operations with filtering, projection, and limit capabilities - Enhanced Error Handling: Improved database connection error handling with specific 503 Service Unavailable responses instead of generic 500 errors
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/services/book_service.py | New service layer module containing business logic for fetching and formatting books from MongoDB |
| app/datastore/mongo_helper.py | Added find_books() helper function to wrap MongoDB find operations with optional filtering and limiting |
| app/routes.py | Refactored GET /books endpoint to use new service layer and improved error handling for database failures |
| tests/test_mongo_helper.py | Added comprehensive unit tests for the new find_books() helper function |
| tests/test_app.py | Updated integration tests to mock the new service layer and test database error scenarios |
| openapi.yml | Added 503 Service Unavailable response specification for database connection failures |
Comments suppressed due to low confidence (1)
app/services/book_service.py:45
- [nitpick] The variable name 'msg_lines_list' is redundant since 'list' is already implied by the type. Consider renaming to 'msg_lines' or 'message_lines'.
msg_lines_list = ["Missing required fields:"]
Fix docstring to reflect parameter name, copilot review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot review- fix spelling error Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Lakorthus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
Description
Trello ticket: https://trello.com/c/O9jraVyM
This is the first of four planned PRs to migrate the API from an in-memory datastore to MongoDB. This PR specifically covers the GET /books endpoint.
This change finalizes the transition of the GET /books endpoint to a full MongoDB-backed implementation. It includes a comprehensive test suite to achieve 100% test coverage for the new logic.
Key changes include:
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Automated testing (pytest), manual testing (cURL), CI/CD with 100% pylint error coverage and 100% test coverage
Checklist: