Fix: Add created_at, created_at to workspace_schedule and workspaces_… #184
Conversation
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
WalkthroughThis pull request introduces a new Changes
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Ashutosh619-sudo
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Ashutosh619-sudo
temporarily deployed
to
Staging
GitHub Actions
Inactive
Ashutosh619-sudo
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (3)
apps/workspaces/migrations/0001_initial.py(2 hunks)apps/workspaces/migrations/0007_workspacesuser_created_at_workspacesuser_updated_at.py(1 hunks)apps/workspaces/models.py(1 hunks)
🔇 Additional comments (2)
apps/workspaces/models.py (1)
51-51: LGTM: ManyToManyField through parameter is correctly configured
The through parameter is properly set to use the new WorkspacesUser model.
apps/workspaces/migrations/0007_workspacesuser_created_at_workspacesuser_updated_at.py (1)
13-22: Verify data integrity before applying migrations
Before applying these changes, verify:
- No orphaned records exist in the database that would violate the new PROTECT constraint
- All existing records have timestamp values to satisfy the non-null constraint
apps/workspaces/models.py
Outdated
| class WorkspacesUser(models.Model): | ||
| id = models.BigAutoField(primary_key=True) | ||
| workspace = models.ForeignKey('Workspace', models.DO_NOTHING) | ||
| user = models.ForeignKey('users.User', models.DO_NOTHING) | ||
| created_at = models.DateTimeField(auto_now_add=True, null=True, help_text='Created at datetime') | ||
| updated_at = models.DateTimeField(auto_now=True, null=True, help_text='Updated at datetime') | ||
|
|
||
| class Meta: | ||
| db_table = 'workspaces_user' | ||
| unique_together = (('workspace', 'user'),) | ||
|
|
There was a problem hiding this comment.
Add docstring and use PROTECT for foreign keys
The new WorkspacesUser model needs the following improvements:
- Add a docstring explaining the purpose of this intermediary model
- Use
models.PROTECTinstead ofmodels.DO_NOTHINGfor foreign keys to maintain referential integrity and be consistent with other models in the codebase
Apply this diff:
class WorkspacesUser(models.Model):
+ """
+ Intermediary model for many-to-many relationship between Workspace and User
+ """
id = models.BigAutoField(primary_key=True)
- workspace = models.ForeignKey('Workspace', models.DO_NOTHING)
- user = models.ForeignKey('users.User', models.DO_NOTHING)
+ workspace = models.ForeignKey('Workspace', models.PROTECT)
+ user = models.ForeignKey('users.User', models.PROTECT)
created_at = models.DateTimeField(auto_now_add=True, null=True, help_text='Created at datetime')
updated_at = models.DateTimeField(auto_now=True, null=True, help_text='Updated at datetime')📝 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.
| class WorkspacesUser(models.Model): | |
| id = models.BigAutoField(primary_key=True) | |
| workspace = models.ForeignKey('Workspace', models.DO_NOTHING) | |
| user = models.ForeignKey('users.User', models.DO_NOTHING) | |
| created_at = models.DateTimeField(auto_now_add=True, null=True, help_text='Created at datetime') | |
| updated_at = models.DateTimeField(auto_now=True, null=True, help_text='Updated at datetime') | |
| class Meta: | |
| db_table = 'workspaces_user' | |
| unique_together = (('workspace', 'user'),) | |
| class WorkspacesUser(models.Model): | |
| """ | |
| Intermediary model for many-to-many relationship between Workspace and User | |
| """ | |
| id = models.BigAutoField(primary_key=True) | |
| workspace = models.ForeignKey('Workspace', models.PROTECT) | |
| user = models.ForeignKey('users.User', models.PROTECT) | |
| created_at = models.DateTimeField(auto_now_add=True, null=True, help_text='Created at datetime') | |
| updated_at = models.DateTimeField(auto_now=True, null=True, help_text='Updated at datetime') | |
| class Meta: | |
| db_table = 'workspaces_user' | |
| unique_together = (('workspace', 'user'),) |
| migrations.CreateModel( | ||
| name='WorkspacesUser', | ||
| fields=[ | ||
| ('id', models.BigAutoField(primary_key=True, serialize=False)), | ||
| ('user', models.ForeignKey(on_delete=django.db.models.deletion.DO_NOTHING, to=settings.AUTH_USER_MODEL)), | ||
| ('workspace', models.ForeignKey(on_delete=django.db.models.deletion.DO_NOTHING, to='workspaces.workspace')), | ||
| ], | ||
| options={ | ||
| 'db_table': 'workspaces_user', | ||
| 'unique_together': {('workspace', 'user')}, | ||
| }, | ||
| ), | ||
| migrations.AlterField( | ||
| model_name='workspace', | ||
| name='user', | ||
| field=models.ManyToManyField(help_text='Reference to users table', through='workspaces.WorkspacesUser', to=settings.AUTH_USER_MODEL), | ||
| ), |
There was a problem hiding this comment.
Update migration to use PROTECT for foreign keys
After updating the model to use models.PROTECT, this migration needs to be regenerated to match the model changes.
Since this is the initial migration, you can:
- Delete all migration files
- Update the models.py as suggested
- Run
python manage.py makemigrationsto generate new migrations
| migrations.AddField( | ||
| model_name='workspacesuser', | ||
| name='created_at', | ||
| field=models.DateTimeField(auto_now_add=True, help_text='Created at datetime', null=True), | ||
| ), | ||
| migrations.AddField( | ||
| model_name='workspacesuser', | ||
| name='updated_at', | ||
| field=models.DateTimeField(auto_now=True, help_text='Updated at datetime', null=True), | ||
| ), |
There was a problem hiding this comment.
Consolidate migrations and make timestamps non-nullable
The timestamp fields should:
- Be part of the initial migration rather than a separate migration
- Be non-nullable to maintain consistency with other models in the codebase (see BaseModel, BaseForeignWorkspaceModel)
After updating the model in models.py, regenerate migrations with these changes:
class WorkspacesUser(models.Model):
id = models.BigAutoField(primary_key=True)
workspace = models.ForeignKey('Workspace', models.PROTECT)
user = models.ForeignKey('users.User', models.PROTECT)
- created_at = models.DateTimeField(auto_now_add=True, null=True, help_text='Created at datetime')
- updated_at = models.DateTimeField(auto_now=True, null=True, help_text='Updated at datetime')
+ created_at = models.DateTimeField(auto_now_add=True, help_text='Created at datetime')
+ updated_at = models.DateTimeField(auto_now=True, help_text='Updated at datetime')📝 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.
| migrations.AddField( | |
| model_name='workspacesuser', | |
| name='created_at', | |
| field=models.DateTimeField(auto_now_add=True, help_text='Created at datetime', null=True), | |
| ), | |
| migrations.AddField( | |
| model_name='workspacesuser', | |
| name='updated_at', | |
| field=models.DateTimeField(auto_now=True, help_text='Updated at datetime', null=True), | |
| ), | |
| migrations.AddField( | |
| model_name='workspacesuser', | |
| name='created_at', | |
| field=models.DateTimeField(auto_now_add=True, help_text='Created at datetime'), | |
| ), | |
| migrations.AddField( | |
| model_name='workspacesuser', | |
| name='updated_at', | |
| field=models.DateTimeField(auto_now=True, help_text='Updated at datetime'), | |
| ), |
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/workspaces/migrations/0009_add_create_at_to_workspaces_user_sql.py (1)
1-1: Consider revising the migration file name for consistency.Currently, the file name says “add_create_at” while the column name is “created_at.” Renaming the file to ensure consistency (e.g., “0009_add_created_at_to_workspaces_user_sql.py”) will make it more intuitive.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/workspaces/migrations/0008_remove_workspacesuser_created_at_and_more.py(1 hunks)apps/workspaces/migrations/0009_add_create_at_to_workspaces_user_sql.py(1 hunks)apps/workspaces/models.py(0 hunks)
💤 Files with no reviewable changes (1)
- apps/workspaces/models.py
✅ Files skipped from review due to trivial changes (1)
- apps/workspaces/migrations/0008_remove_workspacesuser_created_at_and_more.py
🔇 Additional comments (2)
apps/workspaces/migrations/0009_add_create_at_to_workspaces_user_sql.py (2)
18-21: Good inclusion of reverse SQL for rolling back the column.
The reverse SQL cleanly drops the column. This is consistent and will help maintain safe reverts if needed.
13-16: Ensure data correctness and confirm the default applies as intended.
This statement creates a non-nullable column with a default of NOW() for existing rows. Verify that this behavior is desired, because any pre-existing rows will automatically have the current time as their creation timestamp.
|
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
| ], | ||
| options={ | ||
| 'db_table': 'workspaces', | ||
| }, | ||
| ), | ||
| migrations.CreateModel( |
There was a problem hiding this comment.
pls revert changes in this file
| ] | ||
|
|
||
| operations = [ | ||
| migrations.RemoveField( |
There was a problem hiding this comment.
this as well - pls to remove
| @@ -30,6 +30,16 @@ def get_default_onboarding_state(): | |||
| return 'CONNECTION' | |||
|
|
|||
|
|
|||
| class WorkspacesUser(models.Model): | |||
…user table
Description
Please add PR description here, add screenshots if needed
Clickup
Please add link here
https://app.clickup.com/1864988/v/l/6-901605343641-1
Summary by CodeRabbit
New Features
created_attimestamp field for tracking record creation in the workspace user model.Bug Fixes
Documentation