Sequence tool updates (#545)

interim_email JWT claim due to FBOS v6 upgrade (temporary)
Stop users from removing tools from slots when in use
Stop users from deleting slots in use by tools in use by sequences
Remove inactive tools from sequence selection list in the editor.

app/controllers/api/abstract_controller.rb

@@ -14,6 +14,7 @@ module Api
     skip_before_action :verify_authenticity_token
     after_action :skip_set_cookies_header
 
+    rescue_from(User::BadSub) { sorry "Please log out and try again.", 422 }
     rescue_from(User::AlreadyVerified) { sorry "Already verified.", 409 }
 
     rescue_from(JWT::VerificationError) { |e| auth_err }

app/models/celery_script_settings_bag.rb

@@ -45,6 +45,7 @@ module CeleryScriptSettingsBag
   BAD_AXIS              = '"%s" is not a valid axis. Allowed values: %s'
   BAD_POINTER_ID        = "Bad point ID: %s"
   BAD_POINTER_TYPE      = '"%s" is not a type of point. Allowed values: %s'
+  BAD_SPEED             = "Speed must be a percentage between 1-100"
 
   Corpus = CeleryScript::Corpus
       .new
@@ -114,7 +115,9 @@ module CeleryScriptSettingsBag
       .defineArg(:y,               [Integer])
       .defineArg(:z,               [Integer])
       .defineArg(:radius,          [Integer])
-      .defineArg(:speed,           [Integer])
+      .defineArg(:speed,           [Integer]) do |node|
+        node.invalidate!(BAD_SPEED) unless node.value.between?(1, 100)
+      end
       .defineArg(:pin_number,      [Integer])
       .defineArg(:pin_value,       [Integer])
       .defineArg(:milliseconds,    [Integer])

app/mutations/points/destroy.rb

@@ -11,7 +11,6 @@ module Points
     STILL_IN_USE  = "Can't delete point because the following sequences "\
                     "are still using it: %s"
 
-    "Sequence 'foo' still needs to use 'bar' named 'baz'"
     required do
       model :device, class: Device
       array :points,  class: Point
@@ -36,22 +35,28 @@ private
     end
 
     def still_in_use
-      # What do we need to check?
-      #   * Point in use?
-      #   * Tool in use?
       @still_in_use ||= calculate_deps
     end
 
+    def all_deps
+      @all_deps ||= SequenceDependency.where(ALL_SEQ_DEPS, device.id)
+    end
+
     # point => tool_slot => tool
     def calculate_deps
-      SequenceDependency
-        .where(ALL_SEQ_DEPS, device.id)
-        .where(dependency_type: "Point")
-        .where(dependency_id: points.pluck(:id))
+      all_deps
+        .where(dependency_type: "Point", dependency_id: points.pluck(:id)) # NEXT: Add an "OR" for tracking tools
+        .or(refactor_plz)
         .map(&:sequence)
     end
 
-    def points_in_use
+    def refactor_plz
+      all_deps.where(dependency_type: "Tool",
+                     dependency_id: points
+                                      .select { |p| p.pointer_type == "ToolSlot" }
+                                      .map(&:pointer)
+                                      .map(&:tool)
+                                      .map(&:id))
     end
   end
 end

app/mutations/points/tool_removal_check.rb

@@ -0,0 +1,56 @@
+module Points
+  # Logic for tool removal was complicated enough to warrant its own
+  # mutation object.
+  class ToolRemovalCheck < Mutations::Command
+    IN_USE = "Tool in use by the following sequences: %s"
+
+    required do
+      model   :point,  class: Point
+      # Does nil `next_tool_id` mean "I have nothing to say"?
+      # Or does it mean: "I want to clear something out"?
+      boolean :attempting_change
+      integer :next_tool_id, nils: true, empty_is_nil: true
+    end
+
+    def validate
+      nope! if still_in_use?
+    end
+
+    def execute
+      true
+    end
+
+  private
+
+    def is_removal_attempt
+      (attempting_change &&       # Wants to make a change
+       (next_tool_id === nil) &&  # Wants to remove tool_id
+       point.pointer.tool)        # Currently has a tool_id
+    end
+
+    def still_in_use?
+      is_removal_attempt && deps.any?
+    end
+
+    def nope!
+      names = Sequence
+        .where(id: deps.pluck(:sequence_id))
+        .pluck(:name)
+        .join(", ")
+      add_error :in_use, :in_use, (IN_USE % [names])
+    end
+
+    def is_tool_slot?
+      point.pointer_type == "ToolSlot"
+    end
+
+    def current_tool_id
+      point.pointer.tool && point.pointer.tool.id
+    end
+
+    def deps
+      @deps ||= SequenceDependency.where(dependency_type: "Tool",
+                                         dependency_id:   current_tool_id)
+    end
+  end
+end

app/mutations/points/update.rb

@@ -22,6 +22,7 @@ module Points
                         .tools
                         .pluck(:id)
                         .include?(tool_id))
+      prevent_removal_of_in_use_tools
     end
 
     def execute
@@ -41,5 +42,20 @@ module Points
       p = point.pointer
       p && p.assign_attributes(inputs.slice(:tool_id, :openfarm_slug))
     end
+
+    def has_new_tool_id?
+      raw_inputs.key?("tool_id")
+    end
+
+    def prevent_removal_of_in_use_tools
+      results = Points::ToolRemovalCheck.run(point:             point,
+                                             attempting_change: has_new_tool_id?,
+                                             next_tool_id:      tool_id)
+      ok      = results.success?
+      results
+        .errors
+        .values
+        .map { |e| add_error e.symbolic, e.symbolic, e.message } unless ok
+    end
   end
 end

app/serializers/farm_event_serializer.rb

@@ -2,14 +2,13 @@ class FarmEventSerializer < ActiveModel::Serializer
   class BadExe < StandardError; end
   attributes :id, :start_time, :end_time, :repeat, :time_unit,
              :executable_id, :executable_type, :calendar
-
+  BAD_EXE = "Dont know how to calendarize %s"
   def calendar
     case object.executable
       when Sequence then sequence_calendar
       when Regimen  then []
       else
-        msg = "Dont know how to calendarize #{object.executable.class}"
-        throw BadExe.new(msg)
+        throw BadExe.new(BAD_EXE % object.executable.class)
     end
   end
 

db/seeds.rb

@@ -75,7 +75,7 @@ unless Rails.env == "production"
     s = Sequences::Create.run!(device: u.device,
     name: "Goto 0, 0, 0",
     body: [{kind:"move_absolute",args:{location:{kind:"coordinate", args:{x:0,
-    y:0, z:0}}, offset:{kind:"coordinate", args:{x:0, y:0, z:0}}, speed:800}}])
+    y:0, z:0}}, offset:{kind:"coordinate", args:{x:0, y:0, z:0}}, speed:100}}])
     t  = Tools::Create.run!(name: "Trench Digging Tool", device: u.device)
     body_txt = File.read("spec/lib/celery_script/ast_fixture4.json")
                    .gsub("__SEQUENCE_ID__", s.id.to_s)

spec/controllers/api/points/destroy_spec.rb

@@ -8,7 +8,7 @@ describe Api::PointsController do
       "args":{
           "location":{ "kind":"tool", "args":{ "tool_id": --- } },
           "offset":{ "kind":"coordinate", "args":{ "x":0, "y":0, "z":0 } },
-          "speed":800}}]
+          "speed":100}}]
     HEREDOC
     let(:device) { FactoryBot.create(:device) }
     let(:user) { FactoryBot.create(:user, device: device) }

spec/lib/celery_script/ast_has_points.json

@@ -21,7 +21,7 @@
             "z": 0
           }
         },
-        "speed": 800
+        "speed": 100
       }
     }
   ],

spec/lib/celery_script/point_spec.rb

@@ -15,7 +15,7 @@ describe "Celery Script `point` node" do
                       pointer_id:   plant.id }
             },
             offset:{ kind:"coordinate", args:{ x: 0, y: 0, z: 0} },
-            speed:800
+            speed: 100
           }
         }
       ]

spec/mutations/points/destroy_spec.rb

@@ -0,0 +1,12 @@
+require 'spec_helper'
+require_relative "scenario"
+
+describe Points::Destroy do
+  it "prevents deletion of active tool slots" do
+    s      = Points::Scenario.new
+    result = Points::Destroy.run(points: [s.tool_slot], device: s.device)
+    expect(result.success?).to be(false)
+    expect(result.errors.message_list)
+      .to include(Points::Destroy::STILL_IN_USE % s.sequence.name)
+  end
+end

spec/mutations/points/scenario.rb

@@ -0,0 +1,37 @@
+require 'ostruct'
+
+module Points
+  # A scenario complicated (and common) enough to warrant its own wrapper class.
+  # This will create a new Device, Tool, ToolSlot and Sequence which are
+  # interdependent.
+  # Good tool for testing dependency tracking.
+  class Scenario < OpenStruct
+    def  body
+       [ { kind: "move_absolute",
+           args: {
+              location: { kind: "tool", args: { tool_id: self.tool.id } },
+              offset:   { kind: "coordinate", args: { x: 0, y: 0, z: 0 } },
+              speed:    100
+           } } ]
+    end
+
+    def initialize(hash = nil)
+      super(hash)
+      self.device    = FactoryBot.create(:device)
+      self.tool      = Tools::Create.run!(device: self.device,
+                                          name: "Scenario Tool")
+      self.tool_slot = ToolSlots::Create.run!(device: self.device,
+                                              name:   "Scenario Tool Slot",
+                                              x:      0,
+                                              y:      0,
+                                              z:      0)
+      self.sequence  = Sequences::Create.run!(device: self.device,
+                                              body:   self.body,
+                                              name:   "Scenario Sequence")
+      Points::Update.run!(device:  self.device,
+                          point:   self.tool_slot,
+                          tool_id: self.tool.id)
+      self.tool_slot.reload
+    end
+  end
+end

spec/mutations/points/update_spec.rb

@@ -0,0 +1,14 @@
+require 'spec_helper'
+require_relative "scenario"
+
+describe Points::Update do
+  it "prevents remove of tool from actively used tool slots" do
+    s      = Points::Scenario.new
+    result = Points::Update.run(device:  s.device,
+                                point:   s.tool_slot,
+                                tool_id: nil)
+    expect(result.success?).to be(false)
+    expect(result.errors.message_list)
+      .to include(Points::ToolRemovalCheck::IN_USE % s.sequence.name)
+  end
+end