Skip to content

Commit

Permalink
chore(sentry apps): Add backwards compatbility for SentryAppCompenent…
Browse files Browse the repository at this point in the history
… errors (#81480)

In (#81167 ), we want to change the SentryAppComponent endpoint to
return a string of the error to improve end user clarity when debugging.
In the frontend, we currently expect the error response to be a boolean
though, so we need to make the frontend backwards compatible (i.e handle
both boolean and string)
  • Loading branch information
Christinarlong authored Dec 3, 2024
1 parent 3fe00a0 commit 293a435
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 4 deletions.
4 changes: 3 additions & 1 deletion static/app/components/feedback/list/useHasLinkedIssues.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {ExternalIssueComponent} from 'sentry/components/group/externalIssuesList/types';
import useIssueTrackingFilter from 'sentry/components/group/externalIssuesList/useIssueTrackingFilter';
import {sentryAppComponentIsDisabled} from 'sentry/components/sentryAppComponentIcon';
import SentryAppInstallationStore from 'sentry/stores/sentryAppInstallationsStore';
import {useLegacyStore} from 'sentry/stores/useLegacyStore';
import type {Event} from 'sentry/types/event';
Expand All @@ -23,7 +24,8 @@ export default function useExternalIssueData({group, event, project}: Props) {
const renderSentryAppIssues = (): ExternalIssueComponent[] => {
return components
.map<ExternalIssueComponent | null>(component => {
const {sentryApp, error: disabled} = component;
const {sentryApp} = component;
const disabled = sentryAppComponentIsDisabled(component);
const installation = sentryAppInstallations.find(
i => i.app.uuid === sentryApp.uuid
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type {ExternalIssueComponent} from 'sentry/components/group/externalIssue
import {useExternalIssues} from 'sentry/components/group/externalIssuesList/useExternalIssues';
import useFetchIntegrations from 'sentry/components/group/externalIssuesList/useFetchIntegrations';
import useIssueTrackingFilter from 'sentry/components/group/externalIssuesList/useIssueTrackingFilter';
import {sentryAppComponentIsDisabled} from 'sentry/components/sentryAppComponentIcon';
import SentryAppInstallationStore from 'sentry/stores/sentryAppInstallationsStore';
import {useLegacyStore} from 'sentry/stores/useLegacyStore';
import type {Event} from 'sentry/types/event';
Expand Down Expand Up @@ -71,7 +72,8 @@ export default function useExternalIssueData({group, event, project}: Props) {
const renderSentryAppIssues = (): ExternalIssueComponent[] => {
return components
.map<ExternalIssueComponent | null>(component => {
const {sentryApp, error: disabled} = component;
const {sentryApp} = component;
const disabled = sentryAppComponentIsDisabled(component);
const installation = sentryAppInstallations.find(
i => i.app.uuid === sentryApp.uuid
);
Expand Down
24 changes: 24 additions & 0 deletions static/app/components/sentryAppComponentIcon.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import {SentryAppComponentFixture} from 'sentry-fixture/sentryAppComponent';

import {sentryAppComponentIsDisabled} from 'sentry/components/sentryAppComponentIcon';

describe('SentryAppComponentIcon', function () {
it('sentryAppComponentIsDisabled returns false if the error is a non empty string', () => {
const component = SentryAppComponentFixture();
component.error = 'RIP couldnt connect to sentry :C';
expect(sentryAppComponentIsDisabled(component)).toBe(false);
});

it('sentryAppComponentIsDisabled returns true if the error is an empty string', () => {
const component = SentryAppComponentFixture();
component.error = '';
expect(sentryAppComponentIsDisabled(component)).toBe(true);
});

// TODO: Delete after new errors are deployed
it('sentryAppComponentIsDisabled returns itself if the error is a boolean', () => {
const component = SentryAppComponentFixture();
component.error = true;
expect(sentryAppComponentIsDisabled(component)).toBe(true);
});
});
8 changes: 7 additions & 1 deletion static/app/components/sentryAppComponentIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ function SentryAppComponentIcon({sentryAppComponent, size = 20}: Props) {
({color}) => color === false
);
const isDefault = selectedAvatar?.avatarType !== 'upload';
const isDisabled = sentryAppComponent.error;
const isDisabled = sentryAppComponentIsDisabled(sentryAppComponent);

return (
<SentryAppAvatarWrapper
isDark={ConfigStore.get('theme') === 'dark'}
Expand All @@ -34,6 +35,11 @@ function SentryAppComponentIcon({sentryAppComponent, size = 20}: Props) {
);
}

// Patch for backwards compatibility as the change's truth table is inverse to the previous'
export const sentryAppComponentIsDisabled = (component: SentryAppComponent) => {
return typeof component.error === 'boolean' ? component.error : !component.error;
};

export default SentryAppComponentIcon;

const SentryAppAvatarWrapper = styled('span')<{
Expand Down
2 changes: 1 addition & 1 deletion static/app/types/integrations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ export type SentryAppComponent<
};
type: 'issue-link' | 'alert-rule-action' | 'issue-media' | 'stacktrace-link';
uuid: string;
error?: boolean;
error?: string | boolean;
};

export type SentryAppWebhookRequest = {
Expand Down

0 comments on commit 293a435

Please sign in to comment.