Add unique constraint on project name and organization ID, reduce minimum project name length #350
Conversation
WalkthroughAdds a composite unique constraint on project name per organization at the DB/model level, introduces a pre-create duplicate check in project CRUD, relaxes onboarding project_name min length, adds Alembic migration for the constraint, and adds a unit test asserting duplicate-name conflict handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as API (create_project)
participant DB as Database
Client->>API: POST /projects (name, organization_id, ...)
API->>DB: SELECT project WHERE name+organization_id
alt Found duplicate
API-->>Client: 409 Conflict ("Project already exists")
else Not found
API->>DB: INSERT project
DB-->>API: New project row
API-->>Client: 201 Created (project)
end
note over DB,API: DB also enforces UniqueConstraint(name, organization_id)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/crud/project.py (1)
14-33: Handle race with DB unique constraint; translate IntegrityError to HTTP 409.Pre-check is necessary but not sufficient under concurrency. Two requests can pass the check and one will hit a DB unique violation on commit, currently surfacing as a 500. Catch and map to 409.
Apply this refactor:
def create_project(*, session: Session, project_create: ProjectCreate) -> Project: project = get_project_by_name( session=session, organization_id=project_create.organization_id, project_name=project_create.name, ) if project: logger.error( f"[create_project] Project already exists | 'project_id': {project.id}, 'name': {project.name}" ) raise HTTPException(409, "Project already exists") db_project = Project.model_validate(project_create) db_project.inserted_at = now() db_project.updated_at = now() - session.add(db_project) - session.commit() - session.refresh(db_project) + session.add(db_project) + try: + session.commit() + except IntegrityError as e: + session.rollback() + # Best-effort check to ensure we only map the unique constraint violation + if "uq_project_name_org_id" in str(e.orig): + logger.error( + "[create_project] Unique constraint violated on (name, organization_id)" + ) + raise HTTPException(409, "Project already exists") + raise + session.refresh(db_project) logger.info( f"[create_project] Project Created Successfully | 'project_id': {db_project.id}, 'name': {db_project.name}" ) return db_projectAnd add the import:
from sqlalchemy.exc import IntegrityError
🧹 Nitpick comments (7)
backend/app/models/onboarding.py (1)
32-34: Guard against whitespace-only project names.Min length 1 allows " " (space) to pass. Add a validator to enforce at least one non-whitespace character to avoid junk names while honoring the 1-char requirement.
Example validator to add:
# Add alongside other validators @model_validator(mode="after") def _validate_project_name(self): if not (self.project_name and self.project_name.strip()): raise ValueError("project_name must contain at least one non-whitespace character") return selfWould you like me to add an API/CRUD test for a single-character name and a whitespace-only rejection?
backend/app/models/project.py (2)
3-3: Import UniqueConstraint from sqlalchemy for consistency and portability.
UniqueConstraintis typically sourced fromsqlalchemy, and other modules in many codebases follow that. Aligning imports avoids surprises across SQLModel/SQLAlchemy versions.-from sqlmodel import Field, Relationship, SQLModel, UniqueConstraint +from sqlmodel import Field, Relationship, SQLModel +from sqlalchemy import UniqueConstraint
29-31: DB-level uniqueness looks good; consider index hygiene.The composite unique constraint will create a unique index on (name, organization_id). If you don’t query by name alone, consider dropping the legacy single-column index on project.name in a follow-up migration to reduce write overhead.
backend/app/alembic/versions/8725df286943_unique_constraint_on_project_name_and_.py (2)
9-11: Remove unused imports to satisfy linters and keep migration lean.-from alembic import op -import sqlalchemy as sa -import sqlmodel.sql.sqltypes +from alembic import op
20-25: Consider a zero-downtime path for large tables (Postgres).Creating a unique constraint can take an exclusive lock. If the table is large or write-heavy, prefer:
- create unique index concurrently on (name, organization_id),
- attach it as a constraint via
ALTER TABLE ... ADD CONSTRAINT ... UNIQUE USING INDEX ....Alembic example (optional, Postgres-only):
op.create_index( "uqidx_project_name_org_id", "project", ["name", "organization_id"], unique=True, postgresql_concurrently=True ) op.execute( "ALTER TABLE project ADD CONSTRAINT uq_project_name_org_id UNIQUE USING INDEX uqidx_project_name_org_id" )backend/app/tests/crud/test_project.py (2)
37-50: Drop unused variable to satisfy Ruff F841 and keep test clean.- project = create_project(session=db, project_create=project_data) + create_project(session=db, project_create=project_data)
37-50: Add a test to assert same-name projects are allowed across different organizations.This complements the duplicate-in-org test and guards against accidental global uniqueness.
Proposed test:
def test_create_project_same_name_different_orgs(db: Session) -> None: org1 = create_test_organization(db) org2 = create_test_organization(db) name = "shared-name" p1 = create_project(session=db, project_create=ProjectCreate( name=name, description="p1", is_active=True, organization_id=org1.id )) p2 = create_project(session=db, project_create=ProjectCreate( name=name, description="p2", is_active=True, organization_id=org2.id )) assert p1.name == p2.name assert p1.organization_id != p2.organization_id
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
backend/app/alembic/versions/8725df286943_unique_constraint_on_project_name_and_.py(1 hunks)backend/app/crud/project.py(1 hunks)backend/app/models/onboarding.py(1 hunks)backend/app/models/project.py(2 hunks)backend/app/tests/crud/test_project.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
backend/app/models/onboarding.py (2)
backend/app/tests/api/routes/test_onboarding.py (1)
test_onboard_project_with_auto_generated_defaults(131-160)backend/app/tests/crud/test_onboarding.py (2)
test_onboard_project_new_organization_project_user(26-90)test_onboard_project_response_data_integrity(235-264)
backend/app/tests/crud/test_project.py (4)
backend/app/tests/utils/test_data.py (1)
create_test_organization(23-31)backend/app/tests/utils/utils.py (1)
random_lower_string(21-22)backend/app/models/project.py (1)
ProjectCreate(16-17)backend/app/crud/project.py (1)
create_project(13-34)
backend/app/crud/project.py (2)
backend/app/tests/crud/test_onboarding.py (1)
test_onboard_project_duplicate_project_name(154-174)backend/app/tests/api/routes/test_onboarding.py (1)
test_onboard_project_duplicate_project_in_organization(91-128)
backend/app/alembic/versions/8725df286943_unique_constraint_on_project_name_and_.py (1)
backend/app/alembic/versions/99f4fc325617_add_organization_project_setup.py (1)
upgrade(20-72)
backend/app/models/project.py (2)
backend/app/alembic/versions/99f4fc325617_add_organization_project_setup.py (1)
upgrade(20-72)backend/app/models/assistants.py (1)
AssistantBase(11-30)
🪛 Ruff (0.12.2)
backend/app/tests/crud/test_project.py
48-48: Local variable project is assigned to but never used
Remove assignment to unused variable project
(F841)
backend/app/alembic/versions/8725df286943_unique_constraint_on_project_name_and_.py
9-9: sqlalchemy imported but unused
Remove unused import: sqlalchemy
(F401)
10-10: sqlmodel.sql.sqltypes imported but unused
Remove unused import: sqlmodel.sql.sqltypes
(F401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks (3.11.7, 6)
3 participants
Summary
This PR implements a unique constraint to prevent duplicate project names within the same organization and reduces the minimum project name length requirement from 3 to 1 character.
Explain the motivation for making this change. What existing problem does the pull request solve?
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.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit
New Features
Bug Fixes