Abstract LabelPriority away into methods on Label model

3 files changed, 79 insertions, 9 deletions

diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb
index f453b70fb5d..42fd09e9b7e 100644
--- a/app/controllers/projects/labels_controller.rb
+++ b/app/controllers/projects/labels_controller.rb
@@ -84,7 +84,7 @@ class Projects::LabelsController < Projects::ApplicationController
     respond_to do |format|
       label = @available_labels.find(params[:id])
 
-      if label.priorities.where(project_id: project).delete_all
+      if label.unprioritize!(project)
         format.json { render json: label }
       else
         format.json { head :unprocessable_entity }
@@ -94,14 +94,12 @@ class Projects::LabelsController < Projects::ApplicationController
 
   def set_priorities
     Label.transaction do
-      label_ids = @available_labels.where(id: params[:label_ids]).pluck(:id)
+      available_labels_ids = @available_labels.where(id: params[:label_ids]).pluck(:id)
+      label_ids = params[:label_ids].select { |id| available_labels_ids.include?(id.to_i) }
 
-      params[:label_ids].each_with_index do |label_id, index|
-        next unless label_ids.include?(label_id.to_i)
-
-        label_priority = LabelPriority.find_or_initialize_by(project_id: @project.id, label_id: label_id)
-        label_priority.priority = index
-        label_priority.save!
+      label_ids.each_with_index do |label_id, index|
+        label = @available_labels.find(label_id)
+        label.prioritize!(project, index)
       end
     end
 
diff --git a/app/models/label.rb b/app/models/label.rb
index 6fd45d251a8..ae07e8f60e1 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -97,6 +97,20 @@ class Label < ActiveRecord::Base
     merge_requests_count(user, project_id: project.try(:id) || project_id, state: 'opened')
   end
 
+  def prioritize!(project, value)
+    label_priority = priorities.find_or_initialize_by(project_id: project.id)
+    label_priority.priority = value
+    label_priority.save!
+  end
+
+  def unprioritize!(project)
+    priorities.where(project: project).delete_all
+  end
+
+  def priority(project)
+    priorities.find_by(project: project).try(:priority)
+  end
+
   def template?
     template
   end
@@ -135,7 +149,7 @@ class Label < ActiveRecord::Base
 
   def as_json(options = {})
     super(options).tap do |json|
-      json[:priority] = priorities.find_by(project: options[:project]).try(:priority) if options.has_key?(:project)
+      json[:priority] = priority(options[:project]) if options.has_key?(:project)
     end
   end
 
diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb
index 4af0fb6afa9..0c163659a71 100644
--- a/spec/models/label_spec.rb
+++ b/spec/models/label_spec.rb
@@ -46,4 +46,62 @@ describe Label, models: true do
       expect(label.title).to eq('foo & bar?')
     end
   end
+
+  describe 'priorization' do
+    subject(:label) { create(:label) }
+
+    let(:project) { label.project }
+
+    describe '#prioritize!' do
+      context 'when label is not prioritized' do
+        it 'creates a label priority' do
+          expect { label.prioritize!(project, 1) }.to change(label.priorities, :count).by(1)
+        end
+
+        it 'sets label priority' do
+          label.prioritize!(project, 1)
+
+          expect(label.priorities.first.priority).to eq 1
+        end
+      end
+
+      context 'when label is prioritized' do
+        let!(:priority) { create(:label_priority, project: project, label: label, priority: 0) }
+
+        it 'does not create a label priority' do
+          expect { label.prioritize!(project, 1) }.not_to change(label.priorities, :count)
+        end
+
+        it 'updates label priority' do
+          label.prioritize!(project, 1)
+
+          expect(priority.reload.priority).to eq 1
+        end
+      end
+    end
+
+    describe '#unprioritize!' do
+      it 'removes label priority' do
+        create(:label_priority, project: project, label: label, priority: 0)
+
+        expect { label.unprioritize!(project) }.to change(label.priorities, :count).by(-1)
+      end
+    end
+
+    describe '#priority' do
+      context 'when label is not prioritized' do
+        it 'returns nil' do
+          expect(label.priority(project)).to be_nil
+        end
+      end
+
+      context 'when label is prioritized' do
+        it 'returns label priority' do
+          create(:label_priority, project: project, label: label, priority: 1)
+
+          expect(label.priority(project)).to eq 1
+        end
+      end
+    end
+  end
 end