Skip to content

Solution#654

Open
romanbylym wants to merge 1 commit into
mate-academy:mainfrom
romanbylym:develop
Open

Solution#654
romanbylym wants to merge 1 commit into
mate-academy:mainfrom
romanbylym:develop

Conversation

@romanbylym
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

Your Library Management API implementation is approved. All 23 core requirements are satisfied: proper FastAPI setup with SQLAlchemy ORM and SQLite, correct models with relationships (Author with unique name constraint, Book with foreign key to Author), all CRUD operations, and complete endpoint implementations with pagination and filtering support. No functional issues or test failures were found. The only note is that BookCreate schema includes author_id as a required field, which creates some redundancy with the URL path parameter in the /authors/{author_id}/books endpoint. This is a design consideration rather than a blocker—the API will function correctly, though users will need to include author_id in both the path and request body. You may consider making author_id optional in BookCreate if you want cleaner REST API conventions, but this is not required for the implementation to work. Great work on the complete and functional implementation!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread main.py

def get_db():
db = SessionLocal()
try:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The BookCreate schema requires author_id, but in the POST /authors/{author_id}/books endpoint, the author_id is taken from the URL path. Users would need to provide author_id in both the path AND the request body, which is redundant. Consider creating a separate schema for book creation that excludes author_id, or accepting it only from the request body.

Comment thread schemas.py
class BookBase(BaseModel):
title: str
summary: str
publication_date: date
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The author_id field in BookCreate creates a conflict with the endpoint design. When calling POST /authors/{author_id}/books, the author_id comes from the URL path, not the request body. Including it in BookCreate is redundant and could lead to data inconsistency if different values are passed in URL vs body. The author relationship should be established using the URL path parameter instead.

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