From e39a1bb12ee14ff2a6904381293a32cbcfeecf26 Mon Sep 17 00:00:00 2001 From: Ben Ilegbodu Date: Mon, 7 May 2018 15:25:57 -0700 Subject: [PATCH 1/5] initial start on docs --- docs/README.md | 49 +++++++ docs/request.md | 1 + gulpfile.js | 332 ++++++++++++++++++++++++------------------------ 3 files changed, 216 insertions(+), 166 deletions(-) create mode 100644 docs/README.md create mode 100644 docs/request.md diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 0000000..0b87e39 --- /dev/null +++ b/docs/README.md @@ -0,0 +1,49 @@ +# Eventbrite JavaScript SDK Documentation + +This SDK interface closely mirors the [Eventbrite v3 REST API](https://www.eventbrite.com/developer/v3/) endpoints that it wraps. The SDK provides many conveniences for making requests and processing responses to make it easier to use in the JavaScript environment. + +## ToC + +* [Including the package](#including-the-package) +* [Configuring a SDK object](#configuring-a-sdk-object) +* [`request()`](./request) + +## Including the package + +First include the `eventbrite` package (depending on your module environment): + +### Webpack / Rollup / etc ([ECMAScript modules](https://unpkg.com/eventbrite/lib/esm/)): + +```js +import eventbrite from 'eventbrite'; +``` + +### Node / legacy dependency systems ([CommonJS](https://unpkg.com/eventbrite/lib/cjs/) / [Universal Module Definition](https://unpkg.com/eventbrite/lib/umd/)): + +```js +const eventbrite = require('eventbrite'); +``` + +### ` +``` + +NOTE: `window.Eventbrite` will be a reference to the package. + +## Configuring a SDK object + +In order to make requests, we need to configure the SDK object. + +```js +import eventbrite from 'eventbrite'; + +// Create configured Eventbrite SDK +const sdk = eventbrite({token: 'OATH_TOKEN_HERE'}); +``` + +You can configure the SDK object with the following properties; + +* `token` - The Eventbrite [OAuth token](https://www.eventbrite.com/developer/v3/api_overview/authentication/#ebapi-getting-a-token) +* `baseUrl` - The base URL prepending to endpoints when making API requests (defaults to `'https://www.eventbriteapi.com/v3'`). So when using the `'/users/me/'` endpoint, a request would be made to `https://www.eventbriteapi.com/v3/users/me/`. _NOTE: You probably will not need to use this property._ diff --git a/docs/request.md b/docs/request.md new file mode 100644 index 0000000..ba9a603 --- /dev/null +++ b/docs/request.md @@ -0,0 +1 @@ +# `request()` diff --git a/gulpfile.js b/gulpfile.js index 404f435..53a7187 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -1,199 +1,199 @@ -const gulp = require("gulp"); -const babel = require("gulp-babel"); -const uglify = require("gulp-uglify"); -const rename = require("gulp-rename"); -const sourcemaps = require("gulp-sourcemaps"); -const replace = require("gulp-replace"); -const debug = require("gulp-debug"); -const util = require("gulp-util"); - -const { rollup } = require("rollup"); -const rollupBabel = require("rollup-plugin-babel"); -const rollupNodeResolve = require("rollup-plugin-node-resolve"); -const rollupCommonjs = require("rollup-plugin-commonjs"); -const rollupJson = require("rollup-plugin-json"); -const rollupReplace = require("rollup-plugin-replace"); -const rollupUglify = require("rollup-plugin-uglify"); - -const FORMAT_ESM = "esm"; -const FORMAT_CJS = "cjs"; -const FORMAT_UMD = "umd"; - -const MODULE_NAME = "Eventbrite"; - -const SOURCE_ENTRY = "src/index.ts"; +const gulp = require('gulp'); +const babel = require('gulp-babel'); +const uglify = require('gulp-uglify'); +const rename = require('gulp-rename'); +const sourcemaps = require('gulp-sourcemaps'); +const replace = require('gulp-replace'); +const debug = require('gulp-debug'); +const util = require('gulp-util'); + +const {rollup} = require('rollup'); +const rollupBabel = require('rollup-plugin-babel'); +const rollupNodeResolve = require('rollup-plugin-node-resolve'); +const rollupCommonjs = require('rollup-plugin-commonjs'); +const rollupJson = require('rollup-plugin-json'); +const rollupReplace = require('rollup-plugin-replace'); +const rollupUglify = require('rollup-plugin-uglify'); + +const FORMAT_ESM = 'esm'; +const FORMAT_CJS = 'cjs'; +const FORMAT_UMD = 'umd'; + +const MODULE_NAME = 'Eventbrite'; + +const SOURCE_ENTRY = 'src/index.ts'; const FILES_TO_BUILD = [ - // include all the JavaScript files in src/ directory - "src/**/*.@(ts|js)", + // include all the JavaScript files in src/ directory + 'src/**/*.@(ts|js)', - // but exclude the test files - "!src/**/__tests__/**" + // but exclude the test files + '!src/**/__tests__/**', ]; // When transpiling to ES format, we still use the `env` preset // and we want everything transpiled EXCEPT modules -const ESM_ENV_PRESET = ["@babel/env", { modules: false }]; +const ESM_ENV_PRESET = ['@babel/env', {modules: false}]; // When transpiling to UMD, we need the UMD transform plugin. // Need to explicitly list the globals unfortunately const UMD_TRANSFORM_PLUGIN = [ - "@babel/plugin-transform-modules-umd", - { - globals: { - index: MODULE_NAME, - "isomorphic-fetch": "fetch" + '@babel/plugin-transform-modules-umd', + { + globals: { + index: MODULE_NAME, + 'isomorphic-fetch': 'fetch', + }, + exactGlobals: true, }, - exactGlobals: true - } ]; -const _getBabelConfig = format => ({ - babelrc: false, - - presets: [ - format === FORMAT_ESM ? ESM_ENV_PRESET : "@babel/env", - "@babel/typescript" - ], - plugins: [ - "@babel/proposal-class-properties", - "@babel/proposal-object-rest-spread", - "@babel/plugin-external-helpers", - ...(format === FORMAT_UMD ? [UMD_TRANSFORM_PLUGIN] : []) - ] -}); - -const _getBabelStream = format => - gulp - // get a stream of the files to transpile - .src(FILES_TO_BUILD) - // initialize the sourcemaps (used by UMD only) - .pipe(format === FORMAT_UMD ? sourcemaps.init() : util.noop()) - // do the appropriate babel transpile (this is a copy from package.json) - .pipe(babel(_getBabelConfig(format))); - -const _genUmd = ({ minify = false } = {}) => - _getBabelStream(FORMAT_UMD) - // If you're using UMD, you probably don't have `process.env.NODE_ENV` so, we'll replace it. - // If you're using the unminified UMD, you're probably in DEV - // If you're using the unminified UMD, you're probably in production - .pipe( - replace( - "process.env.NODE_ENV", - JSON.stringify(minify ? "production" : "development") - ) - ) - // minify the files and rename to .min.js extension (when minifying) - .pipe(minify ? uglify() : util.noop()) - .pipe(minify ? rename({ extname: ".min.js" }) : util.noop()) - .pipe(sourcemaps.write("./")) - .pipe( - debug({ - title: minify ? "Building + Minifying UMD:" : "Building UMD:" - }) - ) - .pipe(gulp.dest("lib/umd")); - -const _genDist = ({ minify = false } = {}) => - rollup({ - input: SOURCE_ENTRY, +const _getBabelConfig = (format) => ({ + babelrc: false, + presets: [ + format === FORMAT_ESM ? ESM_ENV_PRESET : '@babel/env', + '@babel/typescript', + ], plugins: [ - // Need to replace `process.env.NODE_ENV` in the bundle because most likely the place where this - // would be used doesn't support it. When minified we assume production, dev otherwise - rollupReplace({ - "process.env.NODE_ENV": JSON.stringify( - minify ? "production" : "development" + '@babel/proposal-class-properties', + '@babel/proposal-object-rest-spread', + '@babel/plugin-external-helpers', + ...(format === FORMAT_UMD ? [UMD_TRANSFORM_PLUGIN] : []), + ], +}); + +const _getBabelStream = (format) => + gulp + // get a stream of the files to transpile + .src(FILES_TO_BUILD) + // initialize the sourcemaps (used by UMD only) + .pipe(format === FORMAT_UMD ? sourcemaps.init() : util.noop()) + // do the appropriate babel transpile (this is a copy from package.json) + .pipe(babel(_getBabelConfig(format))); + +const _genUmd = ({minify = false} = {}) => + _getBabelStream(FORMAT_UMD) + // If you're using UMD, you probably don't have `process.env.NODE_ENV` so, we'll replace it. + // If you're using the unminified UMD, you're probably in DEV + // If you're using the unminified UMD, you're probably in production + .pipe( + replace( + 'process.env.NODE_ENV', + JSON.stringify(minify ? 'production' : 'development') + ) ) - }), - - // convert JSON files to ES6 modules, so they can be included in Rollup bundle - rollupJson(), - - // Locate modules using the Node resolution algorithm, for using third party modules in node_modules - rollupNodeResolve({ - // use "module" field for ES6 module if possible - module: true, - - // use (legacy) "jsnext:main" if possible - jsnext: true, - - // use "main" field or index.(ts|js) - main: true, - - // use "browser" field if possible - browser: true, - - // include typescript files as default extensions - extensions: [".ts", ".js"] - }), - - // Convert CommonJS modules to ES6 modules, so they can be included in a Rollup bundle - rollupCommonjs({ - // Node modules are the ones we're trying to get it to understand - include: "node_modules/**" - }), - - // Seamless integration between Rollup and Babel - rollupBabel( - Object.assign( - { - // don't worry about transpiling node_modules when bundling - exclude: "node_modules/**", - - // don't place helpers at the top of the files, but point to reference contained external helpers - externalHelpers: true - }, - _getBabelConfig(FORMAT_ESM) + // minify the files and rename to .min.js extension (when minifying) + .pipe(minify ? uglify() : util.noop()) + .pipe(minify ? rename({extname: '.min.js'}) : util.noop()) + .pipe(sourcemaps.write('./')) + .pipe( + debug({ + title: minify ? 'Building + Minifying UMD:' : 'Building UMD:', + }) ) - ), - - // Minify the code if that option is specified - // `false` will get filtered out below - minify && rollupUglify() - ].filter(Boolean) - }).then(bundle => { - bundle.write({ - format: FORMAT_UMD, - file: `dist/eventbrite${minify ? ".min" : ""}.js`, - - // The name to use for dist bundle - name: MODULE_NAME, - - sourcemap: true + .pipe(gulp.dest('lib/umd')); + +const _genDist = ({minify = false} = {}) => + rollup({ + input: SOURCE_ENTRY, + + plugins: [ + // Need to replace `process.env.NODE_ENV` in the bundle because most likely the place where this + // would be used doesn't support it. When minified we assume production, dev otherwise + rollupReplace({ + 'process.env.NODE_ENV': JSON.stringify( + minify ? 'production' : 'development' + ), + }), + + // convert JSON files to ES6 modules, so they can be included in Rollup bundle + rollupJson(), + + // Locate modules using the Node resolution algorithm, for using third party modules in node_modules + rollupNodeResolve({ + // use "module" field for ES6 module if possible + module: true, + + // use (legacy) "jsnext:main" if possible + jsnext: true, + + // use "main" field or index.(ts|js) + main: true, + + // use "browser" field if possible + browser: true, + + // include typescript files as default extensions + extensions: ['.ts', '.js'], + }), + + // Convert CommonJS modules to ES6 modules, so they can be included in a Rollup bundle + rollupCommonjs({ + // Node modules are the ones we're trying to get it to understand + include: 'node_modules/**', + }), + + // Seamless integration between Rollup and Babel + rollupBabel( + Object.assign( + { + // don't worry about transpiling node_modules when bundling + exclude: 'node_modules/**', + + // don't place helpers at the top of the files, but point to reference contained external helpers + externalHelpers: true, + }, + _getBabelConfig(FORMAT_ESM) + ) + ), + + // Minify the code if that option is specified + // `false` will get filtered out below + minify && rollupUglify(), + ].filter(Boolean), + }).then((bundle) => { + bundle.write({ + format: FORMAT_UMD, + file: `dist/eventbrite${minify ? '.min' : ''}.js`, + + // The window global variable name for the package + name: MODULE_NAME, + + sourcemap: true, + }); }); - }); // Used by modern dependency systems like Webpack or Rollup that can do tree-shaking -gulp.task("build:lib:esm", () => - _getBabelStream(FORMAT_ESM) - .pipe(debug({ title: "Building ESM:" })) - .pipe(gulp.dest("lib/esm")) +gulp.task('build:lib:esm', () => + _getBabelStream(FORMAT_ESM) + .pipe(debug({title: 'Building ESM:'})) + .pipe(gulp.dest('lib/esm')) ); // Used primarily by Node for resolving dependencies -gulp.task("build:lib:cjs", () => - _getBabelStream(FORMAT_CJS) - .pipe(debug({ title: "Building CJS" })) - .pipe(gulp.dest("lib/cjs")) +gulp.task('build:lib:cjs', () => + _getBabelStream(FORMAT_CJS) + .pipe(debug({title: 'Building CJS'})) + .pipe(gulp.dest('lib/cjs')) ); // Used by legacy dependency systems like requireJS -gulp.task("build:dist", () => _genDist()); -gulp.task("build:dist:min", () => _genDist({ minify: true })); +gulp.task('build:dist', () => _genDist()); +gulp.task('build:dist:min', () => _genDist({minify: true})); // Unclear what would use this over the previous 3, but keeping for now // May get removed in later releases -gulp.task("build:lib:umd", () => _genUmd()); -gulp.task("build:lib:umd:min", () => _genUmd({ minify: true })); - -gulp.task("build:lib", [ - "build:lib:esm", - "build:lib:cjs", - "build:lib:umd", - "build:lib:umd:min" +gulp.task('build:lib:umd', () => _genUmd()); +gulp.task('build:lib:umd:min', () => _genUmd({minify: true})); + +gulp.task('build:lib', [ + 'build:lib:esm', + 'build:lib:cjs', + 'build:lib:umd', + 'build:lib:umd:min', ]); -gulp.task("build", ["build:lib", "build:dist", "build:dist:min"]); +gulp.task('build', ['build:lib', 'build:dist', 'build:dist:min']); -gulp.task("default", ["build"]); +gulp.task('default', ['build']); From 8cb87f98e1b19d2c710d00a483884236ed949505 Mon Sep 17 00:00:00 2001 From: Ben Ilegbodu Date: Tue, 15 May 2018 11:42:17 -0700 Subject: [PATCH 2/5] Write request() docs and update implementation/test to follow them --- docs/README.md | 4 +- docs/request.md | 155 ++++++++++++++++++++++++++++ src/__tests__/__fixtures__/index.ts | 13 ++- src/__tests__/request.spec.ts | 129 ++++++++--------------- src/request.ts | 79 ++++++++------ 5 files changed, 261 insertions(+), 119 deletions(-) diff --git a/docs/README.md b/docs/README.md index 0b87e39..5a9b93e 100644 --- a/docs/README.md +++ b/docs/README.md @@ -43,7 +43,9 @@ import eventbrite from 'eventbrite'; const sdk = eventbrite({token: 'OATH_TOKEN_HERE'}); ``` -You can configure the SDK object with the following properties; +You can configure the SDK object with the following properties: * `token` - The Eventbrite [OAuth token](https://www.eventbrite.com/developer/v3/api_overview/authentication/#ebapi-getting-a-token) * `baseUrl` - The base URL prepending to endpoints when making API requests (defaults to `'https://www.eventbriteapi.com/v3'`). So when using the `'/users/me/'` endpoint, a request would be made to `https://www.eventbriteapi.com/v3/users/me/`. _NOTE: You probably will not need to use this property._ + +From then on, you can use `sdk` to make API requests. diff --git a/docs/request.md b/docs/request.md index ba9a603..c690a32 100644 --- a/docs/request.md +++ b/docs/request.md @@ -1 +1,156 @@ # `request()` + +`request()` is the Promise-based, low-level function for making [`fetch`](https://github.com/matthew-andrews/isomorphic-fetch) requests to the Eventbrite v3 REST API, returning responses as JSON. The higher-level convenience endpoint functions use `request()` under the hood for making their requests. We suggest that you use the convenience endpoint functions over `request()`. But there may be cases where new or updated endpoints exist withing the REST API, and the SDK has not yet been updated to provide convenience functions. + +`request()` provides additional request and response handling over `fetch`. + +For requests it: + +* Prepends a configurable base URL to the endpoint you specify (see [Configuring a SDK object](./#configuring-a-sdk-object)) +* Adds your [OAuth token](https://www.eventbrite.com/developer/v3/api_overview/authentication/#ebapi-getting-a-token) in the request `Authorization` header +* Sets the appropriate `Content-type` header depending on the `fetch` `method` (`GET`, `POST`, etc.) configuration you use +* Sets the appropriate `credentials` setting depending on the `fetch` `mode` (`cors`, etc.) configuration you use + +For responses it: + +* Returns a resolved `Promise` with the response data parsed as JSON +* If the HTTP status is in the 400 or 500 range, returns a rejected `Promise` with parsed API errors, [if applicable](#error-handling) + +## API + +The TypeScript function definition of `request()` is: + +``` +(endpoint: string, options?: RequestInit): Promise<{}> +``` + +### Parameters + +`request()` accepts the following parameters: + +* `endpoint`: The Eventbrite v3 API endpoint path, such as `/users/me/`. This will be appended to the `baseUrl` defined when [configuring the SDK object](./#configuring-a-sdk-object). +* `options`: The request initialization options that [`fetch()`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/fetch) accepts. Your [OAuth token](https://www.eventbrite.com/developer/v3/api_overview/authentication/#ebapi-getting-a-token) will be added to the request `Authorization` header for you. Some additional options you may need to pass in are: + * `options.method`: The [HTTP method](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods) (e.g. `GET`, `POST`, etc.) for the fetch. Non-`GET` requests add `application/json` as `Content-Type` by default. + * `options.mode`: The request mode (e.g. `cors`, `same-origin`, etc.) for the fetch. Defaults the `credentials` option to `include` when `mode` is `cors`. Otherwise the `credentials` default to `same-origin`. + +### Response + +The return value from `request()` is a [`Promise`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises) that contains the response from the API call. If the response is successful, the response data will be parsed as JSON. + +See the [Error Handling](#error-handling) section for more information on the default error handling that `request()` provides. + +## Examples + +The simplest approach is to use ES2015 Promises: + +```js +const eventbrite = require('eventbrite'); + +// Create configured Eventbrite SDK +const sdk = eventbrite({token: 'OATH_TOKEN_HERE'}); + +// See: https://www.eventbrite.com/developer/v3/endpoints/users/#ebapi-get-users-id +sdk.request('/users/me').then(user => { + console.log(`Hi ${user.name}!`); +}); +``` + +## Error Handling + +To make interacting with the Eventbrite API easier, the SDK handles and parses some additional errors by default. + +When an error occurs during an API request, the Eventbrite v3 API will send a response with an error HTTP status (in the 400 or 500 range), as well as a JSON response containing more information about the error: + +```json +{ + "status_code": 404, + "error_description": "The user you requested does not exist.", + "error": "NOT_FOUND" +} +``` + +The SDK recognizes that an error has occurred (by inspecting the HTTP status code) and returns a **rejected** promise with an object that contains the errored response as the `response` property and error information in the `parsedError` property. This way you can easily distinguish whether or not your API request succeeded or failed: + +```js +const eventbrite = require('eventbrite'); + +// Create configured Eventbrite SDK +const sdk = eventbrite({token: 'OATH_TOKEN_HERE'}); + +// BAD User ID +const userId = '123456789'; + +// See: https://www.eventbrite.com/developer/v3/endpoints/users/#ebapi-get-users-id +sdk + .request(`/users/${userId}`) + .then(user => { + // Successful response + console.log(`Hi ${user.name}!`); + }) + .catch(errInfo => { + // An error occurred + // Original error response is passed in `response` property + console.error(errInfo.response['error_description']); + + // `ARGUMENT_ERROR` errors are parsed into `parsedError` property + const parsedError = errorInfo.parsedError; + + // equivalent to errorInfo.response.error + console.log(parsedError.error); + + // equivalent to errorInfo.response['error_desscripion'] + console.log(parsedError.description); + }); +``` + +Read more about [Errors](https://www.eventbrite.com/developer/v3/api_overview/errors/) within the Eventbrite v3 API. + +One of the [Common Errors](https://www.eventbrite.com/developer/v3/api_overview/errors/#ebapi-common-errors) in the Eventbrite v3 API, is the `ARGUMENTS_ERROR` error (returned with `400` HTTP code). This happens when one of the parameters passed to the API call is invalid. You would get a response like: + +```json +{ + "status_code": 400, + "error_detail": { + "ARGUMENTS_ERROR": { + "status": ["INVALID"] + } + }, + "error_description": + "There are errors with your arguments: status - INVALID", + "error": "ARGUMENTS_ERROR" +} +``` + +It includes an `error_detail` property that contains additional data about the offending parameters. The SDK parses the `ARGUMENT_ERROR` data within `error_detail` adding it to the `parsedError` property in the rejected promise as the `argumentErrors` property: + +```js +const eventbrite = require('eventbrite'); + +// Create configured Eventbrite SDK +const sdk = eventbrite({token: 'OATH_TOKEN_HERE'}); + +// See: https://www.eventbrite.com/developer/v3/endpoints/users/#ebapi-get-users-id-events +sdk + .request('/users/me/events?status=blah') + .then(user => { + // Successful response + console.log(`Hi ${user.name}!`); + }) + .catch(errInfo => { + // An error occurred + // Original error response is passed in `response` property + console.error(errInfo.response['error_description']); + + // `ARGUMENT_ERROR` errors are parsed into `parsedError` property + const parsedError = errorInfo.parsedError; + + // equivalent to errorInfo.response['error_detail']['ARGUMENT_ERROR'] + console.log(parsedError.argumentErrors); + + // equivalent to errorInfo.response.error (would be "ARGUMENT_ERROR") + console.log(parsedError.error); + + // equivalent to errorInfo.response['error_desscripion'] + console.log(parsedError.description); + }); +``` diff --git a/src/__tests__/__fixtures__/index.ts b/src/__tests__/__fixtures__/index.ts index c16640f..25bc4ef 100644 --- a/src/__tests__/__fixtures__/index.ts +++ b/src/__tests__/__fixtures__/index.ts @@ -14,8 +14,15 @@ export const MOCK_USERS_ME_RESPONSE_DATA = { image_id: null as string, }; -export const MOCK_ERROR_RESPONSE_DATA = { +export const MOCK_INTERNAL_ERROR_RESPONSE_DATA = { + status_code: 500, + error: 'INTERNAL_ERROR', + error_description: 'An unhandled error occured in Eventbrite.', +}; + +export const MOCK_ARGUMENTS_ERROR_RESPOSNE_DATA = { status_code: 400, - error: 'INVALID_TEST', - error_description: 'This is an invalid test', + error_detail: {ARGUMENTS_ERROR: {status: ['INVALID']}}, + error_description: 'There are errors with your arguments: status - INVALID', + error: 'ARGUMENTS_ERROR', }; diff --git a/src/__tests__/request.spec.ts b/src/__tests__/request.spec.ts index 70984c2..4ddfca8 100644 --- a/src/__tests__/request.spec.ts +++ b/src/__tests__/request.spec.ts @@ -1,4 +1,4 @@ -import request, {checkStatus, fetchJSON, catchStatusError} from '../request'; +import request from '../request'; import { mockFetch, getMockFetch, @@ -7,45 +7,30 @@ import { } from './utils'; import { MOCK_USERS_ME_RESPONSE_DATA, - MOCK_ERROR_RESPONSE_DATA + MOCK_INTERNAL_ERROR_RESPONSE_DATA, + MOCK_ARGUMENTS_ERROR_RESPOSNE_DATA } from './__fixtures__'; const TEST_URL = 'https://www.eventbriteapi.com/v3/users/me/'; const getSuccessfulCodeRes = () => getMockResponse(MOCK_USERS_ME_RESPONSE_DATA); -const getUnsuccessfulCodeRes = () => - getMockResponse(MOCK_ERROR_RESPONSE_DATA, {status: 400}); +const getInternalErrorRes = () => + getMockResponse(MOCK_INTERNAL_ERROR_RESPONSE_DATA, {status: 500}); +const getArgumentsErrorRes = () => + getMockResponse(MOCK_ARGUMENTS_ERROR_RESPOSNE_DATA, {status: 400}); -describe('checkStatus', () => { - describe('on receiving an invalid status', () => { - const response = getUnsuccessfulCodeRes(); - - it('returns a rejected promise', async () => { - await expect(checkStatus(response)).rejects.toBe(response); - }); - }); - - describe('on receiving a valid status', () => { - it('returns a fulfilled promise', async () => { - const response = getSuccessfulCodeRes(); - - await expect(checkStatus(response)).resolves.toBe(response); - }); - }); -}); - -describe('fetchJSON', () => { +describe('request', () => { afterEach(() => { restoreMockFetch(); }); - describe('on receiving successful status code', () => { + describe('success cases', () => { beforeEach(() => { mockFetch(getSuccessfulCodeRes()); }); - it('calls fetch with appropriate defaults', async () => { - await fetchJSON(TEST_URL); + it('calls fetch and calls fetch with appropriate defaults', async () => { + await request(TEST_URL); expect(getMockFetch()).toHaveBeenCalledTimes(1); expect(getMockFetch()).toHaveBeenCalledWith( @@ -56,9 +41,10 @@ describe('fetchJSON', () => { ); }); - it('adds "application/json" content type when method is not GET', async () => { - await fetchJSON(TEST_URL, {method: 'POST', body: '{}'}); + it('calls fetch and adds "application/json" content type when method is not GET', async () => { + await request(TEST_URL, {method: 'POST', body: '{}'}); + expect(getMockFetch()).toHaveBeenCalledTimes(1); expect(getMockFetch()).toHaveBeenCalledWith( TEST_URL, expect.objectContaining({ @@ -72,8 +58,8 @@ describe('fetchJSON', () => { ); }); - it('respects overrides in options', async () => { - await fetchJSON(TEST_URL, {credentials: 'omit'}); + it('calls fetch and respects overrides in options', async () => { + await request(TEST_URL, {credentials: 'omit'}); expect(getMockFetch()).toHaveBeenCalledTimes(1); expect(getMockFetch()).toHaveBeenCalledWith( @@ -84,8 +70,8 @@ describe('fetchJSON', () => { ); }); - it('respects overrides in option headers', async () => { - await fetchJSON(TEST_URL, { + it('calls fetch and respects overrides in option headers', async () => { + await request(TEST_URL, { headers: { 'X-TEST': 'testHeader', 'X-CSRFToken': 'testCSRF', @@ -106,8 +92,8 @@ describe('fetchJSON', () => { ); }); - it('merges overrides with defaults in option headers', async () => { - await fetchJSON(TEST_URL, { + it('calls fetch and merges overrides with defaults in option headers', async () => { + await request(TEST_URL, { headers: { 'X-TEST': 'testHeader', 'X-CSRFToken': 'testCSRF', @@ -130,83 +116,58 @@ describe('fetchJSON', () => { }) ); }); - - it('should parse the response JSON', async () => { - await expect(fetchJSON(TEST_URL)).resolves.toEqual( + it('calls fetch and return parsed response JSON data', async () => { + await expect(request(TEST_URL)).resolves.toEqual( MOCK_USERS_ME_RESPONSE_DATA ); - }); - }); - - describe('on receiving an unsuccessful status code', () => { - beforeEach(() => { - mockFetch(getUnsuccessfulCodeRes()); - }); - it('should throw an error', async () => { - await expect(fetchJSON(TEST_URL)).rejects.toEqual( - expect.objectContaining({status: 400}) - ); + expect(getMockFetch()).toHaveBeenCalledTimes(1); }); }); -}); -describe('catchStatusError', () => { - describe('when response is invalid JSON', () => { - it('should reject without parsed errors, only response', async () => { + describe('error handling', () => { + it('should reject only with response when response is invalid JSON', async () => { const response = new Response('{sa;dfsdfi'); - await expect(catchStatusError(response)).rejects.toEqual({ + mockFetch(response); + + await expect(request(TEST_URL)).rejects.toEqual({ response, }); }); - }); - describe('when response is valid JSON', () => { - it('should reject with parsed errors', async () => { - const response = getUnsuccessfulCodeRes(); + it('calls fetch and rejects with parsed error when there is a status error', async () => { + const response = getInternalErrorRes(); + + mockFetch(response); - await expect(catchStatusError(response)).rejects.toEqual({ + await expect(request(TEST_URL)).rejects.toEqual({ response, parsedError: { - error: 'INVALID_TEST', - description: 'This is an invalid test', + error: MOCK_INTERNAL_ERROR_RESPONSE_DATA.error, + description: + MOCK_INTERNAL_ERROR_RESPONSE_DATA['error_description'], }, }); - }); - }); -}); - -describe('request', () => { - afterEach(() => { - restoreMockFetch(); - }); - - describe('when no status error', () => { - beforeEach(() => { - mockFetch(getSuccessfulCodeRes()); - }); - - it('calls fetch and return JSON data', async () => { - await expect(request(TEST_URL)).resolves.toEqual( - MOCK_USERS_ME_RESPONSE_DATA - ); expect(getMockFetch()).toHaveBeenCalledTimes(1); }); - }); - describe('when there is a status error', () => { - it('calls fetch and rejects with parsed error', async () => { - const response = getUnsuccessfulCodeRes(); + it('calls fetch and rejects with parsed argument errors when there is an ARGUMENT_ERROR', async () => { + const response = getArgumentsErrorRes(); mockFetch(response); await expect(request(TEST_URL)).rejects.toEqual({ response, parsedError: { - error: 'INVALID_TEST', - description: 'This is an invalid test', + error: MOCK_ARGUMENTS_ERROR_RESPOSNE_DATA.error, + description: + MOCK_ARGUMENTS_ERROR_RESPOSNE_DATA['error_description'], + argumentErrors: + MOCK_ARGUMENTS_ERROR_RESPOSNE_DATA['error_detail'][ + 'ARGUMENTS_ERROR' + ], }, }); diff --git a/src/request.ts b/src/request.ts index 144f0e4..8a45ced 100644 --- a/src/request.ts +++ b/src/request.ts @@ -5,19 +5,37 @@ import 'isomorphic-fetch'; * Return a promise that is resolved or rejected depending on the response's * status code. */ -export const checkStatus = (res: Response): Promise => { +const _checkStatus = (res: Response): Promise => { if (res.status >= 300) { + // Need to wrap the response in an object so that it matches the same error object + // returned by _catchStatusError return Promise.reject(res); } return Promise.resolve(res); }; +const _tryParseJSON = (res: Response): Promise => { + try { + return ( + res + .json() + + // if JSON cannot parse, it'll return a rejected promise instead + // of throwing an error, so we catch that rejection so that we can rejected + // with the response like everything else expects + .catch(() => Promise.reject(res)) + ); + } catch (error) { + return Promise.reject(res); + } +}; + /** * Calls fetch on provided url with default options necessary for interacting * with our JSON API. Parses the JSON, provides appropriate headers, and asserts * a valid status from the server. */ -export const fetchJSON = ( +export const _fetchJSON = ( url: string, {headers, method, mode, ...options}: RequestInit = {} ): Promise<{}> => { @@ -39,19 +57,11 @@ export const fetchJSON = ( } as RequestInit; return fetch(url, fetchOptions) - .then(checkStatus) - .then((res: Response) => { - let resJSON = {}; - - if ('json' in res) { - resJSON = res.json(); - } - - return resJSON; - }); + .then(_checkStatus) + .then(_tryParseJSON); }; -const hasArgumentsError = (responseData: JSONResponseData): boolean => +const _hasArgumentsError = (responseData: JSONResponseData): boolean => !!( responseData['error_detail'] && responseData['error_detail']['ARGUMENTS_ERROR'] @@ -79,9 +89,7 @@ const hasArgumentsError = (responseData: JSONResponseData): boolean => * } * */ -export const parseError = ( - responseData: JSONResponseData -): ParsedResponseError => { +const _parseError = (responseData: JSONResponseData): ParsedResponseError => { if (!responseData.error) { // Weird error format, return null return null; @@ -92,7 +100,7 @@ export const parseError = ( description: responseData['error_description'], } as ParsedResponseError; - if (hasArgumentsError(responseData)) { + if (_hasArgumentsError(responseData)) { error = { ...error, argumentErrors: responseData['error_detail']['ARGUMENTS_ERROR'], @@ -103,29 +111,38 @@ export const parseError = ( }; /** - * Designed to work with `checkStatus`, or any function that + * Designed to work with `_checkStatus`, or any function that * raises an error on an invalid status. The error raised should have a `response` * property with the original response object. * * Example usage: * - * fetchJSON('/api/v3/test/path', {'body': someData}) - * .catch(catchStatusError) - * .catch(({response, parsedError}) => doSomethingOnError()) - * .then(doSomethingOnSuccess); + * _fetchJSON('/api/v3/test/path', {'body': someData}) + * .catch(_catchStatusError) + * .then(doSomethingOnSuccess) + * .catch(({response, parsedError}) => doSomethingOnError()); */ -export const catchStatusError = (response: Response): Promise => +const _catchStatusError = (res: Response): Promise => new Promise((resolve, reject) => { - response - .json() - .then((responseData) => parseError(responseData)) - .then((parsedError) => reject({response, parsedError})) - .catch(() => reject({response})); + _tryParseJSON(res) + // handled error, so reject with parsed error data along with response + .then((responseData: JSONResponseData) => + reject({ + response: res, + parsedError: _parseError(responseData), + }) + ) + + // Unhandled error + .catch(() => + reject({ + response: res, + }) + ); }); /** - * fetchV3 is a simple wrapper for http/fetchJSON that parses v3 errors received - * by the API. + * Fetch wrapper that parses v3 errors received by the API */ export default (url: string, options?: RequestInit): Promise<{}> => - fetchJSON(url, options).catch(catchStatusError); + _fetchJSON(url, options).catch(_catchStatusError); From 19af030011e248760006375e8063efd1ac59351f Mon Sep 17 00:00:00 2001 From: Ben Ilegbodu Date: Tue, 15 May 2018 14:36:49 -0700 Subject: [PATCH 3/5] Handle errors for HTTP status >= 400 --- src/request.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/request.ts b/src/request.ts index 8a45ced..96cc18b 100644 --- a/src/request.ts +++ b/src/request.ts @@ -6,7 +6,7 @@ import 'isomorphic-fetch'; * status code. */ const _checkStatus = (res: Response): Promise => { - if (res.status >= 300) { + if (res.status >= 400) { // Need to wrap the response in an object so that it matches the same error object // returned by _catchStatusError return Promise.reject(res); From b6b0056da5f315687feee9268ef6bbb41f5a6382 Mon Sep 17 00:00:00 2001 From: Ben Ilegbodu Date: Wed, 16 May 2018 17:12:52 -0700 Subject: [PATCH 4/5] Fixes from @rwholey-eb --- docs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/README.md b/docs/README.md index 5a9b93e..951ed2f 100644 --- a/docs/README.md +++ b/docs/README.md @@ -34,7 +34,7 @@ NOTE: `window.Eventbrite` will be a reference to the package. ## Configuring a SDK object -In order to make requests, we need to configure the SDK object. +In order to make requests, you need to configure the SDK object. ```js import eventbrite from 'eventbrite'; From 352c69082f5226e6f21887f812f073d686b28803 Mon Sep 17 00:00:00 2001 From: Ben Ilegbodu Date: Wed, 16 May 2018 17:13:50 -0700 Subject: [PATCH 5/5] Moving PR template & Contributing changes from #33 --- .github/PULL_REQUEST_TEMPLATE.md | 16 ++++++-- CONTRIBUTING.md | 69 +++++++++++++++++--------------- 2 files changed, 49 insertions(+), 36 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index fef6ff2..5551c29 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,22 +1,30 @@ ## Description + + ## How Has This Been Tested? + + + ## Screenshots (if appropriate): ## Checklist: + + -- [ ] I have read the [**CONTRIBUTING** document](../CONTRIBUTING.md). -- [ ] I have updated the documentation accordingly. -- [ ] I have added tests to cover my changes. -- [ ] I have run yarn validate to ensure that tests, typescript and linting are all in order. + +* [ ] I have read the [**CONTRIBUTING** document](https://github.com/eventbrite/eventbrite-sdk-javascript/blob/master/CONTRIBUTING.md). +* [ ] I have updated the documentation accordingly. +* [ ] I have added tests to cover my changes. +* [ ] I have run `yarn validate` to ensure that tests, typescript and linting are all in order. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 18d2647..f2eb470 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,71 +1,76 @@ -#Contributing +# Contributing Thank you for your interest in contributing to Eventbrite's Javascript SDK! -###Table of Contents -* [Workflow](#workflow) - * [Setup](#setup) - * [Branches](#using-branches-to-submit-changes) - * [Keeping up to date](#keeping-your-local-repo-up-to-date) -* [Creating issues](#creating-issues) -* [Working on and submitting changes](#working-on-and-submitting-changes) - * [Steps to submit](#steps-to-submit) +### Table of Contents + +* [Workflow](#workflow) + * [Setup](#setup) + * [Branches](#using-branches-to-submit-changes) + * [Keeping up to date](#keeping-your-local-repo-up-to-date) +* [Creating issues](#creating-issues) +* [Working on and submitting changes](#working-on-and-submitting-changes) + * [Steps to submit](#steps-to-submit) ## Workflow ### Setup -1. This project uses [yarn](https://yarnpkg.com/en/) as a package management system. If you don't have it installed, you can follow the instructions [here](https://yarnpkg.com/lang/en/docs/install/) -1. Fork the repository. [need help?](https://help.github.com/articles/fork-a-repo/) -1. Clone your new forked repository to your local computer -1. Set the Eventbrite repository as your branches upstream branch -`git remote add upstream https://github.com/eventbrite/eventbrite-sdk-javascript.git` -1. Navigate to the root directory of your newly cloned repository -(for mac `cd /path/to/eventbrite-sdk-javascript`) -1. `yarn install` to install local dependencies +1. This project uses [yarn](https://yarnpkg.com/en/) as a package management system. If you don't have it installed, you can follow the instructions [here](https://yarnpkg.com/lang/en/docs/install/) +1. Fork the repository. [need help?](https://help.github.com/articles/fork-a-repo/) +1. Clone your new forked repository to your local computer +1. Set the Eventbrite repository as your branches upstream branch + `git remote add upstream https://github.com/eventbrite/eventbrite-sdk-javascript.git` +1. Navigate to the root directory of your newly cloned repository + (for mac `cd /path/to/eventbrite-sdk-javascript`) +1. `yarn install` to install local dependencies ### Using branches to submit changes -To work on changes to the Eventbrite repository, create a new branch on your local repository. `git checkout -b ` +To work on changes to the Eventbrite repository, create a new branch on your local repository. `git checkout -b ` ### Keeping your local repo up to date + To ensure your branch never gets out of sync with Eventbrite's master, ensure that you have your upstream set properly (see the [Setup](#setup) step) -1. `git checkout master` (you may have to [stash or commit][stash-docs] your local changes if on a new branch) -1. `git pull upstream master` -1. `git checkout ` -1. `git rebase master` -1. If you've stashed changes, [unstash][stash-docs] them now, otherwise your branch should now be up to date +1. `git checkout master` (you may have to [stash or commit][stash-docs] your local changes if on a new branch) +1. `git pull upstream master` +1. `git checkout ` +1. `git rebase master` +1. If you've stashed changes, [unstash][stash-docs] them now, otherwise your branch should now be up to date Always try to keep your master 'clean' by only pulling changes directly from upstream into your master branch and rebasing those changes onto your working branch. It is always a good idea to pull the upstream branch in to your master branch before creating a new feature branch to work from. This will minimize the chances of encountering merge conflicts. ## Creating Issues + Create issues to file bugs, changes, and proposals. -Before opening a new issue, please [search][issues] to see if there has been previous discussion about the same feature or issue. If so, please contribute to the discussion there. +Before opening a new issue, please [search][issues] to see if there has been previous discussion about the same feature or issue. If so, please contribute to the discussion there. If nothing is found, feel free to [open a new issue][issues] and fill out the issue template to the best of your ability. ## Working on and submitting changes -When starting on improvements or new features that are non-trivial, it is always a good idea to first discuss the changes you wish to implement by [opening a github issue][issues] before getting started. + +When starting on improvements or new features that are non-trivial, it is always a good idea to first discuss the changes you wish to implement by [opening a github issue][issues] before getting started. If you've found a bug or feature you'd like to work on in our [github issue tracker][issues], please comment on the issue to let others know that you'd like to work on it. -While implementing fixes, please try to change as little code as possible. This helps speed up the review process and helps diminish the chance of additional bugs. +While implementing fixes, please try to change as little code as possible. This helps speed up the review process and helps diminish the chance of additional bugs. Please try to conform to the coding style of the code base. ###Steps to submit: -1. Please ensure that your changes are fully covered by one or more unit test(s). -1. Check to make sure that your changes are documented properly (inline comments for interesting lines, READMEs, etc.) -1. Run `yarn validate` to ensure that all tests pass, the linter is satisfied and your changes are typescript compliant. -1. PR titles must be prefixed by the type of changes the PR contains followed by the scope of what the pr touches. We are following the [angular commit guidelines](https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines). Please use one of `feat, fix, docs, style, refactor, perf, test, chore` as the prefix. The scope is the the direct product your changes affect. Example: `chore(build): Add encrypted ssh key for semantic-release` because its a chore and it touches the build. - * For multiple scope items, you can comma separate 2 or 3 but if there are more than that please use a `*` instead. -1. Please use a [closing issue keyword](https://help.github.com/articles/closing-issues-using-keywords/) to indicate the issue that your fix addresses in the description section of the pull request template. Example: `fixes #32` to close issue #32 +1. Please ensure that your changes are fully covered by one or more unit test(s). +1. Check to make sure that your changes are documented properly (inline comments for interesting lines, READMEs, etc.) +1. Run `yarn validate` to ensure that all tests pass, the linter is satisfied and your changes are typescript compliant. +1. PR titles must be prefixed by the type of changes the PR contains followed by the scope of what the pr touches. We are following the [angular commit guidelines](https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines). Please use one of `feat, fix, docs, style, refactor, perf, test, chore` as the prefix. The scope is the the direct product your changes affect. Example: `chore(build): Add encrypted ssh key for semantic-release` because its a chore and it touches the build. + +* For multiple scope items, you can comma separate 2 or 3 but if there are more than that please use a `*` instead. +1. Please use a [closing issue keyword](https://help.github.com/articles/closing-issues-using-keywords/) to indicate the issue that your fix addresses in the description section of the pull request template. Example: `fixes #32` to close issue #32 [issues]: https://github.com/eventbrite/eventbrite-sdk-javascript/issues [stash-docs]: https://git-scm.com/book/en/v1/Git-Tools-Stashing