-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(prevent): send consent URL when no-consent #98571
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
531b791
to
91cab2c
Compare
ticket: prevent-275 There are some assumptions in these changes: 1. The URL for the settings of an org depends on the `sentry_org.slug`. 2. `github_org_name` is not necessarily the same as `sentry_org.slug` 3. Constructing such URL is non-trivial without the base URL for the Sentry instance 4. It's OK to return _any_ of the valid sentry orgs for a prevent-AI request So based on those assumptions these changes include the URL for the org settings such that we can slap that URL in the "no consent given" message - that appears if you request `@sentry review` in a PR without giving the consent. We want this to add a clearer Call To Action in that message, making it more useful
91cab2c
to
27aa7bc
Compare
for org_integration in org_integrations: | ||
try: | ||
org = Organization.objects.get(id=org_integration.organization_id) | ||
if _can_use_prevent_ai_features(org): | ||
return {"consent": True} | ||
# If this is the last org we will return this URL as the consent URL | ||
consent_url = org.absolute_url("/settings/organization/") |
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.
Bug: Non-deterministic URL and Inconsistent Return Structure
This function's consent_url
is non-deterministic when multiple organizations lack consent, as its value depends on the arbitrary processing order of integrations. Additionally, the return structure is inconsistent, including consent_url
only when consent
is False
, which may cause issues for callers.
ticket: prevent-275 There are some assumptions in these changes: 1. The URL for the settings of an org depends on the `sentry_org.slug`. 2. `github_org_name` is not necessarily the same as `sentry_org.slug` 3. Constructing such URL is non-trivial without the base URL for the Sentry instance 4. It's OK to return _any_ of the valid sentry orgs for a prevent-AI request So based on those assumptions these changes include the URL for the org settings such that we can slap that URL in the "no consent given" message - that appears if you request `@sentry review` in a PR without giving the consent. We want this to add a clearer Call To Action in that message, making it more useful **Note:** Point 4 in particular means that we might want to limit the `consent_url` return to cases where there's exactly 1 org found.
ticket: prevent-275
There are some assumptions in these changes:
sentry_org.slug
.github_org_name
is not necessarily the same assentry_org.slug
So based on those assumptions these changes include the URL for the org settings such that we can slap that URL in the "no consent given" message - that appears if you request
@sentry review
in a PR without giving the consent. We want this to add a clearer Call To Action in that message, making it more usefulNote: Point 4 in particular means that we might want to limit the
consent_url
return to cases where there's exactly 1 org found.