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(jira): Add UI for error message when installing in multiple in regions #84377

Merged
merged 5 commits into from
Jan 31, 2025

Conversation

Christinarlong
Copy link
Contributor

Multi-region installs are currently not allowed for the Jira integration. Show an error instead of redirecting without. Also adds integration_id to the logger, so I can match up errors easier.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 31, 2025
@Christinarlong Christinarlong marked this pull request as ready for review January 31, 2025 19:33
@Christinarlong Christinarlong requested review from a team as code owners January 31, 2025 19:33
@cathteng
Copy link
Member

do you have a screenshot of this change, before and after?

@@ -255,16 +255,11 @@ def _finish_pipeline(self, data):
extra={
"organization_id": self.organization.id if self.organization else None,
"provider_key": self.provider.key,
"integration_id": self.integration.id,
Copy link
Member

@ameliahsu ameliahsu Jan 31, 2025

Choose a reason for hiding this comment

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

Just curious, why do you need to add integration_id here to change the error?

Copy link
Contributor Author

@Christinarlong Christinarlong Jan 31, 2025

Choose a reason for hiding this comment

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

we don't this addition just makes my life easier to debug when cust are going why can't I install Jira. For example, last week, because our loggers only catch error cases, it was harder to find logs to tell him yo you have Jira already installed in your US org.

@Christinarlong
Copy link
Contributor Author

We discussed in standup because I can't recreate this due to split silo not working on my local, and this was not a high risk change (since self.error) is previously used behavior to just push through.

@ameliahsu ameliahsu requested a review from a team January 31, 2025 19:38
Copy link

codecov bot commented Jan 31, 2025

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tests/sentry/integrations/test_pipeline.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #84377   +/-   ##
=======================================
  Coverage   87.62%   87.62%           
=======================================
  Files        9611     9611           
  Lines      543591   543563   -28     
  Branches    21333    21326    -7     
=======================================
- Hits       476309   476308    -1     
+ Misses      66928    66901   -27     
  Partials      354      354           

@Christinarlong Christinarlong merged commit 6f4ad69 into master Jan 31, 2025
49 checks passed
@Christinarlong Christinarlong deleted the crl/multi-region-pipeline-error branch January 31, 2025 22:25
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