From 0a6bc905f97bf83b1b5f13512778b1d16d41b17b Mon Sep 17 00:00:00 2001 From: Carlo Camurri Date: Tue, 1 Dec 2020 17:31:51 +0000 Subject: [PATCH] Add url query parameters for filters in job table (#466) --- internal/lookout/ui/package-lock.json | 38 +++- internal/lookout/ui/package.json | 1 + internal/lookout/ui/src/App.tsx | 2 +- .../ui/src/containers/JobsContainer.test.tsx | 183 ++++++++++++++++++ .../ui/src/containers/JobsContainer.tsx | 134 ++++++++++++- .../ui/src/{utils.test.ts => utils.test.tsx} | 2 +- .../lookout/ui/src/{utils.ts => utils.tsx} | 0 7 files changed, 342 insertions(+), 18 deletions(-) create mode 100644 internal/lookout/ui/src/containers/JobsContainer.test.tsx rename internal/lookout/ui/src/{utils.test.ts => utils.test.tsx} (97%) rename internal/lookout/ui/src/{utils.ts => utils.tsx} (100%) diff --git a/internal/lookout/ui/package-lock.json b/internal/lookout/ui/package-lock.json index fd522716cc6..861e0f930c2 100644 --- a/internal/lookout/ui/package-lock.json +++ b/internal/lookout/ui/package-lock.json @@ -10650,6 +10650,22 @@ "prepend-http": "^1.0.0", "query-string": "^4.1.0", "sort-keys": "^1.0.0" + }, + "dependencies": { + "query-string": { + "version": "4.3.4", + "resolved": "https://registry.npmjs.org/query-string/-/query-string-4.3.4.tgz", + "integrity": "sha1-u7aTucqRXCMlFbIosaArYJBD2+s=", + "requires": { + "object-assign": "^4.1.0", + "strict-uri-encode": "^1.0.0" + } + }, + "strict-uri-encode": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/strict-uri-encode/-/strict-uri-encode-1.1.0.tgz", + "integrity": "sha1-J5siXfHVgrH1TmWt3UNS4Y+qBxM=" + } } }, "npm-run-path": { @@ -12468,12 +12484,13 @@ "integrity": "sha512-N5ZAX4/LxJmF+7wN74pUD6qAh9/wnvdQcjq9TZjevvXzSUo7bfmw91saqMjzGS2xq91/odN2dW/WOl7qQHNDGA==" }, "query-string": { - "version": "4.3.4", - "resolved": "https://registry.npmjs.org/query-string/-/query-string-4.3.4.tgz", - "integrity": "sha1-u7aTucqRXCMlFbIosaArYJBD2+s=", + "version": "6.13.7", + "resolved": "https://registry.npmjs.org/query-string/-/query-string-6.13.7.tgz", + "integrity": "sha512-CsGs8ZYb39zu0WLkeOhe0NMePqgYdAuCqxOYKDR5LVCytDZYMGx3Bb+xypvQvPHVPijRXB0HZNFllCzHRe4gEA==", "requires": { - "object-assign": "^4.1.0", - "strict-uri-encode": "^1.0.0" + "decode-uri-component": "^0.2.0", + "split-on-first": "^1.0.0", + "strict-uri-encode": "^2.0.0" } }, "querystring": { @@ -14263,6 +14280,11 @@ "wbuf": "^1.7.3" } }, + "split-on-first": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/split-on-first/-/split-on-first-1.1.0.tgz", + "integrity": "sha512-43ZssAJaMusuKWL8sKUBQXHWOpq8d6CfN/u1p4gUzfJkM05C8rxTmYrkIPTXapZpORA6LkkzcUulJ8FqA7Uudw==" + }, "split-string": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/split-string/-/split-string-3.1.0.tgz", @@ -14438,9 +14460,9 @@ "integrity": "sha512-AiisoFqQ0vbGcZgQPY1cdP2I76glaVA/RauYR4G4thNFgkTqr90yXTo4LYX60Jl+sIlPNHHdGSwo01AvbKUSVQ==" }, "strict-uri-encode": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/strict-uri-encode/-/strict-uri-encode-1.1.0.tgz", - "integrity": "sha1-J5siXfHVgrH1TmWt3UNS4Y+qBxM=" + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/strict-uri-encode/-/strict-uri-encode-2.0.0.tgz", + "integrity": "sha1-ucczDHBChi9rFC3CdLvMWGbONUY=" }, "string-length": { "version": "4.0.1", diff --git a/internal/lookout/ui/package.json b/internal/lookout/ui/package.json index 41e5bf9ffb2..36454ec9bc2 100644 --- a/internal/lookout/ui/package.json +++ b/internal/lookout/ui/package.json @@ -20,6 +20,7 @@ "@types/react-dom": "^16.9.9", "@types/react-router-dom": "^5.1.6", "@types/react-virtualized": "^9.21.10", + "query-string": "^6.13.7", "react": "^17.0.1", "react-dom": "^17.0.1", "react-router-dom": "^5.2.0", diff --git a/internal/lookout/ui/src/App.tsx b/internal/lookout/ui/src/App.tsx index c59bf20727e..67ca662ef52 100644 --- a/internal/lookout/ui/src/App.tsx +++ b/internal/lookout/ui/src/App.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { BrowserRouter as Router, Switch, Route} from 'react-router-dom' import JobService from './services/JobService'; -import { JobsContainer } from "./containers/JobsContainer"; +import JobsContainer from "./containers/JobsContainer"; import { Overview } from './Overview' import NavBar from "./components/NavBar"; diff --git a/internal/lookout/ui/src/containers/JobsContainer.test.tsx b/internal/lookout/ui/src/containers/JobsContainer.test.tsx new file mode 100644 index 00000000000..129e9c86102 --- /dev/null +++ b/internal/lookout/ui/src/containers/JobsContainer.test.tsx @@ -0,0 +1,183 @@ +import { JobFilters, makeFiltersFromQueryString, makeQueryStringFromFilters } from "./JobsContainer"; + +function assertStringHasQueryParams(expected: string[], actual: string) { + const actualQueryParams = actual.split("&") + expect(expected.sort()).toStrictEqual(actualQueryParams.sort()) +} + +describe("makeQueryStringFromFilters", () => { + test("makes string with queue", () => { + const filters: JobFilters = { + queue: "test", + jobSet: "", + jobStates: [], + newestFirst: false, + } + const queryString = makeQueryStringFromFilters(filters) + assertStringHasQueryParams(["queue=test", "newest_first=false"], queryString) + }) + + test("makes string with job set", () => { + const filters: JobFilters = { + queue: "", + jobSet: "job-set", + jobStates: [], + newestFirst: false, + } + const queryString = makeQueryStringFromFilters(filters) + assertStringHasQueryParams(["job_set=job-set", "newest_first=false"], queryString) + }) + + test("makes string with single job state", () => { + const filters: JobFilters = { + queue: "", + jobSet: "", + jobStates: ["Queued"], + newestFirst: false, + } + const queryString = makeQueryStringFromFilters(filters) + assertStringHasQueryParams(["job_states=Queued", "newest_first=false"], queryString) + }) + + test("makes string with multiple job states", () => { + const filters: JobFilters = { + queue: "", + jobSet: "", + jobStates: ["Queued", "Running", "Cancelled"], + newestFirst: false, + } + const queryString = makeQueryStringFromFilters(filters) + assertStringHasQueryParams(["job_states=Queued,Running,Cancelled", "newest_first=false"], queryString) + }) + + test("makes string with ordering", () => { + const filters: JobFilters = { + queue: "", + jobSet: "", + jobStates: [], + newestFirst: true, + } + const queryString = makeQueryStringFromFilters(filters) + assertStringHasQueryParams(["newest_first=true"], queryString) + }) + + test("makes string with all filters", () => { + const filters: JobFilters = { + queue: "other-test", + jobSet: "other-job-set", + jobStates: ["Pending", "Succeeded", "Failed"], + newestFirst: true, + } + const queryString = makeQueryStringFromFilters(filters) + assertStringHasQueryParams([ + "queue=other-test", + "job_set=other-job-set", + "job_states=Pending,Succeeded,Failed", + "newest_first=true", + ], queryString) + }) +}) + +describe("makeFiltersFromQueryString", () => { + test("empty string returns default filters", () => { + const query = "" + const filters = makeFiltersFromQueryString(query) + expect(filters).toStrictEqual({ + queue: "", + jobSet: "", + jobStates: [], + newestFirst: true, + }) + }) + + test("makes filter with queue", () => { + const query = "queue=test" + const filters = makeFiltersFromQueryString(query) + expect(filters).toStrictEqual({ + queue: "test", + jobSet: "", + jobStates: [], + newestFirst: true, + }) + }) + + test("makes filter with job set", () => { + const query = "job_set=job-set" + const filters = makeFiltersFromQueryString(query) + expect(filters).toStrictEqual({ + queue: "", + jobSet: "job-set", + jobStates: [], + newestFirst: true, + }) + }) + + test("makes filter with single job state", () => { + const query = "job_states=Queued" + const filters = makeFiltersFromQueryString(query) + expect(filters).toStrictEqual({ + queue: "", + jobSet: "", + jobStates: ["Queued"], + newestFirst: true, + }) + }) + + test("makes filter with multiple job states", () => { + const query = "job_states=Queued,Pending,Running" + const filters = makeFiltersFromQueryString(query) + expect(filters).toStrictEqual({ + queue: "", + jobSet: "", + jobStates: ["Queued", "Pending", "Running"], + newestFirst: true, + }) + }) + + const orderingsCases = [["newest_first=true", true], ["newest_first=false", false]] + test.each(orderingsCases)("makes filter with ordering %p", (query, expectedOrdering) => { + const filters = makeFiltersFromQueryString(query as string) + expect(filters).toStrictEqual({ + queue: "", + jobSet: "", + jobStates: [], + newestFirst: expectedOrdering as boolean, + }) + }) + + test("makes filter with everything", () => { + const query = "queue=test&job_set=job-set-1&job_states=Queued,Succeeded,Pending&newest_first=false" + const filters = makeFiltersFromQueryString(query) + expect(filters).toStrictEqual({ + queue: "test", + jobSet: "job-set-1", + jobStates: ["Queued", "Succeeded", "Pending"], + newestFirst: false, + }) + }) + + const nonExistentJobStatesCases = [ + ["job_states=SomethingElse", []], + ["job_states=Cancelled,SomethingElse,Succeeded,Failed", ["Cancelled", "Succeeded", "Failed"]], + ] + test.each(nonExistentJobStatesCases)("non existent job states are ignored %p", (query, expectedJobStates) => { + const filters = makeFiltersFromQueryString(query as string) + expect(filters).toStrictEqual({ + queue: "", + jobSet: "", + jobStates: expectedJobStates, + newestFirst: true, + }) + }) + + test("other query parameters are ignored", () => { + const query = "something=else" + const filters = makeFiltersFromQueryString(query) + expect(filters).toStrictEqual({ + queue: "", + jobSet: "", + jobStates: [], + newestFirst: true, + }) + }) +}) diff --git a/internal/lookout/ui/src/containers/JobsContainer.tsx b/internal/lookout/ui/src/containers/JobsContainer.tsx index a5875db6ecd..42de2666710 100644 --- a/internal/lookout/ui/src/containers/JobsContainer.tsx +++ b/internal/lookout/ui/src/containers/JobsContainer.tsx @@ -1,23 +1,111 @@ import React from 'react' +import * as H from "history" +import { match, withRouter } from 'react-router-dom' +import queryString, { ParseOptions, StringifyOptions } from 'query-string' -import JobService, { JobInfoViewModel } from "../services/JobService" +import JobService, { JOB_STATES_FOR_DISPLAY, JobInfoViewModel } from "../services/JobService" import Jobs from "../components/Jobs" import { updateArray } from "../utils"; type JobsContainerProps = { jobService: JobService + history: H.History; + location: H.Location; + match: match; } -type JobsContainerState = { +interface JobsContainerState extends JobFilters { jobInfos: JobInfoViewModel[] + canLoadMore: boolean +} + +export interface JobFilters { queue: string jobSet: string jobStates: string[] newestFirst: boolean - canLoadMore: boolean } -export class JobsContainer extends React.Component { +type JobFiltersQueryParams = { + queue?: string + job_set?: string + job_states?: string[] | string + newest_first?: boolean +} + +const QUERY_STRING_OPTIONS: ParseOptions | StringifyOptions = { + arrayFormat: "comma", + parseBooleans: true, +} + +export function makeQueryStringFromFilters(filters: JobFilters): string { + let queryObject: JobFiltersQueryParams = {} + if (filters.queue) { + queryObject = { + ...queryObject, + queue: filters.queue, + } + } + if (filters.jobSet) { + queryObject = { + ...queryObject, + job_set: filters.jobSet, + } + } + if (filters.jobStates) { + queryObject = { + ...queryObject, + job_states: filters.jobStates, + } + } + if (filters.newestFirst != null) { + queryObject = { + ...queryObject, + newest_first: filters.newestFirst, + } + } + + return queryString.stringify(queryObject, QUERY_STRING_OPTIONS) +} + +export function makeFiltersFromQueryString(query: string): JobFilters { + const params = queryString.parse(query, QUERY_STRING_OPTIONS) as JobFiltersQueryParams + + let filters: JobFilters = { + queue: "", + jobSet: "", + jobStates: [], + newestFirst: true, + } + if (params.queue) { + filters.queue = params.queue + } + if (params.job_set) { + filters.jobSet = params.job_set + } + if (params.job_states) { + filters.jobStates = parseJobStates(params.job_states) + } + if (params.newest_first != null) { + filters.newestFirst = params.newest_first + } + + return filters +} + +function parseJobStates(jobStates: string[] | string): string[] { + if (!Array.isArray(jobStates)) { + if (JOB_STATES_FOR_DISPLAY.includes(jobStates)) { + return [jobStates] + } else { + return [] + } + } + + return jobStates.filter(jobState => JOB_STATES_FOR_DISPLAY.includes(jobState)) +} + +class JobsContainer extends React.Component { constructor(props: JobsContainerProps) { super(props); this.state = { @@ -36,6 +124,15 @@ export class JobsContainer extends React.Component { @@ -74,7 +171,10 @@ export class JobsContainer extends React.Component { + this.setUrlParams() + callback() + }) } jobSetChange(jobSet: string, callback: () => void) { @@ -83,7 +183,10 @@ export class JobsContainer extends React.Component { + this.setUrlParams() + callback() + }) } jobStatesChange(jobStates: string[], callback: () => void) { @@ -92,7 +195,10 @@ export class JobsContainer extends React.Component { + this.setUrlParams() + callback() + }) } orderChange(newestFirst: boolean, callback: () => void) { @@ -101,7 +207,10 @@ export class JobsContainer extends React.Component { + this.setUrlParams() + callback() + }) } refresh(callback: () => void) { @@ -112,6 +221,13 @@ export class JobsContainer extends React.Component { +describe('updateArray', () => { test('does nothing if no new values are provided', () => { const array = [1, 2, 3, 4, 5] updateArray(array, [], 5, 10) diff --git a/internal/lookout/ui/src/utils.ts b/internal/lookout/ui/src/utils.tsx similarity index 100% rename from internal/lookout/ui/src/utils.ts rename to internal/lookout/ui/src/utils.tsx