diff --git a/CHANGELOG.md b/CHANGELOG.md index 349512664..b85c19e91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - A placeholder and help text are now displayed when the Publication Date field is selected in advanced search. - Remove CDN configuration screens. +- Automatically generated lanes can now be renamed. #### Fixed diff --git a/src/components/Lane.tsx b/src/components/Lane.tsx index 1585d9f7f..f2ec7317a 100644 --- a/src/components/Lane.tsx +++ b/src/components/Lane.tsx @@ -47,8 +47,7 @@ export default class Lane extends React.Component { render(): JSX.Element { const { lane, active, snapshot, provided, renderLanes } = this.props; const hasSublanes = lane.sublanes && !!lane.sublanes.length; - const hasCustomLists = - lane.custom_list_ids && !!lane.custom_list_ids.length; + return (
  • {
    {this.state.expanded && (
    - {hasCustomLists && this.renderButton("edit")} + {this.renderButton("edit")} {this.renderButton("create")}
    )} diff --git a/src/components/LaneEditor.tsx b/src/components/LaneEditor.tsx index f00ffebcb..c75735ae3 100644 --- a/src/components/LaneEditor.tsx +++ b/src/components/LaneEditor.tsx @@ -10,6 +10,7 @@ import VisibleIcon from "./icons/VisibleIcon"; import HiddenIcon from "./icons/HiddenIcon"; export interface LaneEditorProps extends React.Props { + editOrCreate?: string; library: string; lane?: LaneData; customLists: CustomListData[]; @@ -77,24 +78,30 @@ export default class LaneEditor extends React.Component< return selectedFilter ? lists.filter(selectedFilter) : lists; } + hasCustomLists() { + return !!this.props.lane?.custom_list_ids?.length; + } + render(): JSX.Element { - const parent = this.props.findParentOfLane(this.props.lane); + const { lane } = this.props; + const parent = this.props.findParentOfLane(lane); + return (
    - {this.props.lane &&

    ID-{this.props.lane.id}

    } + {lane &&

    ID-{lane.id}

    }
    - {this.props.lane && this.props.deleteLane && ( + {lane && this.hasCustomLists() && this.props.deleteLane && (
    + ); + } + + renderCustomListsEditor(): JSX.Element { + const { editOrCreate, lane } = this.props; + + if (editOrCreate === "edit") { + if (!lane) { + // We're editing a lane, but the lane data hasn't loaded yet. Don't render anything until + // we know if this is a custom lane. + return null; + } + + if (!this.hasCustomLists()) { + return ( +
    + This lane was automatically generated. Its contents cannot be + edited. +
    + ); + } + } + + return ( +
    +
    ); } @@ -278,8 +310,12 @@ export default class LaneEditor extends React.Component< } const name = this.laneNameRef.current.getText(); data.append("display_name", name); - const listIds = this.laneCustomListsRef.current.getCustomListIds(); - data.append("custom_list_ids", JSON.stringify(listIds)); + + const listIds = this.laneCustomListsRef.current?.getCustomListIds(); + if (listIds) { + data.append("custom_list_ids", JSON.stringify(listIds)); + } + data.append( "inherit_parent_restrictions", this.state.inheritParentRestrictions @@ -298,7 +334,7 @@ export default class LaneEditor extends React.Component< reset() { this.laneNameRef.current.reset(); - this.laneCustomListsRef.current.reset(this.props.lane.custom_list_ids); + this.laneCustomListsRef.current?.reset(this.props.lane.custom_list_ids); const inheritParentRestrictions = this.props.lane && this.props.lane.inherit_parent_restrictions; this.setState(Object.assign({}, this.state, { inheritParentRestrictions })); diff --git a/src/components/Lanes.tsx b/src/components/Lanes.tsx index 4b6f21993..d31980cfc 100644 --- a/src/components/Lanes.tsx +++ b/src/components/Lanes.tsx @@ -170,6 +170,7 @@ export class Lanes extends React.Component { library: this.props.library, customLists: this.props.customLists, editLane: this.editLane, + editOrCreate, }; const extraProps = { create: { diff --git a/src/components/__tests__/Lane-test.tsx b/src/components/__tests__/Lane-test.tsx index 134eee583..1f1a7ed7f 100644 --- a/src/components/__tests__/Lane-test.tsx +++ b/src/components/__tests__/Lane-test.tsx @@ -91,14 +91,6 @@ describe("Lane", () => { ); expect(editSublane.hasClass("disabled")).to.be.false; - // A lane that wasn't created from custom lists should not be editable. - const notFromCustomLists = { ...laneData, ...{ custom_list_ids: [] } }; - wrapper.setProps({ lane: notFromCustomLists }); - editSublane = wrapper - .find(Link) - .filterWhere((el) => el.find("a").hasClass("edit-lane")); - expect(editSublane.length).to.equal(0); - // The link is disabled if there are lane order changes. wrapper.setProps({ lane: laneData, orderChanged: true }); editSublane = wrapper diff --git a/tests/jest/components/Lane.test.tsx b/tests/jest/components/Lane.test.tsx new file mode 100644 index 000000000..3bd1aa7bd --- /dev/null +++ b/tests/jest/components/Lane.test.tsx @@ -0,0 +1,78 @@ +import * as React from "react"; +import { render, screen } from "@testing-library/react"; +import { stub } from "sinon"; +import { LaneData } from "../../../src/interfaces"; +import Lane from "../../../src/components/Lane"; + +// Mock the Link component from React Router, so we can verify that it gets rendered with the +// expected props. This serves as an example of how to do something analogous to Enzyme's shallow +// rendering, when we don't want/need to render the whole component tree down to HTML elements to +// test something. This technique is useful for testing components in isolation (unit testing), +// instead of the integration testing that RTL focuses on. + +jest.mock("react-router", () => ({ + ...jest.requireActual("react-router"), + Link: (props) => ( +
    + {props.children} +
    + ), +})); + +const renderLanes = stub(); +const toggleLaneVisibility = stub(); + +function createLaneData(displayName: string, isAutomated: boolean): LaneData { + return { + id: 1, + display_name: displayName, + visible: true, + count: 5, + sublanes: [], + // The absence/presence of custom list ids determines if a lane is automated or custom. + custom_list_ids: isAutomated ? [] : [1], + inherit_parent_restrictions: true, + }; +} + +describe("Lane", () => { + it("renders an edit link on a custom lane", () => { + const laneData = createLaneData("Custom Lane", false); + + render( + + ); + + const editLink = screen.getAllByTestId("Link")[0]; + + expect(editLink).toHaveAttribute("data-to", expect.stringMatching(/edit/i)); + expect(editLink).toHaveTextContent(/edit/i); + }); + + it("renders an edit link on an automated lane", async () => { + const laneData = createLaneData("Automated Lane", true); + + render( + + ); + + const editLink = screen.getAllByTestId("Link")[0]; + + expect(editLink).toHaveAttribute("data-to", expect.stringMatching(/edit/i)); + expect(editLink).toHaveTextContent(/edit/i); + }); +}); diff --git a/tests/jest/components/LaneEditor.test.tsx b/tests/jest/components/LaneEditor.test.tsx new file mode 100644 index 000000000..9a0901205 --- /dev/null +++ b/tests/jest/components/LaneEditor.test.tsx @@ -0,0 +1,148 @@ +import * as React from "react"; +import { render, screen } from "@testing-library/react"; +import { stub } from "sinon"; +import { LaneData } from "../../../src/interfaces"; +import LaneEditor from "../../../src/components/LaneEditor"; + +// Mock the LaneCustomListsEditor so we can verify that it is or isn't rendered. This serves as an +// example of how to do something analogous to Enzyme's shallow rendering, when we don't want/need +// to render the whole component tree down to HTML elements to test something. This technique is +// useful for testing components in isolation (unit testing), instead of the integration testing +// that RTL focuses on. + +jest.mock("../../../src/components/LaneCustomListsEditor", () => ({ + __esModule: true, + default: (props) =>
    , +})); + +const customListsData = [ + { id: 1, name: "list 1", entries: [], is_owner: true, is_shared: false }, +]; + +const editLane = stub().returns( + new Promise((resolve) => resolve()) +); + +const deleteLane = stub().returns( + new Promise((resolve) => resolve()) +); + +const toggleLaneVisibility = stub(); + +function createLaneData(displayName: string, isAutomated: boolean): LaneData { + return { + id: 1, + display_name: displayName, + visible: true, + count: 5, + sublanes: [ + { + id: 2, + display_name: `Sublane of ${displayName}`, + visible: true, + count: 3, + sublanes: [], + custom_list_ids: [1], + inherit_parent_restrictions: false, + }, + ], + // The absence/presence of custom list ids determines if a lane is automated or custom. + custom_list_ids: isAutomated ? [] : [1], + inherit_parent_restrictions: true, + }; +} + +describe("LaneEditor", () => { + describe("for a custom lane", () => { + const laneData = createLaneData("Custom Lane", false); + + beforeEach(() => { + render( + + ); + }); + + it("renders a delete button", () => { + expect(screen.getByRole("button", { name: /delete/i })).not.toBeNull(); + }); + + it("renders an inherit parent restrictions checkbox", () => { + expect( + screen.getByRole("checkbox", { + name: /inherit restrictions from parent/i, + }) + ).not.toBeNull(); + }); + + it("renders a custom lists editor", () => { + expect(screen.getByTestId("LaneCustomListsEditor")).not.toBeNull(); + }); + }); + + describe("for an automated lane", () => { + const laneData = createLaneData("Automated Lane", true); + + beforeEach(() => { + render( + + ); + }); + + it("does not render a delete button", () => { + expect(screen.queryByRole("button", { name: /delete/i })).toBeNull(); + }); + + it("does not render an inherit parent restrictions checkbox", () => { + expect( + screen.queryByRole("checkbox", { + name: /inherit restrictions from parent/i, + }) + ).toBeNull(); + }); + + it("does not render a custom lists editor", () => { + expect(screen.queryByTestId("LaneCustomListsEditor")).toBeNull(); + }); + + it("renders an explanation that the lane contents can't be edited", () => { + expect(screen.getByText(/contents cannot be edited/i)).not.toBeNull(); + }); + }); + + it("doesn't render a custom lists editor while a lane is being loaded for editing", () => { + const laneData = null; + + render( + + ); + + expect(screen.queryByTestId("LaneCustomListsEditor")).toBeNull(); + }); +});