From 8cfe74a854793e511ec62e3d0b841e64ab60f118 Mon Sep 17 00:00:00 2001 From: mabashian Date: Fri, 26 Apr 2019 11:02:16 -0400 Subject: [PATCH] Code cleanup, renaming functions, use .all() on config promises --- CONTRIBUTING.md | 1 + .../OrganizationAccessList.test.jsx | 24 +++++--------- src/api.js | 2 +- src/contexts/Config.jsx | 31 +++++++------------ src/pages/Login.jsx | 2 +- .../screens/Organization/Organization.jsx | 3 -- .../screens/OrganizationsList.jsx | 21 +++++++------ 7 files changed, 34 insertions(+), 50 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d6d958fe92..8ec52296db 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -171,6 +171,7 @@ Here are the guidelines for how to name functions. |`replace`| Use for methods that make API `PUT` requests | |`disassociate`| Use for methods that pass `{ disassociate: true }` as a data param to an endpoint | |`associate`| Use for methods that pass a resource id as a data param to an endpoint | +|`can`| Use for props dealing with RBAC to denote whether a user has access to something | ### Default State Initialization When declaring empty initial states, prefer the following instead of leaving them undefined: diff --git a/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx index 24473e3453..1ea9dc9868 100644 --- a/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx +++ b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx @@ -51,9 +51,8 @@ describe('', () => { {}} removeRole={() => {}} - api={api} organization={organization} - /> + />, { context: { network: { api } } } ); }); @@ -62,9 +61,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - api={api} organization={organization} - /> + />, { context: { network: { api } } } ).find('OrganizationAccessList'); setImmediate(() => { @@ -79,9 +77,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - api={api} organization={organization} - /> + />, { context: { network: { api } } } ).find('OrganizationAccessList'); expect(onSort).not.toHaveBeenCalled(); @@ -98,9 +95,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - api={api} organization={organization} - /> + />, { context: { network: { api } } } ).find('OrganizationAccessList'); setImmediate(() => { @@ -120,9 +116,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - api={api} organization={organization} - /> + />, { context: { network: { api } } } ).find('OrganizationAccessList'); expect(handleWarning).not.toHaveBeenCalled(); expect(confirmDelete).not.toHaveBeenCalled(); @@ -145,9 +140,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - api={api} organization={organization} - /> + />, { context: { network: { api } } } ).find('OrganizationAccessList'); setImmediate(() => { @@ -182,9 +176,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - api={api} organization={organization} - /> + />, { context: { network: { api } } } ).find('OrganizationAccessList'); setImmediate(() => { @@ -200,9 +193,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - api={api} organization={readOnlyOrg} - /> + />, { context: { network: { api } } } ).find('OrganizationAccessList'); setImmediate(() => { diff --git a/src/api.js b/src/api.js index 9767632c6a..96842e64b6 100644 --- a/src/api.js +++ b/src/api.js @@ -76,7 +76,7 @@ class APIClient { return this.http.post(API_ORGANIZATIONS, data); } - callOrganizations () { + optionsOrganizations () { return this.http.options(API_ORGANIZATIONS); } diff --git a/src/contexts/Config.jsx b/src/contexts/Config.jsx index ab4bfed202..c5dfa6ed8f 100644 --- a/src/contexts/Config.jsx +++ b/src/contexts/Config.jsx @@ -78,28 +78,19 @@ class Provider extends Component { const { api, handleHttpError } = this.props; try { - const { - data: { - ansible_version, - custom_virtualenvs, - version - } - } = await api.getConfig(); - const { - data: { - custom_logo, - custom_login_info - } - } = await api.getRoot(); - const { data: { results: [me] } } = await api.getMe(); + const [configRes, rootRes, meRes] = await Promise.all([ + api.getConfig(), + api.getRoot(), + api.getMe() + ]); this.setState({ value: { - ansible_version, - custom_virtualenvs, - version, - custom_logo, - custom_login_info, - me + ansible_version: configRes.data.ansible_version, + custom_virtualenvs: configRes.data.custom_virtualenvs, + version: configRes.data.version, + custom_logo: rootRes.data.custom_logo, + custom_login_info: rootRes.data.custom_login_info, + me: meRes.data.results } }); } catch (err) { diff --git a/src/pages/Login.jsx b/src/pages/Login.jsx index 849aaa6cc5..27ca4cfb5f 100644 --- a/src/pages/Login.jsx +++ b/src/pages/Login.jsx @@ -53,7 +53,7 @@ class AWXLogin extends Component { try { const { data } = await api.login(username, password); updateConfig(data); - fetchMe(); + await fetchMe(); this.setState({ isAuthenticated: true, isLoading: false }); } catch (error) { handleHttpError(error) || this.setState({ isInputValid: false, isLoading: false }); diff --git a/src/pages/Organizations/screens/Organization/Organization.jsx b/src/pages/Organizations/screens/Organization/Organization.jsx index e4d8a837ec..cb267f1391 100644 --- a/src/pages/Organizations/screens/Organization/Organization.jsx +++ b/src/pages/Organizations/screens/Organization/Organization.jsx @@ -240,9 +240,6 @@ class Organization extends Component { path="/organizations/:id/notifications" render={() => ( )} diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index f0d0054715..035665ed3b 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -73,7 +73,7 @@ class OrganizationsList extends Component { this.onSelectAll = this.onSelectAll.bind(this); this.onSelect = this.onSelect.bind(this); this.updateUrl = this.updateUrl.bind(this); - this.callOrganizations = this.callOrganizations.bind(this); + this.fetchOptionsOrganizations = this.fetchOptionsOrganizations.bind(this); this.fetchOrganizations = this.fetchOrganizations.bind(this); this.handleOrgDelete = this.handleOrgDelete.bind(this); this.handleOpenOrgDeleteModal = this.handleOpenOrgDeleteModal.bind(this); @@ -82,7 +82,7 @@ class OrganizationsList extends Component { componentDidMount () { const queryParams = this.getQueryParams(); - this.callOrganizations(); + this.fetchOptionsOrganizations(); this.fetchOrganizations(queryParams); } @@ -240,11 +240,11 @@ class OrganizationsList extends Component { } } - async callOrganizations () { + async fetchOptionsOrganizations () { const { api } = this.props; try { - const { data } = await api.callOrganizations(); + const { data } = await api.optionsOrganizations(); const { actions } = data; const stateToUpdate = { @@ -345,11 +345,14 @@ class OrganizationsList extends Component { You dont have permission to delete the following Organizations: - {selected.map(row => ( -
- {row.name} -
- ))} + {selected + .filter(row => !row.summary_fields.user_capabilities.delete) + .map(row => ( +
+ {row.name} +
+ )) + } ) : undefined }