Commit cb8391c5 authored by Christian Couder's avatar Christian Couder Committed by Ash McKenzie

Refactor IssuableBaseService label processing

In issuable_base_service.rb the code processing labels is making
the update() method not as clear as it could be. Let's refactor
this code into a new assign_requested_labels() method.

While at it, lets rename labels_changing? into ids_changing?
as the method is not specific to labels and could be used with
other arrays of ids.
parent e4aec614
...@@ -190,11 +190,7 @@ class IssuableBaseService < BaseService ...@@ -190,11 +190,7 @@ class IssuableBaseService < BaseService
change_additional_attributes(issuable) change_additional_attributes(issuable)
old_associations = associations_before_update(issuable) old_associations = associations_before_update(issuable)
label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) assign_requested_labels(issuable)
if labels_changing?(issuable.label_ids, label_ids)
params[:label_ids] = label_ids
issuable.touch
end
if issuable.changed? || params.present? if issuable.changed? || params.present?
issuable.assign_attributes(params) issuable.assign_attributes(params)
...@@ -297,10 +293,6 @@ class IssuableBaseService < BaseService ...@@ -297,10 +293,6 @@ class IssuableBaseService < BaseService
update_task(issuable) update_task(issuable)
end end
def labels_changing?(old_label_ids, new_label_ids)
old_label_ids.sort != new_label_ids.sort
end
def has_title_or_description_changed?(issuable) def has_title_or_description_changed?(issuable)
issuable.title_changed? || issuable.description_changed? issuable.title_changed? || issuable.description_changed?
end end
...@@ -349,6 +341,20 @@ class IssuableBaseService < BaseService ...@@ -349,6 +341,20 @@ class IssuableBaseService < BaseService
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def assign_requested_labels(issuable)
label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids)
return unless ids_changing?(issuable.label_ids, label_ids)
params[:label_ids] = label_ids
issuable.touch
end
# Arrays of ids are used, but we should really use sets of ids, so
# let's have an helper to properly check if some ids are changing
def ids_changing?(old_array, new_array)
old_array.sort != new_array.sort
end
def toggle_award(issuable) def toggle_award(issuable)
award = params.delete(:emoji_award) award = params.delete(:emoji_award)
AwardEmojis::ToggleService.new(issuable, award, current_user).execute if award AwardEmojis::ToggleService.new(issuable, award, current_user).execute if award
......
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