fix(search): compensate for the search api returning an object instea…

…d of an array closes #335
github-tools · May 27, 2016 · 7c73463 · 7c73463
1 parent 2d102be
commit 7c73463
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,12 +1,13 @@
 # Change Log
 
-## master
+## 2.2.0 - 2016/05/27
 ### Features
 * add `Issue.listIssueEvents`
 
 ### Fixes
+* Search returns results again
 
-## 2.1.0
+## 2.1.0 - 2016/05/26
 ### Features
 Team API
 * `Organization.createTeam`
@@ -40,7 +41,7 @@ User
 ### Fixes
 * `Repository`: Replace invalid references to `postTree` with `createTree`
 
-## 1.2.0 - 2015/05/11
+## 1.2.0 - 2016/05/11
 ### Features
 * Search API now returns all pages of results
 * Added `Repository.listReleases`
@@ -58,7 +59,7 @@ Added functions for issue comments
 ### Fixes
 * all functions now return a Promise
 
-## 1.1.0 - 2015/05/03
+## 1.1.0 - 2016/05/03
 ### Features
 Added methods for commenting on Gists:
 * `Gist.listComments`
@@ -70,7 +71,7 @@ Added methods for commenting on Gists:
 ### Fixes
 * `Repository.deleteFile` now correctly returns a promise.
 
-## 1.0.0 - 2015/04/27
+## 1.0.0 - 2016/04/27
 Complete rewrite in ES2015.
 
 * Promise-ified the API

diff --git a/lib/Requestable.js b/lib/Requestable.js
@@ -16,6 +16,25 @@ if (typeof Promise === 'undefined') {
    polyfill();
 }
 
+/**
+ * The error structure returned when a network call fails
+ */
+class ResponseError extends Error {
+   /**
+    * Construct a new ResponseError
+    * @param {string} message - an message to return instead of the the default error message
+    * @param {string} path - the requested path
+    * @param {Object} response - the object returned by Axios
+    */
+   constructor(message, path, response) {
+      super(message);
+      this.path = path;
+      this.request = response.config;
+      this.response = response;
+      this.status = response.status;
+   }
+}
+
 /**
  * Requestable wraps the logic for making http requests to the API
  */
@@ -208,7 +227,16 @@ class Requestable {
 
       return this._request('GET', path, options)
          .then((response) => {
-            results.push.apply(results, response.data);
+            let thisGroup;
+            if (response.data instanceof Array) {
+               thisGroup = response.data;
+            } else if (response.data.items instanceof Array) {
+               thisGroup = response.data.items;
+            } else {
+               let message = `cannot figure out how to append ${response.data} to the result set`;
+               throw new ResponseError(message, path, response);
+            }
+            results.push.apply(results, thisGroup);
 
             const nextUrl = getNextPage(response.headers.link);
             if (nextUrl) {
@@ -231,24 +259,6 @@ module.exports = Requestable;
 // ////////////////////////// //
 //  Private helper functions  //
 // ////////////////////////// //
-/**
- * The error structure returned when a network call fails
- */
-class ResponseError extends Error {
-   /**
-    * Construct a new ResponseError
-    * @param {string} path - the requested path
-    * @param {Object} response - the object returned by Axios
-    */
-   constructor(path, response) {
-      super(`error making request ${response.config.method} ${response.config.url}`);
-      this.path = path;
-      this.request = response.config;
-      this.response = response;
-      this.status = response.status;
-   }
-}
-
 const METHODS_WITH_NO_BODY = ['GET', 'HEAD', 'DELETE'];
 function methodHasNoBody(method) {
    return METHODS_WITH_NO_BODY.indexOf(method) !== -1;
@@ -267,8 +277,9 @@ function getNextPage(linksHeader = '') {
 
 function callbackErrorOrThrow(cb, path) {
    return function handler(response) {
-      log(`error making request ${response.config.method} ${response.config.url} ${JSON.stringify(response.data)}`);
-      let error = new ResponseError(path, response);
+      let message = `error making request ${response.config.method} ${response.config.url}`;
+      let error = new ResponseError(message, path, response);
+      log(`${message} ${JSON.stringify(response.data)}`);
       if (cb) {
          log('going to error callback');
          cb(error);

diff --git a/test/search.spec.js b/test/search.spec.js
@@ -1,8 +1,10 @@
+import expect from 'must';
+
 import Github from '../lib/GitHub';
 import testUser from './fixtures/user.json';
-import {assertSuccessful} from './helpers/callbacks';
 
 describe('Search', function() {
+   this.timeout(20 * 1000);
    let github;
 
    before(function() {
@@ -13,43 +15,59 @@ describe('Search', function() {
       });
    });
 
-   it('should search repositories', function(done) {
+   it('should search repositories', function() {
       let options;
       let search = github.search({
          q: 'tetris language:assembly',
          sort: 'stars',
          order: 'desc'
       });
 
-      search.forRepositories(options, assertSuccessful(done));
+      return search.forRepositories(options)
+         .then(function({data}) {
+            expect(data).to.be.an.array();
+            expect(data.length).to.be.above(0);
+         });
    });
 
-   it('should search code', function(done) {
+   it('should search code', function() {
       let options;
       let search = github.search({
          q: 'addClass in:file language:js repo:jquery/jquery'
       });
 
-      search.forCode(options, assertSuccessful(done));
+      return search.forCode(options)
+         .then(function({data}) {
+            expect(data).to.be.an.array();
+            expect(data.length).to.be.above(0);
+         });
    });
 
-   it('should search issues', function(done) {
+   it('should search issues', function() {
       let options;
       let search = github.search({
          q: 'windows pip label:bug language:python state:open ',
          sort: 'created',
          order: 'asc'
       });
 
-      search.forIssues(options, assertSuccessful(done));
+      return search.forIssues(options)
+         .then(function({data}) {
+            expect(data).to.be.an.array();
+            expect(data.length).to.be.above(0);
+         });
    });
 
-   it('should search users', function(done) {
+   it('should search users', function() {
       let options;
       let search = github.search({
          q: 'tom repos:>42 followers:>1000'
       });
 
-      search.forUsers(options, assertSuccessful(done));
+      return search.forUsers(options)
+         .then(function({data}) {
+            expect(data).to.be.an.array();
+            expect(data.length).to.be.above(0);
+         });
    });
 });