Remove ProjectUser model and related project user management #393
Conversation
…d routes, update dependencies and relationships in models
WalkthroughRemoved the entire project-user feature: deleted models, CRUD, API routes and router mounting, a dependency function, related model relationships and exports, tests, and added an Alembic migration dropping the projectuser table. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as API Router
participant Auth as Auth/Deps
participant CRUD as CRUD Layer
participant DB as Database
rect rgba(180,220,250,0.20)
note over Client,DB: Previous flow (before removals)
Client->>API: POST /projects/{id}/users
API->>Auth: verify_user_project_organization(project_id, org_id)
Auth->>DB: validate user/project/org membership
DB-->>Auth: result
alt authorized
API->>CRUD: add_user_to_project(...)
CRUD->>DB: insert ProjectUser
DB-->>CRUD: record
CRUD-->>API: success
API-->>Client: 200 OK
else unauthorized
API-->>Client: 4xx Error
end
end
rect rgba(250,200,180,0.20)
note over Client,API: Current flow (after removals)
Client->>API: POST /projects/{id}/users
API-->>Client: 404/route not found (no project_user routes)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![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
🧹 Nitpick comments (1)
backend/app/alembic/versions/585395bb6e5c_refactor_project_user.py (1)
10-10: Remove unused import.The import
sqlmodel.sql.sqltypesis not used in this migration.Apply this diff:
from alembic import op import sqlalchemy as sa -import sqlmodel.sql.sqltypes from sqlalchemy.dialects import postgresql
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
backend/app/alembic/versions/585395bb6e5c_refactor_project_user.py(1 hunks)backend/app/api/deps.py(0 hunks)backend/app/api/main.py(0 hunks)backend/app/api/routes/project_user.py(0 hunks)backend/app/crud/project_user.py(0 hunks)backend/app/models/__init__.py(0 hunks)backend/app/models/project.py(0 hunks)backend/app/models/project_user.py(0 hunks)backend/app/models/user.py(0 hunks)backend/app/tests/api/routes/test_project_user.py(0 hunks)backend/app/tests/api/test_deps.py(0 hunks)backend/app/tests/crud/test_project_user.py(0 hunks)
💤 Files with no reviewable changes (11)
- backend/app/models/init.py
- backend/app/models/project.py
- backend/app/models/user.py
- backend/app/api/routes/project_user.py
- backend/app/models/project_user.py
- backend/app/api/main.py
- backend/app/crud/project_user.py
- backend/app/tests/crud/test_project_user.py
- backend/app/tests/api/routes/test_project_user.py
- backend/app/tests/api/test_deps.py
- backend/app/api/deps.py
🧰 Additional context used
🪛 GitHub Actions: AI Platform CI
backend/app/alembic/versions/585395bb6e5c_refactor_project_user.py
[error] 1-1: Black formatting check failed. The hook reformatted 1 file.
🔇 Additional comments (1)
backend/app/alembic/versions/585395bb6e5c_refactor_project_user.py (1)
20-23: No external foreign keys referenceprojectuser; drop is safe.
backend/app/alembic/versions/585395bb6e5c_refactor_project_user.py
Outdated
Show resolved
Hide resolved
backend/app/alembic/versions/585395bb6e5c_refactor_project_user.py
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/alembic/versions/93d484f5798e_refactor_project_user.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use type hints in Python code (Python 3.11+ project)
Files:
backend/app/alembic/versions/93d484f5798e_refactor_project_user.py
🪛 GitHub Actions: AI Platform CI
backend/app/alembic/versions/93d484f5798e_refactor_project_user.py
[error] 1-1: Black formatting failed: the file was reformatted by Black during the pre-commit hook. Run 'pre-commit run --all-files' locally or 'black' on the file to fix formatting issues.
🔇 Additional comments (2)
backend/app/alembic/versions/93d484f5798e_refactor_project_user.py (2)
22-22: No migration or backup detected for droppedprojectusertable
Ensure any existing data is either backed up or migrated before dropping this table, or confirm that data loss is intentional; the downgrade can only recreate the schema, not restore data.
28-40: No unique constraint to restore
The original migrations do not define a UniqueConstraint on (project_id, user_id); the downgrade correctly recreates the prior schema.Likely an incorrect or invalid review comment.
| def upgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.drop_table('projectuser') | ||
| # ### end Alembic commands ### |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add return type hint.
As per coding guidelines, type hints should be used in Python code for this Python 3.11+ project. Add -> None return type hint to the function.
Apply this diff:
-def upgrade():
+def upgrade() -> None:Based on coding guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def upgrade(): | |
| # ### commands auto generated by Alembic - please adjust! ### | |
| op.drop_table('projectuser') | |
| # ### end Alembic commands ### | |
| def upgrade() -> None: | |
| # ### commands auto generated by Alembic - please adjust! ### | |
| op.drop_table('projectuser') | |
| # ### end Alembic commands ### |
🤖 Prompt for AI Agents
In backend/app/alembic/versions/93d484f5798e_refactor_project_user.py around
lines 20 to 23, the upgrade() function is missing a return type hint; update the
function signature to include the Python 3.11+ type hint "-> None" so it reads
as an upgrade() -> None definition, keeping the existing body unchanged and
ensuring imports or other code are not modified.
| def downgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.create_table('projectuser', | ||
| sa.Column('id', sa.INTEGER(), autoincrement=True, nullable=False), | ||
| sa.Column('project_id', sa.INTEGER(), autoincrement=False, nullable=False), | ||
| sa.Column('is_admin', sa.BOOLEAN(), autoincrement=False, nullable=False), | ||
| sa.Column('is_deleted', sa.BOOLEAN(), autoincrement=False, nullable=False), | ||
| sa.Column('inserted_at', postgresql.TIMESTAMP(), server_default=sa.text('now()'), autoincrement=False, nullable=False), | ||
| sa.Column('updated_at', postgresql.TIMESTAMP(), autoincrement=False, nullable=False), | ||
| sa.Column('deleted_at', postgresql.TIMESTAMP(), autoincrement=False, nullable=True), | ||
| sa.Column('user_id', sa.INTEGER(), autoincrement=False, nullable=False), | ||
| sa.ForeignKeyConstraint(['project_id'], ['project.id'], name='projectuser_project_id_fkey', ondelete='CASCADE'), | ||
| sa.ForeignKeyConstraint(['user_id'], ['user.id'], name='projectuser_user_id_fkey', ondelete='CASCADE'), | ||
| sa.PrimaryKeyConstraint('id', name='projectuser_pkey') | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add return type hint.
As per coding guidelines, type hints should be used in Python code for this Python 3.11+ project. Add -> None return type hint to the function.
Apply this diff:
-def downgrade():
+def downgrade() -> None:Based on coding guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def downgrade(): | |
| # ### commands auto generated by Alembic - please adjust! ### | |
| op.create_table('projectuser', | |
| sa.Column('id', sa.INTEGER(), autoincrement=True, nullable=False), | |
| sa.Column('project_id', sa.INTEGER(), autoincrement=False, nullable=False), | |
| sa.Column('is_admin', sa.BOOLEAN(), autoincrement=False, nullable=False), | |
| sa.Column('is_deleted', sa.BOOLEAN(), autoincrement=False, nullable=False), | |
| sa.Column('inserted_at', postgresql.TIMESTAMP(), server_default=sa.text('now()'), autoincrement=False, nullable=False), | |
| sa.Column('updated_at', postgresql.TIMESTAMP(), autoincrement=False, nullable=False), | |
| sa.Column('deleted_at', postgresql.TIMESTAMP(), autoincrement=False, nullable=True), | |
| sa.Column('user_id', sa.INTEGER(), autoincrement=False, nullable=False), | |
| sa.ForeignKeyConstraint(['project_id'], ['project.id'], name='projectuser_project_id_fkey', ondelete='CASCADE'), | |
| sa.ForeignKeyConstraint(['user_id'], ['user.id'], name='projectuser_user_id_fkey', ondelete='CASCADE'), | |
| sa.PrimaryKeyConstraint('id', name='projectuser_pkey') | |
| ) | |
| def downgrade() -> None: | |
| # ### commands auto generated by Alembic - please adjust! ### | |
| op.create_table('projectuser', | |
| sa.Column('id', sa.INTEGER(), autoincrement=True, nullable=False), | |
| sa.Column('project_id', sa.INTEGER(), autoincrement=False, nullable=False), | |
| sa.Column('is_admin', sa.BOOLEAN(), autoincrement=False, nullable=False), | |
| sa.Column('is_deleted', sa.BOOLEAN(), autoincrement=False, nullable=False), | |
| sa.Column('inserted_at', postgresql.TIMESTAMP(), server_default=sa.text('now()'), autoincrement=False, nullable=False), | |
| sa.Column('updated_at', postgresql.TIMESTAMP(), autoincrement=False, nullable=False), | |
| sa.Column('deleted_at', postgresql.TIMESTAMP(), autoincrement=False, nullable=True), | |
| sa.Column('user_id', sa.INTEGER(), autoincrement=False, nullable=False), | |
| sa.ForeignKeyConstraint(['project_id'], ['project.id'], name='projectuser_project_id_fkey', ondelete='CASCADE'), | |
| sa.ForeignKeyConstraint(['user_id'], ['user.id'], name='projectuser_user_id_fkey', ondelete='CASCADE'), | |
| sa.PrimaryKeyConstraint('id', name='projectuser_pkey') | |
| ) |
🤖 Prompt for AI Agents
In backend/app/alembic/versions/93d484f5798e_refactor_project_user.py around
lines 26 to 40, the downgrade function is missing a return type hint; update the
function signature to include the Python 3.11+ return annotation "-> None"
(i.e., change "def downgrade():" to "def downgrade() -> None:"), leaving the
body unchanged so the function conforms to project typing guidelines.
… projectuser table creation
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
backend/app/alembic/versions/93d484f5798e_refactor_project_user.py (2)
20-20: Add return type hint.As per coding guidelines, type hints should be used in Python code for this Python 3.11+ project. Add
-> Nonereturn type hint to the function.Based on coding guidelines.
26-26: Add return type hint.As per coding guidelines, type hints should be used in Python code for this Python 3.11+ project. Add
-> Nonereturn type hint to the function.Based on coding guidelines.
🧹 Nitpick comments (2)
backend/app/alembic/versions/93d484f5798e_refactor_project_user.py (2)
10-10: Consider removing unused import.The
sqlmodel.sql.sqltypesimport appears to be unused in this migration file, as the migration only uses standard SQLAlchemy types.Apply this diff to remove the unused import:
from alembic import op import sqlalchemy as sa -import sqlmodel.sql.sqltypes from sqlalchemy.dialects import postgresql
26-61: Note: Downgrade recreates structure but not data.The downgrade function recreates the
projectusertable structure with all columns and constraints, but it cannot restore any data that was lost during the upgrade. If a downgrade is ever needed, the table will be empty and may require manual data restoration from backups.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/alembic/versions/93d484f5798e_refactor_project_user.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use type hints in Python code (Python 3.11+ project)
Files:
backend/app/alembic/versions/93d484f5798e_refactor_project_user.py
⏰ 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)
🔇 Additional comments (1)
backend/app/alembic/versions/93d484f5798e_refactor_project_user.py (1)
20-23: Verify data backup before deploying this migration.The upgrade drops the
projectusertable, resulting in permanent data loss. Ensure that:
- This data is no longer needed, or
- A backup has been created if the data might be required for future reference
The downgrade path recreates the table structure but cannot restore the data.
Do you need assistance creating a data export script before running this migration?
3 participants
Summary
Target issue is #392
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