Skip to content

chore(github): Add business+ plan check for multi org #91905

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Christinarlong
Copy link
Contributor

@Christinarlong Christinarlong commented May 19, 2025

This PR

  • Adds the organizations:integrations-scm-multi-org flag to the permanent list of feature flags (there is a getsentry PR too to add the flag to business plans)
  • Passes the plan check result to the organization selection UI
  • Checks the installer's plan before binding the chosen_installation_id (validate the user has access to multi-org feature)

Instead of having multiple conditions for the organizations:github-multi-org flag, we will use the organizations:integrations-scm-multi-org to validate that the feature can only be accessed by business+ plans and organizations:github-multi-org will control which orgs can access the feature.

PR for FF in plans for getsentry: https://github.com/getsentry/getsentry/pull/17517
PR for UI changes: #92036

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 19, 2025
Copy link

codecov bot commented May 19, 2025

⚠️ Parser warning

The parser emitted a warning. Please review your JUnit XML file:

Warning while parsing testcase attributes: Limit of string is 1000 chars, for name, we got 2083 at 1:156718 in /home/runner/work/sentry/sentry/.artifacts/pytest.junit.xml

@Christinarlong Christinarlong changed the title chore(github): Add gh mo to permanent ff list chore(github): Add business+ plan check for multi org May 21, 2025
@Christinarlong Christinarlong marked this pull request as ready for review May 21, 2025 17:13
@Christinarlong Christinarlong requested review from a team as code owners May 21, 2025 17:13
Comment on lines 123 to 128
FeatureDescription(
"""
Connect multiple Sentry organizations to a single GitHub account.
""",
IntegrationFeatures.SCM_MULTI_ORG,
),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we usually feature flag adding this feature description? I was looking at some other PRs that added FeatureDescriptions and they weren't ever feature flagged (??)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm okay waiting until GA to add this, since it's not available to everyone and will show up on the GH integration features page

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed listing the feature in the integration page, we'll do this at GA time

@@ -936,7 +951,10 @@ def dispatch(self, request: HttpRequest, pipeline: Pipeline) -> HttpResponseBase
return self.render_react_view(
request=request,
pipeline_name="githubInstallationSelect",
props={"installation_info": installation_info},
props={
"installation_info": installation_info,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if they don't have multi org, should we still be passing the full list of installations to the FE?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should also error if somebody tries to pass a chosen installation id that's not -1 and they don't have multi org

Copy link
Contributor Author

@Christinarlong Christinarlong May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the passing the full list comment, I think it's useful to keep in case we want to do the per gh org upsell in the dropdown in the UI.

features.has(
"organizations:integrations-scm-multi-org",
organization=self.active_user_organization.organization,
actor=request.user,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qq: whats the purpose of passing user here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I probably yoinked this from a previous feature flag checker, but looking further yeah we dont need to be passing the user/it wont be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants