diff --git a/package.json b/package.json index f55b7a9565f6b9..93adda6c878fe5 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "@sentry/integrations": "5.6.0-beta.4", "@types/echarts": "^4.1.10", "@types/classnames": "^2.2.0", + "@types/jest": "^24.0.17", "@types/jquery": "^2.0.53", "@types/lodash": "^4.14.134", "@types/moment-timezone": "^0.5.12", diff --git a/src/sentry/static/sentry/app/__mocks__/api.jsx b/src/sentry/static/sentry/app/__mocks__/api.tsx similarity index 55% rename from src/sentry/static/sentry/app/__mocks__/api.jsx rename to src/sentry/static/sentry/app/__mocks__/api.tsx index 67f0258bcfd9e3..c17162ae13c698 100644 --- a/src/sentry/static/sentry/app/__mocks__/api.jsx +++ b/src/sentry/static/sentry/app/__mocks__/api.tsx @@ -1,8 +1,10 @@ -const RealClient = require.requireActual('app/api'); +import * as ImportedClient from 'app/api'; + +const RealClient: typeof ImportedClient = jest.requireActual('app/api'); export class Request {} -const respond = (isAsync, fn, ...args) => { +const respond = (isAsync: boolean, fn, ...args): void => { if (fn) { if (isAsync) { setTimeout(() => fn(...args), 1); @@ -16,8 +18,25 @@ const DEFAULT_MOCK_RESPONSE_OPTIONS = { predicate: () => true, }; +const mergeMock = jest.fn(); + +type ResponseType = JQueryXHR & { + url: string; + statusCode: number; + method: string; + callCount: 0; + body: any; + headers: {[key: string]: string}; +}; + class Client { - static mockResponses = []; + static mockResponses: Array< + [ + ResponseType, + jest.Mock, + (url: string, options: Readonly) => boolean + ] + > = []; static clearMockResponses() { Client.mockResponses = []; @@ -42,8 +61,8 @@ class Client { return mock; } - static findMockResponse(url, options) { - return Client.mockResponses.find(([response, mock, predicate]) => { + static findMockResponse(url: string, options: Readonly) { + return Client.mockResponses.find(([response, _mock, predicate]) => { const matchesURL = url === response.url; const matchesMethod = (options.method || 'GET') === response.method; const matchesPredicate = predicate(url, options); @@ -61,8 +80,9 @@ class Client { static mockAsync = false; - wrapCallback(id, error) { + wrapCallback(_id, error) { return (...args) => { + // @ts-ignore if (this.hasProjectBeenRenamed(...args)) { return; } @@ -70,24 +90,33 @@ class Client { }; } - requestPromise(path, {includeAllArgs, ...options} = {}) { + requestPromise( + path, + { + includeAllArgs, + ...options + }: {includeAllArgs?: boolean} & Readonly = {} + ) { return new Promise((resolve, reject) => { this.request(path, { ...options, success: (data, ...args) => { includeAllArgs ? resolve([data, ...args]) : resolve(data); }, - error: (error, ...args) => { + error: (error, ..._args) => { reject(error); }, }); }); } - request(url, options) { - const [response, mock] = Client.findMockResponse(url, options) || []; + request(url, options: Readonly = {}) { + const [response, mock] = Client.findMockResponse(url, options) || [ + undefined, + undefined, + ]; - if (!response) { + if (!response || !mock) { // Endpoints need to be mocked throw new Error( `No mocked response found for request:\n\t${options.method || 'GET'} ${url}` @@ -103,17 +132,33 @@ class Client { if (response.statusCode !== 200) { response.callCount++; - const resp = { - status: response.statusCode, - responseText: JSON.stringify(body), - responseJSON: body, - }; + + const deferred = $.Deferred(); + + const errorResponse: JQueryXHR = Object.assign( + { + status: response.statusCode, + responseText: JSON.stringify(body), + responseJSON: body, + }, + { + overrideMimeType: () => {}, + abort: () => {}, + then: () => {}, + error: () => {}, + }, + deferred, + new XMLHttpRequest() + ); this.handleRequestError( { + id: '1234', path: url, requestOptions: options, }, - resp + errorResponse, + 'error', + 'error' ); } else { response.callCount++; @@ -131,15 +176,18 @@ class Client { respond(Client.mockAsync, options.complete); } -} -Client.prototype.handleRequestError = RealClient.Client.prototype.handleRequestError; -Client.prototype.uniqueId = RealClient.Client.prototype.uniqueId; -Client.prototype.bulkUpdate = RealClient.Client.prototype.bulkUpdate; -Client.prototype._chain = RealClient.Client.prototype._chain; -Client.prototype._wrapRequest = RealClient.Client.prototype._wrapRequest; -Client.prototype.hasProjectBeenRenamed = - RealClient.Client.prototype.hasProjectBeenRenamed; -Client.prototype.merge = RealClient.Client.prototype.merge; + hasProjectBeenRenamed = RealClient.Client.prototype.hasProjectBeenRenamed; + handleRequestError = RealClient.Client.prototype.handleRequestError; + bulkUpdate = RealClient.Client.prototype.bulkUpdate; + _chain = RealClient.Client.prototype._chain; + _wrapRequest = RealClient.Client.prototype._wrapRequest; + + merge(params, options) { + mergeMock(params, options); + + return RealClient.Client.prototype.merge.call(this, params, options); + } +} -export {Client}; +export {Client, mergeMock}; diff --git a/src/sentry/static/sentry/app/actionCreators/events.tsx b/src/sentry/static/sentry/app/actionCreators/events.tsx index bc2c754c1d6569..cfceaad0eb2143 100644 --- a/src/sentry/static/sentry/app/actionCreators/events.tsx +++ b/src/sentry/static/sentry/app/actionCreators/events.tsx @@ -1,3 +1,4 @@ +import {Client} from 'app/api'; import {canIncludePreviousPeriod} from 'app/views/events/utils/canIncludePreviousPeriod'; import {getPeriod} from 'app/utils/getPeriod'; import {EventsStats, Organization} from 'app/types'; @@ -33,8 +34,7 @@ type Options = { * @param {String} options.query Search query */ export const doEventsRequest = ( - // TODO(ts): Update when we type `app/api` - api: any, + api: Client, { organization, project, diff --git a/src/sentry/static/sentry/app/api.jsx b/src/sentry/static/sentry/app/api.tsx similarity index 66% rename from src/sentry/static/sentry/app/api.jsx rename to src/sentry/static/sentry/app/api.tsx index 2cf6e0d9c890dd..75e399042cde99 100644 --- a/src/sentry/static/sentry/app/api.jsx +++ b/src/sentry/static/sentry/app/api.tsx @@ -14,7 +14,10 @@ import GroupActions from 'app/actions/groupActions'; import createRequestError from 'app/utils/requestError/createRequestError'; export class Request { - constructor(xhr) { + alive: boolean; + xhr: JQueryXHR; + + constructor(xhr: JQueryXHR) { this.xhr = xhr; this.alive = true; } @@ -26,12 +29,35 @@ export class Request { } } +type ParamsType = { + itemIds?: Array; + query?: string; + environment?: string | null; + project?: Array | null; +}; + +type QueryArgs = + | { + query: string; + environment?: string; + project?: Array; + } + | { + id: Array; + environment?: string; + project?: Array; + } + | { + environment?: string; + project?: Array; + }; + /** * Converts input parameters to API-compatible query arguments * @param params */ -export function paramsToQueryArgs(params) { - const p = params.itemIds +export function paramsToQueryArgs(params: ParamsType): QueryArgs { + const p: QueryArgs = params.itemIds ? {id: params.itemIds} // items matching array of itemids : params.query ? {query: params.query} // items matching search query @@ -58,8 +84,30 @@ export function paramsToQueryArgs(params) { return p; } +// TODO: move this somewhere +type APIRequestMethod = 'POST' | 'GET' | 'DELETE' | 'PUT'; + +type FunctionCallback = (...args: Args) => void; + +type RequestCallbacks = { + success?: (data: any, textStatus?: string, xhr?: JQueryXHR) => void; + complete?: (jqXHR: JQueryXHR, textStatus: string) => void; + // TODO(ts): Update this when sentry is mostly migrated to TS + error?: FunctionCallback; +}; + +export type RequestOptions = { + method?: APIRequestMethod; + data?: any; + query?: Array | object; + preservedError?: Error; +} & RequestCallbacks; + export class Client { - constructor(options) { + baseUrl: string; + activeRequests: {[ids: string]: Request}; + + constructor(options: {baseUrl?: string} = {}) { if (isUndefined(options)) { options = {}; } @@ -71,7 +119,8 @@ export class Client { * Check if the API response says project has been renamed. * If so, redirect user to new project slug */ - hasProjectBeenRenamed(response) { + // TODO: refine this type later + hasProjectBeenRenamed(response: JQueryXHR) { const code = get(response, 'responseJSON.detail.code'); // XXX(billy): This actually will never happen because we can't intercept the 302 @@ -86,8 +135,12 @@ export class Client { return true; } - wrapCallback(id, func, cleanup) { - return (...args) => { + wrapCallback( + id: string, + func: FunctionCallback | undefined, + cleanup: boolean = false + ) { + return (...args: T) => { const req = this.activeRequests[id]; if (cleanup === true) { delete this.activeRequests[id]; @@ -96,6 +149,7 @@ export class Client { if (req && req.alive) { // Check if API response is a 302 -- means project slug was renamed and user // needs to be redirected + // @ts-ignore if (this.hasProjectBeenRenamed(...args)) { return; } @@ -113,13 +167,22 @@ export class Client { /** * Attempt to cancel all active XHR requests */ - clear() { + clear(): void { for (const id in this.activeRequests) { this.activeRequests[id].cancel(); } } - handleRequestError({id, path, requestOptions}, response, ...responseArgs) { + handleRequestError( + { + id, + path, + requestOptions, + }: {id: string; path: string; requestOptions: Readonly}, + response: JQueryXHR, + textStatus: string, + errorThrown: string + ) { const code = get(response, 'responseJSON.detail.code'); const isSudoRequired = code === SUDO_REQUIRED || code === SUPERUSER_REQUIRED; @@ -129,18 +192,18 @@ export class Client { sudo: code === SUDO_REQUIRED, retryRequest: () => { return this.requestPromise(path, requestOptions) - .then((...args) => { + .then(data => { if (typeof requestOptions.success !== 'function') { return; } - requestOptions.success(...args); + requestOptions.success(data); }) - .catch((...args) => { + .catch(err => { if (typeof requestOptions.error !== 'function') { return; } - requestOptions.error(...args); + requestOptions.error(err); }); }, onClose: () => { @@ -155,14 +218,17 @@ export class Client { } // Call normal error callback - const errorCb = this.wrapCallback(id, requestOptions.error); + const errorCb = this.wrapCallback<[JQueryXHR, string, string]>( + id, + requestOptions.error + ); if (typeof errorCb !== 'function') { return; } - errorCb(response, ...responseArgs); + errorCb(response, textStatus, errorThrown); } - request(path, options = {}) { + request(path: string, options: Readonly = {}): Request { const method = options.method || (options.data ? 'POST' : 'GET'); let data = options.data; @@ -170,7 +236,7 @@ export class Client { data = JSON.stringify(data); } - let query; + let query: string; try { query = $.param(options.query || [], true); } catch (err) { @@ -182,10 +248,10 @@ export class Client { throw err; } - const id = uniqueId(); + const id: string = uniqueId(); metric.mark(`api-request-start-${id}`); - let fullUrl; + let fullUrl: string; if (path.indexOf(this.baseUrl) === -1) { fullUrl = this.baseUrl + path; } else { @@ -218,8 +284,7 @@ export class Client { headers: { Accept: 'application/json; charset=utf-8', }, - success: (...args) => { - const [, , xhr] = args || []; + success: (responseData: any, textStatus: string, xhr: JQueryXHR) => { metric.measure({ name: 'app.api.request-success', start: `api-request-start-${id}`, @@ -228,11 +293,14 @@ export class Client { }, }); if (!isUndefined(options.success)) { - this.wrapCallback(id, options.success)(...args); + this.wrapCallback<[any, string, JQueryXHR]>(id, options.success)( + responseData, + textStatus, + xhr + ); } }, - error: (...args) => { - const [resp] = args || []; + error: (resp: JQueryXHR, textStatus: string, errorThrown: string) => { metric.measure({ name: 'app.api.request-error', start: `api-request-start-${id}`, @@ -248,15 +316,15 @@ export class Client { const errorObjectToUse = createRequestError( resp, preservedError.stack, - options.method, + method, path ); errorObjectToUse.removeFrames(2); // Setting this to warning because we are going to capture all failed requests - scope.setLevel('warning'); - scope.setTag('http.statusCode', resp.status); + scope.setLevel(Sentry.Severity.Warning); + scope.setTag('http.statusCode', String(resp.status)); Sentry.captureException(errorObjectToUse); }); @@ -266,12 +334,17 @@ export class Client { path, requestOptions: options, }, - ...args + resp, + textStatus, + errorThrown ); }, - complete: (...args) => { + complete: (jqXHR: JQueryXHR, textStatus: string) => { Sentry.finishSpan(requestSpan); - return this.wrapCallback(id, options.complete, true)(...args); + return this.wrapCallback<[JQueryXHR, string]>(id, options.complete, true)( + jqXHR, + textStatus + ); }, }) ); @@ -279,7 +352,17 @@ export class Client { return this.activeRequests[id]; } - requestPromise(path, {includeAllArgs, ...options} = {}) { + requestPromise( + path: string, + { + includeAllArgs, + ...options + }: {includeAllArgs?: IncludeAllArgsType} & Readonly = {} + ): Promise< + IncludeAllArgsType extends true + ? [any, string | undefined, JQueryXHR | undefined] + : any + > { // Create an error object here before we make any async calls so // that we have a helpful stacktrace if it errors // @@ -292,10 +375,10 @@ export class Client { this.request(path, { ...options, preservedError, - success: (data, ...args) => { - includeAllArgs ? resolve([data, ...args]) : resolve(data); + success: (data, textStatus, xhr) => { + includeAllArgs ? resolve([data, textStatus, xhr] as any) : resolve(data); }, - error: (resp, ...args) => { + error: (resp: JQueryXHR) => { const errorObjectToUse = createRequestError( resp, preservedError.stack, @@ -312,16 +395,24 @@ export class Client { }); } - _chain(...funcs) { - funcs = funcs.filter(f => !isUndefined(f) && f); - return (...args) => { - funcs.forEach(func => { + _chain(...funcs: Array<((...args: Args) => any) | undefined>) { + const filteredFuncs = funcs.filter( + (f): f is (...args: Args) => any => { + return typeof f === 'function'; + } + ); + return (...args: Args): void => { + filteredFuncs.forEach(func => { func.apply(funcs, args); }); }; } - _wrapRequest(path, options, extraParams) { + _wrapRequest( + path: string, + options: RequestOptions, + extraParams: RequestCallbacks + ): Request { if (isUndefined(extraParams)) { extraParams = {}; } @@ -333,13 +424,16 @@ export class Client { return this.request(path, options); } - bulkDelete(params, options) { - const path = params.projectId + bulkDelete( + params: ParamsType & {orgId: string; projectId?: string}, + options: RequestCallbacks + ): Request { + const path: string = params.projectId ? `/projects/${params.orgId}/${params.projectId}/issues/` : `/organizations/${params.orgId}/issues/`; - const query = paramsToQueryArgs(params); - const id = uniqueId(); + const query: QueryArgs = paramsToQueryArgs(params); + const id: string = uniqueId(); GroupActions.delete(id, params.itemIds); @@ -359,13 +453,21 @@ export class Client { ); } - bulkUpdate(params, options) { - const path = params.projectId + bulkUpdate( + params: ParamsType & { + orgId: string; + projectId?: string; + failSilently?: boolean; + data?: any; + }, + options: RequestCallbacks + ): Request { + const path: string = params.projectId ? `/projects/${params.orgId}/${params.projectId}/issues/` : `/organizations/${params.orgId}/issues/`; - const query = paramsToQueryArgs(params); - const id = uniqueId(); + const query: QueryArgs = paramsToQueryArgs(params); + const id: string = uniqueId(); GroupActions.update(id, params.itemIds, params.data); @@ -386,13 +488,19 @@ export class Client { ); } - merge(params, options) { - const path = params.projectId + merge( + params: ParamsType & { + orgId: string; + projectId?: string; + }, + options: RequestCallbacks + ): Request { + const path: string = params.projectId ? `/projects/${params.orgId}/${params.projectId}/issues/` : `/organizations/${params.orgId}/issues/`; - const query = paramsToQueryArgs(params); - const id = uniqueId(); + const query: QueryArgs = paramsToQueryArgs(params); + const id: string = uniqueId(); GroupActions.merge(id, params.itemIds); diff --git a/src/sentry/static/sentry/app/components/asyncComponent.tsx b/src/sentry/static/sentry/app/components/asyncComponent.tsx index 2151ced43ba4e4..94709d42deaae6 100644 --- a/src/sentry/static/sentry/app/components/asyncComponent.tsx +++ b/src/sentry/static/sentry/app/components/asyncComponent.tsx @@ -163,7 +163,7 @@ export default class AsyncComponent< document.removeEventListener('visibilitychange', this.visibilityReloader); } - api: any; + api: Client = new Client(); private _measurement: any; // XXX: cant call this getInitialState as React whines diff --git a/src/sentry/static/sentry/app/views/events/utils/eventsRequest.tsx b/src/sentry/static/sentry/app/views/events/utils/eventsRequest.tsx index 6a519a7eda6557..feee2d3e41bb16 100644 --- a/src/sentry/static/sentry/app/views/events/utils/eventsRequest.tsx +++ b/src/sentry/static/sentry/app/views/events/utils/eventsRequest.tsx @@ -5,6 +5,7 @@ import React from 'react'; import {Organization, EventsStats, EventsStatsData} from 'app/types'; import {Series, SeriesDataUnit} from 'app/types/echarts'; +import {Client} from 'app/api'; import {addErrorMessage} from 'app/actionCreators/indicator'; import {canIncludePreviousPeriod} from 'app/views/events/utils/canIncludePreviousPeriod'; import {doEventsRequest} from 'app/actionCreators/events'; @@ -28,8 +29,7 @@ type RenderProps = { }; type EventsRequestPartialProps = { - // TODO(ts): Update when we type `app/api` - api: object; + api: Client; organization: Organization; project?: number[]; diff --git a/src/sentry/static/sentry/app/views/settings/projectIncidentRules/actions.ts b/src/sentry/static/sentry/app/views/settings/projectIncidentRules/actions.ts index d960b2e9201a71..7d542235bf704f 100644 --- a/src/sentry/static/sentry/app/views/settings/projectIncidentRules/actions.ts +++ b/src/sentry/static/sentry/app/views/settings/projectIncidentRules/actions.ts @@ -1,8 +1,9 @@ +import {Client} from 'app/api'; import {IncidentRule} from './types'; -// TODO(ts): type api and response +// TODO(ts): type response export function deleteRule( - api: any, + api: Client, orgId: string, projectId: string, rule: IncidentRule diff --git a/tests/js/spec/components/__snapshots__/createProject.spec.jsx.snap b/tests/js/spec/components/__snapshots__/createProject.spec.jsx.snap index 9895aff0fca951..26bf291176173c 100644 --- a/tests/js/spec/components/__snapshots__/createProject.spec.jsx.snap +++ b/tests/js/spec/components/__snapshots__/createProject.spec.jsx.snap @@ -92,7 +92,15 @@ exports[`CreateProject should block if you have access to no teams 1`] = ` exports[`CreateProject should deal with incorrect platform name if its provided by url 1`] = ` Object { @@ -102,7 +108,13 @@ exports[`FormField + model renders with Form 1`] = ` Object { @@ -247,7 +259,13 @@ exports[`FormField + model renders with Form 1`] = ` Object { diff --git a/tests/js/spec/components/projects/__snapshots__/bookmarkStar.spec.jsx.snap b/tests/js/spec/components/projects/__snapshots__/bookmarkStar.spec.jsx.snap index 70161680b6c1a5..0c812a74824543 100644 --- a/tests/js/spec/components/projects/__snapshots__/bookmarkStar.spec.jsx.snap +++ b/tests/js/spec/components/projects/__snapshots__/bookmarkStar.spec.jsx.snap @@ -44,7 +44,15 @@ exports[`BookmarkStar renders 1`] = ` } > Projects