Skip to content
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

fix(autofix): Properly check for existing installation and code mappings #71162

Merged
merged 2 commits into from
May 20, 2024

Conversation

malwilley
Copy link
Member

While running through this, I found an issue with how the setup check is looking at the integration, so made a couple changes:

  1. Pass project in to the integration check so we can find code mappings for that project and error if not there
  2. Checks for the integration's installation instead of the organization integration, which I think is more correct
  3. Stop checking for whether it is disabled since Autofix only really needs the code mappings

@malwilley malwilley requested a review from a team as a code owner May 20, 2024 16:04
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 20, 2024
@@ -119,10 +114,6 @@ def get(self, request: Request, group: Group) -> Response:

return Response(
{
"subprocessorConsent": {
Copy link
Member Author

Choose a reason for hiding this comment

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

I also went ahead and removed this part since we don't actually need it anymore, the genAIConsent flag will be off in this instance and the settings page will alert you that configuration is necessary

return "integration_missing"

if integration.status != ObjectStatus.ACTIVE:
return "integration_inactive"
code_mappings = get_sorted_code_mapping_configs(project)
Copy link
Member

Choose a reason for hiding this comment

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

Oh didn't know this util existed, should be using it for how the other autofix endpoints gets the code mappings too. I'll make a quick refactor

@malwilley malwilley enabled auto-merge (squash) May 20, 2024 16:15
Copy link

codecov bot commented May 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.88%. Comparing base (dfc0962) to head (98520fb).
Report is 3 commits behind head on master.

Current head 98520fb differs from pull request most recent head c5e05f8

Please upload reports for the commit c5e05f8 to get more accurate results.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #71162       +/-   ##
===========================================
+ Coverage   56.76%   77.88%   +21.11%     
===========================================
  Files        6519     6528        +9     
  Lines      290530   290824      +294     
  Branches    50279    50329       +50     
===========================================
+ Hits       164933   226515    +61582     
+ Misses     120851    58067    -62784     
- Partials     4746     6242     +1496     
Files Coverage Δ
.../sentry/api/endpoints/group_autofix_setup_check.py 88.88% <100.00%> (+36.25%) ⬆️

... and 1956 files with indirect coverage changes

@malwilley malwilley merged commit 88094c1 into master May 20, 2024
48 checks passed
@malwilley malwilley deleted the malwilley/fix/autofix-setup-integration-fix branch May 20, 2024 17:03
cmanallen pushed a commit that referenced this pull request May 21, 2024
…ngs (#71162)

While running through this, I found an issue with how the setup check is
looking at the integration, so made a couple changes:

1. Pass `project` in to the integration check so we can find code
mappings for that project and error if not there
2. Checks for the integration's installation instead of the organization
integration, which I think is more correct
3. Stop checking for whether it is disabled since Autofix only really
needs the code mappings
@github-actions github-actions bot locked and limited conversation to collaborators Jun 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants