fix(code-review): Check if org has disabled default code review triggers

[suejung-sentry]

Fix trigger check to look within settings for seat-based-seer orgs (post GA)

Closes https://linear.app/getsentry/issue/CW-311/add-check-for-the-trigger-enabled-downstream-of-preflight

[linear]

CW-311 Add check for the trigger enabled downstream of preflight

[armenzg]

You can import this within pull_request rather than passing it down.

[suejung-sentry]

Could you clarify what you mean there?
The settings depend on org type logic that's already computed in the preflight check, and duplicating that in pull_request.py seems worse than passing it as a parameter. Also making it explicit instead of part of the kwargs seems better for type hints. So probably missing what you mean on that.

In theory "settings" can grow based on how our product's configurability grows. Right now we have pretty coarse controls of just the repo enabled or not and upon which triggers it runs on. But in the future other things we considered are like what sensitivity level of reviews you want, or what "flavor" of code review (a general code reviewer, or one more focused on bugs, or perhaps one day one more focused on performance bugs). All that is meant to be collected into this one spot of the settings object. It's true that today it's only read on pull_request events, but in my time working on this it's been asked to be added to other ones here and there (our current state only has it in pull_request tho, correct)

[armenzg]

You don't need yet another data structure. You could do this:

settings: CodeReviewSettings | None = self._repo_settings
if self._is_code_review_beta_org() or self._is_legacy_usage_based_seer_plan_org():
    settings = CodeReviewSettings(
        enabled=True,
        triggers=[
            CodeReviewTrigger.ON_NEW_COMMIT,
            CodeReviewTrigger.ON_READY_FOR_REVIEW,
        ],
    )

[ajay-sentry]

I kind of like having an explicit const denote what the default settings are. Alternatively can do what you suggested but add a comment denoting that is the default

[suejung-sentry]

I liked the explicit const too but fine with this too. Moved it down and added a comment denoting it is a default

[armenzg]

This should not be happening as part of tests set up. We need to rely on the code to do this for us.

[suejung-sentry]

This is the equivalent of the user picking these options. In the same way we are using test setup for create_repo we to do the same for create_repo_settings. If we want to go crazy we have to call the endpoint to either manually or auto-set this up from way upstream in the onboarding flow just as the user would. Seems outside the scope of what we should be testing here

[armenzg]

In order to speed up merging this PR I'm going to approve it.

I'm going to propose some changes but I first need to make sure they make sense.

[cursor]

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.}

[cursor]

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.}

[cursor]

Tests for seat-based orgs missing billing setup

Medium Severity

The new tests for seat-based orgs (seat-based-seer-enabled feature) are missing required billing infrastructure setup. For seat-based orgs, the preflight _check_billing method requires OrganizationContributors to exist and check_seer_quota to pass. Without this setup, tests expecting Seer to be called (test_pull_request_closed_bypasses_trigger_check_post_ga, test_pull_request_opened_works_when_trigger_enabled_post_ga, test_pull_request_ready_for_review_works_when_trigger_enabled_post_ga) will fail due to billing check failure. Tests expecting Seer NOT to be called may pass but for the wrong reason (billing failure instead of trigger check).

Additional Locations (1)

• tests/sentry/seer/code_review/webhooks/test_pull_request.py#L277-L292