Skip to content

♻️ Remove redundant item deletion logic in backend/app/api/routes/users.py‎#2253

Open
raphaeltannous wants to merge 8 commits into
fastapi:masterfrom
raphaeltannous:fix/cascade-delete-redundancy
Open

♻️ Remove redundant item deletion logic in backend/app/api/routes/users.py‎#2253
raphaeltannous wants to merge 8 commits into
fastapi:masterfrom
raphaeltannous:fix/cascade-delete-redundancy

Conversation

@raphaeltannous
Copy link
Copy Markdown

Hey!

Removed redundant manual item deletion in the user deletion endpoint, since the SQLModel relationship already has ON DELETE CASCADE configured, which automatically deletes all related items when a user is deleted.

owner_id: uuid.UUID = Field(
foreign_key="user.id", nullable=False, ondelete="CASCADE"
)

Copy link
Copy Markdown
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

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

Makes sense!
But we should then add tests that check that items are also deleted.
Currently tests don't check that: https://github.com/fastapi/full-stack-fastapi-template/blob/master/backend/tests/api/routes/test_users.py

@YuriiMotov YuriiMotov changed the title Refactor redundant user deletion logic in users.py ♻️ Remove redundant item deletion logic in backend/app/api/routes/users.py‎ May 21, 2026
@raphaeltannous
Copy link
Copy Markdown
Author

Makes sense! But we should then add tests that check that items are also deleted. Currently tests don't check that: https://github.com/fastapi/full-stack-fastapi-template/blob/master/backend/tests/api/routes/test_users.py

Hey! I've added 2 tests to check that the items are deleted.

@github-actions github-actions Bot removed the waiting label May 25, 2026
YuriiMotov

This comment was marked as resolved.

@raphaeltannous
Copy link
Copy Markdown
Author

LGTM!

I would simplify new added tests by excluding part that is already covered by previous tests

Updated.

@YuriiMotov YuriiMotov dismissed their stale review May 25, 2026 08:24

Found an issue in the test implementation

Comment thread backend/tests/api/routes/test_users.py Outdated
@raphaeltannous
Copy link
Copy Markdown
Author

You are right! I did not notice that.

I created create_random_item_for_user(db: Session, owner_id: uuid.UUID) -> Item, and used it. I also added an assertion for the item creation.

Comment thread backend/tests/utils/item.py Outdated
Copy link
Copy Markdown
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

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

LGTM!

@raphaeltannous, thanks!
Passing this to Sebastian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants