diff --git a/src/components/common/Alert.tsx b/src/components/common/Alert.tsx index a36f131..414252e 100644 --- a/src/components/common/Alert.tsx +++ b/src/components/common/Alert.tsx @@ -8,7 +8,7 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ -import React, { FunctionComponent } from "react"; +import React from "react"; export type AlertType = "info" | "success" | "danger"; @@ -16,23 +16,27 @@ export interface AlertProps { type: AlertType; onClick: () => void; message: string; + dismissible: boolean; } -const Alert: FunctionComponent = (props: AlertProps) => { +const Alert = (props: AlertProps) => { + const dismissClass = props.dismissible ? "alert-dismissible" : ""; + return ( -
- +
+ {props.dismissible ? ( + + ) : null} {props.message}
); }; Alert.defaultProps = { - onClick: () => {} + onClick: () => {}, + dismissible: true }; export default Alert; diff --git a/src/components/list/DomainList.tsx b/src/components/list/DomainList.tsx index ab9e4cf..4863803 100644 --- a/src/components/list/DomainList.tsx +++ b/src/components/list/DomainList.tsx @@ -12,6 +12,7 @@ import React from "react"; import { WithNamespaces, withNamespaces } from "react-i18next"; import api from "../../util/api"; import { Button } from "reactstrap"; +import Alert from "../common/Alert"; export interface DomainListProps extends WithNamespaces { domains: string[]; @@ -54,9 +55,11 @@ const DomainList = ({ domains, onRemove, t }: DomainListProps) => { body = domains.map(mapDomainsToListItems); } else { body = ( -
- {t("There are no domains in this list")} -
+ ); } diff --git a/src/components/list/ListPage.tsx b/src/components/list/ListPage.tsx index 336beca..6450a1d 100644 --- a/src/components/list/ListPage.tsx +++ b/src/components/list/ListPage.tsx @@ -23,9 +23,9 @@ export interface ListPageProps extends WithNamespaces { title: string; note?: {} | string; placeholder: string; - add: (domain: string) => Promise; - refresh: () => Promise; - remove: (domain: string) => Promise; + onAdd: (domain: string) => Promise; + onRefresh: () => Promise; + onRemove: (domain: string) => Promise; isValid: (domain: string) => boolean; validationErrorMsg: string; } @@ -60,7 +60,7 @@ export class ListPage extends Component { const prevDomains = this.state.domains.slice(); // Try to add the domain - this.addHandler = makeCancelable(this.props.add(domain)); + this.addHandler = makeCancelable(this.props.onAdd(domain)); this.addHandler.promise .then(() => { this.onAdded(domain); @@ -117,7 +117,7 @@ export class ListPage extends Component { if (this.state.domains.includes(domain)) { const prevDomains = this.state.domains.slice(); - this.removeHandler = makeCancelable(this.props.remove(domain)); + this.removeHandler = makeCancelable(this.props.onRemove(domain)); this.removeHandler.promise.catch(ignoreCancel).catch(() => { this.onRemoveFailed(domain, prevDomains); }); @@ -127,7 +127,7 @@ export class ListPage extends Component { }; onRefresh = () => { - this.refreshHandler = makeCancelable(this.props.refresh()); + this.refreshHandler = makeCancelable(this.props.onRefresh()); this.refreshHandler.promise .then(data => { this.setState({ domains: data }); diff --git a/src/components/list/__tests__/DomainList.test.tsx b/src/components/list/__tests__/DomainList.test.tsx index eb9b828..c66681c 100644 --- a/src/components/list/__tests__/DomainList.test.tsx +++ b/src/components/list/__tests__/DomainList.test.tsx @@ -12,6 +12,7 @@ import React from "react"; import { shallow } from "enzyme"; import DomainList from "../DomainList"; import api from "../../../util/api"; +import Alert from "../../common/Alert"; const domains = ["domain1.com", "domain2.com", "domain3.com"]; @@ -26,9 +27,11 @@ it("shows a list of domains", () => { it("shows an alert if there are no domains", () => { const wrapper = shallow(); - expect(wrapper.find("li")).toHaveLength(0); - expect(wrapper.find("ul").childAt(0)).toHaveClassName("alert-info"); - expect(wrapper).toIncludeText("There are no domains in this list"); + expect(wrapper.find("li")).not.toExist(); + expect(wrapper.find(Alert)).toExist(); + expect(wrapper.find(Alert).props().message).toEqual( + "There are no domains in this list" + ); }); it("does not have a delete button when not logged in", () => { @@ -36,12 +39,7 @@ it("does not have a delete button when not logged in", () => { ); - expect( - wrapper - .find("ul") - .childAt(0) - .find("button") - ).not.toExist(); + expect(wrapper.find("Button")).not.toExist(); }); it("has a delete button when logged in", () => { diff --git a/src/components/list/__tests__/ListPage.test.tsx b/src/components/list/__tests__/ListPage.test.tsx index 5f42384..4f1a013 100644 --- a/src/components/list/__tests__/ListPage.test.tsx +++ b/src/components/list/__tests__/ListPage.test.tsx @@ -15,6 +15,9 @@ import ListPage, { ListPageProps, ListPageState } from "../ListPage"; +import Alert from "../../common/Alert"; +import DomainInput from "../DomainInput"; +import DomainList from "../DomainList"; const ignoreAPI = global.ignoreAPI; const tick = global.tick; @@ -32,9 +35,9 @@ it("shows the title", () => { title={title} placeholder="" note="" - add={ignoreAPI} - refresh={ignoreAPI} - remove={ignoreAPI} + onAdd={ignoreAPI} + onRefresh={ignoreAPI} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> @@ -50,15 +53,15 @@ it("shows the placeholder", () => { title="" placeholder={placeholder} note="" - add={ignoreAPI} - refresh={ignoreAPI} - remove={ignoreAPI} + onAdd={ignoreAPI} + onRefresh={ignoreAPI} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> ); - expect(wrapper.find("DomainInput")).toHaveProp("placeholder", placeholder); + expect(wrapper.find(DomainInput)).toHaveProp("placeholder", placeholder); }); it("shows the note", () => { @@ -68,9 +71,9 @@ it("shows the note", () => { title="" placeholder="" note={note} - add={ignoreAPI} - refresh={ignoreAPI} - remove={ignoreAPI} + onAdd={ignoreAPI} + onRefresh={ignoreAPI} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> @@ -85,15 +88,105 @@ it("starts with no alerts shown", () => { title="" placeholder="" note="" - add={ignoreAPI} - refresh={ignoreAPI} - remove={ignoreAPI} + onAdd={ignoreAPI} + onRefresh={ignoreAPI} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> ); - expect(wrapper.find("Alert")).toHaveLength(0); + expect(wrapper.find(Alert)).not.toExist(); +}); + +it("hides the alert if closed", () => { + const wrapper: ListPageWrapper = shallow( + + ); + + // Show an error message + wrapper.instance().onAlreadyAdded("domain"); + + // Now the alert is shown + const alert = wrapper.find(Alert); + expect(alert).toExist(); + + // Hide the alert + alert.props().onClick(); + + expect(wrapper.find(Alert)).not.toExist(); +}); + +it("cancels requests when un-mounting", async () => { + const wrapper: ListPageWrapper = shallow( + Promise.resolve(["domain"])} + onRemove={ignoreAPI} + isValid={jest.fn()} + validationErrorMsg="" + /> + ); + + // Load the domains into state + await tick(); + + // Initiate requests to refresh, add, and remove domains + wrapper.instance().onRefresh(); + wrapper.instance().onEnter("domain2"); + wrapper.instance().onRemove("domain"); + + // Spy on the handlers + // Casting instance to any to access private fields + const instance = wrapper.instance() as any; + const cancelRefreshSpy = jest.spyOn(instance.refreshHandler, "cancel"); + const cancelAddSpy = jest.spyOn(instance.addHandler, "cancel"); + const cancelRemoveSpy = jest.spyOn(instance.removeHandler, "cancel"); + + // Unmount, which should cancel the requests + wrapper.unmount(); + + expect(cancelRefreshSpy).toHaveBeenCalled(); + expect(cancelAddSpy).toHaveBeenCalled(); + expect(cancelRemoveSpy).toHaveBeenCalled(); +}); + +it("shows a validation message as an error", () => { + const validationError = "test message"; + const wrapper = shallow( + + ); + + wrapper + .find(DomainInput) + .props() + .onValidationError(); + + const alert = wrapper.find(Alert); + expect(alert).toExist(); + expect(alert.props().message).toEqual(validationError); + expect(alert.props().type).toEqual("danger"); }); it("loads domains after mounting", async () => { @@ -103,42 +196,38 @@ it("loads domains after mounting", async () => { title="" placeholder="" note="" - add={ignoreAPI} - refresh={() => Promise.resolve(domains)} - remove={ignoreAPI} + onAdd={ignoreAPI} + onRefresh={() => Promise.resolve(domains)} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> ); await tick(); - wrapper.update(); - expect(wrapper.find("DomainList")).toHaveProp("domains", domains); + expect(wrapper.find(DomainList)).toHaveProp("domains", domains); }); it("checks if the domain was already added", async () => { const domains = ["domain1", "domain2.com", "domain3.net"]; - const onAlreadyAdded = jest.fn(); const wrapper: ListPageWrapper = shallow( Promise.resolve(domains)} - remove={ignoreAPI} + onAdd={ignoreAPI} + onRefresh={() => Promise.resolve(domains)} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> ); + const onAlreadyAdded = jest.spyOn(wrapper.instance(), "onAlreadyAdded"); - // Setup with domains (wait for promise to resolve) and mock function + // Setup with domains (wait for promise to resolve) await tick(); - wrapper.instance().onAlreadyAdded = onAlreadyAdded; - wrapper.update(); - // Test onEnter wrapper.instance().onEnter(domains[0]); expect(onAlreadyAdded).toHaveBeenCalledWith(domains[0]); @@ -146,15 +235,15 @@ it("checks if the domain was already added", async () => { it("calls the add prop when adding a domain", () => { const domain = "domain"; - const add = jest.fn(ignoreAPI); + const onAdd = jest.fn(ignoreAPI); const wrapper: ListPageWrapper = shallow( @@ -162,27 +251,25 @@ it("calls the add prop when adding a domain", () => { wrapper.instance().onEnter(domain); - expect(add).toHaveBeenCalledWith(domain); + expect(onAdd).toHaveBeenCalledWith(domain); }); it("calls onAdding when adding a domain", () => { const domain = "domain"; - const onAdding = jest.fn(); const wrapper: ListPageWrapper = shallow( ); + const onAdding = jest.spyOn(wrapper.instance(), "onAdding"); - wrapper.instance().onAdding = onAdding; - wrapper.update(); wrapper.instance().onEnter(domain); expect(onAdding).toHaveBeenCalledWith(domain); @@ -190,22 +277,20 @@ it("calls onAdding when adding a domain", () => { it("calls onAdded after API request succeeds", async () => { const domain = "domain"; - const onAdded = jest.fn(); const wrapper: ListPageWrapper = shallow( Promise.resolve()} - refresh={ignoreAPI} - remove={ignoreAPI} + onAdd={() => Promise.resolve()} + onRefresh={ignoreAPI} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> ); + const onAdded = jest.spyOn(wrapper.instance(), "onAdded"); - wrapper.instance().onAdded = onAdded; - wrapper.update(); wrapper.instance().onEnter(domain); await tick(); @@ -214,22 +299,20 @@ it("calls onAdded after API request succeeds", async () => { it("calls onAddFailed after API request fails", async () => { const domain = "domain"; - const onAddFailed = jest.fn(); const wrapper: ListPageWrapper = shallow( Promise.reject({})} - refresh={ignoreAPI} - remove={ignoreAPI} + onAdd={() => Promise.reject({})} + onRefresh={ignoreAPI} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> ); + const onAddFailed = jest.spyOn(wrapper.instance(), "onAddFailed"); - wrapper.instance().onAddFailed = onAddFailed; - wrapper.update(); wrapper.instance().onEnter(domain); await tick(); @@ -243,16 +326,15 @@ it("adds the domain in onAdded", async () => { title="" placeholder="" note="" - add={() => Promise.resolve()} - refresh={ignoreAPI} - remove={ignoreAPI} + onAdd={() => Promise.resolve()} + onRefresh={ignoreAPI} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> ); wrapper.instance().onEnter(domain); - wrapper.update(); await tick(); expect(wrapper.state().domains).toEqual([domain]); @@ -265,60 +347,84 @@ it("resets the domains when adding failed", async () => { title="" placeholder="" note="" - add={() => Promise.reject({})} - refresh={ignoreAPI} - remove={ignoreAPI} + onAdd={() => Promise.reject({})} + onRefresh={ignoreAPI} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> ); wrapper.instance().onEnter(domain); - wrapper.update(); await tick(); expect(wrapper.state().domains).toEqual([]); }); -it("removes the domain when onRemoved is called", async () => { - const domain = "domain"; - const domains = [domain]; +it("does not remove the domain if it is not present", async () => { + const domain = "domain1"; + const domains = ["domain2"]; const wrapper: ListPageWrapper = shallow( Promise.resolve(domains)} - remove={ignoreAPI} + onAdd={ignoreAPI} + onRefresh={() => Promise.resolve(domains)} + onRemove={ignoreAPI} isValid={jest.fn()} validationErrorMsg="" /> ); - wrapper.instance().onRemoved(domain); - wrapper.update(); + await tick(); + wrapper.instance().onRemove(domain); - expect(wrapper.state().domains).toEqual([]); + expect(wrapper.state().domains).toEqual(domains); }); -it("resets the domains when removal failed", () => { +it("removes the domain from state when onRemove is called", async () => { const domain = "domain"; + const domain2 = "domain2"; + const domains = [domain, domain2]; + const wrapper: ListPageWrapper = shallow( + Promise.resolve(domains)} + onRemove={() => Promise.resolve()} + isValid={jest.fn()} + validationErrorMsg="" + /> + ); + + await tick(); + wrapper.instance().onRemove(domain); + + expect(wrapper.state().domains).toEqual([domain2]); +}); + +it("resets the domains when removal failed", async () => { + const domain = "domain"; const domains = [domain]; const wrapper: ListPageWrapper = shallow( Promise.resolve(domains)} - remove={ignoreAPI} + onAdd={ignoreAPI} + onRefresh={() => Promise.resolve(domains)} + onRemove={() => Promise.reject()} isValid={jest.fn()} validationErrorMsg="" /> ); - wrapper.instance().onRemoveFailed(domain, domains); - wrapper.update(); + + await tick(); + wrapper.instance().onRemove(domain); + await tick(); expect(wrapper.state().domains).toEqual(domains); }); diff --git a/src/views/Blacklist.tsx b/src/views/Blacklist.tsx index f24479b..8a1db81 100644 --- a/src/views/Blacklist.tsx +++ b/src/views/Blacklist.tsx @@ -21,9 +21,9 @@ const Blacklist: FunctionComponent = props => { = props => { = props => {