From f520be71d672d86bc05303ea46175682dddf7a48 Mon Sep 17 00:00:00 2001 From: kialam Date: Mon, 12 Nov 2018 13:24:17 -0500 Subject: [PATCH 1/4] Begin using async/await. --- src/App.jsx | 9 ++++----- src/api.js | 38 +++++++++---------------------------- src/endpoints.jsx | 7 +++++++ src/index.jsx | 11 ++++------- src/pages/Organizations.jsx | 7 ++++--- 5 files changed, 28 insertions(+), 44 deletions(-) create mode 100644 src/endpoints.jsx diff --git a/src/App.jsx b/src/App.jsx index 3952904781..26cf0724b2 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -22,6 +22,7 @@ import { import { global_breakpoint_md as breakpointMd } from '@patternfly/react-tokens'; import api from './api'; +import { API_LOGOUT } from './endpoints'; // import About from './components/About'; import LogoutButton from './components/LogoutButton'; @@ -79,11 +80,9 @@ class App extends React.Component { this.setState({ activeGroup: 'views_group', activeItem: 'views_group_dashboard' }); } - onDevLogout = () => { - api.logout() - .then(() => { - this.setState({ activeGroup: 'views_group', activeItem: 'views_group_dashboard' }); - }); + onDevLogout = async () => { + await api.BaseGet(API_LOGOUT); + this.setState({ activeGroup: 'views_group', activeItem: 'views_group_dashboard' }); } render () { diff --git a/src/api.js b/src/api.js index 45b330755c..0671d6ea65 100644 --- a/src/api.js +++ b/src/api.js @@ -1,12 +1,6 @@ import axios from 'axios'; -const API_ROOT = '/api/'; -const API_LOGIN = `${API_ROOT}login/`; -const API_LOGOUT = `${API_ROOT}logout/`; -const API_V2 = `${API_ROOT}v2/`; -const API_CONFIG = `${API_V2}config/`; -const API_PROJECTS = `${API_V2}projects/`; -const API_ORGANIZATIONS = `${API_V2}organizations/`; +import * as constant from './endpoints'; const CSRF_COOKIE_NAME = 'csrftoken'; const CSRF_HEADER_NAME = 'X-CSRFToken'; @@ -38,7 +32,7 @@ class APIClient { return authenticated; } - login (username, password, redirect = API_CONFIG) { + async login (username, password, redirect = constant.API_CONFIG) { const un = encodeURIComponent(username); const pw = encodeURIComponent(password); const next = encodeURIComponent(redirect); @@ -46,29 +40,15 @@ class APIClient { const data = `username=${un}&password=${pw}&next=${next}`; const headers = { 'Content-Type': LOGIN_CONTENT_TYPE }; - return this.http.get(API_LOGIN, { headers }) - .then(() => this.http.post(API_LOGIN, data, { headers })); + try { + await this.http.get(constant.API_LOGIN, { headers }); + await this.http.post(constant.API_LOGIN, data, { headers }); + } catch (err) { + alert(`There was a problem logging in: ${err}`); + } } - logout () { - return this.http.get(API_LOGOUT); - } - - getConfig () { - return this.http.get(API_CONFIG); - } - - getProjects () { - return this.http.get(API_PROJECTS); - } - - getOrganizations () { - return this.http.get(API_ORGANIZATIONS); - } - - getRoot () { - return this.http.get(API_ROOT); - } + BaseGet = (endpoint) => this.http.get(endpoint); } export default new APIClient(); diff --git a/src/endpoints.jsx b/src/endpoints.jsx new file mode 100644 index 0000000000..d1499b00d6 --- /dev/null +++ b/src/endpoints.jsx @@ -0,0 +1,7 @@ +export const API_ROOT = '/api/'; +export const API_LOGIN = `${API_ROOT}login/`; +export const API_LOGOUT = `${API_ROOT}logout/`; +export const API_V2 = `${API_ROOT}v2/`; +export const API_CONFIG = `${API_V2}config/`; +export const API_PROJECTS = `${API_V2}projects/`; +export const API_ORGANIZATIONS = `${API_V2}organizations/`; \ No newline at end of file diff --git a/src/index.jsx b/src/index.jsx index aac9291f39..f5d675b04f 100644 --- a/src/index.jsx +++ b/src/index.jsx @@ -3,6 +3,7 @@ import { render } from 'react-dom'; import App from './App'; import api from './api'; +import { API_ROOT } from './endpoints'; import '@patternfly/react-core/dist/styles/base.css'; import '@patternfly/patternfly-next/patternfly.css'; @@ -11,13 +12,9 @@ import './app.scss'; const el = document.getElementById('app'); -const main = () => { - api.getRoot() - .then(({ data }) => { - const { custom_logo, custom_login_info } = data; - - render(, el); - }); +const main = async () => { + const { custom_logo, custom_login_info } = await api.BaseGet(API_ROOT); + render(, el); }; main(); diff --git a/src/pages/Organizations.jsx b/src/pages/Organizations.jsx index d987ede528..99cad1650d 100644 --- a/src/pages/Organizations.jsx +++ b/src/pages/Organizations.jsx @@ -9,6 +9,7 @@ import { import OrganizationCard from '../components/OrganizationCard'; import api from '../api'; +import { API_ORGANIZATIONS } from '../endpoints'; class Organizations extends Component { constructor (props) { @@ -17,9 +18,9 @@ class Organizations extends Component { this.state = { organizations: [] }; } - componentDidMount () { - api.getOrganizations() - .then(({ data }) => this.setState({ organizations: data.results })); + async componentDidMount () { + const { data } = await api.BaseGet(API_ORGANIZATIONS); + this.setState({ organizations: data.results }); } render () { From 44e9d3919d3ef51fd2ec4b2fae08cd635d7302ad Mon Sep 17 00:00:00 2001 From: kialam Date: Tue, 13 Nov 2018 09:46:43 -0500 Subject: [PATCH 2/4] Update unit tests. --- __tests__/App.test.jsx | 7 +++-- __tests__/api.test.js | 48 ++++--------------------------- __tests__/pages/Organizations.jsx | 6 ++++ src/api.js | 9 ++---- 4 files changed, 20 insertions(+), 50 deletions(-) diff --git a/__tests__/App.test.jsx b/__tests__/App.test.jsx index 01e216659d..58c8d57056 100644 --- a/__tests__/App.test.jsx +++ b/__tests__/App.test.jsx @@ -2,6 +2,8 @@ import React from 'react'; import { shallow, mount } from 'enzyme'; import App from '../src/App'; import api from '../src/api'; +import * as constant from '../src/endpoints'; + import Dashboard from '../src/pages/Dashboard'; import Login from '../src/pages/Login'; import { asyncFlush } from '../jest.setup'; @@ -64,12 +66,13 @@ describe('', () => { }); test('api.logout called from logout button', async () => { - api.logout = jest.fn().mockImplementation(() => Promise.resolve({})); + api.BaseGet = jest.fn().mockImplementation(() => Promise.resolve({})); const appWrapper = mount(); const logoutButton = appWrapper.find('LogoutButton'); logoutButton.props().onDevLogout(); appWrapper.setState({ activeGroup: 'foo', activeItem: 'bar' }); - expect(api.logout).toHaveBeenCalledTimes(1); + expect(api.BaseGet).toHaveBeenCalledTimes(1); + expect(api.BaseGet).toHaveBeenCalledWith(constant.API_LOGOUT); await asyncFlush(); expect(appWrapper.state().activeItem).toBe(DEFAULT_ACTIVE_ITEM); expect(appWrapper.state().activeGroup).toBe(DEFAULT_ACTIVE_GROUP); diff --git a/__tests__/api.test.js b/__tests__/api.test.js index 5d14b20503..f4aa913153 100644 --- a/__tests__/api.test.js +++ b/__tests__/api.test.js @@ -1,14 +1,7 @@ import mockAxios from 'axios'; import APIClient from '../src/api'; - -const API_ROOT = '/api/'; -const API_LOGIN = `${API_ROOT}login/`; -const API_LOGOUT = `${API_ROOT}logout/`; -const API_V2 = `${API_ROOT}v2/`; -const API_CONFIG = `${API_V2}config/`; -const API_PROJECTS = `${API_V2}projects/`; -const API_ORGANIZATIONS = `${API_V2}organizations/`; +import * as constant from '../src/endpoints'; const CSRF_COOKIE_NAME = 'csrftoken'; const CSRF_HEADER_NAME = 'X-CSRFToken'; @@ -52,9 +45,9 @@ describe('APIClient (api.js)', () => { APIClient.setCookie = jest.fn(); APIClient.login(un, pw, next).then(() => { expect(mockAxios.get).toHaveBeenCalledTimes(1); - expect(mockAxios.get).toHaveBeenCalledWith(API_LOGIN, { headers }); + expect(mockAxios.get).toHaveBeenCalledWith(constant.API_LOGIN, { headers }); expect(mockAxios.post).toHaveBeenCalledTimes(1); - expect(mockAxios.post).toHaveBeenCalledWith(API_LOGIN, data, { headers }); + expect(mockAxios.post).toHaveBeenCalledWith(constant.API_LOGIN, data, { headers }); done(); }); }); @@ -67,7 +60,7 @@ describe('APIClient (api.js)', () => { const data = `username=${encodeURIComponent(un)}&password=${encodeURIComponent(pw)}&next=${encodeURIComponent(next)}`; APIClient.login(un, pw, next).then(() => { expect(mockAxios.post).toHaveBeenCalledTimes(1); - expect(mockAxios.post).toHaveBeenCalledWith(API_LOGIN, data, { headers }); + expect(mockAxios.post).toHaveBeenCalledWith(constant.API_LOGIN, data, { headers }); done(); }); }); @@ -76,42 +69,13 @@ describe('APIClient (api.js)', () => { const un = 'foo'; const pw = 'bar'; const headers = { 'Content-Type': LOGIN_CONTENT_TYPE }; - const data = `username=${un}&password=${pw}&next=${encodeURIComponent(API_CONFIG)}`; + const data = `username=${un}&password=${pw}&next=${encodeURIComponent(constant.API_CONFIG)}`; APIClient.setCookie = jest.fn(); APIClient.login(un, pw).then(() => { expect(mockAxios.post).toHaveBeenCalledTimes(1); - expect(mockAxios.post).toHaveBeenCalledWith(API_LOGIN, data, { headers }); + expect(mockAxios.post).toHaveBeenCalledWith(constant.API_LOGIN, data, { headers }); done(); }); }); - test('logout calls get to logout route', () => { - APIClient.logout(); - expect(mockAxios.get).toHaveBeenCalledTimes(1); - expect(mockAxios.get).toHaveBeenCalledWith(API_LOGOUT); - }); - - test('getConfig calls get to config route', () => { - APIClient.getConfig(); - expect(mockAxios.get).toHaveBeenCalledTimes(1); - expect(mockAxios.get).toHaveBeenCalledWith(API_CONFIG); - }); - - test('getProjects calls get to projects route', () => { - APIClient.getProjects(); - expect(mockAxios.get).toHaveBeenCalledTimes(1); - expect(mockAxios.get).toHaveBeenCalledWith(API_PROJECTS); - }); - - test('getOrganigzations calls get to organizations route', () => { - APIClient.getOrganizations(); - expect(mockAxios.get).toHaveBeenCalledTimes(1); - expect(mockAxios.get).toHaveBeenCalledWith(API_ORGANIZATIONS); - }); - - test('getRoot calls get to root route', () => { - APIClient.getRoot(); - expect(mockAxios.get).toHaveBeenCalledTimes(1); - expect(mockAxios.get).toHaveBeenCalledWith(API_ROOT); - }); }); diff --git a/__tests__/pages/Organizations.jsx b/__tests__/pages/Organizations.jsx index f41b830091..87efbf95f1 100644 --- a/__tests__/pages/Organizations.jsx +++ b/__tests__/pages/Organizations.jsx @@ -1,5 +1,6 @@ import React from 'react'; import { mount } from 'enzyme'; +import { API_ORGANIZATIONS } from '../../src/endpoints'; import Organizations from '../../src/pages/Organizations'; describe('', () => { @@ -51,4 +52,9 @@ describe('', () => { expect(galleryItems.length).toBe(3); expect(orgCards.length).toBe(3); }); + + test('API Organization endpoint is valid', () => { + expect(API_ORGANIZATIONS).toBeDefined(); + }); + }); diff --git a/src/api.js b/src/api.js index 0671d6ea65..0b0513cc85 100644 --- a/src/api.js +++ b/src/api.js @@ -40,15 +40,12 @@ class APIClient { const data = `username=${un}&password=${pw}&next=${next}`; const headers = { 'Content-Type': LOGIN_CONTENT_TYPE }; - try { - await this.http.get(constant.API_LOGIN, { headers }); - await this.http.post(constant.API_LOGIN, data, { headers }); - } catch (err) { - alert(`There was a problem logging in: ${err}`); - } + await this.http.get(constant.API_LOGIN, { headers }); + await this.http.post(constant.API_LOGIN, data, { headers }); } BaseGet = (endpoint) => this.http.get(endpoint); + } export default new APIClient(); From 03f6e52cf1a3ae9d23568e0d3ccfa05d90bb60d7 Mon Sep 17 00:00:00 2001 From: kialam Date: Tue, 13 Nov 2018 09:53:36 -0500 Subject: [PATCH 3/4] Address PR review comments. --- __tests__/App.test.jsx | 8 ++++---- __tests__/api.test.js | 12 ++++++------ src/App.jsx | 2 +- src/api.js | 10 +++++----- src/index.jsx | 2 +- src/pages/Organizations.jsx | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/__tests__/App.test.jsx b/__tests__/App.test.jsx index 58c8d57056..e07a1984d5 100644 --- a/__tests__/App.test.jsx +++ b/__tests__/App.test.jsx @@ -2,7 +2,7 @@ import React from 'react'; import { shallow, mount } from 'enzyme'; import App from '../src/App'; import api from '../src/api'; -import * as constant from '../src/endpoints'; +import { API_LOGOUT } from '../src/endpoints'; import Dashboard from '../src/pages/Dashboard'; import Login from '../src/pages/Login'; @@ -66,13 +66,13 @@ describe('', () => { }); test('api.logout called from logout button', async () => { - api.BaseGet = jest.fn().mockImplementation(() => Promise.resolve({})); + api.get = jest.fn().mockImplementation(() => Promise.resolve({})); const appWrapper = mount(); const logoutButton = appWrapper.find('LogoutButton'); logoutButton.props().onDevLogout(); appWrapper.setState({ activeGroup: 'foo', activeItem: 'bar' }); - expect(api.BaseGet).toHaveBeenCalledTimes(1); - expect(api.BaseGet).toHaveBeenCalledWith(constant.API_LOGOUT); + expect(api.get).toHaveBeenCalledTimes(1); + expect(api.get).toHaveBeenCalledWith(API_LOGOUT); await asyncFlush(); expect(appWrapper.state().activeItem).toBe(DEFAULT_ACTIVE_ITEM); expect(appWrapper.state().activeGroup).toBe(DEFAULT_ACTIVE_GROUP); diff --git a/__tests__/api.test.js b/__tests__/api.test.js index f4aa913153..9bc5a7f218 100644 --- a/__tests__/api.test.js +++ b/__tests__/api.test.js @@ -1,7 +1,7 @@ import mockAxios from 'axios'; import APIClient from '../src/api'; -import * as constant from '../src/endpoints'; +import * as endpoints from '../src/endpoints'; const CSRF_COOKIE_NAME = 'csrftoken'; const CSRF_HEADER_NAME = 'X-CSRFToken'; @@ -45,9 +45,9 @@ describe('APIClient (api.js)', () => { APIClient.setCookie = jest.fn(); APIClient.login(un, pw, next).then(() => { expect(mockAxios.get).toHaveBeenCalledTimes(1); - expect(mockAxios.get).toHaveBeenCalledWith(constant.API_LOGIN, { headers }); + expect(mockAxios.get).toHaveBeenCalledWith(endpoints.API_LOGIN, { headers }); expect(mockAxios.post).toHaveBeenCalledTimes(1); - expect(mockAxios.post).toHaveBeenCalledWith(constant.API_LOGIN, data, { headers }); + expect(mockAxios.post).toHaveBeenCalledWith(endpoints.API_LOGIN, data, { headers }); done(); }); }); @@ -60,7 +60,7 @@ describe('APIClient (api.js)', () => { const data = `username=${encodeURIComponent(un)}&password=${encodeURIComponent(pw)}&next=${encodeURIComponent(next)}`; APIClient.login(un, pw, next).then(() => { expect(mockAxios.post).toHaveBeenCalledTimes(1); - expect(mockAxios.post).toHaveBeenCalledWith(constant.API_LOGIN, data, { headers }); + expect(mockAxios.post).toHaveBeenCalledWith(endpoints.API_LOGIN, data, { headers }); done(); }); }); @@ -69,11 +69,11 @@ describe('APIClient (api.js)', () => { const un = 'foo'; const pw = 'bar'; const headers = { 'Content-Type': LOGIN_CONTENT_TYPE }; - const data = `username=${un}&password=${pw}&next=${encodeURIComponent(constant.API_CONFIG)}`; + const data = `username=${un}&password=${pw}&next=${encodeURIComponent(endpoints.API_CONFIG)}`; APIClient.setCookie = jest.fn(); APIClient.login(un, pw).then(() => { expect(mockAxios.post).toHaveBeenCalledTimes(1); - expect(mockAxios.post).toHaveBeenCalledWith(constant.API_LOGIN, data, { headers }); + expect(mockAxios.post).toHaveBeenCalledWith(endpoints.API_LOGIN, data, { headers }); done(); }); }); diff --git a/src/App.jsx b/src/App.jsx index 26cf0724b2..98adc2d5c4 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -81,7 +81,7 @@ class App extends React.Component { } onDevLogout = async () => { - await api.BaseGet(API_LOGOUT); + await api.get(API_LOGOUT); this.setState({ activeGroup: 'views_group', activeItem: 'views_group_dashboard' }); } diff --git a/src/api.js b/src/api.js index 0b0513cc85..8063b24c19 100644 --- a/src/api.js +++ b/src/api.js @@ -1,6 +1,6 @@ import axios from 'axios'; -import * as constant from './endpoints'; +import * as endpoints from './endpoints'; const CSRF_COOKIE_NAME = 'csrftoken'; const CSRF_HEADER_NAME = 'X-CSRFToken'; @@ -32,7 +32,7 @@ class APIClient { return authenticated; } - async login (username, password, redirect = constant.API_CONFIG) { + async login (username, password, redirect = endpoints.API_CONFIG) { const un = encodeURIComponent(username); const pw = encodeURIComponent(password); const next = encodeURIComponent(redirect); @@ -40,11 +40,11 @@ class APIClient { const data = `username=${un}&password=${pw}&next=${next}`; const headers = { 'Content-Type': LOGIN_CONTENT_TYPE }; - await this.http.get(constant.API_LOGIN, { headers }); - await this.http.post(constant.API_LOGIN, data, { headers }); + await this.http.get(endpoints.API_LOGIN, { headers }); + await this.http.post(endpoints.API_LOGIN, data, { headers }); } - BaseGet = (endpoint) => this.http.get(endpoint); + get = (endpoint) => this.http.get(endpoint); } diff --git a/src/index.jsx b/src/index.jsx index f5d675b04f..d20574471f 100644 --- a/src/index.jsx +++ b/src/index.jsx @@ -13,7 +13,7 @@ import './app.scss'; const el = document.getElementById('app'); const main = async () => { - const { custom_logo, custom_login_info } = await api.BaseGet(API_ROOT); + const { custom_logo, custom_login_info } = await api.get(API_ROOT); render(, el); }; diff --git a/src/pages/Organizations.jsx b/src/pages/Organizations.jsx index 99cad1650d..84708f91a8 100644 --- a/src/pages/Organizations.jsx +++ b/src/pages/Organizations.jsx @@ -19,7 +19,7 @@ class Organizations extends Component { } async componentDidMount () { - const { data } = await api.BaseGet(API_ORGANIZATIONS); + const { data } = await api.get(API_ORGANIZATIONS); this.setState({ organizations: data.results }); } From b0855ee33d3233ec1e3a131c5f5a32d7599962b0 Mon Sep 17 00:00:00 2001 From: kialam Date: Tue, 13 Nov 2018 12:11:52 -0500 Subject: [PATCH 4/4] Fix unhandled promise reject from jenkins. --- src/api.js | 1 - src/pages/Organizations.jsx | 16 ++++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/api.js b/src/api.js index 8063b24c19..1650c5d8df 100644 --- a/src/api.js +++ b/src/api.js @@ -45,7 +45,6 @@ class APIClient { } get = (endpoint) => this.http.get(endpoint); - } export default new APIClient(); diff --git a/src/pages/Organizations.jsx b/src/pages/Organizations.jsx index 84708f91a8..2a332bced1 100644 --- a/src/pages/Organizations.jsx +++ b/src/pages/Organizations.jsx @@ -15,17 +15,24 @@ class Organizations extends Component { constructor (props) { super(props); - this.state = { organizations: [] }; + this.state = { + organizations: [], + error: false, + }; } async componentDidMount () { - const { data } = await api.get(API_ORGANIZATIONS); - this.setState({ organizations: data.results }); + try { + const { data } = await api.get(API_ORGANIZATIONS); + this.setState({ organizations: data.results }); + } catch (err) { + this.setState({ error: err }); + } } render () { const { light, medium } = PageSectionVariants; - const { organizations } = this.state; + const { organizations, error } = this.state; return ( @@ -39,6 +46,7 @@ class Organizations extends Component { ))} + { error ?
error
: '' }