From 58444a75b970fc5e3760e1a8c276496392b4c320 Mon Sep 17 00:00:00 2001 From: Kia Lam Date: Mon, 1 Jul 2019 10:55:11 -0400 Subject: [PATCH 1/4] Fix Org list returning a 404 by redirecting user to current page. - Update itemCount after an org has been successfully deleted. - Update PaginatedDataList to get current page when the number of items has changed. --- .../PaginatedDataList/PaginatedDataList.jsx | 25 +++++++++++++++++-- .../OrganizationList/OrganizationList.jsx | 7 +++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index f273971034..466b006ea2 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -38,7 +38,15 @@ class PaginatedDataList extends React.Component { this.handleSort = this.handleSort.bind(this); } - getSortOrder() { + componentDidUpdate(prevProps) { + const { itemCount: prevItemCount } = prevProps; + const { itemCount } = this.props + if (prevItemCount !== itemCount) { + this.getCurrPage(itemCount); + } + } + + getSortOrder () { const { qsConfig, location } = this.props; const queryParams = parseNamespacedQueryString(qsConfig, location.search); if (queryParams.order_by && queryParams.order_by.startsWith('-')) { @@ -47,7 +55,20 @@ class PaginatedDataList extends React.Component { return [queryParams.order_by, 'ascending']; } - handleSetPage(event, pageNumber) { + getCurrPage (itemCount) { + if (itemCount < 0) { + return; + } + const { qsConfig, location } = this.props; + const queryParams = parseNamespacedQueryString(qsConfig, location.search); + const maxPages = Math.ceil(itemCount / queryParams.page_size); + const currPage = queryParams.page; + if (currPage > maxPages) { + this.pushHistoryState({ page: (currPage + (maxPages - currPage)) || 1 }) + } + } + + handleSetPage (event, pageNumber) { this.pushHistoryState({ page: pageNumber }); } diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx index fb07b0e7f1..3819601803 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx @@ -75,12 +75,13 @@ class OrganizationsList extends Component { this.setState({ deletionError: null }); } - async handleOrgDelete() { - const { selected } = this.state; + async handleOrgDelete () { + const { selected, itemCount } = this.state; this.setState({ hasContentLoading: true }); try { - await Promise.all(selected.map(org => OrganizationsAPI.destroy(org.id))); + await Promise.all(selected.map((org) => OrganizationsAPI.destroy(org.id))); + this.setState({ itemCount: itemCount - selected.length }); } catch (err) { this.setState({ deletionError: err }); } finally { From d22cafc42e2262c9e293fb19775985fb1f23339c Mon Sep 17 00:00:00 2001 From: Kia Lam Date: Mon, 1 Jul 2019 10:57:15 -0400 Subject: [PATCH 2/4] Add unit test. --- .../PaginatedDataList.test.jsx | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx index e8182d7616..9ef2231484 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx @@ -10,6 +10,8 @@ const mockData = [ { id: 3, name: 'three', url: '/org/team/3' }, { id: 4, name: 'four', url: '/org/team/4' }, { id: 5, name: 'five', url: '/org/team/5' }, + { id: 6, name: 'six', url: '/org/team/6' }, + { id: 7, name: 'seven', url: '/org/team/7' }, ]; const qsConfig = { @@ -123,4 +125,34 @@ describe('', () => { pagination.prop('onPerPageSelect')(null, 25); expect(history.location.search).toEqual('?item.page_size=25'); }); + test('should navigate to correct current page when list items change', () => { + const customQSConfig = { + namespace: 'foo', + defaultParams: { page: 7, page_size: 1 }, // show only 1 item per page + integerFields: [], + }; + const testParams = [5, 25, 0, -1]; // number of items + const expected = [5, 5, 1, 1] // expected current page + const history = createMemoryHistory({ + initialEntries: ['/organizations/1/teams'], + }); + const wrapper = mountWithContexts( + , { context: { router: { history } } } + ); + testParams.forEach((param, i) => { + wrapper.setProps({ itemCount: param }); + expect(history.location.search).toEqual(`?${customQSConfig.namespace}.page=${expected[i]}`) + wrapper.update(); + }) + wrapper.unmount(); + }); }); From d3ed6ac73a57ce3afb27e9a1b04244066dab60de Mon Sep 17 00:00:00 2001 From: Kia Lam Date: Mon, 1 Jul 2019 10:57:49 -0400 Subject: [PATCH 3/4] Fix Template list as well. --- .../Template/TemplateList/TemplateList.jsx | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index 4ac2956c5e..db8636fd96 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -77,22 +77,21 @@ class TemplatesList extends Component { } } - async handleTemplateDelete() { - const { selected } = this.state; + async handleTemplateDelete () { + const { selected, itemCount } = this.state; this.setState({ hasContentLoading: true }); try { - await Promise.all( - selected.map(({ type, id }) => { - let deletePromise; - if (type === 'job_template') { - deletePromise = JobTemplatesAPI.destroy(id); - } else if (type === 'workflow_job_template') { - deletePromise = WorkflowJobTemplatesAPI.destroy(id); - } - return deletePromise; - }) - ); + await Promise.all(selected.map(({ type, id }) => { + let deletePromise; + if (type === 'job_template') { + deletePromise = JobTemplatesAPI.destroy(id); + } else if (type === 'workflow_job_template') { + deletePromise = WorkflowJobTemplatesAPI.destroy(id); + } + return deletePromise; + })); + this.setState({ itemCount: itemCount - selected.length }); } catch (err) { this.setState({ deletionError: err }); } finally { From 74e4c17b632021cc96c0c5b2b270de32607bf4bd Mon Sep 17 00:00:00 2001 From: Kia Lam Date: Tue, 2 Jul 2019 09:54:07 -0400 Subject: [PATCH 4/4] Address PR feedback and format. --- .../PaginatedDataList/PaginatedDataList.jsx | 20 +++++++++-------- .../PaginatedDataList.test.jsx | 11 ++++++---- .../OrganizationList/OrganizationList.jsx | 4 ++-- .../Template/TemplateList/TemplateList.jsx | 22 ++++++++++--------- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index 466b006ea2..48bc97bf73 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -40,13 +40,13 @@ class PaginatedDataList extends React.Component { componentDidUpdate(prevProps) { const { itemCount: prevItemCount } = prevProps; - const { itemCount } = this.props + const { itemCount } = this.props; if (prevItemCount !== itemCount) { this.getCurrPage(itemCount); } } - getSortOrder () { + getSortOrder() { const { qsConfig, location } = this.props; const queryParams = parseNamespacedQueryString(qsConfig, location.search); if (queryParams.order_by && queryParams.order_by.startsWith('-')) { @@ -55,20 +55,22 @@ class PaginatedDataList extends React.Component { return [queryParams.order_by, 'ascending']; } - getCurrPage (itemCount) { + getCurrPage(itemCount) { if (itemCount < 0) { return; } const { qsConfig, location } = this.props; - const queryParams = parseNamespacedQueryString(qsConfig, location.search); - const maxPages = Math.ceil(itemCount / queryParams.page_size); - const currPage = queryParams.page; - if (currPage > maxPages) { - this.pushHistoryState({ page: (currPage + (maxPages - currPage)) || 1 }) + const { page_size, page: currPage } = parseNamespacedQueryString( + qsConfig, + location.search + ); + const lastPage = Math.ceil(itemCount / page_size); + if (currPage > lastPage) { + this.pushHistoryState({ page: lastPage || 1 }); } } - handleSetPage (event, pageNumber) { + handleSetPage(event, pageNumber) { this.pushHistoryState({ page: pageNumber }); } diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx index 9ef2231484..1c9a22c374 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx @@ -132,7 +132,7 @@ describe('', () => { integerFields: [], }; const testParams = [5, 25, 0, -1]; // number of items - const expected = [5, 5, 1, 1] // expected current page + const expected = [5, 5, 1, 1]; // expected current page const history = createMemoryHistory({ initialEntries: ['/organizations/1/teams'], }); @@ -146,13 +146,16 @@ describe('', () => { order_by: 'name', }} qsConfig={customQSConfig} - />, { context: { router: { history } } } + />, + { context: { router: { history } } } ); testParams.forEach((param, i) => { wrapper.setProps({ itemCount: param }); - expect(history.location.search).toEqual(`?${customQSConfig.namespace}.page=${expected[i]}`) + expect(history.location.search).toEqual( + `?${customQSConfig.namespace}.page=${expected[i]}` + ); wrapper.update(); - }) + }); wrapper.unmount(); }); }); diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx index 3819601803..037241a156 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx @@ -75,12 +75,12 @@ class OrganizationsList extends Component { this.setState({ deletionError: null }); } - async handleOrgDelete () { + async handleOrgDelete() { const { selected, itemCount } = this.state; this.setState({ hasContentLoading: true }); try { - await Promise.all(selected.map((org) => OrganizationsAPI.destroy(org.id))); + await Promise.all(selected.map(org => OrganizationsAPI.destroy(org.id))); this.setState({ itemCount: itemCount - selected.length }); } catch (err) { this.setState({ deletionError: err }); diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index db8636fd96..93c3352790 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -77,20 +77,22 @@ class TemplatesList extends Component { } } - async handleTemplateDelete () { + async handleTemplateDelete() { const { selected, itemCount } = this.state; this.setState({ hasContentLoading: true }); try { - await Promise.all(selected.map(({ type, id }) => { - let deletePromise; - if (type === 'job_template') { - deletePromise = JobTemplatesAPI.destroy(id); - } else if (type === 'workflow_job_template') { - deletePromise = WorkflowJobTemplatesAPI.destroy(id); - } - return deletePromise; - })); + await Promise.all( + selected.map(({ type, id }) => { + let deletePromise; + if (type === 'job_template') { + deletePromise = JobTemplatesAPI.destroy(id); + } else if (type === 'workflow_job_template') { + deletePromise = WorkflowJobTemplatesAPI.destroy(id); + } + return deletePromise; + }) + ); this.setState({ itemCount: itemCount - selected.length }); } catch (err) { this.setState({ deletionError: err });