From aa12e323b42bdec30470e155da9b0ba7f8f7f69c Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 3 Jun 2020 11:11:40 -0700 Subject: [PATCH 01/17] ignore .env.local file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 2f3635eabe..406382197f 100644 --- a/.gitignore +++ b/.gitignore @@ -31,6 +31,7 @@ awx/ui/templates/ui/installing.html awx/ui_next/node_modules/ awx/ui_next/coverage/ awx/ui_next/build +awx/ui_next/.env.local rsyslog.pid /tower-license /tower-license/** From f2641de260707fa76f3651791dbea96c7c6fbaf2 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 3 Jun 2020 11:14:32 -0700 Subject: [PATCH 02/17] rough out jobs list websockets --- .../src/components/JobList/JobList.jsx | 76 ++++++--- .../src/components/JobList/useWebSocket.js | 151 ++++++++++++++++++ 2 files changed, 202 insertions(+), 25 deletions(-) create mode 100644 awx/ui_next/src/components/JobList/useWebSocket.js diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index 403f5bfc9a..68bbabd9fb 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -11,6 +11,7 @@ import PaginatedDataList, { ToolbarDeleteButton } from '../PaginatedDataList'; import useRequest, { useDeleteItems } from '../../util/useRequest'; import { getQSConfig, parseQueryString } from '../../util/qs'; import JobListItem from './JobListItem'; +import useWebSocket from './useWebSocket'; import { AdHocCommandsAPI, InventoryUpdatesAPI, @@ -36,34 +37,59 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { const [selected, setSelected] = useState([]); const location = useLocation(); - + const params = parseQueryString(QS_CONFIG, location.search); const { - result: { jobs, itemCount }, - error: contentError, + jobs, + count: itemCount, + contentError, isLoading, - request: fetchJobs, - } = useRequest( - useCallback(async () => { - const params = parseQueryString(QS_CONFIG, location.search); + fetchJobs, + } = useWebSocket(params); + // should this happen automatically inside the hook? + // useEffect(() => { + // fetchJobs(); + // }, [fetchJobs]); + // useCallback(async () => { + // const params = parseQueryString(QS_CONFIG, location.search); + // + // const { + // data: { count, results }, + // } = await UnifiedJobsAPI.read({ ...params }); + // + // return { + // itemCount: count, + // jobs: results, + // }; + // }, [QS_CONFIG, location]) + // ); // eslint-disable-line react-hooks/exhaustive-deps - const { - data: { count, results }, - } = await UnifiedJobsAPI.read({ ...params }); - - return { - itemCount: count, - jobs: results, - }; - }, [location]), // eslint-disable-line react-hooks/exhaustive-deps - { - jobs: [], - itemCount: 0, - } - ); - - useEffect(() => { - fetchJobs(); - }, [fetchJobs]); + // const { + // result: { jobs, itemCount }, + // error: contentError, + // isLoading, + // request: fetchJobs, + // } = useRequest( + // useCallback(async () => { + // const params = parseQueryString(QS_CONFIG, location.search); + // + // const { + // data: { count, results }, + // } = await UnifiedJobsAPI.read({ ...params }); + // + // return { + // itemCount: count, + // jobs: results, + // }; + // }, [location]), // eslint-disable-line react-hooks/exhaustive-deps + // { + // jobs: [], + // itemCount: 0, + // } + // ); + // + // useEffect(() => { + // fetchJobs(); + // }, [fetchJobs]); const isAllSelected = selected.length === jobs.length && selected.length > 0; const { diff --git a/awx/ui_next/src/components/JobList/useWebSocket.js b/awx/ui_next/src/components/JobList/useWebSocket.js new file mode 100644 index 0000000000..79c9430537 --- /dev/null +++ b/awx/ui_next/src/components/JobList/useWebSocket.js @@ -0,0 +1,151 @@ +import { useState, useEffect, useCallback, useRef } from 'react'; +import useRequest from '../../util/useRequest'; +import { UnifiedJobsAPI } from '../../api'; + +// rename: useWsJobs ? +export default function useWebSocket(params) { + const [jobs, setJobs] = useState([]); + const { + result: { results, count }, + error: contentError, + isLoading, + request: fetchJobs, + } = useRequest( + useCallback(async () => { + console.log('fetching...'); + const { data } = await UnifiedJobsAPI.read({ ...params }); + return data; + }, [params]), + { results: [], count: 0 } + ); + useEffect(() => { + fetchJobs(); + }, []); // eslint-disable-line react-hooks/exhaustive-deps + useEffect(() => { + setJobs(results); + }, [results]); + const ws = useRef(null); + + useEffect(() => { + ws.current = new WebSocket(`wss://${window.location.host}/websocket/`); + + const connect = () => { + const xrftoken = `; ${document.cookie}` + .split('; csrftoken=') + .pop() + .split(';') + .shift(); + ws.current.send( + JSON.stringify({ + xrftoken, + groups: { + jobs: ['status_changed'], + schedules: ['changed'], + control: ['limit_reached_1'], + }, + }) + ); + }; + ws.current.onopen = connect; + + ws.current.onmessage = e => { + const data = JSON.parse(e.data); + // console.log(JSON.parse(e.data)); + const jobId = data.unified_job_id; + if (!jobId) { + return; + } + + // TODO: Pull this function up... jobs is being closed over + const index = jobs.findIndex(j => j.id === jobId); + console.log('index', index); + console.log(jobId, typeof jobId, jobs); + if (index > -1) { + const job = { + ...jobs[index], + status: data.status, + finished: data.finished, + }; + console.log('updated job:', job); + const newJobs = [ + ...jobs.slice(0, index), + job, + ...jobs.slice(index + 1), + ]; + console.log('updating jobs: ', newJobs); + setJobs(newJobs); + } + }; + + ws.current.onclose = e => { + // eslint-disable-next-line no-console + console.debug('Socket closed. Reconnecting...', e); + setTimeout(() => { + connect(); + }, 1000); + }; + + ws.current.onerror = err => { + // eslint-disable-next-line no-console + console.debug('Socket error: ', err, 'Disconnecting...'); + ws.close(); + }; + + return () => { + ws.current.close(); + }; + }, []); + + return { + jobs, + count, + contentError, + isLoading, + fetchJobs, + }; +} + +const initial = { + groups_current: [ + 'schedules-changed', + 'control-limit_reached_1', + 'jobs-status_changed', + ], + groups_left: [], + groups_joined: [ + 'schedules-changed', + 'control-limit_reached_1', + 'jobs-status_changed', + ], +}; + +const one = { + unified_job_id: 292, + status: 'pending', + type: 'job', + group_name: 'jobs', + unified_job_template_id: 26, +}; +const two = { + unified_job_id: 292, + status: 'waiting', + instance_group_name: 'tower', + type: 'job', + group_name: 'jobs', + unified_job_template_id: 26, +}; +const three = { + unified_job_id: 293, + status: 'running', + type: 'job', + group_name: 'jobs', + unified_job_template_id: 26, +}; +const four = { + unified_job_id: 293, + status: 'successful', + finished: '2020-06-01T21:49:28.704114Z', + type: 'job', + group_name: 'jobs', + unified_job_template_id: 26, +}; From b055d34139a39eca510e5022ad4b7dc9b72d9893 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 4 Jun 2020 11:30:01 -0700 Subject: [PATCH 03/17] update jobs in list based on websockets --- .../src/components/JobList/JobList.jsx | 69 ++------ .../src/components/JobList/JobListItem.jsx | 2 +- .../src/components/JobList/useWebSocket.js | 151 ------------------ .../src/components/JobList/useWsJobs.js | 138 ++++++++++++++++ 4 files changed, 156 insertions(+), 204 deletions(-) delete mode 100644 awx/ui_next/src/components/JobList/useWebSocket.js create mode 100644 awx/ui_next/src/components/JobList/useWsJobs.js diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index 68bbabd9fb..ccbb32159b 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -11,7 +11,7 @@ import PaginatedDataList, { ToolbarDeleteButton } from '../PaginatedDataList'; import useRequest, { useDeleteItems } from '../../util/useRequest'; import { getQSConfig, parseQueryString } from '../../util/qs'; import JobListItem from './JobListItem'; -import useWebSocket from './useWebSocket'; +import useWsJobs from './useWsJobs'; import { AdHocCommandsAPI, InventoryUpdatesAPI, @@ -37,59 +37,24 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { const [selected, setSelected] = useState([]); const location = useLocation(); - const params = parseQueryString(QS_CONFIG, location.search); const { - jobs, - count: itemCount, - contentError, + result: { results, count }, + error: contentError, isLoading, - fetchJobs, - } = useWebSocket(params); - // should this happen automatically inside the hook? - // useEffect(() => { - // fetchJobs(); - // }, [fetchJobs]); - // useCallback(async () => { - // const params = parseQueryString(QS_CONFIG, location.search); - // - // const { - // data: { count, results }, - // } = await UnifiedJobsAPI.read({ ...params }); - // - // return { - // itemCount: count, - // jobs: results, - // }; - // }, [QS_CONFIG, location]) - // ); // eslint-disable-line react-hooks/exhaustive-deps + request: fetchJobs, + } = useRequest( + useCallback(async () => { + const params = parseQueryString(QS_CONFIG, location.search); + const { data } = await UnifiedJobsAPI.read({ ...params }); + return data; + }, [location]), // eslint-disable-line react-hooks/exhaustive-deps + { results: [], count: 0 } + ); + useEffect(() => { + fetchJobs(); + }, [fetchJobs]); - // const { - // result: { jobs, itemCount }, - // error: contentError, - // isLoading, - // request: fetchJobs, - // } = useRequest( - // useCallback(async () => { - // const params = parseQueryString(QS_CONFIG, location.search); - // - // const { - // data: { count, results }, - // } = await UnifiedJobsAPI.read({ ...params }); - // - // return { - // itemCount: count, - // jobs: results, - // }; - // }, [location]), // eslint-disable-line react-hooks/exhaustive-deps - // { - // jobs: [], - // itemCount: 0, - // } - // ); - // - // useEffect(() => { - // fetchJobs(); - // }, [fetchJobs]); + const jobs = useWsJobs(results, fetchJobs); const isAllSelected = selected.length === jobs.length && selected.length > 0; const { @@ -151,7 +116,7 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { contentError={contentError} hasContentLoading={isLoading || isDeleteLoading} items={jobs} - itemCount={itemCount} + itemCount={count} pluralizedItemName={i18n._(t`Jobs`)} qsConfig={QS_CONFIG} onRowClick={handleSelect} diff --git a/awx/ui_next/src/components/JobList/JobListItem.jsx b/awx/ui_next/src/components/JobList/JobListItem.jsx index 296ecba9c7..a813641a7f 100644 --- a/awx/ui_next/src/components/JobList/JobListItem.jsx +++ b/awx/ui_next/src/components/JobList/JobListItem.jsx @@ -75,7 +75,7 @@ function JobListItem({ ] : []), - {formatDateString(job.finished)} + {job.finished ? formatDateString(job.finished) : ''} , ]} /> diff --git a/awx/ui_next/src/components/JobList/useWebSocket.js b/awx/ui_next/src/components/JobList/useWebSocket.js deleted file mode 100644 index 79c9430537..0000000000 --- a/awx/ui_next/src/components/JobList/useWebSocket.js +++ /dev/null @@ -1,151 +0,0 @@ -import { useState, useEffect, useCallback, useRef } from 'react'; -import useRequest from '../../util/useRequest'; -import { UnifiedJobsAPI } from '../../api'; - -// rename: useWsJobs ? -export default function useWebSocket(params) { - const [jobs, setJobs] = useState([]); - const { - result: { results, count }, - error: contentError, - isLoading, - request: fetchJobs, - } = useRequest( - useCallback(async () => { - console.log('fetching...'); - const { data } = await UnifiedJobsAPI.read({ ...params }); - return data; - }, [params]), - { results: [], count: 0 } - ); - useEffect(() => { - fetchJobs(); - }, []); // eslint-disable-line react-hooks/exhaustive-deps - useEffect(() => { - setJobs(results); - }, [results]); - const ws = useRef(null); - - useEffect(() => { - ws.current = new WebSocket(`wss://${window.location.host}/websocket/`); - - const connect = () => { - const xrftoken = `; ${document.cookie}` - .split('; csrftoken=') - .pop() - .split(';') - .shift(); - ws.current.send( - JSON.stringify({ - xrftoken, - groups: { - jobs: ['status_changed'], - schedules: ['changed'], - control: ['limit_reached_1'], - }, - }) - ); - }; - ws.current.onopen = connect; - - ws.current.onmessage = e => { - const data = JSON.parse(e.data); - // console.log(JSON.parse(e.data)); - const jobId = data.unified_job_id; - if (!jobId) { - return; - } - - // TODO: Pull this function up... jobs is being closed over - const index = jobs.findIndex(j => j.id === jobId); - console.log('index', index); - console.log(jobId, typeof jobId, jobs); - if (index > -1) { - const job = { - ...jobs[index], - status: data.status, - finished: data.finished, - }; - console.log('updated job:', job); - const newJobs = [ - ...jobs.slice(0, index), - job, - ...jobs.slice(index + 1), - ]; - console.log('updating jobs: ', newJobs); - setJobs(newJobs); - } - }; - - ws.current.onclose = e => { - // eslint-disable-next-line no-console - console.debug('Socket closed. Reconnecting...', e); - setTimeout(() => { - connect(); - }, 1000); - }; - - ws.current.onerror = err => { - // eslint-disable-next-line no-console - console.debug('Socket error: ', err, 'Disconnecting...'); - ws.close(); - }; - - return () => { - ws.current.close(); - }; - }, []); - - return { - jobs, - count, - contentError, - isLoading, - fetchJobs, - }; -} - -const initial = { - groups_current: [ - 'schedules-changed', - 'control-limit_reached_1', - 'jobs-status_changed', - ], - groups_left: [], - groups_joined: [ - 'schedules-changed', - 'control-limit_reached_1', - 'jobs-status_changed', - ], -}; - -const one = { - unified_job_id: 292, - status: 'pending', - type: 'job', - group_name: 'jobs', - unified_job_template_id: 26, -}; -const two = { - unified_job_id: 292, - status: 'waiting', - instance_group_name: 'tower', - type: 'job', - group_name: 'jobs', - unified_job_template_id: 26, -}; -const three = { - unified_job_id: 293, - status: 'running', - type: 'job', - group_name: 'jobs', - unified_job_template_id: 26, -}; -const four = { - unified_job_id: 293, - status: 'successful', - finished: '2020-06-01T21:49:28.704114Z', - type: 'job', - group_name: 'jobs', - unified_job_template_id: 26, -}; diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js new file mode 100644 index 0000000000..839ccd02af --- /dev/null +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -0,0 +1,138 @@ +import { useState, useEffect, useRef } from 'react'; + +export default function useWsJobs(initialJobs, refetchJobs) { + const [jobs, setJobs] = useState(initialJobs); + const [lastMessage, setLastMessage] = useState(null); + useEffect(() => { + setJobs(initialJobs); + }, [initialJobs]); + + const ws = useRef(null); + + useEffect(() => { + if (!lastMessage || !lastMessage.unified_job_id) { + return; + } + + const jobId = lastMessage.unified_job_id; + const index = jobs.findIndex(j => j.id === jobId); + if (index > -1) { + setJobs(updateJob(jobs, index, lastMessage)); + } else { + setJobs(addJobStub(jobs, lastMessage)); + refetchJobs(); + } + }, [lastMessage]); // eslint-disable-line react-hooks/exhaustive-deps + + useEffect(() => { + ws.current = new WebSocket(`wss://${window.location.host}/websocket/`); + + const connect = () => { + const xrftoken = `; ${document.cookie}` + .split('; csrftoken=') + .pop() + .split(';') + .shift(); + ws.current.send( + JSON.stringify({ + xrftoken, + groups: { + jobs: ['status_changed'], + schedules: ['changed'], + control: ['limit_reached_1'], + }, + }) + ); + }; + ws.current.onopen = connect; + + ws.current.onmessage = e => { + setLastMessage(JSON.parse(e.data)); + }; + + ws.current.onclose = e => { + // eslint-disable-next-line no-console + console.debug('Socket closed. Reconnecting...', e); + setTimeout(() => { + connect(); + }, 1000); + }; + + ws.current.onerror = err => { + // eslint-disable-next-line no-console + console.debug('Socket error: ', err, 'Disconnecting...'); + ws.close(); + }; + + return () => { + ws.current.close(); + }; + }, []); + + return jobs; +} + +function updateJob(jobs, index, message) { + const job = { + ...jobs[index], + status: message.status, + finished: message.finished, + }; + return [...jobs.slice(0, index), job, ...jobs.slice(index + 1)]; +} + +function addJobStub(jobs, message) { + const job = { + id: message.unified_job_id, + status: message.status, + type: message.type, + url: `/api/v2/jobs/${message.unified_job_id}`, + }; + return [job, ...jobs]; +} + +// +// const initial = { +// groups_current: [ +// 'schedules-changed', +// 'control-limit_reached_1', +// 'jobs-status_changed', +// ], +// groups_left: [], +// groups_joined: [ +// 'schedules-changed', +// 'control-limit_reached_1', +// 'jobs-status_changed', +// ], +// }; +// +// const one = { +// unified_job_id: 292, +// status: 'pending', +// type: 'job', +// group_name: 'jobs', +// unified_job_template_id: 26, +// }; +// const two = { +// unified_job_id: 292, +// status: 'waiting', +// instance_group_name: 'tower', +// type: 'job', +// group_name: 'jobs', +// unified_job_template_id: 26, +// }; +// const three = { +// unified_job_id: 293, +// status: 'running', +// type: 'job', +// group_name: 'jobs', +// unified_job_template_id: 26, +// }; +// const four = { +// unified_job_id: 293, +// status: 'successful', +// finished: '2020-06-01T21:49:28.704114Z', +// type: 'job', +// group_name: 'jobs', +// unified_job_template_id: 26, +// }; From 38079b2ad58919b4b69fe96a8d315e71774ffe98 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 5 Jun 2020 14:57:45 -0700 Subject: [PATCH 04/17] =?UTF-8?q?don=E2=80=99t=20add=20still=20running=20j?= =?UTF-8?q?obs=20to=20some=20jobs=20lists?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- awx/ui_next/src/components/JobList/JobList.jsx | 2 +- awx/ui_next/src/components/JobList/useWsJobs.js | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index ccbb32159b..694d250b32 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -54,7 +54,7 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { fetchJobs(); }, [fetchJobs]); - const jobs = useWsJobs(results, fetchJobs); + const jobs = useWsJobs(results, fetchJobs, !!defaultParams); const isAllSelected = selected.length === jobs.length && selected.length > 0; const { diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index 839ccd02af..8296dbd4de 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -1,6 +1,6 @@ import { useState, useEffect, useRef } from 'react'; -export default function useWsJobs(initialJobs, refetchJobs) { +export default function useWsJobs(initialJobs, refetchJobs, filtersApplied) { const [jobs, setJobs] = useState(initialJobs); const [lastMessage, setLastMessage] = useState(null); useEffect(() => { @@ -13,6 +13,12 @@ export default function useWsJobs(initialJobs, refetchJobs) { if (!lastMessage || !lastMessage.unified_job_id) { return; } + if (filtersApplied) { + if (['completed', 'failed', 'error'].includes(lastMessage.status)) { + refetchJobs(); + } + return; + } const jobId = lastMessage.unified_job_id; const index = jobs.findIndex(j => j.id === jobId); From 58b954df3ef8222f0c5608494b87a1e3d3694af5 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Mon, 8 Jun 2020 16:26:39 -0700 Subject: [PATCH 05/17] fetch individual jobs based on websocket messages --- .../src/components/JobList/JobList.jsx | 35 ++++++++-- .../src/components/JobList/useWsJobs.js | 65 ++----------------- 2 files changed, 33 insertions(+), 67 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index 694d250b32..933284c776 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -42,19 +42,42 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { error: contentError, isLoading, request: fetchJobs, + setValue, } = useRequest( - useCallback(async () => { - const params = parseQueryString(QS_CONFIG, location.search); - const { data } = await UnifiedJobsAPI.read({ ...params }); - return data; - }, [location]), // eslint-disable-line react-hooks/exhaustive-deps + useCallback( + async () => { + const params = parseQueryString(QS_CONFIG, location.search); + const { data } = await UnifiedJobsAPI.read({ ...params }); + return data; + }, + [location] // eslint-disable-line react-hooks/exhaustive-deps + ), { results: [], count: 0 } ); useEffect(() => { fetchJobs(); }, [fetchJobs]); - const jobs = useWsJobs(results, fetchJobs, !!defaultParams); + const { request: addJobsById } = useRequest( + useCallback( + async ids => { + const params = parseQueryString(QS_CONFIG, location.search); + params.id__in = ids; + const { data } = await UnifiedJobsAPI.read({ ...params }); + const mergedJobsList = [...data.results, ...results].slice( + 0, + params.page_size + ); + setValue({ + results: mergedJobsList, + count: count + data.count, + }); + }, + [location, setValue, QS_CONFIG] // eslint-disable-line react-hooks/exhaustive-deps + ) + ); + + const jobs = useWsJobs(results, addJobsById, !!defaultParams); const isAllSelected = selected.length === jobs.length && selected.length > 0; const { diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index 8296dbd4de..f96295d0d7 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -1,6 +1,6 @@ import { useState, useEffect, useRef } from 'react'; -export default function useWsJobs(initialJobs, refetchJobs, filtersApplied) { +export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { const [jobs, setJobs] = useState(initialJobs); const [lastMessage, setLastMessage] = useState(null); useEffect(() => { @@ -15,7 +15,7 @@ export default function useWsJobs(initialJobs, refetchJobs, filtersApplied) { } if (filtersApplied) { if (['completed', 'failed', 'error'].includes(lastMessage.status)) { - refetchJobs(); + fetchJobsById([lastMessage.unified_job_id]); } return; } @@ -25,8 +25,7 @@ export default function useWsJobs(initialJobs, refetchJobs, filtersApplied) { if (index > -1) { setJobs(updateJob(jobs, index, lastMessage)); } else { - setJobs(addJobStub(jobs, lastMessage)); - refetchJobs(); + fetchJobsById([lastMessage.unified_job_id]); } }, [lastMessage]); // eslint-disable-line react-hooks/exhaustive-deps @@ -67,7 +66,7 @@ export default function useWsJobs(initialJobs, refetchJobs, filtersApplied) { ws.current.onerror = err => { // eslint-disable-next-line no-console console.debug('Socket error: ', err, 'Disconnecting...'); - ws.close(); + ws.current.close(); }; return () => { @@ -86,59 +85,3 @@ function updateJob(jobs, index, message) { }; return [...jobs.slice(0, index), job, ...jobs.slice(index + 1)]; } - -function addJobStub(jobs, message) { - const job = { - id: message.unified_job_id, - status: message.status, - type: message.type, - url: `/api/v2/jobs/${message.unified_job_id}`, - }; - return [job, ...jobs]; -} - -// -// const initial = { -// groups_current: [ -// 'schedules-changed', -// 'control-limit_reached_1', -// 'jobs-status_changed', -// ], -// groups_left: [], -// groups_joined: [ -// 'schedules-changed', -// 'control-limit_reached_1', -// 'jobs-status_changed', -// ], -// }; -// -// const one = { -// unified_job_id: 292, -// status: 'pending', -// type: 'job', -// group_name: 'jobs', -// unified_job_template_id: 26, -// }; -// const two = { -// unified_job_id: 292, -// status: 'waiting', -// instance_group_name: 'tower', -// type: 'job', -// group_name: 'jobs', -// unified_job_template_id: 26, -// }; -// const three = { -// unified_job_id: 293, -// status: 'running', -// type: 'job', -// group_name: 'jobs', -// unified_job_template_id: 26, -// }; -// const four = { -// unified_job_id: 293, -// status: 'successful', -// finished: '2020-06-01T21:49:28.704114Z', -// type: 'job', -// group_name: 'jobs', -// unified_job_template_id: 26, -// }; From 7aa8495d1aba3047f490066026305f583d3d3b40 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Tue, 9 Jun 2020 15:24:15 -0700 Subject: [PATCH 06/17] debounce fetching of individual jobs --- .../src/components/JobList/JobList.jsx | 23 ++++++++----- .../src/components/JobList/useWsJobs.js | 34 +++++++++++++++++-- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index 933284c776..bdd17cd46c 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -61,19 +61,24 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { const { request: addJobsById } = useRequest( useCallback( async ids => { + if (!ids.length) { + return; + } const params = parseQueryString(QS_CONFIG, location.search); - params.id__in = ids; + params.id__in = ids.join(','); const { data } = await UnifiedJobsAPI.read({ ...params }); - const mergedJobsList = [...data.results, ...results].slice( - 0, - params.page_size - ); - setValue({ - results: mergedJobsList, - count: count + data.count, + setValue(prev => { + const mergedJobsList = [...data.results, ...prev.results].slice( + 0, + params.page_size + ); + return { + results: mergedJobsList, + count: prev.count + data.count, + }; }); }, - [location, setValue, QS_CONFIG] // eslint-disable-line react-hooks/exhaustive-deps + [location, setValue] // eslint-disable-line react-hooks/exhaustive-deps ) ); diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index f96295d0d7..02a9c1671b 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -3,10 +3,24 @@ import { useState, useEffect, useRef } from 'react'; export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { const [jobs, setJobs] = useState(initialJobs); const [lastMessage, setLastMessage] = useState(null); + const [jobsToFetch, setJobsToFetch] = useState([]); + const debouncedJobsToFetch = useDebounce(jobsToFetch, 5000); useEffect(() => { setJobs(initialJobs); }, [initialJobs]); + const enqueueJobId = id => { + if (!jobsToFetch.includes(id)) { + setJobsToFetch(ids => ids.concat(id)); + } + }; + useEffect(() => { + if (debouncedJobsToFetch.length) { + fetchJobsById(debouncedJobsToFetch); + setJobsToFetch([]); + } + }, [debouncedJobsToFetch, fetchJobsById]); + const ws = useRef(null); useEffect(() => { @@ -15,7 +29,7 @@ export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { } if (filtersApplied) { if (['completed', 'failed', 'error'].includes(lastMessage.status)) { - fetchJobsById([lastMessage.unified_job_id]); + enqueueJobId(lastMessage.unified_job_id); } return; } @@ -25,7 +39,7 @@ export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { if (index > -1) { setJobs(updateJob(jobs, index, lastMessage)); } else { - fetchJobsById([lastMessage.unified_job_id]); + enqueueJobId(lastMessage.unified_job_id); } }, [lastMessage]); // eslint-disable-line react-hooks/exhaustive-deps @@ -85,3 +99,19 @@ function updateJob(jobs, index, message) { }; return [...jobs.slice(0, index), job, ...jobs.slice(index + 1)]; } + +function useDebounce(value, delay) { + const [debouncedValue, setDebouncedValue] = useState(value); + + useEffect(() => { + const timeout = setTimeout(() => { + setDebouncedValue(value); + }, delay); + + return () => { + clearTimeout(timeout); + }; + }, [value, delay]); + + return debouncedValue; +} From 48977e50df7db3b84794523520ddf8200a3fd539 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 11 Jun 2020 10:52:35 -0700 Subject: [PATCH 07/17] change jobs debounce to throttle; prevent duplicate rows --- .../src/components/JobList/JobList.jsx | 8 ++--- .../src/components/JobList/useWsJobs.js | 30 +++++++++++-------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index bdd17cd46c..435b20d96d 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -68,12 +68,12 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { params.id__in = ids.join(','); const { data } = await UnifiedJobsAPI.read({ ...params }); setValue(prev => { - const mergedJobsList = [...data.results, ...prev.results].slice( - 0, - params.page_size + const deDuplicated = data.results.filter( + job => !prev.results.find(j => j.id === job.id) ); + const mergedJobsList = [...deDuplicated, ...prev.results]; return { - results: mergedJobsList, + results: mergedJobsList.slice(0, params.page_size), count: prev.count + data.count, }; }); diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index 02a9c1671b..8602bc98c3 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -4,7 +4,9 @@ export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { const [jobs, setJobs] = useState(initialJobs); const [lastMessage, setLastMessage] = useState(null); const [jobsToFetch, setJobsToFetch] = useState([]); - const debouncedJobsToFetch = useDebounce(jobsToFetch, 5000); + // const debouncedJobsToFetch = useDebounce(jobsToFetch, 5000); + const throttleJobsToFetch = useThrottle(jobsToFetch, 5000); + useEffect(() => { setJobs(initialJobs); }, [initialJobs]); @@ -15,11 +17,11 @@ export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { } }; useEffect(() => { - if (debouncedJobsToFetch.length) { - fetchJobsById(debouncedJobsToFetch); + if (throttleJobsToFetch.length) { + fetchJobsById(throttleJobsToFetch); setJobsToFetch([]); } - }, [debouncedJobsToFetch, fetchJobsById]); + }, [throttleJobsToFetch, fetchJobsById]); const ws = useRef(null); @@ -100,18 +102,22 @@ function updateJob(jobs, index, message) { return [...jobs.slice(0, index), job, ...jobs.slice(index + 1)]; } -function useDebounce(value, delay) { - const [debouncedValue, setDebouncedValue] = useState(value); +function useThrottle(value, limit) { + const [throttledValue, setThrottledValue] = useState(value); + const lastRan = useRef(Date.now()); useEffect(() => { - const timeout = setTimeout(() => { - setDebouncedValue(value); - }, delay); + const handler = setTimeout(() => { + if (Date.now() - lastRan.current >= limit) { + setThrottledValue(value); + lastRan.current = Date.now(); + } + }, limit - (Date.now() - lastRan.current)); return () => { - clearTimeout(timeout); + clearTimeout(handler); }; - }, [value, delay]); + }, [value, limit]); - return debouncedValue; + return throttledValue; } From 0bedd6fbd89788f87414e7109ff3ba22ef6fe268 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Mon, 15 Jun 2020 14:24:44 -0700 Subject: [PATCH 08/17] mock websockets; test useWsJobs --- awx/ui_next/package-lock.json | 15 +++ awx/ui_next/package.json | 2 + .../src/components/JobList/useThrottle.js | 21 +++ .../components/JobList/useThrottle.test.jsx | 0 .../src/components/JobList/useWsJobs.js | 22 +-- .../src/components/JobList/useWsJobs.test.jsx | 126 ++++++++++++++++++ 6 files changed, 165 insertions(+), 21 deletions(-) create mode 100644 awx/ui_next/src/components/JobList/useThrottle.js create mode 100644 awx/ui_next/src/components/JobList/useThrottle.test.jsx create mode 100644 awx/ui_next/src/components/JobList/useWsJobs.test.jsx diff --git a/awx/ui_next/package-lock.json b/awx/ui_next/package-lock.json index 049b48ad8d..12df9b3259 100644 --- a/awx/ui_next/package-lock.json +++ b/awx/ui_next/package-lock.json @@ -8988,6 +8988,12 @@ } } }, + "jest-websocket-mock": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/jest-websocket-mock/-/jest-websocket-mock-2.0.2.tgz", + "integrity": "sha512-SFTUI8O/LDGqROOMnfAzbrrX5gQ8GDhRqkzVrt8Y67evnFKccRPFI3ymS05tKcMONvVfxumat4pX/LRjM/CjVg==", + "dev": true + }, "jest-worker": { "version": "24.9.0", "resolved": "https://registry.npmjs.org/jest-worker/-/jest-worker-24.9.0.tgz", @@ -9893,6 +9899,15 @@ "minimist": "^1.2.5" } }, + "mock-socket": { + "version": "9.0.3", + "resolved": "https://registry.npmjs.org/mock-socket/-/mock-socket-9.0.3.tgz", + "integrity": "sha512-SxIiD2yE/By79p3cNAAXyLQWTvEFNEzcAO7PH+DzRqKSFaplAPFjiQLmw8ofmpCsZf+Rhfn2/xCJagpdGmYdTw==", + "dev": true, + "requires": { + "url-parse": "^1.4.4" + } + }, "moo": { "version": "0.5.1", "resolved": "https://registry.npmjs.org/moo/-/moo-0.5.1.tgz", diff --git a/awx/ui_next/package.json b/awx/ui_next/package.json index e5ca6eb7f2..74f857ce63 100644 --- a/awx/ui_next/package.json +++ b/awx/ui_next/package.json @@ -72,6 +72,8 @@ "eslint-plugin-react": "^7.11.1", "eslint-plugin-react-hooks": "^2.2.0", "http-proxy-middleware": "^1.0.3", + "jest-websocket-mock": "^2.0.2", + "mock-socket": "^9.0.3", "prettier": "^1.18.2" }, "jest": { diff --git a/awx/ui_next/src/components/JobList/useThrottle.js b/awx/ui_next/src/components/JobList/useThrottle.js new file mode 100644 index 0000000000..cfdedfecfc --- /dev/null +++ b/awx/ui_next/src/components/JobList/useThrottle.js @@ -0,0 +1,21 @@ +import { useState, useEffect, useRef } from 'react'; + +export default function useThrottle(value, limit) { + const [throttledValue, setThrottledValue] = useState(value); + const lastRan = useRef(Date.now()); + + useEffect(() => { + const handler = setTimeout(() => { + if (Date.now() - lastRan.current >= limit) { + setThrottledValue(value); + lastRan.current = Date.now(); + } + }, limit - (Date.now() - lastRan.current)); + + return () => { + clearTimeout(handler); + }; + }, [value, limit]); + + return throttledValue; +} diff --git a/awx/ui_next/src/components/JobList/useThrottle.test.jsx b/awx/ui_next/src/components/JobList/useThrottle.test.jsx new file mode 100644 index 0000000000..e69de29bb2 diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index 8602bc98c3..ab543d6d8e 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -1,10 +1,10 @@ import { useState, useEffect, useRef } from 'react'; +import useThrottle from './useThrottle'; export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { const [jobs, setJobs] = useState(initialJobs); const [lastMessage, setLastMessage] = useState(null); const [jobsToFetch, setJobsToFetch] = useState([]); - // const debouncedJobsToFetch = useDebounce(jobsToFetch, 5000); const throttleJobsToFetch = useThrottle(jobsToFetch, 5000); useEffect(() => { @@ -101,23 +101,3 @@ function updateJob(jobs, index, message) { }; return [...jobs.slice(0, index), job, ...jobs.slice(index + 1)]; } - -function useThrottle(value, limit) { - const [throttledValue, setThrottledValue] = useState(value); - const lastRan = useRef(Date.now()); - - useEffect(() => { - const handler = setTimeout(() => { - if (Date.now() - lastRan.current >= limit) { - setThrottledValue(value); - lastRan.current = Date.now(); - } - }, limit - (Date.now() - lastRan.current)); - - return () => { - clearTimeout(handler); - }; - }, [value, limit]); - - return throttledValue; -} diff --git a/awx/ui_next/src/components/JobList/useWsJobs.test.jsx b/awx/ui_next/src/components/JobList/useWsJobs.test.jsx new file mode 100644 index 0000000000..1dead6d5da --- /dev/null +++ b/awx/ui_next/src/components/JobList/useWsJobs.test.jsx @@ -0,0 +1,126 @@ +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { mount } from 'enzyme'; +import WS from 'jest-websocket-mock'; +import useWsJobs from './useWsJobs'; + +/* + Jest mock timers don’t play well with jest-websocket-mock, + so we'll stub out throttling to resolve immediately +*/ +jest.mock('./useThrottle', () => ({ + __esModule: true, + default: jest.fn(val => val), +})); + +function TestInner() { + return
; +} +function Test({ jobs, fetch }) { + const syncedJobs = useWsJobs(jobs, fetch); + return ; +} + +describe('useWsJobs hook', () => { + let debug; + let wrapper; + beforeEach(() => { + debug = global.console.debug; // eslint-disable-line prefer-destructuring + global.console.debug = () => {}; + }); + + afterEach(() => { + global.console.debug = debug; + }); + + test('should return jobs list', () => { + const jobs = [{ id: 1 }]; + wrapper = mount(); + + expect(wrapper.find('TestInner').prop('jobs')).toEqual(jobs); + WS.clean(); + }); + + test('should establish websocket connection', async () => { + global.document.cookie = 'csrftoken=abc123'; + const mockServer = new WS('wss://localhost/websocket/'); + + const jobs = [{ id: 1 }]; + await act(async () => { + wrapper = await mount(); + }); + + await mockServer.connected; + await expect(mockServer).toReceiveMessage( + JSON.stringify({ + xrftoken: 'abc123', + groups: { + jobs: ['status_changed'], + schedules: ['changed'], + control: ['limit_reached_1'], + }, + }) + ); + WS.clean(); + }); + + test('should update job status', async () => { + global.document.cookie = 'csrftoken=abc123'; + const mockServer = new WS('wss://localhost/websocket/'); + + const jobs = [{ id: 1, status: 'running' }]; + await act(async () => { + wrapper = await mount(); + }); + + await mockServer.connected; + await expect(mockServer).toReceiveMessage( + JSON.stringify({ + xrftoken: 'abc123', + groups: { + jobs: ['status_changed'], + schedules: ['changed'], + control: ['limit_reached_1'], + }, + }) + ); + expect(wrapper.find('TestInner').prop('jobs')[0].status).toEqual('running'); + act(() => { + mockServer.send( + JSON.stringify({ + unified_job_id: 1, + status: 'successful', + }) + ); + }); + wrapper.update(); + + expect(wrapper.find('TestInner').prop('jobs')[0].status).toEqual( + 'successful' + ); + WS.clean(); + }); + + test('should fetch new job', async () => { + global.document.cookie = 'csrftoken=abc123'; + const mockServer = new WS('wss://localhost/websocket/'); + const jobs = [{ id: 1 }]; + const fetch = jest.fn(); + await act(async () => { + wrapper = await mount(); + }); + + await mockServer.connected; + act(() => { + mockServer.send( + JSON.stringify({ + unified_job_id: 2, + status: 'running', + }) + ); + }); + + expect(fetch).toHaveBeenCalledWith([2]); + WS.clean(); + }); +}); From e50576c8207f9c2adf6fa3ffc392e25db599d9d8 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Mon, 15 Jun 2020 16:03:35 -0700 Subject: [PATCH 09/17] failed attempt at useThrottle tests --- .../components/JobList/useThrottle.test.jsx | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/awx/ui_next/src/components/JobList/useThrottle.test.jsx b/awx/ui_next/src/components/JobList/useThrottle.test.jsx index e69de29bb2..0f5e9aa88a 100644 --- a/awx/ui_next/src/components/JobList/useThrottle.test.jsx +++ b/awx/ui_next/src/components/JobList/useThrottle.test.jsx @@ -0,0 +1,46 @@ +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { mount } from 'enzyme'; +import useThrottle from './useThrottle'; + +function TestInner() { + return
; +} +function Test({ value }) { + const throttled = useThrottle(value, 500); + return ; +} + +jest.useFakeTimers(); + +describe('useThrottle', () => { + test.skip('should throttle value', async () => { + const mockTime = { value: 1000 }; + const realDateNow = Date.now.bind(global.Date); + const dateNowStub = jest.fn(() => mockTime.value); + global.Date.now = dateNowStub; + let wrapper; + await act(async () => { + wrapper = await mount(); + }); + + wrapper.setProps({ + value: 4, + }); + + expect(wrapper.find('TestInner').prop('throttled')).toEqual(3); + jest.advanceTimersByTime(501); + mockTime.value = 1510; + wrapper.setProps({ + value: 2, + }); + wrapper.setProps({ + value: 4, + }); + wrapper.update(); + expect(wrapper.find('TestInner').prop('throttled')).toEqual(4); + + // Date.now.restore(); + global.Date.now = realDateNow; + }); +}); From b4a6749699f47879fe4b191fc8bb05a9153dacd3 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 19 Jun 2020 16:38:07 -0700 Subject: [PATCH 10/17] refactor getJobsById into useWsJobs hook --- .../src/components/JobList/JobList.jsx | 34 ++++++------------- .../src/components/JobList/JobList.test.jsx | 2 +- .../src/components/JobList/useWsJobs.js | 19 ++++++++--- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index 435b20d96d..029e42d0cd 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -42,7 +42,6 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { error: contentError, isLoading, request: fetchJobs, - setValue, } = useRequest( useCallback( async () => { @@ -58,31 +57,18 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { fetchJobs(); }, [fetchJobs]); - const { request: addJobsById } = useRequest( - useCallback( - async ids => { - if (!ids.length) { - return; - } - const params = parseQueryString(QS_CONFIG, location.search); - params.id__in = ids.join(','); - const { data } = await UnifiedJobsAPI.read({ ...params }); - setValue(prev => { - const deDuplicated = data.results.filter( - job => !prev.results.find(j => j.id === job.id) - ); - const mergedJobsList = [...deDuplicated, ...prev.results]; - return { - results: mergedJobsList.slice(0, params.page_size), - count: prev.count + data.count, - }; - }); - }, - [location, setValue] // eslint-disable-line react-hooks/exhaustive-deps - ) + // TODO: update QS_CONFIG to be safe for deps array + const fetchJobsById = useCallback( + async ids => { + const params = parseQueryString(QS_CONFIG, location.search); + params.id__in = ids.join(','); + const { data } = await UnifiedJobsAPI.read(params); + return data.results; + }, + [location.search] // eslint-disable-line react-hooks/exhaustive-deps ); - const jobs = useWsJobs(results, addJobsById, !!defaultParams); + const jobs = useWsJobs(results, fetchJobsById, !!defaultParams); const isAllSelected = selected.length === jobs.length && selected.length > 0; const { diff --git a/awx/ui_next/src/components/JobList/JobList.test.jsx b/awx/ui_next/src/components/JobList/JobList.test.jsx index f2d1e6f71d..bd5b3f8cde 100644 --- a/awx/ui_next/src/components/JobList/JobList.test.jsx +++ b/awx/ui_next/src/components/JobList/JobList.test.jsx @@ -218,7 +218,7 @@ describe('', () => { jest.restoreAllMocks(); }); - test('error is shown when job not successfully deleted from api', async () => { + test.only('error is shown when job not successfully deleted from api', async () => { JobsAPI.destroy.mockImplementation(() => { throw new Error({ response: { diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index ab543d6d8e..574c50d1b4 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -5,7 +5,7 @@ export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { const [jobs, setJobs] = useState(initialJobs); const [lastMessage, setLastMessage] = useState(null); const [jobsToFetch, setJobsToFetch] = useState([]); - const throttleJobsToFetch = useThrottle(jobsToFetch, 5000); + const throttledJobsToFetch = useThrottle(jobsToFetch, 5000); useEffect(() => { setJobs(initialJobs); @@ -17,11 +17,20 @@ export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { } }; useEffect(() => { - if (throttleJobsToFetch.length) { - fetchJobsById(throttleJobsToFetch); + (async () => { + if (!throttledJobsToFetch.length) { + return; + } setJobsToFetch([]); - } - }, [throttleJobsToFetch, fetchJobsById]); + const newJobs = await fetchJobsById(throttledJobsToFetch); + const deduplicated = newJobs.filter( + job => !jobs.find(j => j.id === job.id) + ); + if (deduplicated.length) { + setJobs([...deduplicated, ...jobs]); + } + })(); + }, [throttledJobsToFetch, fetchJobsById]); // eslint-disable-line react-hooks/exhaustive-deps const ws = useRef(null); From 638a6fdaa170e5a0091e99d61ac3b03245d15d72 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Mon, 22 Jun 2020 11:06:38 -0700 Subject: [PATCH 11/17] remove extra logging from JobList tests --- awx/ui_next/src/components/JobList/JobList.test.jsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/awx/ui_next/src/components/JobList/JobList.test.jsx b/awx/ui_next/src/components/JobList/JobList.test.jsx index bd5b3f8cde..31bae1ed95 100644 --- a/awx/ui_next/src/components/JobList/JobList.test.jsx +++ b/awx/ui_next/src/components/JobList/JobList.test.jsx @@ -105,6 +105,16 @@ function waitForLoaded(wrapper) { } describe('', () => { + let debug; + beforeEach(() => { + debug = global.console.debug; // eslint-disable-line prefer-destructuring + global.console.debug = () => {}; + }); + + afterEach(() => { + global.console.debug = debug; + }); + test('initially renders succesfully', async () => { let wrapper; await act(async () => { @@ -218,7 +228,7 @@ describe('', () => { jest.restoreAllMocks(); }); - test.only('error is shown when job not successfully deleted from api', async () => { + test('error is shown when job not successfully deleted from api', async () => { JobsAPI.destroy.mockImplementation(() => { throw new Error({ response: { From a1f257bd4a32d19abff7300ee3f2ce4b889596bd Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Mon, 22 Jun 2020 15:08:10 -0700 Subject: [PATCH 12/17] fix joblist updating completed jobs lists --- awx/ui_next/src/components/JobList/useWsJobs.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index 574c50d1b4..e1d8d2d033 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -38,10 +38,10 @@ export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { if (!lastMessage || !lastMessage.unified_job_id) { return; } - if (filtersApplied) { - if (['completed', 'failed', 'error'].includes(lastMessage.status)) { - enqueueJobId(lastMessage.unified_job_id); - } + if ( + filtersApplied && + !['completed', 'failed', 'error'].includes(lastMessage.status) + ) { return; } From 25fe090e67e7cff528b853add8dbf6af7a01cf22 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 24 Jun 2020 11:38:28 -0700 Subject: [PATCH 13/17] delete un-workable test for useThrottle --- .../components/JobList/useThrottle.test.jsx | 46 ------------------- 1 file changed, 46 deletions(-) delete mode 100644 awx/ui_next/src/components/JobList/useThrottle.test.jsx diff --git a/awx/ui_next/src/components/JobList/useThrottle.test.jsx b/awx/ui_next/src/components/JobList/useThrottle.test.jsx deleted file mode 100644 index 0f5e9aa88a..0000000000 --- a/awx/ui_next/src/components/JobList/useThrottle.test.jsx +++ /dev/null @@ -1,46 +0,0 @@ -import React from 'react'; -import { act } from 'react-dom/test-utils'; -import { mount } from 'enzyme'; -import useThrottle from './useThrottle'; - -function TestInner() { - return
; -} -function Test({ value }) { - const throttled = useThrottle(value, 500); - return ; -} - -jest.useFakeTimers(); - -describe('useThrottle', () => { - test.skip('should throttle value', async () => { - const mockTime = { value: 1000 }; - const realDateNow = Date.now.bind(global.Date); - const dateNowStub = jest.fn(() => mockTime.value); - global.Date.now = dateNowStub; - let wrapper; - await act(async () => { - wrapper = await mount(); - }); - - wrapper.setProps({ - value: 4, - }); - - expect(wrapper.find('TestInner').prop('throttled')).toEqual(3); - jest.advanceTimersByTime(501); - mockTime.value = 1510; - wrapper.setProps({ - value: 2, - }); - wrapper.setProps({ - value: 4, - }); - wrapper.update(); - expect(wrapper.find('TestInner').prop('throttled')).toEqual(4); - - // Date.now.restore(); - global.Date.now = realDateNow; - }); -}); From 5610309a88a8b704950cc28455ecf8c34eaeac7c Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 25 Jun 2020 10:23:15 -0700 Subject: [PATCH 14/17] fix sorting jobs by finished date --- .../src/components/JobList/JobList.jsx | 2 +- .../src/components/JobList/useWsJobs.js | 34 +++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index 029e42d0cd..6e6d57653b 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -68,7 +68,7 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { [location.search] // eslint-disable-line react-hooks/exhaustive-deps ); - const jobs = useWsJobs(results, fetchJobsById, !!defaultParams); + const jobs = useWsJobs(results, fetchJobsById, QS_CONFIG); const isAllSelected = selected.length === jobs.length && selected.length > 0; const { diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index e1d8d2d033..0733e7bfc6 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -1,7 +1,10 @@ import { useState, useEffect, useRef } from 'react'; +import { useLocation } from 'react-router-dom'; import useThrottle from './useThrottle'; +import { parseQueryString } from '../../util/qs'; -export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { +export default function useWsJobs(initialJobs, fetchJobsById, qsConfig) { + const location = useLocation(); const [jobs, setJobs] = useState(initialJobs); const [lastMessage, setLastMessage] = useState(null); const [jobsToFetch, setJobsToFetch] = useState([]); @@ -38,6 +41,8 @@ export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { if (!lastMessage || !lastMessage.unified_job_id) { return; } + const params = parseQueryString(qsConfig, location.search); + const filtersApplied = Object.keys(params).length > 4; if ( filtersApplied && !['completed', 'failed', 'error'].includes(lastMessage.status) @@ -48,7 +53,7 @@ export default function useWsJobs(initialJobs, fetchJobsById, filtersApplied) { const jobId = lastMessage.unified_job_id; const index = jobs.findIndex(j => j.id === jobId); if (index > -1) { - setJobs(updateJob(jobs, index, lastMessage)); + setJobs(sortJobs(updateJob(jobs, index, lastMessage), params.order_by)); } else { enqueueJobId(lastMessage.unified_job_id); } @@ -110,3 +115,28 @@ function updateJob(jobs, index, message) { }; return [...jobs.slice(0, index), job, ...jobs.slice(index + 1)]; } + +function sortJobs(jobs, orderBy) { + if (orderBy !== '-finished') { + return jobs; + } + + return jobs.sort((a, b) => { + if (!a.finished) { + return -1; + } + if (!b.finished) { + return 1; + } + + const dateA = new Date(a.finished); + const dateB = new Date(b.finished); + if (dateA < dateB) { + return 1; + } + if (dateA > dateB) { + return -1; + } + return 0; + }); +} From 9705f7bec63499af3f34878a99df8320bd218d20 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 25 Jun 2020 16:23:38 -0700 Subject: [PATCH 15/17] sort jobs list by selection sort option --- .../src/components/JobList/sortJobs.js | 78 +++++++++++++++++++ .../src/components/JobList/useWsJobs.js | 26 +------ 2 files changed, 79 insertions(+), 25 deletions(-) create mode 100644 awx/ui_next/src/components/JobList/sortJobs.js diff --git a/awx/ui_next/src/components/JobList/sortJobs.js b/awx/ui_next/src/components/JobList/sortJobs.js new file mode 100644 index 0000000000..9b761f1c90 --- /dev/null +++ b/awx/ui_next/src/components/JobList/sortJobs.js @@ -0,0 +1,78 @@ +const sortFns = { + finished: byFinished, + id: byId, + name: byName, + created_by__id: byCreatedBy, + unified_job_template__project__id: byProject, + started: byStarted, +}; + +export default function sortJobs(jobs, orderBy) { + const key = orderBy.replace('-', ''); + const fn = sortFns[key]; + if (!fn) { + return jobs; + } + + return orderBy[0] === '-' ? jobs.sort(reverse(fn)) : jobs.sort(fn); +} + +function reverse(fn) { + return (a, b) => fn(a, b) * -1; +} + +function byFinished(a, b) { + if (!a.finished) { + return 1; + } + if (!b.finished) { + return -1; + } + return sort(new Date(a.finished), new Date(b.finished)); +} + +function byStarted(a, b) { + if (!a.started) { + return -1; + } + if (!b.started) { + return 1; + } + return sort(new Date(a.started), new Date(b.started)); +} + +function byId(a, b) { + return sort(a.id, b.id); +} + +function byName(a, b) { + return sort(a.name, b.name); +} + +function byCreatedBy(a, b) { + const nameA = a.summary_fields?.created_by?.username; + const nameB = b.summary_fields?.created_by?.username; + return sort(nameA, nameB); +} + +function byProject(a, b) { + const projA = a.summary_fields?.project?.id; + const projB = b.summary_fields?.project?.id; + return sort(projA, projB); +} + +function sort(a, b) { + if (!a) { + return -1; + } + if (!b) { + return 1; + } + if (a < b) { + return -1; + } + if (a > b) { + return 1; + } + return 0; +} diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index 0733e7bfc6..c06ea7c88f 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -2,6 +2,7 @@ import { useState, useEffect, useRef } from 'react'; import { useLocation } from 'react-router-dom'; import useThrottle from './useThrottle'; import { parseQueryString } from '../../util/qs'; +import sortJobs from './sortJobs'; export default function useWsJobs(initialJobs, fetchJobsById, qsConfig) { const location = useLocation(); @@ -115,28 +116,3 @@ function updateJob(jobs, index, message) { }; return [...jobs.slice(0, index), job, ...jobs.slice(index + 1)]; } - -function sortJobs(jobs, orderBy) { - if (orderBy !== '-finished') { - return jobs; - } - - return jobs.sort((a, b) => { - if (!a.finished) { - return -1; - } - if (!b.finished) { - return 1; - } - - const dateA = new Date(a.finished); - const dateB = new Date(b.finished); - if (dateA < dateB) { - return 1; - } - if (dateA > dateB) { - return -1; - } - return 0; - }); -} From a3e0ae66bae25097772db90c51497f2ab12335cc Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 26 Jun 2020 12:00:11 -0700 Subject: [PATCH 16/17] clean up sorting order discrepancies --- .../src/components/JobList/sortJobs.js | 24 +++++++++---------- .../src/components/JobList/useWsJobs.js | 5 ++-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/awx/ui_next/src/components/JobList/sortJobs.js b/awx/ui_next/src/components/JobList/sortJobs.js index 9b761f1c90..92218994ae 100644 --- a/awx/ui_next/src/components/JobList/sortJobs.js +++ b/awx/ui_next/src/components/JobList/sortJobs.js @@ -7,14 +7,16 @@ const sortFns = { started: byStarted, }; -export default function sortJobs(jobs, orderBy) { - const key = orderBy.replace('-', ''); +export default function sortJobs(jobs, params) { + const { order_by = '-finished', page_size = 20 } = params; + const key = order_by.replace('-', ''); const fn = sortFns[key]; if (!fn) { - return jobs; + return jobs.slice(0, page_size); } - return orderBy[0] === '-' ? jobs.sort(reverse(fn)) : jobs.sort(fn); + const sorted = order_by[0] === '-' ? jobs.sort(reverse(fn)) : jobs.sort(fn); + return sorted.slice(0, page_size); } function reverse(fn) { @@ -33,10 +35,10 @@ function byFinished(a, b) { function byStarted(a, b) { if (!a.started) { - return -1; + return 1; } if (!b.started) { - return 1; + return -1; } return sort(new Date(a.started), new Date(b.started)); } @@ -50,15 +52,13 @@ function byName(a, b) { } function byCreatedBy(a, b) { - const nameA = a.summary_fields?.created_by?.username; - const nameB = b.summary_fields?.created_by?.username; - return sort(nameA, nameB); + const nameA = a.summary_fields?.created_by?.id; + const nameB = b.summary_fields?.created_by?.id; + return sort(nameA, nameB) * -1; } function byProject(a, b) { - const projA = a.summary_fields?.project?.id; - const projB = b.summary_fields?.project?.id; - return sort(projA, projB); + return sort(a.unified_job_template, b.unified_job_template); } function sort(a, b) { diff --git a/awx/ui_next/src/components/JobList/useWsJobs.js b/awx/ui_next/src/components/JobList/useWsJobs.js index c06ea7c88f..46068353af 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.js +++ b/awx/ui_next/src/components/JobList/useWsJobs.js @@ -31,7 +31,8 @@ export default function useWsJobs(initialJobs, fetchJobsById, qsConfig) { job => !jobs.find(j => j.id === job.id) ); if (deduplicated.length) { - setJobs([...deduplicated, ...jobs]); + const params = parseQueryString(qsConfig, location.search); + setJobs(sortJobs([...deduplicated, ...jobs], params)); } })(); }, [throttledJobsToFetch, fetchJobsById]); // eslint-disable-line react-hooks/exhaustive-deps @@ -54,7 +55,7 @@ export default function useWsJobs(initialJobs, fetchJobsById, qsConfig) { const jobId = lastMessage.unified_job_id; const index = jobs.findIndex(j => j.id === jobId); if (index > -1) { - setJobs(sortJobs(updateJob(jobs, index, lastMessage), params.order_by)); + setJobs(sortJobs(updateJob(jobs, index, lastMessage), params)); } else { enqueueJobId(lastMessage.unified_job_id); } From 53047929490c3d2071a3b5c231ee149f03a0391e Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 26 Jun 2020 14:40:24 -0700 Subject: [PATCH 17/17] update useWsJobs tests --- .../src/components/JobList/useWsJobs.test.jsx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/awx/ui_next/src/components/JobList/useWsJobs.test.jsx b/awx/ui_next/src/components/JobList/useWsJobs.test.jsx index 1dead6d5da..c7782b2427 100644 --- a/awx/ui_next/src/components/JobList/useWsJobs.test.jsx +++ b/awx/ui_next/src/components/JobList/useWsJobs.test.jsx @@ -1,7 +1,7 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; -import { mount } from 'enzyme'; import WS from 'jest-websocket-mock'; +import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; import useWsJobs from './useWsJobs'; /* @@ -17,7 +17,8 @@ function TestInner() { return
; } function Test({ jobs, fetch }) { - const syncedJobs = useWsJobs(jobs, fetch); + const qsConfig = {}; + const syncedJobs = useWsJobs(jobs, fetch, qsConfig); return ; } @@ -35,7 +36,7 @@ describe('useWsJobs hook', () => { test('should return jobs list', () => { const jobs = [{ id: 1 }]; - wrapper = mount(); + wrapper = mountWithContexts(); expect(wrapper.find('TestInner').prop('jobs')).toEqual(jobs); WS.clean(); @@ -47,7 +48,7 @@ describe('useWsJobs hook', () => { const jobs = [{ id: 1 }]; await act(async () => { - wrapper = await mount(); + wrapper = await mountWithContexts(); }); await mockServer.connected; @@ -70,7 +71,7 @@ describe('useWsJobs hook', () => { const jobs = [{ id: 1, status: 'running' }]; await act(async () => { - wrapper = await mount(); + wrapper = await mountWithContexts(); }); await mockServer.connected; @@ -105,9 +106,9 @@ describe('useWsJobs hook', () => { global.document.cookie = 'csrftoken=abc123'; const mockServer = new WS('wss://localhost/websocket/'); const jobs = [{ id: 1 }]; - const fetch = jest.fn(); + const fetch = jest.fn(() => []); await act(async () => { - wrapper = await mount(); + wrapper = await mountWithContexts(); }); await mockServer.connected;