Refactor job skill assignment a bit

app/controllers/api/v1/parties_controller.rb

@@ -20,36 +20,32 @@ class Api::V1::PartiesController < Api::V1::ApiController
     if @party.user != current_user
       render_unauthorized_response
     else
-      @party.attributes =
+      @party.attributes = party_params.except(:skill1_id, :skill2_id, :skill3_id)
-        party_params.except(:skill1_id, :skill2_id, :skill3_id)
+      ap party_params
-
       # Determine which incoming keys contain new skills
-      new_skill_keys =
+      skill_keys = %w[skill1_id skill2_id skill3_id]
-        party_params.keys - %i[skill0_id skill1_id skill2_id skill3_id]
+      if (party_params.keys & skill_keys).any?
+        new_skill_keys = party_params.keys - skill_keys
 
-      if !new_skill_keys.empty?
         # If there are new skills, merge them with the existing skills
-        existing_skills = [@party.skill1, @party.skill2, @party.skill3]
+        unless new_skill_keys.empty?
-        new_skill_ids = new_skill_keys.map { |key| party_params[key] }
+          existing_skills = [@party.skill1, @party.skill2, @party.skill3]
-        positions = extract_positions_from_keys(new_skill_keys)
+          new_skill_ids = new_skill_keys.map { |key| party_params[key] }
+          positions = extract_positions_from_keys(new_skill_keys)
 
-        new_skills =
+          new_skills = merge_skills_with_existing_skills(existing_skills, new_skill_ids, positions)
-          merge_skills_with_existing_skills(
-            existing_skills,
-            new_skill_ids,
-            positions,
-          )
 
-        new_skill_ids = {}
+          new_skill_ids = {}
-        new_skills.each_with_index do |skill, index|
+          new_skills.each_with_index do |skill, index|
-          new_skill_ids["skill#{index + 1}_id"] = skill.id
+            new_skill_ids["skill#{index + 1}_id"] = skill.id
+          end
+
+          @party.attributes = new_skill_ids
         end
-
-        @party.attributes = new_skill_ids
       end
-
-      render :update, status: :ok if @party.save!
     end
+
+    render :update, status: :ok if @party.save!
   end
 
   def index
@@ -79,9 +75,9 @@ class Api::V1::PartiesController < Api::V1::ApiController
         .order(created_at: :desc)
         .paginate(page: request.params[:page], per_page: @per_page)
         .each do |party|
-          party.favorited =
+        party.favorited =
-            (current_user) ? party.is_favorited(current_user) : false
+          current_user ? party.is_favorited(current_user) : false
-        end
+      end
     @count = Party.where(conditions).count
 
     render :all, status: :ok
@@ -160,26 +156,49 @@ class Api::V1::PartiesController < Api::V1::ApiController
       progress = place_skill_in_existing_skills(progress, skill, positions[0])
     end
 
-    return progress
+    progress
   end
 
   def place_skill_in_existing_skills(existing_skills, skill, position)
     old_position = existing_skills.index { |x| x.id == skill.id }
 
     if old_position
-      # Check desired position for a skill
+      existing_skills = swap_skills_at_position(existing_skills, skill, position, old_position)
-      displaced_skill = existing_skills[position] if existing_skills[
-        position
-      ].present?
-
-      # Put skill in new position
-      existing_skills[position] = skill
-      existing_skills[old_position] = displaced_skill
     else
+      # Test if skill will exceed allowances of skill types
+      skill_type = skill.sub ? 'sub' : 'emp'
+      unless can_add_skill_of_type(existing_skills, position, skill_type)
+        raise Api::V1::TooManySkillsOfTypeError.new(skill_type: skill_type)
+      end
+
       existing_skills[position] = skill
     end
 
-    return existing_skills
+    existing_skills
+  end
+
+  def swap_skills_at_position(skills, new_skill, position1, position2)
+    # Check desired position for a skill
+    displaced_skill = skills[position1] if skills[position1].present?
+
+    # Put skill in new position
+    skills[position1] = new_skill
+    skills[position2] = displaced_skill
+
+    skills
+  end
+
+  def can_add_skill_of_type(skills, position, type)
+    max_skill_of_type = 2
+
+    count = skills.reject
+                  .with_index { |_el, index| index == position }
+                  .reduce(0) do |sum, skill|
+      sum + 1 if type == 'emp' && skill.emp
+      sum + 1 if type == 'sub' && skill.sub
+    end
+
+    count + 1 <= max_skill_of_type
   end
 
   def extract_positions_from_keys(keys)
@@ -196,7 +215,7 @@ class Api::V1::PartiesController < Api::V1::ApiController
   def set_from_slug
     @party = Party.where("shortcode = ?", params[:id]).first
     @party.favorited =
-      (current_user && @party) ? @party.is_favorited(current_user) : false
+      current_user && @party ? @party.is_favorited(current_user) : false
   end
 
   def set
@@ -211,9 +230,10 @@ class Api::V1::PartiesController < Api::V1::ApiController
       :description,
       :raid_id,
       :job_id,
+      :skill0_id,
       :skill1_id,
       :skill2_id,
-      :skill3_id,
+      :skill3_id
     )
   end
 end