Commit d07b179b authored by Jan Provaznik's avatar Jan Provaznik

Minor fixes

parent 76607cfa
...@@ -2,9 +2,7 @@ ...@@ -2,9 +2,7 @@
# == LabelEventable concern # == LabelEventable concern
# #
# Contains functionality related to objects that support adding/removing events. # Contains functionality related to objects that support adding/removing labels.
#
# Used by Issue and MergeRequest.
# #
module LabelEventable module LabelEventable
......
...@@ -3,8 +3,6 @@ ...@@ -3,8 +3,6 @@
class ResourceLabelEvent < ActiveRecord::Base class ResourceLabelEvent < ActiveRecord::Base
prepend EE::ResourceLabelEvent prepend EE::ResourceLabelEvent
ISSUABLE_COLUMNS = %i(issue_id merge_request_id).freeze
belongs_to :user belongs_to :user
belongs_to :issue belongs_to :issue
belongs_to :merge_request belongs_to :merge_request
...@@ -12,28 +10,26 @@ class ResourceLabelEvent < ActiveRecord::Base ...@@ -12,28 +10,26 @@ class ResourceLabelEvent < ActiveRecord::Base
validates :user, presence: true, on: :create validates :user, presence: true, on: :create
validates :label, presence: true, on: :create validates :label, presence: true, on: :create
validate :issuable_id_is_present validate :exactly_one_issuable
enum action: { enum action: {
add: 1, add: 1,
remove: 2 remove: 2
} }
def self.issuable_columns
%i(issue_id merge_request_id).freeze
end
def issuable def issuable
issue || merge_request issue || merge_request
end end
private private
def issuable_columns def exactly_one_issuable
ISSUABLE_COLUMNS if self.class.issuable_columns.count { |attr| self[attr] } != 1
end errors.add(:base, "Exactly one of #{self.class.issuable_columns.join(', ')} is required")
def issuable_id_is_present
ids = issuable_columns.find_all {|attr| self[attr]}
if ids.size != 1
errors.add(:base, "Exactly one of #{issuable_columns.join(', ')} is required")
end end
end end
end end
# frozen_string_literal: true # frozen_string_literal: true
module ResourceLabelEventService module ResourceLabelEventService
prepend EE::ResourceLabelEventService
extend self extend self
def change_labels(resource, user, added_labels, removed_labels) def change_labels(resource, user, added_labels, removed_labels)
...@@ -23,9 +25,10 @@ module ResourceLabelEventService ...@@ -23,9 +25,10 @@ module ResourceLabelEventService
private private
def resource_column(resource) def resource_column(resource)
if resource.is_a?(Issue) case resource
when Issue
:issue_id :issue_id
elsif resource.is_a?(MergeRequest) when MergeRequest
:merge_request_id :merge_request_id
else else
raise ArgumentError, "Unknown resource type #{resource.class.name}" raise ArgumentError, "Unknown resource type #{resource.class.name}"
......
---
title: Add new model for tracking label events.
merge_request:
author:
type: added
...@@ -5,16 +5,19 @@ module EE ...@@ -5,16 +5,19 @@ module EE
extend ActiveSupport::Concern extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
EE_ISSUABLE_COLUMNS = %i(epic_id).freeze prepended do
belongs_to :epic
end
class_methods do
def issuable_columns
%i(epic_id).freeze + super
end
end
override :issuable override :issuable
def issuable def issuable
epic || super epic || super
end end
override :issuable_columns
def issuable_columns
EE_ISSUABLE_COLUMNS + super
end
end end
end end
# frozen_string_literal: true
module EE
module ResourceLabelEventService
extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override
override :resource_column
def resource_column(resource)
resource.is_a?(Epic) ? :epic_id : super
end
end
end
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe ResourceLabelEvent, type: :model do
subject { build(:resource_label_event) }
let(:epic) { create(:epic) }
describe 'validations' do
describe 'Issuable validation' do
it 'is valid if only epic_id is set' do
subject.attributes = { epic: epic, issue: nil, merge_request: nil }
expect(subject).to be_valid
end
end
end
end
...@@ -11,12 +11,43 @@ describe ResourceLabelEventService do ...@@ -11,12 +11,43 @@ describe ResourceLabelEventService do
subject { described_class.change_labels(resource, author, added, removed) } subject { described_class.change_labels(resource, author, added, removed) }
let(:labels) { create_list(:label, 2, project: project) } let(:labels) { create_list(:label, 2, project: project) }
let(:added) { [labels[0]] }
let(:removed) { [labels[1]] }
it 'creates an event for each label in single query' do def expect_label_event(event, label, action)
expect(Gitlab::Database).to receive(:bulk_insert).once.and_call_original expect(event.user).to eq(author)
expect { subject }.to change { resource.resource_label_events.count }.from(0).to(2) expect(event.label).to eq(label)
expect(event.action).to eq(action)
end
context 'when adding a label' do
let(:added) { [labels[0]] }
let(:removed) { [] }
it 'creates new label event' do
expect { subject }.to change { resource.resource_label_events.count }.from(0).to(1)
expect_label_event(resource.resource_label_events.first, labels[0], 'add')
end
end
context 'when removing a label' do
let(:added) { [] }
let(:removed) { [labels[1]] }
it 'creates new label event' do
expect { subject }.to change { resource.resource_label_events.count }.from(0).to(1)
expect_label_event(resource.resource_label_events.first, labels[1], 'remove')
end
end
context 'when both adding and removing labels' do
let(:added) { [labels[0]] }
let(:removed) { [labels[1]] }
it 'creates all label events in a single query' do
expect(Gitlab::Database).to receive(:bulk_insert).once.and_call_original
expect { subject }.to change { resource.resource_label_events.count }.from(0).to(2)
end
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment