Allow automatically generated lanes to be renamed.

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
 

src/components/Lane.tsx

@@ -47,8 +47,7 @@ export default class Lane extends React.Component<LaneProps, LaneState> {
   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 (
       <li key={lane.id}>
         <div
@@ -79,7 +78,7 @@ export default class Lane extends React.Component<LaneProps, LaneState> {
           </div>
           {this.state.expanded && (
             <div className="lane-buttons">
-              {hasCustomLists && this.renderButton("edit")}
+              {this.renderButton("edit")}
               {this.renderButton("create")}
             </div>
           )}

src/components/LaneEditor.tsx

@@ -78,23 +78,26 @@ export default class LaneEditor extends React.Component<
   }
 
   render(): JSX.Element {
-    const parent = this.props.findParentOfLane(this.props.lane);
+    const { lane } = this.props;
+    const hasCustomLists = !!lane?.custom_list_ids?.length;
+    const parent = this.props.findParentOfLane(lane);
+
     return (
       <div className="lane-editor">
         <div className="lane-editor-header">
           <div>
             <div className="save-or-edit">
               <TextWithEditMode
-                text={this.props.lane && this.props.lane.display_name}
+                text={lane?.display_name}
                 placeholder="name"
                 onUpdate={this.changeName}
                 ref={this.laneNameRef}
                 aria-label="Enter a name for this lane"
               />
-              {this.props.lane && <h4>ID-{this.props.lane.id}</h4>}
+              {lane && <h4>ID-{lane.id}</h4>}
             </div>
             <span className="lane-buttons">
-              {this.props.lane && this.props.deleteLane && (
+              {lane && hasCustomLists && this.props.deleteLane && (
                 <Button
                   className="danger delete-lane"
                   callback={this.delete}
@@ -122,7 +125,7 @@ export default class LaneEditor extends React.Component<
           </div>
           <div className="lane-details">
             {this.renderInfo(parent)}
-            {parent && (
+            {hasCustomLists && parent && (
               <EditableInput
                 type="checkbox"
                 name="inherit_parent_restrictions"
@@ -133,17 +136,19 @@ export default class LaneEditor extends React.Component<
             )}
           </div>
         </div>
-        <div className="lane-editor-body">
-          <LaneCustomListsEditor
-            allCustomLists={this.props.customLists}
-            customListIds={this.state.customListIds}
-            filter={this.state.filter}
-            filteredCustomLists={this.filterLists()}
-            changeFilter={this.changeFilter}
-            onUpdate={this.changeCustomLists}
-            ref={this.laneCustomListsRef}
-          />
-        </div>
+        {(!lane || hasCustomLists) && (
+          <div className="lane-editor-body">
+            <LaneCustomListsEditor
+              allCustomLists={this.props.customLists}
+              customListIds={this.state.customListIds}
+              filter={this.state.filter}
+              filteredCustomLists={this.filterLists()}
+              changeFilter={this.changeFilter}
+              onUpdate={this.changeCustomLists}
+              ref={this.laneCustomListsRef}
+            />
+          </div>
+        )}
       </div>
     );
   }
@@ -278,8 +283,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 +307,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 }));

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

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) => (
+    <div data-testid="Link" data-to={props.to}>
+      {props.children}
+    </div>
+  ),
+}));
+
+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(
+      <Lane
+        lane={laneData}
+        active={false}
+        library="test_library"
+        orderChanged={false}
+        renderLanes={renderLanes}
+        toggleLaneVisibility={toggleLaneVisibility}
+      />
+    );
+
+    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(
+      <Lane
+        lane={laneData}
+        active={false}
+        library="test_library"
+        orderChanged={false}
+        renderLanes={renderLanes}
+        toggleLaneVisibility={toggleLaneVisibility}
+      />
+    );
+
+    const editLink = screen.getAllByTestId("Link")[0];
+
+    expect(editLink).toHaveAttribute("data-to", expect.stringMatching(/edit/i));
+    expect(editLink).toHaveTextContent(/edit/i);
+  });
+});

tests/jest/components/LaneEditor.test.tsx

@@ -0,0 +1,121 @@
+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) => <div data-testid="LaneCustomListsEditor" />,
+}));
+
+const customListsData = [
+  { id: 1, name: "list 1", entries: [], is_owner: true, is_shared: false },
+];
+
+const editLane = stub().returns(
+  new Promise<void>((resolve) => resolve())
+);
+
+const deleteLane = stub().returns(
+  new Promise<void>((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(
+        <LaneEditor
+          library="library"
+          lane={laneData}
+          customLists={customListsData}
+          editLane={editLane}
+          deleteLane={deleteLane}
+          findParentOfLane={stub().returns(laneData)}
+          toggleLaneVisibility={toggleLaneVisibility}
+        />
+      );
+    });
+
+    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(() => {
+      <LaneEditor
+        library="library"
+        lane={laneData}
+        customLists={customListsData}
+        editLane={editLane}
+        deleteLane={deleteLane}
+        findParentOfLane={stub().returns(laneData)}
+        toggleLaneVisibility={toggleLaneVisibility}
+      />;
+    });
+
+    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();
+    });
+  });
+});