Skip to content

Commit d708fba

Browse files
committed
fix(app-platform): Render unpublished integrations
A bug was causing Issue pages in Orgs that had an unpublished Integration installed to error out. This was only case for one or two Orgs, as far as I can tell (our test Orgs). This fixes that bug and refactors some related things: - Retrieve SentryAppInstallations from Store instead of passing down through components - Handle when the "give me all SentryApps" and "give me all MY SentryApps" endpoints return the same records
1 parent 996bc67 commit d708fba

File tree

6 files changed

+140
-72
lines changed

6 files changed

+140
-72
lines changed

src/sentry/static/sentry/app/actionCreators/sentryAppComponents.jsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import SentryAppComponentsActions from 'app/actions/sentryAppComponentActions';
33
export function fetchSentryAppComponents(api, orgSlug, projectId) {
44
const componentsUri = `/organizations/${orgSlug}/sentry-app-components/?projectId=${projectId}`;
55

6-
const promise = api.requestPromise(componentsUri);
7-
promise.then(res => SentryAppComponentsActions.loadComponents(res));
8-
return promise;
6+
return api.requestPromise(componentsUri).then(res => {
7+
SentryAppComponentsActions.loadComponents(res);
8+
return res;
9+
});
910
}

src/sentry/static/sentry/app/components/group/externalIssuesList.jsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,8 @@ class ExternalIssueList extends AsyncComponent {
123123

124124
return components.map(component => {
125125
const {sentryApp} = component;
126-
127126
const installation = sentryAppInstallations.find(
128-
i => i.sentryApp.uuid === sentryApp.uuid
127+
i => i.app.uuid === sentryApp.uuid
129128
);
130129

131130
const issue = (externalIssues || []).find(i => i.serviceType == sentryApp.slug);

src/sentry/static/sentry/app/components/group/sidebar.jsx

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ const GroupSidebar = createReactClass({
2727
group: SentryTypes.Group,
2828
event: SentryTypes.Event,
2929
environments: PropTypes.arrayOf(SentryTypes.Environment),
30-
sentryAppInstallations: PropTypes.array,
3130
},
3231

3332
getInitialState() {
@@ -228,13 +227,7 @@ const GroupSidebar = createReactClass({
228227
},
229228

230229
render() {
231-
const {
232-
group,
233-
organization,
234-
project,
235-
environments,
236-
sentryAppInstallations,
237-
} = this.props;
230+
const {group, organization, project, environments} = this.props;
238231
const projectId = project.slug;
239232

240233
return (
@@ -254,7 +247,6 @@ const GroupSidebar = createReactClass({
254247
group={this.props.group}
255248
project={project}
256249
orgId={organization.slug}
257-
sentryAppInstallations={sentryAppInstallations}
258250
/>
259251
</ErrorBoundary>
260252

src/sentry/static/sentry/app/stores/sentryAppStore.jsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import Reflux from 'reflux';
2+
import {uniqBy} from 'lodash';
23

34
const SentryAppStore = Reflux.createStore({
45
init() {
@@ -11,7 +12,14 @@ const SentryAppStore = Reflux.createStore({
1112

1213
load(items) {
1314
this.items = items;
14-
this.trigger(items);
15+
this.deDup();
16+
this.trigger(this.items);
17+
},
18+
19+
add(...apps) {
20+
apps.forEach(app => this.items.push(app));
21+
this.deDup();
22+
this.trigger(this.items);
1523
},
1624

1725
get(slug) {
@@ -21,6 +29,10 @@ const SentryAppStore = Reflux.createStore({
2129
getAll() {
2230
return this.items;
2331
},
32+
33+
deDup() {
34+
this.items = uniqBy(this.items, i => i.uuid);
35+
},
2436
});
2537

2638
export default SentryAppStore;

src/sentry/static/sentry/app/utils/fetchSentryAppInstallations.jsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,17 @@ import SentryAppStore from 'app/stores/sentryAppStore';
33

44
const fetchSentryAppInstallations = (api, orgSlug) => {
55
const sentryAppsUri = '/sentry-apps/';
6+
const ownedSentryAppsUri = `/organizations/${orgSlug}/sentry-apps/`;
67
const installsUri = `/organizations/${orgSlug}/sentry-app-installations/`;
78

89
function updateSentryAppStore(sentryApps) {
910
SentryAppStore.load(sentryApps);
1011
}
1112

13+
function fetchOwnedSentryApps() {
14+
api.requestPromise(ownedSentryAppsUri).then(apps => SentryAppStore.add(...apps));
15+
}
16+
1217
function fetchInstalls() {
1318
api
1419
.requestPromise(installsUri)
@@ -28,6 +33,7 @@ const fetchSentryAppInstallations = (api, orgSlug) => {
2833
api
2934
.requestPromise(sentryAppsUri)
3035
.then(updateSentryAppStore)
36+
.then(fetchOwnedSentryApps)
3137
.then(fetchInstalls);
3238
};
3339

tests/js/spec/views/organizationGroupDetails/groupEventDetails.spec.jsx

Lines changed: 115 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,7 @@ describe('groupEventDetails', () => {
1212
let group;
1313
let event;
1414

15-
beforeEach(() => {
16-
const props = initializeOrg();
17-
org = props.organization;
18-
project = props.project;
19-
project.organization = org;
20-
routerContext = props.routerContext;
21-
22-
group = TestStubs.Group();
23-
event = TestStubs.Event({
24-
size: 1,
25-
dateCreated: '2019-03-20T00:00:00.000Z',
26-
errors: [],
27-
entries: [],
28-
tags: [{key: 'environment', value: 'dev'}],
29-
});
30-
15+
const mockGroupApis = () => {
3116
MockApiClient.addMockResponse({
3217
url: `/issues/${group.id}/`,
3318
body: group,
@@ -67,12 +52,36 @@ describe('groupEventDetails', () => {
6752
url: `/groups/${group.id}/integrations/`,
6853
body: [],
6954
});
55+
};
56+
57+
beforeEach(() => {
58+
const props = initializeOrg();
59+
org = props.organization;
60+
project = props.project;
61+
project.organization = org;
62+
routerContext = props.routerContext;
63+
64+
group = TestStubs.Group();
65+
event = TestStubs.Event({
66+
size: 1,
67+
dateCreated: '2019-03-20T00:00:00.000Z',
68+
errors: [],
69+
entries: [],
70+
tags: [{key: 'environment', value: 'dev'}],
71+
});
72+
73+
mockGroupApis();
7074

7175
MockApiClient.addMockResponse({
7276
url: '/sentry-apps/',
7377
body: [],
7478
});
7579

80+
MockApiClient.addMockResponse({
81+
url: `/organizations/${org.slug}/sentry-apps/`,
82+
body: [],
83+
});
84+
7685
MockApiClient.addMockResponse({
7786
url: `/organizations/${org.slug}/sentry-app-installations/`,
7887
body: [],
@@ -195,51 +204,100 @@ describe('groupEventDetails', () => {
195204
});
196205
});
197206

198-
it('loads Sentry Apps', () => {
199-
const request = MockApiClient.addMockResponse({
200-
url: '/sentry-apps/',
201-
body: [],
207+
describe('Platform Integrations', () => {
208+
let wrapper; // eslint-disable-line
209+
let integrationsRequest;
210+
let orgIntegrationsRequest;
211+
let componentsRequest;
212+
213+
const mountWrapper = () => {
214+
return mount(
215+
<GroupEventDetails
216+
api={new MockApiClient()}
217+
group={group}
218+
project={project}
219+
organization={org}
220+
environments={[{id: '1', name: 'dev', displayName: 'Dev'}]}
221+
params={{orgId: org.slug, groupId: group.id, eventId: '1'}}
222+
location={{query: {environment: 'dev'}}}
223+
/>,
224+
routerContext
225+
);
226+
};
227+
228+
beforeEach(() => {
229+
const integration = TestStubs.SentryApp();
230+
const unpublishedIntegration = TestStubs.SentryApp({status: 'unpublished'});
231+
const internalIntegration = TestStubs.SentryApp({status: 'internal'});
232+
233+
const unpublishedInstall = TestStubs.SentryAppInstallation({
234+
app: {
235+
slug: unpublishedIntegration.slug,
236+
uuid: unpublishedIntegration.uuid,
237+
},
238+
});
239+
240+
const internalInstall = TestStubs.SentryAppInstallation({
241+
app: {
242+
slug: internalIntegration.slug,
243+
uuid: internalIntegration.uuid,
244+
},
245+
});
246+
247+
const component = TestStubs.SentryAppComponent({
248+
sentryApp: {
249+
uuid: unpublishedIntegration.uuid,
250+
slug: unpublishedIntegration.slug,
251+
name: unpublishedIntegration.name,
252+
},
253+
});
254+
255+
MockApiClient.clearMockResponses();
256+
mockGroupApis();
257+
258+
MockApiClient.addMockResponse({
259+
url: `/projects/${org.slug}/${project.slug}/events/1/`,
260+
body: event,
261+
});
262+
263+
componentsRequest = MockApiClient.addMockResponse({
264+
url: `/organizations/${org.slug}/sentry-app-components/?projectId=${project.id}`,
265+
body: [component],
266+
});
267+
268+
MockApiClient.addMockResponse({
269+
url: `/projects/${org.slug}/${project.slug}/events/1/`,
270+
body: event,
271+
});
272+
273+
integrationsRequest = MockApiClient.addMockResponse({
274+
url: '/sentry-apps/',
275+
body: [integration],
276+
});
277+
278+
MockApiClient.addMockResponse({
279+
url: `/organizations/${org.slug}/sentry-app-installations/`,
280+
body: [unpublishedInstall, internalInstall],
281+
});
282+
283+
orgIntegrationsRequest = MockApiClient.addMockResponse({
284+
url: `/organizations/${org.slug}/sentry-apps/`,
285+
body: [unpublishedIntegration, internalIntegration],
286+
});
287+
288+
wrapper = mountWrapper();
202289
});
203290

204-
project.organization = org;
205-
206-
mount(
207-
<GroupEventDetails
208-
api={new MockApiClient()}
209-
group={group}
210-
project={project}
211-
organization={org}
212-
environments={[{id: '1', name: 'dev', displayName: 'Dev'}]}
213-
params={{}}
214-
location={{}}
215-
/>,
216-
routerContext
217-
);
218-
219-
expect(request).toHaveBeenCalledTimes(1);
220-
});
221-
222-
it('loads sentry app components when flagged in', () => {
223-
const request = MockApiClient.addMockResponse({
224-
url: `/organizations/${org.slug}/sentry-app-components/?projectId=${project.id}`,
225-
body: [],
291+
it('loads Integrations', () => {
292+
expect(integrationsRequest).toHaveBeenCalled();
226293
});
227294

228-
project.organization = org;
229-
230-
mount(
231-
<GroupEventDetails
232-
api={new MockApiClient()}
233-
group={group}
234-
project={project}
235-
organization={org}
236-
environments={[{id: '1', name: 'dev', displayName: 'Dev'}]}
237-
params={{}}
238-
location={{}}
239-
/>,
240-
routerContext
241-
);
295+
it('loads unpublished and internal Integrations', () => {
296+
expect(orgIntegrationsRequest).toHaveBeenCalled();
297+
});
242298

243-
expect(request).toHaveBeenCalledTimes(1);
299+
it('loads Integration UI components', () => {
300+
expect(componentsRequest).toHaveBeenCalled();
301+
});
244302
});
245303
});

0 commit comments

Comments
 (0)