Skip to content

Commit

Permalink
fix (regression): GET and PUT requests should send params in URI quer…
Browse files Browse the repository at this point in the history
…y string
  • Loading branch information
citizensas committed Apr 12, 2017
1 parent 41a5edc commit f5ec9bd
Show file tree
Hide file tree
Showing 21 changed files with 271 additions and 222 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ coverage

# dist
dist
typings
es
61 changes: 50 additions & 11 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,53 @@
2.0.2 - April 11, 2017



2.0.1 - April 4, 2017



2.0.0 - April 4, 2017


2.0.2 - April 12, 2017

* 2.0.2 (Sassoun Derderian)
* chore: update package keywords (Sassoun Derderian)
* fix: adding dist/ folder in npm package (Sassoun Derderian)
* chore(package): update tslint to version 5.1.0 (greenkeeper[bot])
* fix(package): update form-data to version 2.1.4 (greenkeeper[bot])
* chore(package): update tslint-loader to version 3.5.2 (greenkeeper[bot])
* chore(package): update tslint-loader to version 3.5.1 (greenkeeper[bot])
* chore(package): update karma to version 1.6.0 (greenkeeper[bot])
* chore: greenkeeper initial PR (Sassoun Derderian)
* docs(readme): add Greenkeeper badge (greenkeeper[bot])
* chore(package): update dependencies (greenkeeper[bot])
* 2.0.1 (Sassoun Derderian)

v2.0.0 - April 4, 2017

* 2.0.0 (Sassoun Derderian)
* chore: remove docs/ before re-generating it (Sassoun Derderian)
* fix: typo in package.json (Sassoun Derderian)
* feat: setApiKey, clearApiKey fix: setSessionID chore: codecov badge (Sassoun Derderian)
* fix: corrected lcov file name (Hovhannes Babayan)
* chore: config tweak (Hovhannes Babayan)
* separate upload methods for browser and node env (Sassoun Derderian)
* no need for gulp tasks, use `np` instead (Sassoun Derderian)
* get rid of ApiFactory (Sassoun Derderian)
* Saucelabs integration (#21) (Sassoun Derderian)
* chore: thank browserstack (Sassoun Derderian)
* chore: matrix (Sassoun Derderian)
* chore: global env (Sassoun Derderian)
* chore: set CI variable (Sassoun Derderian)
* chore: travis.yaml set CI env variable (Sassoun Derderian)
* chore: set BrowserStack username and accesskey (Sassoun Derderian)
* chore: set BrowserStack username and accesskey (Sassoun Derderian)
* chore: setup BrowserStack (Sassoun Derderian)
* chore: set sourceMap=true in ts-loader compilerOptions (Sassoun Derderian)
* chore: set sourceMap=true in ts-loader compilerOptions (Sassoun Derderian)
* chore: include karma coverageReporter lcov type (Sassoun Derderian)
* chore: npm-run-all shorthands (Sassoun Derderian)
* chore: run tests on PhantomJS (Sassoun Derderian)
* fix: handle fetch response chore: add login tests (Sassoun Derderian)
* remove dist/ (Sassoun Derderian)
* try another config for karma-sauce-launcher (Sassoun Derderian)
* fix: TRAVIS_JOB_NUMBER as tunnelIdentifier (Sassoun Derderian)
* fix: wrong version of should-sinon (Sassoun Derderian)
* fix: should-sinon not installed (Sassoun Derderian)
* fix: build with webpack (Sassoun Derderian)
* refactor: Replace Browserify to Webpack, code in Typescript (Sassoun Derderian)
* refactor: Replace Browserify to Webpack, code in Typescript (Sassoun Derderian)
* feat: set sessionID in headers via setSessionID method (Sassoun Derderian)

v1.4.1 - March 7, 2017

Expand Down
2 changes: 1 addition & 1 deletion docs/assets/js/search.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/classes/_api_.api.html
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ <h2>Properties</h2>
<section class="tsd-panel tsd-member tsd-kind-property tsd-parent-kind-class">
<a name="_httpoptions" class="tsd-anchor"></a>
<h3>_http<wbr>Options</h3>
<div class="tsd-signature tsd-kind-icon">_http<wbr>Options<span class="tsd-signature-symbol">:</span> <a href="../modules/_api_.html#thttpoptions" class="tsd-signature-type">THttpOptions</a></div>
<div class="tsd-signature tsd-kind-icon">_http<wbr>Options<span class="tsd-signature-symbol">:</span> <a href="../modules/_api_.html#thttpoptions" class="tsd-signature-type">IHttpOptions</a></div>
<aside class="tsd-sources">
<ul>
<li>Defined in <a href="https://github.com/Workfront/workfront-api/blob/fa4aa8a/src/Api.ts#L56">Api.ts:56</a></li>
Expand Down
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@
"version": "2.0.2",
"description": "A Workfront API for the Node.js and the Web",
"main": "dist/workfront.js",
"module": "es/index.js",
"files": [
"dist"
"dist",
"es",
"typings"
],
"typings": "typings/index.d.ts",
"dependencies": {
"es6-promise": "4.1.0",
"form-data": "2.1.4",
Expand Down Expand Up @@ -49,7 +53,7 @@
"scripts": {
"test": "karma start karma.conf.js",
"debug": "npm run test -- --auto-watch --browsers Chrome --no-single-run",
"build": "webpack -p --bail",
"build": "webpack -p --bail && tsc -p ./tsconfig.json -d --declarationDir ./typings",
"docs": "rm -rf ./docs/ && typedoc --out ./docs/ ./src/ --excludePrivate --excludeExternals --exclude \"**/*.spec.ts\"",
"prepublish": "npm run build",
"version": "npm run docs && node ./generate-changelog.js && git add docs CHANGELOG.md",
Expand Down
43 changes: 22 additions & 21 deletions src/Api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ import 'isomorphic-fetch'
import * as stream from 'stream'
import {INTERNAL_PREFIX} from 'workfront-api-constants'

type THttpParams = any
type THttpOptions = {
export type THttpParams = any
export interface IHttpOptions {
path?: string
method?: string
url: string
alwaysUseGet?: boolean
headers: {sessionID?: string}
}
type TFields = string | string[]
export type TFields = string | string[]

const GlobalScope = Function('return this')()

Expand All @@ -53,7 +53,7 @@ export class Api {
POST: 'POST'
}

_httpOptions: THttpOptions
_httpOptions: IHttpOptions
_httpParams: THttpParams

constructor(config) {
Expand Down Expand Up @@ -110,7 +110,7 @@ export class Api {
* @return {Promise} A promise which will resolved with results if everything went ok and rejected otherwise
*/
copy(objCode: string, objID: string, updates: object, fields?: TFields) {
let params: {
const params: {
copySourceID: string,
updates?: object
} = {
Expand All @@ -130,7 +130,7 @@ export class Api {
* @return {Promise}
*/
count(objCode: string, query?: object): Promise<number> {
return this.request(objCode + '/count', query, null, Api.Methods.GET).then(function (data) {
return this.request(objCode + '/count', query, null, Api.Methods.GET).then(function(data) {
return data.count
})
}
Expand All @@ -143,7 +143,7 @@ export class Api {
*/
clearApiKey() {
return new Promise((resolve, reject) => {
this.execute('USER', null, 'clearApiKey').then(function (result) {
this.execute('USER', null, 'clearApiKey').then(function(result) {
if (result) {
delete this._httpParams.apiKey
resolve()
Expand Down Expand Up @@ -324,7 +324,7 @@ export class Api {

const alwaysUseGet = this._httpOptions.alwaysUseGet

let options = Object.assign({}, this._httpOptions)
const options = Object.assign({}, this._httpOptions)
if (alwaysUseGet) {
params.method = method
} else {
Expand All @@ -346,12 +346,12 @@ export class Api {
params.fields = fields.join()
}

let headers = new Headers()
const headers = new Headers()
if (this._httpOptions.headers.sessionID) {
headers.append('sessionID', this._httpOptions.headers.sessionID)
}

let bodyParams
let bodyParams = null, queryString = ''
if (NodeFormData && params instanceof NodeFormData) {
bodyParams = params
}
Expand All @@ -360,13 +360,19 @@ export class Api {
}
else {
headers.append('Content-Type', 'application/x-www-form-urlencoded')
bodyParams = Object.keys(params).reduce(function (a, k) {
bodyParams = Object.keys(params).reduce(function(a, k) {
a.push(k + '=' + encodeURIComponent(params[k]))
return a
}, []).join('&')
if (method === Api.Methods.GET || method === Api.Methods.PUT) {
if (bodyParams) {
queryString = '?' + bodyParams
}
bodyParams = null
}
}

return fetch(options.url + options.path, {
return fetch(options.url + options.path + queryString, {
method: method,
headers: headers,
body: bodyParams
Expand All @@ -380,11 +386,11 @@ export class Api {
* Used for object retrieval by multiple search criteria.
* @memberOf Api
* @param {String} objCode One of object codes from {@link https://developers.attask.com/api-docs/api-explorer/|Workfront API Explorer}
* @param {Object} query An object with search criteria
* @param {Object} [query] An object with search criteria
* @param {String|String[]} [fields] Which fields to return. See {@link https://developers.attask.com/api-docs/api-explorer/|Workfront API Explorer} for the list of available fields for the given objCode.
* @return {Promise} A promise which will resolved with search results if everything went ok and rejected otherwise
*/
search(objCode: string, query: object, fields?: TFields) {
search(objCode: string, query?: object, fields?: TFields) {
return this.request(objCode + '/search', query, fields, Api.Methods.GET)
}

Expand Down Expand Up @@ -423,23 +429,18 @@ export class Api {
* @param {String} filename Override the filename
*/
uploadFromStream(stream: stream.Readable, filename: string) {
let data = new NodeFormData()
const data = new NodeFormData()
data.append('uploadedFile', stream, filename)
return this.request('upload', data, null, Api.Methods.POST)
}

uploadFileContent(fileContent, filename: string) {
let data = new GlobalScope.FormData()
const data = new GlobalScope.FormData()
data.append('uploadedFile', fileContent, filename)
return this.request('upload', data, null, Api.Methods.POST)
}
}

// if (typeof(window) === 'undefined') {
// These plugins only work in node
// require('./plugins/upload')(Api)
// }

const ResponseHandler = {
success: (response) => {
if (response.ok) {
Expand Down
10 changes: 5 additions & 5 deletions test/Api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import * as should from 'should'
import {Api} from '../src/index'


describe('Create new instance for API', function () {
it('should have methods', function () {
describe('Create new instance for API', function() {
it('should have methods', function() {
const api = new Api({url: 'http://localhost'})
should(api.copy).be.a.Function()
should(api.count).be.a.Function()
Expand All @@ -41,17 +41,17 @@ describe('Create new instance for API', function () {
should(api.clearApiKey).be.a.Function()
})

it('should set correct API path based on passed configuration (version is passed)', function () {
it('should set correct API path based on passed configuration (version is passed)', function() {
const api = new Api({url: 'http://localhost', version: '2.0'})
should(api._httpOptions.path).equal('/attask/api/v2.0')
})

it('should set correct API path based on passed configuration (version is not passed)', function () {
it('should set correct API path based on passed configuration (version is not passed)', function() {
const api = new Api({url: 'http://localhost'})
should(api._httpOptions.path).equal('/attask/api')
})

it('should set correct API path based on passed configuration (version="internal")', function () {
it('should set correct API path based on passed configuration (version="internal")', function() {
const api = new Api({url: 'http://localhost', version: 'internal'})
should(api._httpOptions.path).equal('/attask/api-internal')
})
Expand Down
20 changes: 10 additions & 10 deletions test/integration/copy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,22 @@ import * as Workfront from '../../src/index'

const API_URL = 'http://foobar:8080'

describe('Copy', function () {
describe('Copy', function() {

afterEach(fetchMock.reset)
afterEach(fetchMock.restore)

beforeEach(function () {
beforeEach(function() {
this.api = new Workfront.Api({
url: API_URL
})
})
afterEach(function () {
afterEach(function() {
this.api = undefined
})

describe('success', function () {
beforeEach(function () {
describe('success', function() {
beforeEach(function() {
fetchMock.mock(
`begin:${API_URL}/attask/api/`,
require('../../fixtures/copy.json'),
Expand All @@ -45,16 +45,16 @@ describe('Copy', function () {
}
)
})
it('makes request to objCode with copySourceID in the params', function () {
return this.api.copy('foo', 'bar').then(function () {
let [url, opts] = fetchMock.lastCall('copy')
it('makes request to objCode with copySourceID in the params', function() {
return this.api.copy('foo', 'bar').then(function() {
const [url, opts] = fetchMock.lastCall('copy')
should(url).endWith('foo')
should(opts.method).equal('POST')
should(opts.body).containEql('copySourceID=bar')
})
})
it('returns data with a new ID', function () {
return this.api.copy('foo', 'bar').then(function (data) {
it('returns data with a new ID', function() {
return this.api.copy('foo', 'bar').then(function(data) {
should(data).have.property('ID')
should(data.ID).not.containEql('bar')
})
Expand Down
28 changes: 14 additions & 14 deletions test/integration/count.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,21 @@ import * as Workfront from '../../src/index'

const API_URL = 'http://foobar:8080'

describe('Count', function () {
describe('Count', function() {

afterEach(fetchMock.reset)
afterEach(fetchMock.restore)

beforeEach(function () {
beforeEach(function() {
this.api = new Workfront.Api({
url: API_URL
})
})
afterEach(function () {
afterEach(function() {
this.api = undefined
})

beforeEach(function () {
beforeEach(function() {
fetchMock.mock(
`begin:${API_URL}/attask/api/`,
require('../../fixtures/count.json'),
Expand All @@ -45,23 +45,23 @@ describe('Count', function () {
)
})

it('makes request with proper params with search criteria', function () {
return this.api.count('foo', {foo: 'bar'}).then(function () {
let [url, opts] = fetchMock.lastCall('count')
should(url).endWith('foo/count')
it('makes request with proper params with search criteria', function() {
return this.api.count('foo', {foo: 'bar'}).then(function() {
const [url, opts] = fetchMock.lastCall('count')
should(url).endWith('foo/count?foo=bar')
should(opts.method).equal('GET')
should(opts.body).containEql('foo=bar')
should(opts.body).be.null()
})
})
it('makes request with proper params without a search criteria', function () {
return this.api.count('foo').then(function () {
let opts = fetchMock.lastOptions('count')
it('makes request with proper params without a search criteria', function() {
return this.api.count('foo').then(function() {
const opts = fetchMock.lastOptions('count')
should(opts.method).equal('GET')
should(opts.body).equal('')
should(opts.body).be.null()
})
})

it('should return a promise with count', function () {
it('should return a promise with count', function() {
return this.api.count('foo').should.be.finally.a.Number().and.equal(147)
})
})
Loading

0 comments on commit f5ec9bd

Please sign in to comment.