Remove ArgsValidator: Services - Interactors Part 2

app/services/interactors/set_effort_status.rb

@@ -3,17 +3,17 @@ class SetEffortStatus
     # This class expects to be given an effort loaded with split_times: :splits, or in the alternative, it expects
     # :ordered_split_times and :lap_splits to be provided in tne options hash.
 
-    def self.perform(effort, options = {})
-      new(effort, options).perform
+    def self.perform(effort, **)
+      new(effort, **).perform
     end
 
-    def initialize(effort, options = {})
-      ArgsValidator.validate(subject: effort, subject_class: Effort, params: options,
-                             exclusive: [:ordered_split_times, :lap_splits, :times_container], class: self)
+    def initialize(effort, ordered_split_times: nil, lap_splits: nil, times_container: nil)
       @effort = effort
-      @ordered_split_times = options[:ordered_split_times] || effort.ordered_split_times.reject(&:destroyed?)
-      @lap_splits = options[:lap_splits] || effort.lap_splits
-      @times_container = options[:times_container] || SegmentTimesContainer.new(calc_model: :stats)
+      validate_setup
+
+      @ordered_split_times = ordered_split_times || effort.ordered_split_times.reject(&:destroyed?)
+      @lap_splits = lap_splits || effort.lap_splits
+      @times_container = times_container || SegmentTimesContainer.new(calc_model: :stats)
     end
 
     def perform
@@ -40,7 +40,10 @@ def set_split_time_status(split_time)
     end
 
     def set_effort_status
-      worst_split_time_status = ordered_split_times.map(&:data_status_numeric).push(Effort.data_statuses[:good]).compact.min
+      worst_split_time_status = ordered_split_times.map(&:data_status_numeric)
+                                                   .push(Effort.data_statuses[:good])
+                                                   .compact
+                                                   .min
       effort_status = times_without_start_time? ? ::Effort.data_statuses[:bad] : ::Effort.data_statuses[:good]
       effort.data_status = [worst_split_time_status, effort_status].min
     end
@@ -105,5 +108,10 @@ def changed_split_times
     def unconfirmed_split_times
       @unconfirmed_split_times ||= ordered_split_times.reject(&:confirmed?)
     end
+
+    def validate_setup
+      raise ArgumentError, "set_effort_status must include effort" unless effort
+      raise ArgumentError, "effort must be an Effort" unless effort.is_a?(Effort)
+    end
   end
 end

app/services/interactors/set_effort_stop.rb

@@ -1,14 +1,13 @@
 module Interactors
   class SetEffortStop
-    def self.perform(effort, options = {})
-      new(effort, options).perform
+    def self.perform(effort, **)
+      new(effort, **).perform
     end
 
-    def initialize(effort, options = {})
-      ArgsValidator.validate(subject: effort, subject_class: Effort, params: options, exclusive: [:stop_status, :split_time_id])
+    def initialize(effort, stop_status: nil, split_time_id: nil)
       @effort = effort
-      @stop_status = options[:stop_status].nil? ? true : options[:stop_status]
-      @split_time_id = options[:split_time_id]
+      @stop_status = stop_status.nil? || stop_status
+      @split_time_id = split_time_id
       validate_setup
     end
 
@@ -39,9 +38,11 @@ def resources
     end
 
     def validate_setup
-      if split_time_id && !found_split_time
-        raise ArgumentError, "split_time_id #{split_time_id} does not exist for #{effort}"
-      end
+      raise ArgumentError, "set_effort_stop must include effort" unless effort
+      raise ArgumentError, "effort must be an Effort" unless effort.is_a?(Effort)
+      return unless split_time_id && !found_split_time
+
+      raise ArgumentError, "split_time_id #{split_time_id} does not exist for #{effort}"
     end
   end
 end

app/services/interactors/shift_event_start_time.rb

@@ -3,26 +3,23 @@ class ShiftEventStartTime
     include Interactors::Errors
     include TimeFormats
 
-    def self.perform!(event, args)
-      new(event, args).perform!
+    def self.perform!(event, new_start_time:)
+      new(event, new_start_time: new_start_time).perform!
     end
 
-    def initialize(event, args)
-      ArgsValidator.validate(subject: event,
-                             subject_class: Event,
-                             params: args,
-                             required: [:new_start_time],
-                             exclusive: [:new_start_time],
-                             class: self.class)
+    def initialize(event, new_start_time:)
       @event = event
-      @new_start_time = args[:new_start_time].in_time_zone(event.home_time_zone)
+      @non_localized_new_start_time = new_start_time
+      validate_setup
+
+      @new_start_time = new_start_time.in_time_zone(event.home_time_zone)
       @old_start_time = event.scheduled_start_time_local
       @current_user = User.current
       @errors = []
     end
 
     def perform!
-      unless errors.present? || shift_seconds == 0
+      unless errors.present? || shift_seconds.zero?
         ActiveRecord::Base.transaction do
           update_event
           update_efforts
@@ -35,6 +32,8 @@ def perform!
 
     private
 
+    attr_reader :event, :non_localized_new_start_time, :new_start_time, :old_start_time, :current_user, :errors
+
     def update_event
       event.update!(scheduled_start_time: new_start_time)
     rescue ActiveRecord::ActiveRecordError => e
@@ -53,8 +52,6 @@ def update_split_times
       errors << active_record_error(e)
     end
 
-    attr_reader :event, :new_start_time, :old_start_time, :current_user, :errors
-
     def effort_query
       EffortQuery.shift_event_scheduled_times(event, shift_seconds)
     end
@@ -73,14 +70,24 @@ def shift_direction
 
     def response_message
       if errors.present?
-        "The start time for #{event.name} could not be shifted from #{flexible_format(old_start_time, new_start_time)} to #{flexible_format(new_start_time, old_start_time)}. "
-      elsif shift_seconds == 0
-        "The new start time for #{event.name} was #{flexible_format(new_start_time, old_start_time)}, unchanged from the old start time. "
+        "The start time for #{event.name} could not be shifted " \
+          "from #{flexible_format(old_start_time, new_start_time)} " \
+          "to #{flexible_format(new_start_time, old_start_time)}. "
+      elsif shift_seconds.zero?
+        "The new start time for #{event.name} was #{flexible_format(new_start_time, old_start_time)}, " \
+          "unchanged from the old start time. "
       else
-        "The start time for #{event.name} was shifted #{shift_direction} " +
-          "from #{flexible_format(old_start_time, new_start_time)} to #{flexible_format(new_start_time, old_start_time)}. " +
+        "The start time for #{event.name} was shifted #{shift_direction} " \
+          "from #{flexible_format(old_start_time, new_start_time)} " \
+          "to #{flexible_format(new_start_time, old_start_time)}. " \
           "All related scheduled start times and split times were shifted #{shift_direction} by the same amount."
       end
     end
+
+    def validate_setup
+      raise ArgumentError, "shift_event_start_time must include event" unless event
+      raise ArgumentError, "event must be an Event" unless event.is_a?(Event)
+      raise ArgumentError, "shift_event_start_time must include new_start_time" unless non_localized_new_start_time
+    end
   end
 end

app/services/interactors/start_efforts.rb

@@ -3,24 +3,22 @@ class StartEfforts
     include Interactors::Errors
     include ActionView::Helpers::TextHelper
 
-    def self.perform!(args)
-      new(args).perform!
+    def self.perform!(efforts:, start_time: nil)
+      new(efforts: efforts, start_time: start_time).perform!
     end
 
-    def initialize(args)
-      ArgsValidator.validate(params: args,
-                             required: [:efforts],
-                             exclusive: [:efforts, :start_time],
-                             class: self.class)
-      @efforts = args[:efforts]
-      @start_time = args[:start_time]
+    def initialize(efforts:, start_time: nil)
+      raise ArgumentError, "start_efforts must include efforts" unless efforts
+
+      @efforts = efforts
+      @start_time = start_time
       @errors = []
       @saved_split_times = []
       validate_setup
     end
 
     def perform!
-      unless errors.present?
+      if errors.blank?
         SplitTime.transaction do
           efforts.each(&method(:start_effort))
           raise ActiveRecord::Rollback if errors.present?
@@ -69,8 +67,20 @@ def time_zone
     end
 
     def response_message
-      started_time = converted_start_time ? I18n.l(converted_start_time, format: :datetime_input) : "the scheduled start #{pluralize(saved_split_times.size, 'time')}"
-      errors.present? ? "No efforts were started" : "Started #{pluralize(saved_split_times.size, 'effort')} at #{started_time}"
+      started_time = if converted_start_time
+                       I18n.l(converted_start_time,
+                              format: :datetime_input)
+                     else
+                       "the scheduled start #{pluralize(
+                         saved_split_times.size, 'time'
+                       )}"
+                     end
+      if errors.present?
+        "No efforts were started"
+      else
+        "Started #{pluralize(saved_split_times.size,
+                             'effort')} at #{started_time}"
+      end
     end
 
     def validate_setup
@@ -82,8 +92,8 @@ def validate_setup
       errors << multiple_event_groups_error(event_group_ids) if event_group_ids.uniq.many?
       errors << invalid_start_time_error(start_time) if invalid_start_time?
       errors << invalid_start_time_error(start_time || "nil") unless converted_start_time ||
-        efforts.all?(&:scheduled_start_time?) ||
-        efforts.all?(&:event)
+                                                                     efforts.all?(&:scheduled_start_time?) ||
+                                                                     efforts.all?(&:event)
     end
 
     def event_group_ids

app/services/interactors/submit_raw_time_rows.rb

@@ -2,25 +2,27 @@ module Interactors
   class SubmitRawTimeRows
     include Interactors::Errors
 
-    def self.perform!(args)
-      new(args).perform!
+    def self.perform!(raw_time_rows:, event_group:, force_submit:, mark_as_reviewed:, current_user_id: nil)
+      new(
+        raw_time_rows: raw_time_rows,
+        event_group: event_group,
+        force_submit: force_submit,
+        mark_as_reviewed: mark_as_reviewed,
+        current_user_id: current_user_id
+      ).perform!
     end
 
-    def initialize(args)
-      ArgsValidator.validate(params: args,
-                             required: [:raw_time_rows, :event_group, :force_submit, :mark_as_reviewed],
-                             exclusive: [:raw_time_rows, :event_group, :force_submit, :mark_as_reviewed,
-                                         :current_user_id],
-                             class: self.class)
-      @raw_time_rows = args[:raw_time_rows]
-      @event_group = args[:event_group]
-      @force_submit = args[:force_submit]
-      @mark_as_reviewed = args[:mark_as_reviewed]
-      @current_user_id = args[:current_user_id]
+    def initialize(raw_time_rows:, event_group:, force_submit:, mark_as_reviewed:, current_user_id: nil)
+      @raw_time_rows = raw_time_rows
+      @event_group = event_group
+      @force_submit = force_submit
+      @mark_as_reviewed = mark_as_reviewed
+      @current_user_id = current_user_id
       @times_container = SegmentTimesContainer.new(calc_model: :stats)
       @problem_rows = []
       @upserted_split_times = []
       @errors = []
+      validate_setup
     end
 
     def perform!
@@ -99,5 +101,12 @@ def bib_numbers
     def resources
       { problem_rows: problem_rows, upserted_split_times: upserted_split_times }
     end
+
+    def validate_setup
+      raise ArgumentError, "submit_raw_time_rows must include raw_time_rows" unless raw_time_rows
+      raise ArgumentError, "submit_raw_time_rows must include event_group" unless event_group
+      raise ArgumentError, "submit_raw_time_rows must include force_submit" if force_submit.nil?
+      raise ArgumentError, "submit_raw_time_rows must include mark_as_reviewed" if mark_as_reviewed.nil?
+    end
   end
 end

spec/services/interactors/set_effort_status_spec.rb

@@ -1,7 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Interactors::SetEffortStatus do
-  subject { Interactors::SetEffortStatus.new(effort, times_container: times_container) }
+  subject { described_class.new(effort, times_container: times_container) }
+
   let(:effort) { efforts(:hardrock_2014_finished_first) }
   let(:subject_split_times) { effort.ordered_split_times }
   let(:times_container) { SegmentTimesContainer.new(calc_model: :terrain) }
@@ -19,7 +20,7 @@
       let(:effort) { nil }
 
       it "raises an ArgumentError" do
-        expect { subject }.to raise_error(/must include a subject/)
+        expect { subject }.to raise_error(ArgumentError, /must include effort/)
       end
     end
   end
@@ -30,7 +31,7 @@
       subject_split_times.each { |st| st.update(data_status: nil) }
     end
 
-    context "for an effort that has not yet started" do
+    context "when the effort has not yet started" do
       let(:effort) { efforts(:hardrock_2014_not_started) }
 
       it "sets effort data_status to good and does not attempt to change split_times" do
@@ -73,7 +74,7 @@
 
       it 'sets data_status of all split_times correctly and sets effort to "questionable"' do
         subject.perform
-        expect(subject_split_times.map(&:data_status)).to eq(%w[good questionable] + ["good"] * 10)
+        expect(subject_split_times.map(&:data_status)).to eq(%w[good questionable] + (["good"] * 10))
         expect(effort.data_status).to eq("questionable")
       end
     end
@@ -82,12 +83,14 @@
       let(:split_time_1) { subject_split_times.second }
       let(:split_time_2) { subject_split_times.third }
 
-      before { split_time_1.update(absolute_time: split_time_1.absolute_time - 4.hours) }
-      before { split_time_2.update(absolute_time: split_time_2.absolute_time - 3.hours) }
+      before do
+        split_time_1.update(absolute_time: split_time_1.absolute_time - 4.hours)
+        split_time_2.update(absolute_time: split_time_2.absolute_time - 3.hours)
+      end
 
       it 'sets data_status of all split_times correctly and sets effort to "bad"' do
         subject.perform
-        expect(subject_split_times.map(&:data_status)).to eq(%w[good bad questionable] + ["good"] * 9)
+        expect(subject_split_times.map(&:data_status)).to eq(%w[good bad questionable] + (["good"] * 9))
         expect(effort.data_status).to eq("bad")
       end
     end
@@ -97,15 +100,17 @@
 
       it 'sets data_status of all subsequent split_times to "questionable"' do
         subject.perform
-        expect(subject_split_times.map(&:data_status)).to eq(%w[good good good] + ["questionable"] * 9)
+        expect(subject_split_times.map(&:data_status)).to eq(%w[good good good] + (["questionable"] * 9))
         expect(effort.data_status).to eq("questionable")
       end
     end
 
     context "when all split_times are confirmed" do
       let(:split_time) { subject_split_times.third }
-      before { split_time.update(absolute_time: split_time.absolute_time - 4.hours) } # Bad time
-      before { subject_split_times.each { |st| st.data_status = :confirmed } }
+      before do
+        split_time.update(absolute_time: split_time.absolute_time - 4.hours) # Bad time
+        subject_split_times.each { |st| st.data_status = :confirmed }
+      end
 
       it 'sets data_status of the effort to "good"' do
         subject.perform
@@ -120,20 +125,20 @@
         split_time.update(absolute_time: split_time.absolute_time - 4.hours) # Bad time
         effort.update(data_status: :good)
 
-        pre_set_statuses = %w[bad questionable good questionable good bad] + ["good"] * 6
+        pre_set_statuses = %w[bad questionable good questionable good bad] + (["good"] * 6)
         subject_split_times.zip(pre_set_statuses).each do |st, status|
           st.update(data_status: status)
         end
       end
 
       it "sets data_status of the split_times and effort correctly" do
         subject.perform
-        expect(subject_split_times.map(&:data_status)).to eq(%w[good good bad] + ["good"] * 9)
+        expect(subject_split_times.map(&:data_status)).to eq(%w[good good bad] + (["good"] * 9))
         expect(effort.data_status).to eq("bad")
       end
     end
 
-    context "for a multi-lap event" do
+    context "when the event is multi-lap" do
       let(:effort) { efforts(:rufa_2017_24h_finished_last) }
 
       context "when all times are good" do
@@ -150,7 +155,7 @@
 
         it "works as expected" do
           subject.perform
-          expect(subject_split_times.map(&:data_status)).to eq(["good"] * 4 + ["bad"] + ["good"])
+          expect(subject_split_times.map(&:data_status)).to eq((["good"] * 4) + ["bad"] + ["good"])
           expect(effort.data_status).to eq("bad")
         end
       end