Merge pull request #1744 from FarmBot/bugfix/add-tool

Fix add tool bug

frontend/css/farm_designer/farm_designer_panels.scss

@@ -650,6 +650,7 @@
     margin-top: 1rem;
     &.red {
       float: left;
+      margin-bottom: 1rem;
     }
   }
   svg {
@@ -659,6 +660,19 @@
     height: 10rem;
     margin-top: 2rem;
   }
+  .edit-tool,
+  .add-new-tool {
+    margin-bottom: 3rem;
+    .name-error {
+      margin-top: 1.2rem;
+      margin-right: 1rem;
+      color: $dark_red;
+      float: right;
+    }
+    .save-btn {
+      float: right;
+    }
+  }
   .add-stock-tools {
     .filter-search {
       margin-bottom: 1rem;

frontend/farm_designer/tools/__tests__/add_tool_test.tsx

@@ -1,4 +1,10 @@
-jest.mock("../../../api/crud", () => ({ initSave: jest.fn() }));
+let mockSave = () => Promise.resolve();
+jest.mock("../../../api/crud", () => ({
+  initSave: jest.fn(),
+  init: jest.fn(() => ({ payload: { uuid: "fake uuid" } })),
+  save: jest.fn(() => mockSave),
+  destroy: jest.fn(),
+}));
 
 jest.mock("../../../history", () => ({ history: { push: jest.fn() } }));
 
@@ -7,7 +13,7 @@ import { mount, shallow } from "enzyme";
 import { RawAddTool as AddTool, mapStateToProps } from "../add_tool";
 import { fakeState } from "../../../__test_support__/fake_state";
 import { SaveBtn } from "../../../ui";
-import { initSave } from "../../../api/crud";
+import { initSave, init, destroy } from "../../../api/crud";
 import { history } from "../../../history";
 import { FirmwareHardware } from "farmbot";
 import { AddToolProps } from "../interfaces";
@@ -32,11 +38,47 @@ describe("<AddTool />", () => {
     expect(wrapper.state().toolName).toEqual("new name");
   });
 
-  it("saves", () => {
-    const wrapper = shallow(<AddTool {...fakeProps()} />);
+  it("disables save until name in entered", () => {
+    const wrapper = shallow<AddTool>(<AddTool {...fakeProps()} />);
+    expect(wrapper.state().toolName).toEqual("");
+    expect(wrapper.find("SaveBtn").first().props().disabled).toBeTruthy();
+    wrapper.setState({ toolName: "fake tool name" });
+    expect(wrapper.find("SaveBtn").first().props().disabled).toBeFalsy();
+  });
+
+  it("shows name collision message", () => {
+    const p = fakeProps();
+    p.existingToolNames = ["tool"];
+    const wrapper = shallow<AddTool>(<AddTool {...p} />);
+    wrapper.setState({ toolName: "tool" });
+    expect(wrapper.find("p").first().text()).toEqual("Already added.");
+    expect(wrapper.find("SaveBtn").first().props().disabled).toBeTruthy();
+  });
+
+  it("saves", async () => {
+    mockSave = () => Promise.resolve();
+    const p = fakeProps();
+    p.dispatch = jest.fn(x => typeof x === "function" && x());
+    const wrapper = shallow<AddTool>(<AddTool {...p} />);
     wrapper.setState({ toolName: "Foo" });
-    wrapper.find(SaveBtn).simulate("click");
-    expect(initSave).toHaveBeenCalledWith("Tool", { name: "Foo" });
+    await wrapper.find(SaveBtn).simulate("click");
+    expect(init).toHaveBeenCalledWith("Tool", { name: "Foo" });
+    expect(wrapper.state().uuid).toEqual(undefined);
+    expect(history.push).toHaveBeenCalledWith("/app/designer/tools");
+  });
+
+  it("removes unsaved tool on exit", async () => {
+    mockSave = () => Promise.reject();
+    const p = fakeProps();
+    p.dispatch = jest.fn(x => typeof x === "function" && x());
+    const wrapper = shallow<AddTool>(<AddTool {...p} />);
+    wrapper.setState({ toolName: "Foo" });
+    await wrapper.find(SaveBtn).simulate("click");
+    expect(init).toHaveBeenCalledWith("Tool", { name: "Foo" });
+    expect(wrapper.state().uuid).toEqual("fake uuid");
+    expect(history.push).not.toHaveBeenCalled();
+    wrapper.unmount();
+    expect(destroy).toHaveBeenCalledWith("fake uuid");
   });
 
   it.each<[FirmwareHardware, number]>([

frontend/farm_designer/tools/__tests__/edit_tool_test.tsx

@@ -38,6 +38,7 @@ describe("<EditTool />", () => {
     dispatch: jest.fn(),
     mountedToolId: undefined,
     isActive: jest.fn(),
+    existingToolNames: [],
   });
 
   it("renders", () => {
@@ -71,6 +72,23 @@ describe("<EditTool />", () => {
     expect(wrapper.state().toolName).toEqual("new name");
   });
 
+  it("disables save until name in entered", () => {
+    const wrapper = shallow<EditTool>(<EditTool {...fakeProps()} />);
+    wrapper.setState({ toolName: "" });
+    expect(wrapper.find("SaveBtn").first().props().disabled).toBeTruthy();
+    wrapper.setState({ toolName: "fake tool name" });
+    expect(wrapper.find("SaveBtn").first().props().disabled).toBeFalsy();
+  });
+
+  it("shows name collision message", () => {
+    const p = fakeProps();
+    p.existingToolNames = ["tool"];
+    const wrapper = shallow<EditTool>(<EditTool {...p} />);
+    wrapper.setState({ toolName: "tool" });
+    expect(wrapper.find("p").first().text()).toEqual("Name already taken.");
+    expect(wrapper.find("SaveBtn").first().props().disabled).toBeTruthy();
+  });
+
   it("saves", () => {
     const wrapper = shallow(<EditTool {...fakeProps()} />);
     wrapper.find(SaveBtn).simulate("click");

frontend/farm_designer/tools/__tests__/index_test.tsx

@@ -188,7 +188,9 @@ describe("<Tools />", () => {
 
   it("displays tool as active", () => {
     const p = fakeProps();
-    p.tools = [fakeTool()];
+    const tool = fakeTool();
+    tool.body.id = 1;
+    p.tools = [tool];
     p.isActive = () => true;
     p.device.body.mounted_tool_id = undefined;
     const wrapper = mount(<Tools {...p} />);

frontend/farm_designer/tools/add_tool.tsx

@@ -7,7 +7,7 @@ import { Everything } from "../../interfaces";
 import { t } from "../../i18next_wrapper";
 import { SaveBtn } from "../../ui";
 import { SpecialStatus } from "farmbot";
-import { initSave } from "../../api/crud";
+import { initSave, destroy, init, save } from "../../api/crud";
 import { Panel } from "../panel_header";
 import { history } from "../../history";
 import { selectAllTools } from "../../resources/selectors";
@@ -27,7 +27,7 @@ export const mapStateToProps = (props: Everything): AddToolProps => ({
 });
 
 export class RawAddTool extends React.Component<AddToolProps, AddToolState> {
-  state: AddToolState = { toolName: "", toAdd: [] };
+  state: AddToolState = { toolName: "", toAdd: [], uuid: undefined };
 
   filterExisting = (n: string) => !this.props.existingToolNames.includes(n);
 
@@ -41,15 +41,23 @@ export class RawAddTool extends React.Component<AddToolProps, AddToolState> {
     toAdd: this.stockToolNames().filter(this.filterExisting)
   });
 
-  newTool = (name: string) => {
-    this.props.dispatch(initSave("Tool", { name }));
-  };
+  newTool = (name: string) => this.props.dispatch(initSave("Tool", { name }));
 
   save = () => {
-    this.newTool(this.state.toolName);
-    history.push("/app/designer/tools");
+    const initTool = init("Tool", { name: this.state.toolName });
+    this.props.dispatch(initTool);
+    const { uuid } = initTool.payload;
+    this.setState({ uuid });
+    this.props.dispatch(save(uuid))
+      .then(() => {
+        this.setState({ uuid: undefined });
+        history.push("/app/designer/tools");
+      }).catch(() => { });
   }
 
+  componentWillUnmount = () =>
+    this.state.uuid && this.props.dispatch(destroy(this.state.uuid));
+
   stockToolNames = () => {
     switch (this.props.firmwareHardware) {
       case "arduino":
@@ -123,6 +131,8 @@ export class RawAddTool extends React.Component<AddToolProps, AddToolState> {
   }
 
   render() {
+    const { toolName, uuid } = this.state;
+    const alreadyAdded = !uuid && !this.filterExisting(toolName);
     const panelName = "add-tool";
     return <DesignerPanel panelName={panelName} panel={Panel.Tools}>
       <DesignerPanelHeader
@@ -138,7 +148,13 @@ export class RawAddTool extends React.Component<AddToolProps, AddToolState> {
             name="name"
             onChange={e =>
               this.setState({ toolName: e.currentTarget.value })} />
-          <SaveBtn onClick={this.save} status={SpecialStatus.DIRTY} />
+          <SaveBtn
+            onClick={this.save}
+            disabled={!this.state.toolName || alreadyAdded}
+            status={SpecialStatus.DIRTY} />
+          <p className="name-error">
+            {alreadyAdded ? t("Already added.") : ""}
+          </p>
         </div>
         <this.AddStockTools />
       </DesignerPanelContent>

frontend/farm_designer/tools/edit_tool.tsx

@@ -9,6 +9,7 @@ import { getPathArray } from "../../history";
 import { TaggedTool, SpecialStatus, TaggedToolSlotPointer } from "farmbot";
 import {
   maybeFindToolById, getDeviceAccountSettings, selectAllToolSlotPointers,
+  selectAllTools,
 } from "../../resources/selectors";
 import { SaveBtn } from "../../ui";
 import { edit, destroy } from "../../api/crud";
@@ -17,6 +18,7 @@ import { Panel } from "../panel_header";
 import { ToolSVG } from "../map/layers/tool_slots/tool_graphics";
 import { error } from "../../toast/toast";
 import { EditToolProps, EditToolState } from "./interfaces";
+import { betterCompact } from "../../util";
 
 export const isActive = (toolSlots: TaggedToolSlotPointer[]) =>
   (toolId: number | undefined) =>
@@ -29,6 +31,8 @@ export const mapStateToProps = (props: Everything): EditToolProps => ({
   mountedToolId: getDeviceAccountSettings(props.resources.index)
     .body.mounted_tool_id,
   isActive: isActive(selectAllToolSlotPointers(props.resources.index)),
+  existingToolNames: betterCompact(selectAllTools(props.resources.index)
+    .map(tool => tool.body.name)),
 });
 
 export class RawEditTool extends React.Component<EditToolProps, EditToolState> {
@@ -53,18 +57,26 @@ export class RawEditTool extends React.Component<EditToolProps, EditToolState> {
       ? t("Cannot delete while mounted.")
       : t("Cannot delete while in a slot.");
     const activeOrMounted = this.props.isActive(tool.body.id) || isMounted;
+    const nameTaken = this.props.existingToolNames
+      .filter(x => x != tool.body.name).includes(this.state.toolName);
     return <this.PanelWrapper>
-      <ToolSVG toolName={this.state.toolName} />
-      <label>{t("Name")}</label>
-      <input name="name"
-        value={toolName}
-        onChange={e => this.setState({ toolName: e.currentTarget.value })} />
-      <SaveBtn
-        onClick={() => {
-          dispatch(edit(tool, { name: toolName }));
-          history.push("/app/designer/tools");
-        }}
-        status={SpecialStatus.DIRTY} />
+      <div className="edit-tool">
+        <ToolSVG toolName={this.state.toolName} />
+        <label>{t("Name")}</label>
+        <input name="name"
+          value={toolName}
+          onChange={e => this.setState({ toolName: e.currentTarget.value })} />
+        <SaveBtn
+          onClick={() => {
+            this.props.dispatch(edit(tool, { name: toolName }));
+            history.push("/app/designer/tools");
+          }}
+          disabled={!this.state.toolName || nameTaken}
+          status={SpecialStatus.DIRTY} />
+        <p className="name-error">
+          {nameTaken ? t("Name already taken.") : ""}
+        </p>
+      </div>
       <button
         className={`fb-button red no-float ${activeOrMounted
           ? "pseudo-disabled" : ""}`}

frontend/farm_designer/tools/index.tsx

@@ -143,6 +143,7 @@ export class RawTools extends React.Component<ToolsProps, ToolsState> {
         .filter(tool => !tool.body.name ||
           tool.body.name && tool.body.name.toLowerCase()
             .includes(this.state.searchTerm.toLowerCase()))
+        .filter(tool => tool.body.id)
         .map(tool =>
           <ToolInventoryItem key={tool.uuid}
             toolId={tool.body.id}

frontend/farm_designer/tools/interfaces.ts

@@ -19,6 +19,7 @@ export interface AddToolProps {
 export interface AddToolState {
   toolName: string;
   toAdd: string[];
+  uuid: UUID | undefined;
 }
 
 export interface EditToolProps {
@@ -26,6 +27,7 @@ export interface EditToolProps {
   dispatch: Function;
   mountedToolId: number | undefined;
   isActive(id: number | undefined): boolean;
+  existingToolNames: string[];
 }
 
 export interface EditToolState {

frontend/ui/save_button.tsx

@@ -7,6 +7,7 @@ interface SaveBtnProps {
   onClick?: (e: React.MouseEvent<{}>) => void;
   status: SpecialStatus;
   dirtyText?: string;
+  disabled?: boolean;
   /** Optional alternative to "SAVING" */
   savingText?: string;
   /** Optional alternative to "SAVED" */
@@ -31,13 +32,14 @@ export function SaveBtn(props: SaveBtnProps) {
     [SpecialStatus.SAVING]: props.savingText || t("Saving")
   };
 
-  const { savedText, onClick, hidden } = props;
+  const { savedText, onClick, hidden, disabled } = props;
   const statusClass = STATUS_TRANSLATION[props.status || ""] || "is-saved";
   const klass = `${props.color || "green"} ${statusClass} save-btn fb-button`;
   const spinnerEl = (props.status === SpecialStatus.SAVING) ?
     spinner : "";
 
   return <button onClick={onClick} title={t("save")}
+    disabled={disabled}
     hidden={!!hidden} className={klass}>
     {CAPTIONS[props.status] || (savedText || t("Saved") + " ✔")} {spinnerEl}
   </button>;