-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
ref(code review): add date_updated and date_added columns to RepositorySettings #106977
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This PR has a migration; here is the generated SQL for for --
-- Add field date_added to repositorysettings
--
ALTER TABLE "sentry_repositorysettings" ADD COLUMN "date_added" timestamp with time zone DEFAULT (STATEMENT_TIMESTAMP()) NOT NULL;
--
-- Add field date_updated to repositorysettings
--
ALTER TABLE "sentry_repositorysettings" ADD COLUMN "date_updated" timestamp with time zone DEFAULT (STATEMENT_TIMESTAMP()) NOT NULL; |
cc7c2c4 to
8e9d743
Compare
src/sentry/integrations/api/endpoints/organization_repository_settings.py
Outdated
Show resolved
Hide resolved
src/sentry/integrations/api/endpoints/organization_repository_settings.py
Outdated
Show resolved
Hide resolved
src/sentry/integrations/api/endpoints/organization_repository_settings.py
Outdated
Show resolved
Hide resolved
1cc7571 to
0517e7c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| default=list, | ||
| ) | ||
| date_added = models.DateTimeField(db_default=Now()) | ||
| date_updated = models.DateTimeField(db_default=Now(), auto_now=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
date_updated not updated during bulk upsert operations
Medium Severity
The new date_updated field uses auto_now=True, but auto_now only triggers on .save() calls. The existing OrganizationRepositorySettingsEndpoint uses bulk_create with update_conflicts=True and doesn't include date_updated in update_fields. For existing rows updated via this bulk endpoint, date_updated will remain unchanged, defeating the purpose of tracking update times "for easier debugging." The db_default=Now() only applies to INSERT operations, not UPDATE operations in the upsert.
followup to #106977 Add `"date_updated"` to `update_fields` when bulk creating repo settings. Otherwise the value will not update.
…rySettings (#106977) fixes CW-683 Add `date_updated` and `date_added` columns to `RepositorySettings` for easier debugging. Also, remove `test_1016_remove_on_command_phrase_trigger.py` as it was failing due to using a direct import of `RepositorySettings`, and we don't need this test anymore.
followup to #106977 Add `"date_updated"` to `update_fields` when bulk creating repo settings. Otherwise the value will not update.
fixes CW-683
Add
date_updatedanddate_addedcolumns toRepositorySettingsfor easier debugging.Also, remove
test_1016_remove_on_command_phrase_trigger.pyas it was failing due to using a direct import ofRepositorySettings, and we don't need this test anymore.