feat(code-review): Handle auto enable code review on repoCreated #104666
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #104666 +/- ##
===========================================
+ Coverage 79.21% 80.55% +1.34%
===========================================
Files 9364 9335 -29
Lines 401865 400792 -1073
Branches 25834 25610 -224
===========================================
+ Hits 318352 322876 +4524
+ Misses 83071 77468 -5603
- Partials 442 448 +6 |
src/sentry/models/repository.py
Outdated
| if OrganizationOption.objects.get_value( | ||
| organization=instance.organization_id, | ||
| key="sentry:auto_enable_code_review", | ||
| default=False, | ||
| ): | ||
| triggers = OrganizationOption.objects.get_value( | ||
| organization=instance.organization_id, | ||
| key="sentry:default_code_review_triggers", | ||
| default=[], | ||
| ) |
There was a problem hiding this comment.
Pointing out that we're only going to save triggers when the default is to enable code review. So we won't have extra rows representing repo's that might never have code-review enabled.
Therefore: If the default is Not to automatically enable code-review for a this repo, then we don't save a record into out RepositorySettings table.
Instead, later the user can still manually enable code-review for this repo, and at that time, in the UI, default trigger setting will be presented to the user. 💯
There was a problem hiding this comment.
Yup correct, that was my intent - good summary. I wanted to minimize the number of writes we're doing to this new table and err on the side of if there is no row, no user has ever actively chosen a setting.
I thought this paradigm would give us more flexibility in the future to change any programmatic default behavior
|
Pinged integrations team for review, too - will merge after that! |
src/sentry/models/repository.py
Outdated
| handle_auto_enable_code_review(instance) | ||
|
|
||
|
|
||
| post_save.connect(on_repository_created, sender=Repository, weak=False) |
There was a problem hiding this comment.
After talking through this a bit more with @kcons, I think we should move this to override the Repository model's save method, ensuring both of these models are created within a transaction. It's a bit safer, and it's a pattern we follow for our outboxes, which have similar concerns.
There was a problem hiding this comment.
That makes sense to me. Making the changes now!
For seer launch, there's a toggle where an org can turn on auto-enable code review on repos. As in sentry org turns this toggle on, they add a new repo in github that has sentry github app installed on it, that means we should consider this repo enabled for code review.
For new repos, they get written to
sentry_repositorytable upon repositoryCreated gh webhook, or if the user manually adds the repo using sentry UI.Instead of doing this by finding every existing repo.objects.create (and remembering to do it if any new cases arise), probably most durable to add as a signal. There are downsides to using signals but it seems like a common pattern across sentry codebase to use these.
Add that signal handler in this PR
Closes https://linear.app/getsentry/issue/ENG-6094/handle-auto-enable-code-review-org-setting
Depends on #104645