Collections: Customizable collection names#878
Open
Ayush8923 wants to merge 4 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds PATCH /collections/{collection_id} using a new CollectionUpdate model; extends CollectionPublic with optional name and description; updates to_collection_public to include those fields; implements CollectionCrud.update with name-uniqueness and IntegrityError handling; adds docs, re-export, and API/CRUD tests. ChangesCollection update flow
sequenceDiagram
participant Client
participant API as update_collection route
participant Crud as CollectionCrud.update
participant DB as Database
participant Helper as to_collection_public
Client->>API: PATCH /collections/{id} with CollectionUpdate
API->>Crud: CollectionCrud.update(collection_id, patch)
Crud->>DB: load collection / persist changes
Crud->>Helper: to_collection_public(updated_collection)
Helper->>API: CollectionPublic
API->>Client: 200 OK with CollectionPublic
🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels: Suggested reviewers:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/api/routes/collections.py`:
- Around line 211-234: The update_collection function is missing an explicit
return type; add a return type hint (e.g., -> APIResponse) to the
update_collection signature to reflect that it returns
APIResponse.success_response(...), and if APIResponse is not already imported in
backend/app/api/routes/collections.py, add the appropriate import; keep the
existing parameter annotations and ensure the signature uses the exact function
name update_collection and the same parameters (session: SessionDep,
current_user: AuthContextDep, patch: CollectionUpdate, collection_id: UUID =
FastPath(...)).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6747a73-5f99-43f4-8f5a-0f72b5e39d52
📒 Files selected for processing (5)
backend/app/api/docs/collections/update.mdbackend/app/api/routes/collections.pybackend/app/crud/collection/collection.pybackend/app/models/__init__.pybackend/app/models/collection.py
✅ Files skipped from review due to trivial changes (1)
- backend/app/api/docs/collections/update.md
AkhileshNegi
requested changes
May 22, 2026
| description: str | None = Field( | ||
| default=None, | ||
| description="Description of the collection", | ||
| ) |
Collaborator
There was a problem hiding this comment.
can you add testcases for this as well
Collaborator
Author
There was a problem hiding this comment.
Yes, added the testcases.
|
|
||
| class CollectionPublic(SQLModel): | ||
| id: UUID | ||
| name: str | None = Field( |
Collaborator
There was a problem hiding this comment.
while this is fine can we add some restriction like name should be there instead of name and minimum and maximum length
Comment on lines
+104
to
+107
| raise HTTPException( | ||
| status_code=409, | ||
| detail=f"Collection '{changes['name']}' already exists. Choose a different name.", | ||
| ) |
Collaborator
There was a problem hiding this comment.
can you check other code, usually HTTPException belongs in routes, not crud/
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/app/tests/api/routes/collections/test_collection_update.py (1)
13-91: ⚖️ Poor tradeoffConsider using factory pattern for test data creation.
Similar to the CRUD tests, these API tests use helper functions
get_project()andget_assistant_collection()rather than the factory pattern. As per coding guidelines, test fixtures inbackend/app/tests/should use the factory pattern.As per coding guidelines: Use factory pattern for test fixtures in
backend/app/tests/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/tests/api/routes/collections/test_collection_update.py` around lines 13 - 91, These API tests (test_update_collection_returns_updated_fields, test_update_collection_partial_update_preserves_other_fields, test_update_collection_rename_to_existing_name_returns_409, test_update_collection_not_found_returns_404) use helper functions get_project() and get_assistant_collection(); update them to follow the repository guideline by using the factory pattern instead: import and use the appropriate ProjectFactory and CollectionFactory (or equivalent factories used in CRUD tests) to create projects and collections in each test, replace calls to get_project() and get_assistant_collection() with factory.create(...) (or the factory's create/ build API), and remove reliance on those helpers so fixtures align with backend/app/tests/ conventions while keeping the same test assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/app/tests/api/routes/collections/test_collection_update.py`:
- Around line 13-91: These API tests
(test_update_collection_returns_updated_fields,
test_update_collection_partial_update_preserves_other_fields,
test_update_collection_rename_to_existing_name_returns_409,
test_update_collection_not_found_returns_404) use helper functions get_project()
and get_assistant_collection(); update them to follow the repository guideline
by using the factory pattern instead: import and use the appropriate
ProjectFactory and CollectionFactory (or equivalent factories used in CRUD
tests) to create projects and collections in each test, replace calls to
get_project() and get_assistant_collection() with factory.create(...) (or the
factory's create/ build API), and remove reliance on those helpers so fixtures
align with backend/app/tests/ conventions while keeping the same test
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 664e3b2c-41f7-4cc8-ace1-81b6d3b4ea31
📒 Files selected for processing (4)
backend/app/api/routes/collections.pybackend/app/crud/collection/collection.pybackend/app/tests/api/routes/collections/test_collection_update.pybackend/app/tests/crud/collections/collection/test_crud_collection_update.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue: ProjectTech4DevAI/kaapi-frontend#172
Summary
nameanddescriptionfields, providing more detailed collection information in public endpoints.Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Summary by CodeRabbit