Skip to content

ref(browser reporting): Use option to control Reporting-Endpoints header #93560

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

Merged

Conversation

lobsterkatie
Copy link
Member

This switches our check for whether to add the Reporting-Endpoints header from using staff.is_active (which seems to be false even if the user has is_staff = True in the database) to using a new option, issues.browser_reporting.reporting_endpoints_header_enabled.

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

codecov bot commented Jun 13, 2025

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   #93560       +/-   ##
===========================================
+ Coverage   46.30%   88.00%   +41.69%     
===========================================
  Files       10306    10322       +16     
  Lines      594236   594964      +728     
  Branches    23141    23141               
===========================================
+ Hits       275175   523608   +248433     
+ Misses     318568    70863   -247705     
  Partials      493      493               

@lobsterkatie lobsterkatie force-pushed the kmclb-use-option-for-adding-reporting-endpoints-header branch from 46da06e to c8b92a1 Compare June 13, 2025 22:37
@lobsterkatie lobsterkatie marked this pull request as ready for review June 13, 2025 23:01
@lobsterkatie lobsterkatie requested a review from a team as a code owner June 13, 2025 23:01
@armenzg armenzg merged commit 74dfdaf into master Jun 16, 2025
64 checks passed
@armenzg armenzg deleted the kmclb-use-option-for-adding-reporting-endpoints-header branch June 16, 2025 12:48
Copy link

sentry-io bot commented Jun 16, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ RuntimeError: Database access not allowed, use the "django_db" mark, or the "db" or "transactional_db" fixtures to enable it. pytest.runtest.protocol tests/sentry/api/endpoi... View Issue

Did you find this useful? React with a 👍 or 👎

@asottile-sentry asottile-sentry added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Jun 16, 2025
@asottile-sentry
Copy link
Member

reverting: this is breaking CI

@getsentry-bot
Copy link
Contributor

PR reverted: c57c013

getsentry-bot added a commit that referenced this pull request Jun 16, 2025
…oints` header (#93560)"

This reverts commit 74dfdaf.

Co-authored-by: asottile-sentry <[email protected]>
armenzg pushed a commit that referenced this pull request Jun 16, 2025
…eader (#93560)

This switches our check for whether to add the `Reporting-Endpoints`
header from using `staff.is_active` (which seems to be false even if the
user has `is_staff = True` in the database) to using a new option,
`issues.browser_reporting.reporting_endpoints_header_enabled`.
armenzg added a commit that referenced this pull request Jun 18, 2025
)

Based on @lobsterkatie 's work (#93560).

Only set the `Reporting-Endpoints` header if the option
`issues.browser_reporting.reporting_endpoints_header_enabled` is set
(since checking for `staff.is_active` is not working as expected.

Handle when the DB is not accessible by the request.

---------

Co-authored-by: Katie Byers <[email protected]>
billyvg pushed a commit that referenced this pull request Jun 18, 2025
…eader (#93560)

This switches our check for whether to add the `Reporting-Endpoints`
header from using `staff.is_active` (which seems to be false even if the
user has `is_staff = True` in the database) to using a new option,
`issues.browser_reporting.reporting_endpoints_header_enabled`.
billyvg pushed a commit that referenced this pull request Jun 18, 2025
…oints` header (#93560)"

This reverts commit 74dfdaf.

Co-authored-by: asottile-sentry <[email protected]>
andrewshie-sentry pushed a commit that referenced this pull request Jun 19, 2025
…eader (#93560)

This switches our check for whether to add the `Reporting-Endpoints`
header from using `staff.is_active` (which seems to be false even if the
user has `is_staff = True` in the database) to using a new option,
`issues.browser_reporting.reporting_endpoints_header_enabled`.
andrewshie-sentry pushed a commit that referenced this pull request Jun 19, 2025
…oints` header (#93560)"

This reverts commit 74dfdaf.

Co-authored-by: asottile-sentry <[email protected]>
andrewshie-sentry pushed a commit that referenced this pull request Jun 19, 2025
)

Based on @lobsterkatie 's work (#93560).

Only set the `Reporting-Endpoints` header if the option
`issues.browser_reporting.reporting_endpoints_header_enabled` is set
(since checking for `staff.is_active` is not working as expected.

Handle when the DB is not accessible by the request.

---------

Co-authored-by: Katie Byers <[email protected]>
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 Trigger: Revert Add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants