Merge branch 'security-2773-milestones-fix-ee' into 'master'

[master] Port of: Check issue milestone availability

See merge request gitlab/gitlab-ee!857

app/models/concerns/issuable.rb

@@ -75,6 +75,7 @@ module Issuable
 
     validates :author, presence: true
     validates :title, presence: true, length: { maximum: 255 }
+    validate :milestone_is_valid
 
     scope :authored, ->(user) { where(author_id: user) }
     scope :recent, -> { reorder(id: :desc) }
@@ -118,6 +119,18 @@ module Issuable
     def has_multiple_assignees?
       assignees.count > 1
     end
+
+    def milestone_available?
+      return if is_a?(Epic)
+
+      project_id == milestone&.project_id || project.ancestors_upto.compact.include?(milestone&.group)
+    end
+
+    private
+
+    def milestone_is_valid
+      errors.add(:milestone_id, message: "is invalid") if milestone_id.present? && !milestone_available?
+    end
   end
 
   class_methods do

app/models/merge_request.rb

@@ -191,6 +191,9 @@ class MergeRequest < ActiveRecord::Base
 
   after_save :keep_around_commit
 
+  alias_attribute :project, :target_project
+  alias_attribute :project_id, :target_project_id
+
   def self.reference_prefix
     '!'
   end
@@ -849,10 +852,6 @@ class MergeRequest < ActiveRecord::Base
     target_project != source_project
   end
 
-  def project
-    target_project
-  end
-
   # If the merge request closes any issues, save this information in the
   # `MergeRequestsClosingIssues` model. This is a performance optimization.
   # Calculating this information for a number of merge requests requires

app/services/issuable_base_service.rb

@@ -387,6 +387,12 @@ class IssuableBaseService < BaseService
   def parent
     project
   end
+
+  # we need to check this because milestone from milestone_id param is displayed on "new" page
+  # where private project milestone could leak without this check
+  def ensure_milestone_available(issuable)
+    issuable.milestone_id = nil unless issuable.milestone_available?
+  end
 end
 
 IssuableBaseService.prepend(EE::IssuableBaseService)

app/services/issues/build_service.rb

@@ -6,7 +6,9 @@ module Issues
 
     def execute
       filter_resolve_discussion_params
-      @issue = project.issues.new(issue_params)
+      @issue = project.issues.new(issue_params).tap do |issue|
+        ensure_milestone_available(issue)
+      end
     end
 
     def issue_params_with_info_from_discussions

app/services/merge_requests/build_service.rb

@@ -19,6 +19,7 @@ module MergeRequests
       merge_request.target_project  = find_target_project
       merge_request.target_branch   = find_target_branch
       merge_request.can_be_created  = projects_and_branches_valid?
+      ensure_milestone_available(merge_request)
 
       # compare branches only if branches are valid, otherwise
       # compare_branches may raise an error

changelogs/unreleased/51971-milestones-visibility.yml

+---
+title: Check if desired milestone for an issue is available
+merge_request:
+author:
+type: security

ee/spec/models/burndown_spec.rb

@@ -133,26 +133,26 @@ describe Burndown do
   end
 
   describe 'group milestone burndown' do
-    let(:group) { create(:group) }
-    let(:nested_group) { create(:group, parent: group) }
+    let(:parent_group) { create(:group) }
+    let(:group) { create(:group, parent: parent_group) }
+    let(:parent_group_project) { create(:project, group: parent_group) }
     let(:group_project) { create(:project, group: group) }
-    let(:nested_group_project) { create(:project, group: nested_group) }
-    let(:group_milestone) { create(:milestone, project: nil, group: group, start_date: start_date, due_date: due_date) }
-    let(:nested_group_milestone) { create(:milestone, group: nested_group, start_date: start_date, due_date: due_date) }
+    let(:parent_group_milestone) { create(:milestone, project: nil, group: parent_group, start_date: start_date, due_date: due_date) }
+    let(:group_milestone) { create(:milestone, group: group, start_date: start_date, due_date: due_date) }
 
     context 'when nested group milestone', :nested_groups do
       before do
-        group.add_developer(user)
+        parent_group.add_developer(user)
       end
 
       it_behaves_like 'burndown for milestone' do
-        let(:milestone) { nested_group_milestone }
-        let(:project) { nested_group_project }
+        let(:milestone) { group_milestone }
+        let(:project) { group_project }
         let(:issue_params) do
           {
             milestone: milestone,
             weight: 2,
-            project_id: nested_group_project.id,
+            project_id: group_project.id,
             author: user,
             created_at: milestone.start_date
           }

ee/spec/models/epic_spec.rb

 require 'spec_helper'
 
 describe Epic do
+  let(:group) { create(:group) }
+  let(:project) { create(:project, group: group) }
+
   describe 'associations' do
     subject { build(:epic) }
 
@@ -221,29 +224,32 @@ describe Epic do
     end
 
     context 'fixed date is not set' do
-      subject { create(:epic, start_date: nil, end_date: nil) }
+      subject { create(:epic, start_date: nil, end_date: nil, group: group) }
 
       let(:milestone1) do
         create(
           :milestone,
           start_date: Date.new(2000, 1, 1),
-          due_date: Date.new(2000, 1, 10)
+          due_date: Date.new(2000, 1, 10),
+          group: group
         )
       end
       let(:milestone2) do
         create(
           :milestone,
           start_date: Date.new(2000, 1, 3),
-          due_date: Date.new(2000, 1, 20)
+          due_date: Date.new(2000, 1, 20),
+          group: group
         )
       end
 
       context 'multiple milestones' do
         before do
-          epic_issue1 = create(:epic_issue, epic: subject)
-          epic_issue1.issue.update(milestone: milestone1)
-          epic_issue2 = create(:epic_issue, epic: subject)
-          epic_issue2.issue.update(milestone: milestone2)
+          issue1 = create(:issue, project: project, milestone: milestone1)
+          issue2 = create(:issue, project: project, milestone: milestone2)
+
+          create(:epic_issue, epic: subject, issue: issue1)
+          create(:epic_issue, epic: subject, issue: issue2)
         end
 
         context 'complete start and due dates' do
@@ -260,14 +266,16 @@ describe Epic do
             create(
               :milestone,
               start_date: Date.new(2000, 1, 1),
-              due_date: nil
+              due_date: nil,
+              group: group
             )
           end
           let(:milestone2) do
             create(
               :milestone,
               start_date: Date.new(2000, 1, 3),
-              due_date: nil
+              due_date: nil,
+              group: group
             )
           end
 
@@ -284,14 +292,16 @@ describe Epic do
             create(
               :milestone,
               start_date: nil,
-              due_date: nil
+              due_date: nil,
+              group: group
             )
           end
           let(:milestone2) do
             create(
               :milestone,
               start_date: nil,
-              due_date: nil
+              due_date: nil,
+              group: group
             )
           end
 
@@ -322,7 +332,7 @@ describe Epic do
       context 'single milestone' do
         before do
           epic_issue1 = create(:epic_issue, epic: subject)
-          epic_issue1.issue.update(milestone: milestone1)
+          epic_issue1.issue.update(milestone: milestone1, project: project)
         end
 
         context 'complete start and due dates' do
@@ -339,7 +349,8 @@ describe Epic do
             create(
               :milestone,
               start_date: Date.new(2000, 1, 1),
-              due_date: nil
+              due_date: nil,
+              group: group
             )
           end
 
@@ -356,7 +367,8 @@ describe Epic do
             create(
               :milestone,
               start_date: nil,
-              due_date: nil
+              due_date: nil,
+              group: group
             )
           end
 
@@ -373,12 +385,12 @@ describe Epic do
 
   describe '.update_start_and_due_dates' do
     def link_epic_to_milestone(epic, milestone)
-      create(:issue, epic: epic, milestone: milestone)
+      create(:issue, epic: epic, milestone: milestone, project: project)
     end
 
     it 'updates in bulk' do
-      milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10))
-      milestone2 = create(:milestone, due_date: Date.new(2000, 1, 30))
+      milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10), group: group)
+      milestone2 = create(:milestone, due_date: Date.new(2000, 1, 30), group: group)
 
       epics = [
         create(:epic),
@@ -414,8 +426,8 @@ describe Epic do
     end
 
     context 'query count check' do
-      let(:milestone) { create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10)) }
-      let!(:epics) { [create(:epic)] }
+      let(:milestone) { create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10), group: group) }
+      let!(:epics) { [create(:epic, group: group)] }
 
       def setup_control_group
         link_epic_to_milestone(epics[0], milestone)

ee/spec/services/ee/boards/issues/list_service_spec.rb

@@ -20,7 +20,8 @@ describe Boards::Issues::ListService, services: true do
     let(:p3) { create(:group_label, title: 'P3', group: group) }
 
     let(:user_list) { create(:user_list, board: board, position: 2) }
-    let(:milestone_list) { create(:milestone_list, board: board, position: 3) }
+    let(:milestone) { create(:milestone, group: group) }
+    let(:milestone_list) { create(:milestone_list, board: board, position: 3, milestone: milestone) }
     let(:backlog)   { create(:backlog_list, board: board) }
     let(:list1)     { create(:list, board: board, label: development, position: 0) }
     let(:list2)     { create(:list, board: board, label: testing, position: 1) }

ee/spec/services/ee/issuable/clone/attributes_rewriter_spec.rb

@@ -37,15 +37,6 @@ describe Issuable::Clone::AttributesRewriter do
 
         expect(new_epic.reload.milestone).to be_nil
       end
-
-      it 'copies the milestone when old issue milestone is a group milestone' do
-        milestone = create(:milestone, title: 'milestone', group: group)
-        original_issue.update(milestone: milestone)
-
-        subject.execute
-
-        expect(new_epic.reload.milestone).to eq(milestone)
-      end
     end
   end
 end

ee/spec/services/ee/issues/update_service_spec.rb

@@ -14,10 +14,10 @@ describe Issues::UpdateService do
 
     context 'refresh epic dates' do
       let(:epic) { create(:epic) }
-      let(:issue) { create(:issue, epic: epic) }
+      let(:issue) { create(:issue, epic: epic, project: project) }
 
       context 'updating milestone' do
-        let(:milestone) { create(:milestone) }
+        let(:milestone) { create(:milestone, project: project) }
 
         it 'calls epic#update_start_and_due_dates' do
           expect(epic).to receive(:update_start_and_due_dates).twice

ee/spec/services/epic_issues/list_service_spec.rb

@@ -55,9 +55,8 @@ describe EpicIssues::ListService do
         control_count = ActiveRecord::QueryRecorder.new { list_service.execute }.count
 
         new_group2 = create(:group, :private)
-        new_project2 = create(:project, namespace: new_group2)
         new_group2.add_developer(user)
-        milestone3 = create(:milestone, project: new_project2)
+        milestone3 = create(:milestone, project: new_project)
         new_issue4 = create(:issue, project: new_project, milestone: milestone3)
         create(:epic_issue, issue: new_issue4, epic: epic, relative_position: 6)
 

ee/spec/services/epics/issue_promote_service_spec.rb

@@ -64,7 +64,6 @@ describe Epics::IssuePromoteService do
             expect(epic.description).to eq(issue.description)
             expect(epic.author).to eq(user)
             expect(epic.group).to eq(group)
-            expect(epic.milestone).to eq(milestone)
           end
 
           it 'copies group labels assigned to the issue' do

ee/spec/services/milestones/update_service_spec.rb

@@ -9,7 +9,7 @@ describe Milestones::UpdateService do
         user = build(:user)
         milestone = create(:milestone, project: project)
         epic = create(:epic)
-        create(:issue, milestone: milestone, epic: epic)
+        create(:issue, milestone: milestone, epic: epic, project: project)
         due_date = 3.days.from_now.to_date
 
         described_class.new(project, user, { due_date: due_date }).execute(milestone)

spec/controllers/dashboard/milestones_controller_spec.rb

@@ -13,7 +13,7 @@ describe Dashboard::MilestonesController do
     )
   end
   let(:issue) { create(:issue, project: project, milestone: project_milestone) }
-  let(:group_issue) { create(:issue, milestone: group_milestone) }
+  let(:group_issue) { create(:issue, milestone: group_milestone, project: create(:project, group: group)) }
 
   let!(:label) { create(:label, project: project, title: 'Issue Label', issues: [issue]) }
   let!(:group_label) { create(:group_label, group: group, title: 'Group Issue Label', issues: [group_issue]) }

spec/features/issues_spec.rb

@@ -244,8 +244,8 @@ describe 'Issues' do
                          created_at: Time.now - (index * 60))
         end
       end
-      let(:newer_due_milestone) { create(:milestone, due_date: '2013-12-11') }
-      let(:later_due_milestone) { create(:milestone, due_date: '2013-12-12') }
+      let(:newer_due_milestone) { create(:milestone, project: project, due_date: '2013-12-11') }
+      let(:later_due_milestone) { create(:milestone, project: project, due_date: '2013-12-12') }
 
       it 'sorts by newest' do
         visit project_issues_path(project, sort: sort_value_created_date)

spec/features/merge_requests/user_lists_merge_requests_spec.rb

@@ -13,7 +13,7 @@ describe 'Merge requests > User lists merge requests' do
                   source_project: project,
                   source_branch: 'fix',
                   assignee: user,
-                  milestone: create(:milestone, due_date: '2013-12-11'),
+                  milestone: create(:milestone, project: project, due_date: '2013-12-11'),
                   created_at: 1.minute.ago,
                   updated_at: 1.minute.ago)
     create(:merge_request,
@@ -21,7 +21,7 @@ describe 'Merge requests > User lists merge requests' do
            source_project: project,
            source_branch: 'markdown',
            assignee: user,
-           milestone: create(:milestone, due_date: '2013-12-12'),
+           milestone: create(:milestone, project: project, due_date: '2013-12-12'),
            created_at: 2.minutes.ago,
            updated_at: 2.minutes.ago)
     create(:merge_request,

spec/models/concerns/issuable_spec.rb

@@ -32,17 +32,56 @@ describe Issuable do
   end
 
   describe "Validation" do
-    subject { build(:issue) }
+    context 'general validations' do
+      subject { build(:issue) }
 
-    before do
-      allow(InternalId).to receive(:generate_next).and_return(nil)
+      before do
+        allow(InternalId).to receive(:generate_next).and_return(nil)
+      end
+
+      it { is_expected.to validate_presence_of(:project) }
+      it { is_expected.to validate_presence_of(:iid) }
+      it { is_expected.to validate_presence_of(:author) }
+      it { is_expected.to validate_presence_of(:title) }
+      it { is_expected.to validate_length_of(:title).is_at_most(255) }
     end
 
-    it { is_expected.to validate_presence_of(:project) }
-    it { is_expected.to validate_presence_of(:iid) }
-    it { is_expected.to validate_presence_of(:author) }
-    it { is_expected.to validate_presence_of(:title) }
-    it { is_expected.to validate_length_of(:title).is_at_most(255) }
+    describe 'milestone' do
+      let(:project) { create(:project) }
+      let(:milestone_id) { create(:milestone, project: project).id }
+      let(:params) do
+        {
+          title: 'something',
+          project: project,
+          author: build(:user),
+          milestone_id: milestone_id
+        }
+      end
+
+      subject { issuable_class.new(params) }
+
+      context 'with correct params' do
+        it { is_expected.to be_valid }
+      end
+
+      context 'with empty string milestone' do
+        let(:milestone_id) { '' }
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'with nil milestone id' do
+        let(:milestone_id) { nil }
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'with a milestone id from another project' do
+        let(:milestone_id) { create(:milestone).id }
+
+        it { is_expected.to be_invalid }
+      end
+    end
   end
 
   describe "Scope" do
@@ -66,6 +105,48 @@ describe Issuable do
     end
   end
 
+  describe '#milestone_available?' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project, group: group) }
+    let(:issue) { create(:issue, project: project) }
+
+    def build_issuable(milestone_id)
+      issuable_class.new(project: project, milestone_id: milestone_id)
+    end
+
+    it 'returns true with a milestone from the issue project' do
+      milestone = create(:milestone, project: project)
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_truthy
+    end
+
+    it 'returns true with a milestone from the issue project group' do
+      milestone = create(:milestone, group: group)
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_truthy
+    end
+
+    it 'returns true with a milestone from the the parent of the issue project group', :nested_groups do
+      parent = create(:group)
+      group.update(parent: parent)
+      milestone = create(:milestone, group: parent)
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_truthy
+    end
+
+    it 'returns false with a milestone from another project' do
+      milestone = create(:milestone)
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_falsey
+    end
+
+    it 'returns false with a milestone from another group' do
+      milestone = create(:milestone, group: create(:group))
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_falsey
+    end
+  end
+
   describe ".search" do
     let!(:searchable_issue) { create(:issue, title: "Searchable awesome issue") }
     let!(:searchable_issue2) { create(:issue, title: 'Aw') }

spec/models/issue/metrics_spec.rb

@@ -9,7 +9,7 @@ describe Issue::Metrics do
     context "milestones" do
       it "records the first time an issue is associated with a milestone" do
         time = Time.now
-        Timecop.freeze(time) { subject.update(milestone: create(:milestone)) }
+        Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) }
         metrics = subject.metrics
 
         expect(metrics).to be_present
@@ -18,9 +18,9 @@ describe Issue::Metrics do
 
       it "does not record the second time an issue is associated with a milestone" do
         time = Time.now
-        Timecop.freeze(time) { subject.update(milestone: create(:milestone)) }
+        Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) }
         Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) }
-        Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone)) }
+        Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone, project: project)) }
         metrics = subject.metrics
 
         expect(metrics).to be_present

spec/models/milestone_spec.rb

@@ -184,7 +184,7 @@ describe Milestone do
   describe '#total_items_count' do
     before do
       create :closed_issue, milestone: milestone, project: project
-      create :merge_request, milestone: milestone
+      create :merge_request, milestone: milestone, source_project: project
     end
 
     it 'returns total count of issues and merge requests assigned to milestone' do
@@ -194,10 +194,10 @@ describe Milestone do
 
   describe '#can_be_closed?' do
     before do
-      milestone = create :milestone
-      create :closed_issue, milestone: milestone
+      milestone = create :milestone, project: project
+      create :closed_issue, milestone: milestone, project: project
 
-      create :issue
+      create :issue, project: project
     end
 
     it 'returns true if milestone active and all nested issues closed' do

spec/requests/api/issues_spec.rb

@@ -49,7 +49,7 @@ describe API::Issues do
     create(:label, title: 'label', color: '#FFAABB', project: project)
   end
   let!(:label_link) { create(:label_link, label: label, target: issue) }
-  set(:milestone) { create(:milestone, title: '1.0.0', project: project) }
+  let(:milestone) { create(:milestone, title: '1.0.0', project: project) }
   set(:empty_milestone) do
     create(:milestone, title: '2.0.0', project: project)
   end

spec/services/issuable/common_system_notes_service_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Issuable::CommonSystemNotesService do
   let(:user) { create(:user) }
   let(:project) { create(:project) }
-  let(:issuable) { create(:issue) }
+  let(:issuable) { create(:issue, project: project) }
 
   context 'on issuable update' do
     it_behaves_like 'system note creation', { title: 'New title' }, 'changed title'
@@ -70,7 +70,7 @@ describe Issuable::CommonSystemNotesService do
   end
 
   context 'on issuable create' do
-    let(:issuable) { build(:issue) }
+    let(:issuable) { build(:issue, project: project) }
 
     subject { described_class.new(project, user).execute(issuable, old_labels: [], is_update: false) }
 

spec/services/issues/build_service_spec.rb

@@ -8,29 +8,29 @@ describe Issues::BuildService do
     project.add_developer(user)
   end
 
+  def build_issue(issue_params = {})
+    described_class.new(project, user, issue_params).execute
+  end
+
   context 'for a single discussion' do
     describe '#execute' do
       let(:merge_request) { create(:merge_request, title: "Hello world", source_project: project) }
       let(:discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, note: "Almost done").to_discussion }
-      let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) }
 
-      it 'references the noteable title in the issue title' do
-        issue = service.execute
+      subject { build_issue(merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) }
 
-        expect(issue.title).to include('Hello world')
+      it 'references the noteable title in the issue title' do
+        expect(subject.title).to include('Hello world')
       end
 
       it 'adds the note content to the description' do
-        issue = service.execute
-
-        expect(issue.description).to include('Almost done')
+        expect(subject.description).to include('Almost done')
       end
     end
   end
 
   context 'for discussions in a merge request' do
     let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) }
-    let(:issue) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute }
 
     describe '#items_for_discussions' do
       it 'has an item for each discussion' do
@@ -66,28 +66,30 @@ describe Issues::BuildService do
     end
 
     describe '#execute' do
-      it 'has the merge request reference in the title' do
-        expect(issue.title).to include(merge_request.title)
-      end
+      let(:base_params) { { merge_request_to_resolve_discussions_of: merge_request.iid } }
 
-      it 'has the reference of the merge request in the description' do
-        expect(issue.description).to include(merge_request.to_reference)
+      context 'without additional params' do
+        subject { build_issue(base_params) }
+
+        it 'has the merge request reference in the title' do
+          expect(subject.title).to include(merge_request.title)
+        end
+
+        it 'has the reference of the merge request in the description' do
+          expect(subject.description).to include(merge_request.to_reference)
+        end
       end
 
-      it 'does not assign title when a title was given' do
-        issue = described_class.new(project, user,
-                                    merge_request_to_resolve_discussions_of: merge_request,
-                                    title: 'What an issue').execute
+      it 'uses provided title if title param given' do
+        issue = build_issue(base_params.merge(title: 'What an issue'))
 
         expect(issue.title).to eq('What an issue')
       end
 
-      it 'does not assign description when a description was given' do
-        issue = described_class.new(project, user,
-                                    merge_request_to_resolve_discussions_of: merge_request,
-                                    description: 'Fix at your earliest conveignance').execute
+      it 'uses provided description if description param given' do
+        issue = build_issue(base_params.merge(description: 'Fix at your earliest convenience'))
 
-        expect(issue.description).to eq('Fix at your earliest conveignance')
+        expect(issue.description).to eq('Fix at your earliest convenience')
       end
 
       describe 'with multiple discussions' do
@@ -96,20 +98,20 @@ describe Issues::BuildService do
         it 'mentions all the authors in the description' do
           authors = merge_request.resolvable_discussions.map(&:author)
 
-          expect(issue.description).to include(*authors.map(&:to_reference))
+          expect(build_issue(base_params).description).to include(*authors.map(&:to_reference))
         end
 
         it 'has a link for each unresolved discussion in the description' do
           notes = merge_request.resolvable_discussions.map(&:first_note)
           links = notes.map { |note| Gitlab::UrlBuilder.build(note) }
 
-          expect(issue.description).to include(*links)
+          expect(build_issue(base_params).description).to include(*links)
         end
 
         it 'mentions additional notes' do
           create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, in_reply_to: diff_note)
 
-          expect(issue.description).to include('(+2 comments)')
+          expect(build_issue(base_params).description).to include('(+2 comments)')
         end
       end
     end
@@ -120,7 +122,7 @@ describe Issues::BuildService do
 
     describe '#execute' do
       it 'mentions the merge request in the description' do
-        issue = described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute
+        issue = build_issue(merge_request_to_resolve_discussions_of: merge_request.iid)
 
         expect(issue.description).to include("Review the conversation in #{merge_request.to_reference}")
       end
@@ -128,20 +130,18 @@ describe Issues::BuildService do
   end
 
   describe '#execute' do
-    let(:milestone) { create(:milestone, project: project) }
-
     it 'builds a new issues with given params' do
-      issue = described_class.new(
-        project,
-        user,
-        title: 'Issue #1',
-        description: 'Issue description',
-        milestone_id: milestone.id
-      ).execute
-
-      expect(issue.title).to eq('Issue #1')
-      expect(issue.description).to eq('Issue description')
+      milestone = create(:milestone, project: project)
+      issue = build_issue(milestone_id: milestone.id)
+
       expect(issue.milestone).to eq(milestone)
     end
+
+    it 'sets milestone to nil if it is not available for the project' do
+      milestone = create(:milestone, project: create(:project))
+      issue = build_issue(milestone_id: milestone.id)
+
+      expect(issue.milestone).to be_nil
+    end
   end
 end

spec/services/issues/update_service_spec.rb

@@ -356,7 +356,7 @@ describe Issues::UpdateService, :mailer do
         it_behaves_like 'system notes for milestones'
 
         it 'sends notifications for subscribers of changed milestone' do
-          issue.milestone = create(:milestone)
+          issue.milestone = create(:milestone, project: project)
 
           issue.save
 
@@ -380,7 +380,7 @@ describe Issues::UpdateService, :mailer do
         end
 
         it 'marks todos as done' do
-          update_issue(milestone: create(:milestone))
+          update_issue(milestone: create(:milestone, project: project))
 
           expect(todo.reload.done?).to eq true
         end
@@ -389,7 +389,7 @@ describe Issues::UpdateService, :mailer do
 
         it 'sends notifications for subscribers of changed milestone' do
           perform_enqueued_jobs do
-            update_issue(milestone: create(:milestone))
+            update_issue(milestone: create(:milestone, project: project))
           end
 
           should_email(subscriber)

spec/services/merge_requests/build_service_spec.rb

@@ -229,6 +229,15 @@ describe MergeRequests::BuildService do
             end
           end
         end
+
+        context 'when a milestone is from another project' do
+          let(:milestone) { create(:milestone, project: create(:project)) }
+          let(:milestone_id) { milestone.id }
+
+          it 'sets milestone to nil' do
+            expect(merge_request.milestone).to be_nil
+          end
+        end
       end
     end
 

spec/services/merge_requests/update_service_spec.rb

@@ -328,7 +328,7 @@ describe MergeRequests::UpdateService, :mailer do
         it_behaves_like 'system notes for milestones'
 
         it 'sends notifications for subscribers of changed milestone' do
-          merge_request.milestone = create(:milestone)
+          merge_request.milestone = create(:milestone, project: project)
 
           merge_request.save
 
@@ -352,7 +352,7 @@ describe MergeRequests::UpdateService, :mailer do
         end
 
         it 'marks pending todos as done' do
-          update_merge_request({ milestone: create(:milestone) })
+          update_merge_request({ milestone: create(:milestone, project: project) })
 
           expect(pending_todo.reload).to be_done
         end
@@ -361,7 +361,7 @@ describe MergeRequests::UpdateService, :mailer do
 
         it 'sends notifications for subscribers of changed milestone' do
           perform_enqueued_jobs do
-            update_merge_request(milestone: create(:milestone))
+            update_merge_request(milestone: create(:milestone, project: project))
           end
 
           should_email(subscriber)

spec/support/shared_examples/issuable_shared_examples.rb

@@ -31,7 +31,7 @@ shared_examples 'system notes for milestones' do
   context 'project milestones' do
     it 'creates a system note' do
       expect do
-        update_issuable(milestone: create(:milestone))
+        update_issuable(milestone: create(:milestone, project: project))
       end.to change { Note.system.count }.by(1)
     end
   end