fix(seer onboarding): make on_command_phrase CR trigger always on

[srest2021]

relates to ENG-6194

Backend counterpart to #105696

Since we are not charging by review, let's make sure the on_command_phrase trigger is always turned on in RepositorySettings and organization defaultCodeReviewTriggers.

• Update RepositorySettings.save() to add the on_command_phrase trigger if it's not present
• Update PUT in OrganizationRepositorySettingsEndpoint -- bulk_create() doesn't hit the modified save(), so we just add it to all new settings, and to all existing settings if it's not present
• Update OrganizationDetailsPutSerializer to add the on_command_phrase trigger to defaultCodeReviewTriggers if it's not present
• Migration: Backfill all existing RepositorySettings triggers and organization defaultCodeReviewTriggers to include on_command_phrase

[linear]

ENG-6194 Make "on_command_phrase" always just ON (not part of the multi select)

[github-actions]

This PR has a migration; here is the generated SQL for src/sentry/migrations/1015_alter_code_review_triggers_default.py

for 1015_alter_code_review_triggers_default in sentry

--
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #105364   +/-   ##
========================================
  Coverage   80.56%    80.57%           
========================================
  Files        9432      9432           
  Lines      404548    404557    +9     
  Branches    25675     25675           
========================================
+ Hits       325944    325954   +10     
+ Misses      78135     78134    -1     
  Partials      469       469           

[github-actions]

This PR has a migration; here is the generated SQL for src/sentry/migrations/1015_backfill_on_command_phrase_trigger.py

for 1015_backfill_on_command_phrase_trigger in sentry

--
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL

[suejung-sentry]

This logic makes sense to me. # TIL about django signals on save() not working on the bulk_create, good to know. Thanks for even thinking of doing the migration for our sparse test org data.

If this ends up a source of any difficulties or if hiding it as "hidden triggers" on frontend ends up hard, we can revisit removing it as a concept from this list and having all downstream consumers understand it is implicitly there

[markstory]

How frequent are writes to this table? There will be a small time window (a few minutes) between your backfill running and this code being deployed to all servers. Any rows created in that time window will not have on_command_phrase stored. If you have a low volume of writes you could get lucky. If you want to completely avoid missed writes you'd need to make the migration a post_deploy instead.

[srest2021]

Ok, I expect write freq to be low but if there's no downside to doing it post-deploy, I'll just do so for peace of mind!

[github-actions]

This PR has a migration; here is the generated SQL for src/sentry/migrations/1016_backfill_on_command_phrase_trigger.py

for 1016_backfill_on_command_phrase_trigger in sentry

--
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL

[evanpurkhiser]

Are we planning to remove this at some point? With this logic as written you're no longer able to turn off the ON_COMMAND_PHRASE trigger right? So presumably we're going to remove this trigger completely (make it no longer configurable), at which point this validation can go away

[evanpurkhiser]

Left a more broad note in this regard as a whole PR review comment.

[evanpurkhiser]

If the goal here is to make it such that the on_command_trigger is always enabled for users, we should instead just be removing whatever conditional is checking that defaultCodeReviewTriggers contains on_command_phrase, and then running a migration to remove all on_command_phrase's from the defaultCodeReviewTriggers from the database.

Removing conditional checks for features should result in less code not more.

[ryan953]

If the goal here is to make it such that the on_command_trigger is always enabled for users, we should instead just be removing whatever conditional is checking that defaultCodeReviewTriggers contains on_command_phrase, and then running a migration to remove all on_command_phrase's from the defaultCodeReviewTriggers from the database.

Removing conditional checks for features should result in less code not more.

Short answer is yes, we'll target removing ON_COMMAND_PHRASE from the enums and stuff soonish actually.

The constraint right now is that we have a separate repo in play: overwatch, and it would need to be updated in sync with this PR. That's not the end of the world on it's own, but we're actually pulling the functional parts of overwatch directly into sentry itself. So after a few weeks there won't be that separate repo to coordinate with. For that reason we decided to go with this approach now for now. This PR is also a template for what we'd want to do to clean things up: another migration, along with removing the API validation stuff; so we won't have to think to hard about it.