diff --git a/.gitignore b/.gitignore index a76211389..aed6cef3a 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ .coverage .tests bindata_assetfs.go +coverage.txt unsee ui/build ui/coverage diff --git a/.travis.yml b/.travis.yml index a6062640b..3813bc178 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,12 +30,16 @@ jobs: before_script: - make mock-assets script: make test-go + after_success: + - bash <(curl -s https://codecov.io/bash) -F backend - stage: Test JavaScript code <<: *DEFAULTS_JS env: - NODE_ENV=test script: make test-js + after_success: + - bash <(curl -s https://codecov.io/bash) -F ui -s ui - stage: Lint Go code <<: *DEFAULTS_GO diff --git a/Makefile b/Makefile index 226bb41f2..59c70948d 100644 --- a/Makefile +++ b/Makefile @@ -118,12 +118,22 @@ lint: lint-go lint-js lint-docs .PHONY: test-go test-go: .build/vendor.ok - go test -v -bench=. -benchmem -cover `go list ./... | grep -v /vendor/` + go test -v \ + -bench=. -benchmem \ + -cover -coverprofile=coverage.txt -covermode=atomic \ + `go list ./... | grep -v /vendor/` .PHONY: test-js test-js: .build/deps-build-node.ok cd ui && CI=true npm test -- --coverage +.PHONY: test-js-watch +test-js-watch: .build/deps-build-node.ok + @# hitting issues with jest --watch due to + @# https://github.com/facebook/jest/issues/3436 + @# use onchange for now + cd ui && ./node_modules/onchange/cli.js 'src/*.js' 'src/**/*.js' -- npm test -- --coverage + .PHONY: test test: lint test-go test-js diff --git a/ui/package-lock.json b/ui/package-lock.json index 10dfb8db3..4c88ef967 100644 --- a/ui/package-lock.json +++ b/ui/package-lock.json @@ -7482,6 +7482,12 @@ "resolved": "https://registry.npmjs.org/next-tick/-/next-tick-1.0.0.tgz", "integrity": "sha1-yobR/ogoFpsBICCOPchCS524NCw=" }, + "nice-try": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/nice-try/-/nice-try-1.0.4.tgz", + "integrity": "sha512-2NpiFHqC87y/zFke0fC0spBXL3bBsoh/p5H1EFhshxjCR5+0g2d6BiXbUFz9v1sAcxsk2htp2eQnNIci2dIYcA==", + "dev": true + }, "no-case": { "version": "2.3.2", "resolved": "https://registry.npmjs.org/no-case/-/no-case-2.3.2.tgz", @@ -8028,6 +8034,40 @@ "wrappy": "1.0.2" } }, + "onchange": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/onchange/-/onchange-4.1.0.tgz", + "integrity": "sha512-xSuX6stJmPO+V1CBFYUdVS7AzuH2nyXXzW6OW7HLXaBa9Pc7bFGcjCXTha575wTeEe2N8p1gIQhwLMDNU15YPg==", + "dev": true, + "requires": { + "arrify": "1.0.1", + "chokidar": "2.0.3", + "cross-spawn": "6.0.5", + "minimist": "1.2.0", + "tree-kill": "1.2.0" + }, + "dependencies": { + "cross-spawn": { + "version": "6.0.5", + "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-6.0.5.tgz", + "integrity": "sha512-eTVLrBSt7fjbDygz805pMnstIs2VTBNkRm0qxZd+M7A5XDdxVRWO5MxGBXZhjY4cqLYLdtrGqRf8mBPmzwSpWQ==", + "dev": true, + "requires": { + "nice-try": "1.0.4", + "path-key": "2.0.1", + "semver": "5.5.0", + "shebang-command": "1.2.0", + "which": "1.3.1" + } + }, + "minimist": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", + "dev": true + } + } + }, "onetime": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/onetime/-/onetime-2.0.1.tgz", @@ -11680,6 +11720,12 @@ "resolved": "https://registry.npmjs.org/traverse/-/traverse-0.6.6.tgz", "integrity": "sha1-y99WD9e5r2MlAv7UD5GMFX6pcTc=" }, + "tree-kill": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/tree-kill/-/tree-kill-1.2.0.tgz", + "integrity": "sha512-DlX6dR0lOIRDFxI0mjL9IYg6OTncLm/Zt+JiBhE5OlFcAR8yc9S7FFXU9so0oda47frdM/JFsk7UjNt9vscKcg==", + "dev": true + }, "trim-newlines": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/trim-newlines/-/trim-newlines-1.0.0.tgz", diff --git a/ui/package.json b/ui/package.json index e5d73b874..d56241d54 100644 --- a/ui/package.json +++ b/ui/package.json @@ -54,6 +54,7 @@ "eslint-plugin-react": "7.11.1", "markdownlint-cli": "0.13.0", "node-sass-chokidar": "1.3.3", + "onchange": "4.1.0", "react-test-renderer": "16.4.2" } } diff --git a/ui/src/App.js b/ui/src/App.js index c501387a7..6ad6cc7e4 100644 --- a/ui/src/App.js +++ b/ui/src/App.js @@ -29,7 +29,7 @@ class App extends Component { let filters; // parse and decode request query args - const p = DecodeLocationSearch(); + const p = DecodeLocationSearch(window.location.search); // p.defaultsUsed means that unsee URI didn't have ?q=foo query args if (p.defaultsUsed) { diff --git a/ui/src/Stores/AlertStore.js b/ui/src/Stores/AlertStore.js index 912c36ec0..d8c3ee582 100644 --- a/ui/src/Stores/AlertStore.js +++ b/ui/src/Stores/AlertStore.js @@ -9,7 +9,9 @@ import qs from "qs"; // generate URL for the UI with a set of filters function FormatAPIFilterQuery(filters) { return qs.stringify( - Object.assign(DecodeLocationSearch().params, { q: filters }), + Object.assign(DecodeLocationSearch(window.location.search).params, { + q: filters + }), { encodeValuesOnly: true, // don't encode q[] indices: false // go-gin doesn't support parsing q[0]=foo&q[1]=bar @@ -23,12 +25,14 @@ function FormatUnseeBackendURI(path) { return `${uri}/${path}`; } -function DecodeLocationSearch() { +// takes the '?foo=bar&foo=baz' part of http://example.com?foo=bar&foo=baz +// and decodes it into a dict with some extra metadata +function DecodeLocationSearch(searchString) { let defaultsUsed = true; let params = { q: [] }; - if (window.location.search !== "") { - const parsed = qs.parse(window.location.search.split("?")[1]); + if (searchString !== "") { + const parsed = qs.parse(searchString.split("?")[1]); params = Object.assign(params, parsed); if (parsed.q !== undefined) { @@ -36,7 +40,8 @@ function DecodeLocationSearch() { if (parsed.q === "") { params.q = []; } else if (Array.isArray(parsed.q)) { - params.q = parsed.q; + // filter out empty strings, so 'q=' doesn't end up [""] but rather [] + params.q = parsed.q.filter(v => v !== ""); } else { params.q = [parsed.q]; } @@ -97,8 +102,15 @@ class AlertStore { replaceFilter(oldRaw, newRaw) { const index = this.values.findIndex(e => e.raw === oldRaw); if (index >= 0) { - this.values[index] = newUnappliedFilter(newRaw); - UpdateLocationSearch({ q: this.values.map(f => f.raw) }); + // first check if we would create a duplicated filter + if (this.values.findIndex(e => e.raw === newRaw) >= 0) { + // we already have newRaw, simply drop oldRaw + this.removeFilter(oldRaw); + } else { + // no dups, continue with a swap + this.values[index] = newUnappliedFilter(newRaw); + UpdateLocationSearch({ q: this.values.map(f => f.raw) }); + } } }, setFilters(raws) { diff --git a/ui/src/Stores/AlertStore.test.js b/ui/src/Stores/AlertStore.test.js index 1e7561a1d..f168b0686 100644 --- a/ui/src/Stores/AlertStore.test.js +++ b/ui/src/Stores/AlertStore.test.js @@ -1,6 +1,11 @@ -import { AlertStore, AlertStoreStatuses } from "Stores/AlertStore"; +import { + AlertStore, + AlertStoreStatuses, + FormatUnseeBackendURI, + DecodeLocationSearch +} from "Stores/AlertStore"; -describe("AlertStore", () => { +describe("AlertStore.status", () => { it("status is initially idle with no error", () => { const store = new AlertStore([]); expect(store.status.value).toEqual(AlertStoreStatuses.Idle); @@ -37,3 +42,143 @@ describe("AlertStore", () => { expect(store.status.error).toBeNull(); }); }); + +describe("AlertStore.filters", () => { + const formatEmptyFilter = raw => ({ + applied: false, + isValid: true, + raw: raw, + hits: 0, + name: "", + matcher: "", + value: "" + }); + + it("addFilter('foo') should create a correct empty filter", () => { + const store = new AlertStore([]); + store.filters.addFilter("foo"); + expect(store.filters.values).toHaveLength(1); + expect(store.filters.values[0]).toMatchObject(formatEmptyFilter("foo")); + }); + + it("removeFilter('foo') should remove passed filter if it's defined", () => { + const store = new AlertStore([]); + store.filters.addFilter("foo"); + store.filters.removeFilter("foo"); + expect(store.filters.values).toHaveLength(0); + }); + + it("removeFilter('foo') should not remove filters other than 'foo'", () => { + const store = new AlertStore([]); + store.filters.addFilter("bar"); + store.filters.addFilter("foo"); + store.filters.addFilter("baz"); + store.filters.removeFilter("foo"); + expect(store.filters.values).toHaveLength(2); + expect(store.filters.values[0]).toMatchObject(formatEmptyFilter("bar")); + expect(store.filters.values[1]).toMatchObject(formatEmptyFilter("baz")); + }); + + it("removeFilter('foo') should not remove any filter if 'foo' isn't defined", () => { + const store = new AlertStore([]); + store.filters.addFilter("bar"); + store.filters.removeFilter("foo"); + expect(store.filters.values).toHaveLength(1); + expect(store.filters.values[0]).toMatchObject(formatEmptyFilter("bar")); + }); + + it("replaceFilter('foo', 'bar') should not replace anything if filter list is empty", () => { + const store = new AlertStore([]); + store.filters.replaceFilter("foo", "bar"); + expect(store.filters.values).toHaveLength(0); + }); + + it("replaceFilter('foo', 'new') should replace correct filter", () => { + const store = new AlertStore([]); + store.filters.addFilter("bar"); + store.filters.addFilter("foo"); + store.filters.addFilter("baz"); + store.filters.replaceFilter("foo", "new"); + expect(store.filters.values).toHaveLength(3); + expect(store.filters.values[0]).toMatchObject(formatEmptyFilter("bar")); + expect(store.filters.values[1]).toMatchObject(formatEmptyFilter("new")); + expect(store.filters.values[2]).toMatchObject(formatEmptyFilter("baz")); + }); + + it("replaceFilter('foo', 'bar') should not allow duplicates", () => { + const store = new AlertStore([]); + store.filters.addFilter("foo"); + store.filters.addFilter("bar"); + store.filters.replaceFilter("foo", "bar"); + expect(store.filters.values).toHaveLength(1); + expect(store.filters.values[0]).toMatchObject(formatEmptyFilter("bar")); + }); +}); + +describe("FormatUnseeBackendURI", () => { + beforeEach(() => { + // wipe REACT_APP_BACKEND_URI env on each run as it's used by some tests + delete process.env.REACT_APP_BACKEND_URI; + }); + + it("FormatUnseeBackendURI without REACT_APP_BACKEND_URI env returns ./ prefixed URIs", () => { + const uri = FormatUnseeBackendURI("foo/bar"); + expect(uri).toEqual("./foo/bar"); + }); + + it("FormatUnseeBackendURI with REACT_APP_BACKEND_URI env returns env value prefixed URIs", () => { + process.env.REACT_APP_BACKEND_URI = "http://localhost:1234"; + const uri = FormatUnseeBackendURI("foo/bar"); + expect(uri).toEqual("http://localhost:1234/foo/bar"); + }); +}); + +describe("DecodeLocationSearch", () => { + const defaultParams = { + defaultsUsed: true, + params: { q: [] } + }; + + it("empty ('') search param is decoded correctly", () => { + expect(DecodeLocationSearch("")).toMatchObject(defaultParams); + }); + + it("empty ('?') search param is decoded correctly", () => { + expect(DecodeLocationSearch("?")).toMatchObject(defaultParams); + }); + + it("no value q[]= search param is decoded correctly", () => { + expect(DecodeLocationSearch("?q[]=")).toMatchObject({ + defaultsUsed: false, + params: { q: [] } + }); + }); + + it("no value q= search param is decoded correctly", () => { + expect(DecodeLocationSearch("?q=")).toMatchObject({ + defaultsUsed: false, + params: { q: [] } + }); + }); + + it("single value q=foo search param is decoded correctly", () => { + expect(DecodeLocationSearch("?q=foo")).toMatchObject({ + defaultsUsed: false, + params: { q: ["foo"] } + }); + }); + + it("single value q[]=foo search param is decoded correctly", () => { + expect(DecodeLocationSearch("?q[]=foo")).toMatchObject({ + defaultsUsed: false, + params: { q: ["foo"] } + }); + }); + + it("multi value q[]=foo&q[]=bar search param is decoded correctly", () => { + expect(DecodeLocationSearch("?q[]=foo&q[]=bar")).toMatchObject({ + defaultsUsed: false, + params: { q: ["foo", "bar"] } + }); + }); +});