From 4a2328b2f10ea678184391e4caf235b41323cd3e Mon Sep 17 00:00:00 2001 From: Erez Rokah Date: Mon, 11 Nov 2019 18:33:20 +0200 Subject: [PATCH] fix(git-gateway): unpublished entries not loaded for git-gateway(GitHub) (#2856) --- jest.config.js | 1 + .../src/GitHubAPI.js | 30 +------ .../src/__tests__/GitHubAPI.spec.js | 84 +++++++++++++++++++ .../netlify-cms-backend-github/src/API.js | 16 ++-- .../src/__tests__/API.spec.js | 70 ++++++++++++++++ 5 files changed, 168 insertions(+), 33 deletions(-) create mode 100644 packages/netlify-cms-backend-git-gateway/src/__tests__/GitHubAPI.spec.js diff --git a/jest.config.js b/jest.config.js index a3770199..3e0bf8c8 100644 --- a/jest.config.js +++ b/jest.config.js @@ -7,6 +7,7 @@ module.exports = { 'netlify-cms-lib-auth': '/packages/netlify-cms-lib-auth/src/index.js', 'netlify-cms-lib-util': '/packages/netlify-cms-lib-util/src/index.js', 'netlify-cms-ui-default': '/packages/netlify-cms-ui-default/src/index.js', + 'netlify-cms-backend-github': '/packages/netlify-cms-backend-github/src/index.js', }, testURL: 'http://localhost:8080', }; diff --git a/packages/netlify-cms-backend-git-gateway/src/GitHubAPI.js b/packages/netlify-cms-backend-git-gateway/src/GitHubAPI.js index 424cd3eb..bef41d15 100644 --- a/packages/netlify-cms-backend-git-gateway/src/GitHubAPI.js +++ b/packages/netlify-cms-backend-git-gateway/src/GitHubAPI.js @@ -41,7 +41,7 @@ export default class API extends GithubAPI { }); } - getRequestHeaders(headers = {}) { + requestHeaders(headers = {}) { return this.tokenPromise().then(jwtToken => { const baseHeader = { Authorization: `Bearer ${jwtToken}`, @@ -53,38 +53,14 @@ export default class API extends GithubAPI { }); } - urlFor(path, options) { - const cacheBuster = new Date().getTime(); - const params = [`ts=${cacheBuster}`]; - if (options.params) { - for (const key in options.params) { - params.push(`${key}=${encodeURIComponent(options.params[key])}`); - } - } - if (params.length) { - path += `?${params.join('&')}`; - } - return this.api_root + path; + handleRequestError(error, responseStatus) { + throw new APIError(error.message || error.msg, responseStatus, 'Git Gateway'); } user() { return Promise.resolve(this.commitAuthor); } - request(path, options = {}, parseResponse = response => this.parseResponse(response)) { - const url = this.urlFor(path, options); - let responseStatus; - return this.getRequestHeaders(options.headers || {}) - .then(headers => fetch(url, { ...options, headers })) - .then(response => { - responseStatus = response.status; - return parseResponse(response); - }) - .catch(error => { - throw new APIError(error.message || error.msg, responseStatus, 'Git Gateway'); - }); - } - commit(message, changeTree) { const commitParams = { message, diff --git a/packages/netlify-cms-backend-git-gateway/src/__tests__/GitHubAPI.spec.js b/packages/netlify-cms-backend-git-gateway/src/__tests__/GitHubAPI.spec.js new file mode 100644 index 00000000..884cb954 --- /dev/null +++ b/packages/netlify-cms-backend-git-gateway/src/__tests__/GitHubAPI.spec.js @@ -0,0 +1,84 @@ +import API from '../GitHubAPI'; + +describe('github API', () => { + describe('request', () => { + beforeEach(() => { + const fetch = jest.fn(); + global.fetch = fetch; + global.Date = jest.fn(() => ({ getTime: () => 1000 })); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should fetch url with authorization header', async () => { + const api = new API({ + api_root: 'https://site.netlify.com/.netlify/git/github', + tokenPromise: () => Promise.resolve('token'), + }); + + fetch.mockResolvedValue({ + text: jest.fn().mockResolvedValue('some response'), + ok: true, + status: 200, + headers: { get: () => '' }, + }); + const result = await api.request('/some-path'); + expect(result).toEqual('some response'); + expect(fetch).toHaveBeenCalledTimes(1); + expect(fetch).toHaveBeenCalledWith( + 'https://site.netlify.com/.netlify/git/github/some-path?ts=1000', + { + headers: { Authorization: 'Bearer token', 'Content-Type': 'application/json' }, + }, + ); + }); + + it('should throw error on not ok response with message property', async () => { + const api = new API({ + api_root: 'https://site.netlify.com/.netlify/git/github', + tokenPromise: () => Promise.resolve('token'), + }); + + fetch.mockResolvedValue({ + text: jest.fn().mockResolvedValue({ message: 'some error' }), + ok: false, + status: 404, + headers: { get: () => '' }, + }); + + await expect(api.request('some-path')).rejects.toThrow( + expect.objectContaining({ + message: 'some error', + name: 'API_ERROR', + status: 404, + api: 'Git Gateway', + }), + ); + }); + + it('should throw error on not ok response with msg property', async () => { + const api = new API({ + api_root: 'https://site.netlify.com/.netlify/git/github', + tokenPromise: () => Promise.resolve('token'), + }); + + fetch.mockResolvedValue({ + text: jest.fn().mockResolvedValue({ msg: 'some error' }), + ok: false, + status: 404, + headers: { get: () => '' }, + }); + + await expect(api.request('some-path')).rejects.toThrow( + expect.objectContaining({ + message: 'some error', + name: 'API_ERROR', + status: 404, + api: 'Git Gateway', + }), + ); + }); + }); +}); diff --git a/packages/netlify-cms-backend-github/src/API.js b/packages/netlify-cms-backend-github/src/API.js index b98d02a2..005aca70 100644 --- a/packages/netlify-cms-backend-github/src/API.js +++ b/packages/netlify-cms-backend-github/src/API.js @@ -109,8 +109,13 @@ export default class API { return textPromise; } - request(path, options = {}, parseResponse = response => this.parseResponse(response)) { - const headers = this.requestHeaders(options.headers || {}); + handleRequestError(error, responseStatus) { + throw new APIError(error.message, responseStatus, 'GitHub'); + } + + async request(path, options = {}, parseResponse = response => this.parseResponse(response)) { + // overriding classes can return a promise from requestHeaders + const headers = await this.requestHeaders(options.headers || {}); const url = this.urlFor(path, options); let responseStatus; return fetch(url, { ...options, headers }) @@ -118,13 +123,12 @@ export default class API { responseStatus = response.status; return parseResponse(response); }) - .catch(error => { - throw new APIError(error.message, responseStatus, 'GitHub'); - }); + .catch(error => this.handleRequestError(error, responseStatus)); } async requestAllPages(url, options = {}) { - const headers = this.requestHeaders(options.headers || {}); + // overriding classes can return a promise from requestHeaders + const headers = await this.requestHeaders(options.headers || {}); const processedURL = this.urlFor(url, options); const allResponses = await getAllResponses(processedURL, { ...options, headers }); const pages = await Promise.all(allResponses.map(res => this.parseResponse(res))); diff --git a/packages/netlify-cms-backend-github/src/__tests__/API.spec.js b/packages/netlify-cms-backend-github/src/__tests__/API.spec.js index b7edb3bc..e5c63cc9 100644 --- a/packages/netlify-cms-backend-github/src/__tests__/API.spec.js +++ b/packages/netlify-cms-backend-github/src/__tests__/API.spec.js @@ -36,4 +36,74 @@ describe('github API', () => { .then(() => prBaseBranch), ).resolves.toEqual('gh-pages'); }); + + describe('request', () => { + beforeEach(() => { + const fetch = jest.fn(); + global.fetch = fetch; + global.Date = jest.fn(() => ({ getTime: () => 1000 })); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should fetch url with authorization header', async () => { + const api = new API({ branch: 'gh-pages', repo: 'my-repo', token: 'token' }); + + fetch.mockResolvedValue({ + text: jest.fn().mockResolvedValue('some response'), + ok: true, + status: 200, + headers: { get: () => '' }, + }); + const result = await api.request('/some-path'); + expect(result).toEqual('some response'); + expect(fetch).toHaveBeenCalledTimes(1); + expect(fetch).toHaveBeenCalledWith('https://api.github.com/some-path?ts=1000', { + headers: { Authorization: 'token token', 'Content-Type': 'application/json' }, + }); + }); + + it('should throw error on not ok response', async () => { + const api = new API({ branch: 'gh-pages', repo: 'my-repo', token: 'token' }); + + fetch.mockResolvedValue({ + text: jest.fn().mockResolvedValue({ message: 'some error' }), + ok: false, + status: 404, + headers: { get: () => '' }, + }); + + await expect(api.request('some-path')).rejects.toThrow( + expect.objectContaining({ + message: 'some error', + name: 'API_ERROR', + status: 404, + api: 'GitHub', + }), + ); + }); + + it('should allow overriding requestHeaders to return a promise ', async () => { + const api = new API({ branch: 'gh-pages', repo: 'my-repo', token: 'token' }); + + api.requestHeaders = jest + .fn() + .mockResolvedValue({ Authorization: 'promise-token', 'Content-Type': 'application/json' }); + + fetch.mockResolvedValue({ + text: jest.fn().mockResolvedValue('some response'), + ok: true, + status: 200, + headers: { get: () => '' }, + }); + const result = await api.request('/some-path'); + expect(result).toEqual('some response'); + expect(fetch).toHaveBeenCalledTimes(1); + expect(fetch).toHaveBeenCalledWith('https://api.github.com/some-path?ts=1000', { + headers: { Authorization: 'promise-token', 'Content-Type': 'application/json' }, + }); + }); + }); });