Code improvements

3 files changed, 13 insertions, 16 deletions

diff --git a/app/services/milestones/promote_service.rb b/app/services/milestones/promote_service.rb
index 0490122d14e..091c5c667d1 100644
--- a/app/services/milestones/promote_service.rb
+++ b/app/services/milestones/promote_service.rb
@@ -11,7 +11,7 @@ module Milestones
 
         group_milestone = clone_project_milestone(milestone)
 
-        move_issuables_to_group_milestone(group_milestone)
+        move_children_to_group_milestone(group_milestone)
 
         # Just to be safe
         unless group_milestone.valid?
@@ -27,11 +27,14 @@ module Milestones
     def milestone_ids_for_merge(group_milestone)
       # Pluck need to be used here instead of select so the array of ids
       # is persistent after old milestones gets deleted.
-      @milestone_ids_for_merge ||=
-        Milestone.where(project_id: group_project_ids, title: group_milestone.title).pluck(:id)
+      @milestone_ids_for_merge ||= begin
+        search_params = { title: group_milestone.title, project_ids: group_project_ids, state: 'all' }
+        milestones = MilestonesFinder.new(search_params).execute
+        milestones.pluck(:id)
+      end
     end
 
-    def move_issuables_to_group_milestone(group_milestone)
+    def move_children_to_group_milestone(group_milestone)
       milestone_ids_for_merge(group_milestone).in_groups_of(100) do |milestone_ids|
         update_issuables(group_milestone, milestone_ids)
       end
@@ -59,13 +62,7 @@ module Milestones
     end
 
     def group
-      @group ||= begin
-        group = parent.group
-
-        raise_error('Project does not belong to a group.') unless group
-
-        group
-      end
+      @group ||= parent.group || raise_error('Project does not belong to a group.')
     end
 
     def destroy_old_milestones(group_milestone)
diff --git a/doc/user/project/milestones/index.md b/doc/user/project/milestones/index.md
index 5782e05ade6..83adbd8cce2 100644
--- a/doc/user/project/milestones/index.md
+++ b/doc/user/project/milestones/index.md
@@ -29,7 +29,7 @@ In addition to that you will be able to filter issues or merge requests by group
 
 ## Milestone promotion
 
-Project milestones can be promoted to group milestones if its project belongs to a group. When a milestone is promoted all other milestones across the group projects with the same title will be merged into it, which means all milestones children like issues, merge requests and boards will be moved into the new promoted milestone.
+Project milestones can be promoted to group milestones if its project belongs to a group. When a milestone is promoted all other milestones across the group projects with the same title will be merged into it, which means all milestone's children like issues, merge requests and boards will be moved into the new promoted milestone.
 The promote button can be found in the milestone view or milestones list.
 
 ## Special milestone filters
diff --git a/spec/services/milestones/promote_service_spec.rb b/spec/services/milestones/promote_service_spec.rb
index 5b69c4f58bb..9f2df6d6d19 100644
--- a/spec/services/milestones/promote_service_spec.rb
+++ b/spec/services/milestones/promote_service_spec.rb
@@ -15,7 +15,7 @@ describe Milestones::PromoteService do
 
     context 'validations' do
       it 'raises error if milestone does not belong to a project' do
-        allow_any_instance_of(Milestone).to receive(:project_milestone?).and_return(false)
+        allow(milestone).to receive(:project_milestone?).and_return(false)
 
         expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError)
       end
@@ -31,7 +31,7 @@ describe Milestones::PromoteService do
       it 'promotes project milestone to group milestone' do
         promoted_milestone = service.execute(milestone)
 
-        expect(promoted_milestone.group_milestone?).to be_truthy
+        expect(promoted_milestone).to be_group_milestone
       end
 
       it 'sets issuables with new promoted milestone' do
@@ -40,7 +40,7 @@ describe Milestones::PromoteService do
 
         promoted_milestone = service.execute(milestone)
 
-        expect(promoted_milestone.group_milestone?).to be_truthy
+        expect(promoted_milestone).to be_group_milestone
         expect(issue.reload.milestone).to eq(promoted_milestone)
         expect(merge_request.reload.milestone).to eq(promoted_milestone)
       end
@@ -53,7 +53,7 @@ describe Milestones::PromoteService do
       it 'deletes project milestones with the same title' do
         promoted_milestone = service.execute(milestone)
 
-        expect(promoted_milestone.group_milestone?).to be_truthy
+        expect(promoted_milestone).to be_group_milestone
         expect(promoted_milestone).to be_valid
         expect(Milestone.exists?(milestone.id)).to be_falsy
         expect(Milestone.exists?(milestone_2.id)).to be_falsy