Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into revert-c676283b-existing

13 files changed, 270 insertions, 58 deletions

diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index 92b97bf3d0c..a0870891cf4 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -87,10 +87,10 @@ describe Groups::GroupMembersController do
     context 'when member is not found' do
       before { sign_in(user) }
 
-      it 'returns 403' do
+      it 'returns 404' do
         delete :leave, group_id: group
 
-        expect(response).to have_http_status(403)
+        expect(response).to have_http_status(404)
       end
     end
 
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index 5e2a8cf3849..074f85157de 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -135,11 +135,11 @@ describe Projects::ProjectMembersController do
     context 'when member is not found' do
       before { sign_in(user) }
 
-      it 'returns 403' do
+      it 'returns 404' do
         delete :leave, namespace_id: project.namespace,
                        project_id: project
 
-        expect(response).to have_http_status(403)
+        expect(response).to have_http_status(404)
       end
     end
 
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 8f27e616c3e..48d69377461 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -109,6 +109,44 @@ describe SessionsController do
             end
           end
 
+          context 'when the user is on their last attempt' do
+            before do
+              user.update(failed_attempts: User.maximum_attempts.pred)
+            end
+
+            context 'when OTP is valid' do
+              it 'authenticates correctly' do
+                authenticate_2fa(otp_attempt: user.current_otp)
+
+                expect(subject.current_user).to eq user
+              end
+            end
+
+            context 'when OTP is invalid' do
+              before { authenticate_2fa(otp_attempt: 'invalid') }
+
+              it 'does not authenticate' do
+                expect(subject.current_user).not_to eq user
+              end
+
+              it 'warns about invalid login' do
+                expect(response).to set_flash.now[:alert]
+                  .to /Invalid Login or password/
+              end
+
+              it 'locks the user' do
+                expect(user.reload).to be_access_locked
+              end
+
+              it 'keeps the user locked on future login attempts' do
+                post(:create, user: { login: user.username, password: user.password })
+
+                expect(response)
+                  .to set_flash.now[:alert].to /Invalid Login or password/
+              end
+            end
+          end
+
           context 'when another user does not have 2FA enabled' do
             let(:another_user) { create(:user) }
 
diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb
index f76c4fe8b57..cd79c4f512d 100644
--- a/spec/features/projects/issuable_templates_spec.rb
+++ b/spec/features/projects/issuable_templates_spec.rb
@@ -26,7 +26,7 @@ feature 'issuable templates', feature: true, js: true do
     scenario 'user selects "bug" template' do
       select_template 'bug'
       wait_for_ajax
-      preview_template
+      preview_template(template_content)
       save_changes
     end
 
@@ -42,6 +42,26 @@ feature 'issuable templates', feature: true, js: true do
     end
   end
 
+  context 'user creates an issue using templates, with a prior description' do
+    let(:prior_description) { 'test issue description' }
+    let(:template_content) { 'this is a test "bug" template' }
+    let(:issue) { create(:issue, author: user, assignee: user, project: project) }
+
+    background do
+      project.repository.commit_file(user, '.gitlab/issue_templates/bug.md', template_content, 'added issue template', 'master', false)
+      visit edit_namespace_project_issue_path project.namespace, project, issue
+      fill_in :'issue[title]', with: 'test issue title'
+      fill_in :'issue[description]', with: prior_description
+    end
+
+    scenario 'user selects "bug" template' do
+      select_template 'bug'
+      wait_for_ajax
+      preview_template("#{prior_description}\n\n#{template_content}")
+      save_changes
+    end
+  end
+
   context 'user creates a merge request using templates' do
     let(:template_content) { 'this is a test "feature-proposal" template' }
     let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) }
@@ -55,7 +75,7 @@ feature 'issuable templates', feature: true, js: true do
     scenario 'user selects "feature-proposal" template' do
       select_template 'feature-proposal'
       wait_for_ajax
-      preview_template
+      preview_template(template_content)
       save_changes
     end
   end
@@ -82,16 +102,16 @@ feature 'issuable templates', feature: true, js: true do
         scenario 'user selects template' do
           select_template 'feature-proposal'
           wait_for_ajax
-          preview_template
+          preview_template(template_content)
           save_changes
         end
       end
     end
   end
 
-  def preview_template
+  def preview_template(expected_content)
     click_link 'Preview'
-    expect(page).to have_content template_content
+    expect(page).to have_content expected_content
   end
 
   def save_changes
diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb
index fc555a74f30..bf93c1d1251 100644
--- a/spec/features/todos/todos_spec.rb
+++ b/spec/features/todos/todos_spec.rb
@@ -4,7 +4,7 @@ describe 'Dashboard Todos', feature: true do
   let(:user)    { create(:user) }
   let(:author)  { create(:user) }
   let(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
-  let(:issue)   { create(:issue) }
+  let(:issue)   { create(:issue, due_date: Date.today) }
 
   describe 'GET /dashboard/todos' do
     context 'User does not have todos' do
@@ -28,6 +28,12 @@ describe 'Dashboard Todos', feature: true do
         expect(page).to have_selector('.todos-list .todo', count: 1)
       end
 
+      it 'shows due date as today' do
+        page.within first('.todo') do
+          expect(page).to have_content 'Due today'
+        end
+      end
+
       describe 'deleting the todo' do
         before do
           first('.done-todo').click
diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb
index 549b0042038..132858950d5 100644
--- a/spec/models/concerns/mentionable_spec.rb
+++ b/spec/models/concerns/mentionable_spec.rb
@@ -1,18 +1,27 @@
 require 'spec_helper'
 
 describe Mentionable do
-  include Mentionable
+  class Example
+    include Mentionable
 
-  def author
-    nil
+    attr_accessor :project, :message
+    attr_mentionable :message
+
+    def author
+      nil
+    end
   end
 
   describe 'references' do
     let(:project) { create(:project) }
+    let(:mentionable) { Example.new }
 
     it 'excludes JIRA references' do
       allow(project).to receive_messages(jira_tracker?: true)
-      expect(referenced_mentionables(project, 'JIRA-123')).to be_empty
+
+      mentionable.project = project
+      mentionable.message = 'JIRA-123'
+      expect(mentionable.referenced_mentionables).to be_empty
     end
   end
 end
@@ -39,9 +48,8 @@ describe Issue, "Mentionable" do
       let(:user) { create(:user) }
 
       def referenced_issues(current_user)
-        text = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}"
-
-        issue.referenced_mentionables(current_user, text)
+        issue.title = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}"
+        issue.referenced_mentionables(current_user)
       end
 
       context 'when the current user can see the issue' do
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 9d7be2429ed..38b6da50168 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -86,6 +86,30 @@ describe MergeRequest, models: true do
     end
   end
 
+  describe '#cache_merge_request_closes_issues!' do
+    before do
+      subject.project.team << [subject.author, :developer]
+      subject.target_branch = subject.project.default_branch
+    end
+
+    it 'caches closed issues' do
+      issue  = create :issue, project: subject.project
+      commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
+      allow(subject).to receive(:commits).and_return([commit])
+
+      expect { subject.cache_merge_request_closes_issues! }.to change(subject.merge_requests_closing_issues, :count).by(1)
+    end
+
+    it 'does not cache issues from external trackers' do
+      subject.project.update_attribute(:has_external_issue_tracker, true)
+      issue  = ExternalIssue.new('JIRA-123', subject.project)
+      commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
+      allow(subject).to receive(:commits).and_return([commit])
+
+      expect { subject.cache_merge_request_closes_issues! }.not_to change(subject.merge_requests_closing_issues, :count)
+    end
+  end
+
   describe '#source_branch_sha' do
     let(:last_branch_commit) { subject.source_project.repository.commit(subject.source_branch) }
 
@@ -522,7 +546,7 @@ describe MergeRequest, models: true do
   end
 
   it_behaves_like 'an editable mentionable' do
-    subject { create(:merge_request) }
+    subject { create(:merge_request, :simple) }
 
     let(:backref_text) { "merge request #{subject.to_reference}" }
     let(:set_mentionable_text) { ->(txt){ subject.description = txt } }
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index db29f4d353b..98c64c079b9 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -320,6 +320,16 @@ describe Repository, models: true do
     end
   end
 
+  describe '#create_ref' do
+    it 'redirects the call to fetch_ref' do
+      ref, ref_path = '1', '2'
+
+      expect(repository).to receive(:fetch_ref).with(repository.path_to_repo, ref, ref_path)
+
+      repository.create_ref(ref, ref_path)
+    end
+  end
+
   describe "#changelog" do
     before do
       repository.send(:cache).expire(:changelog)
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 05056a4bb47..ed1bc9271ae 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -203,6 +203,23 @@ describe Service, models: true do
     end
   end
 
+  describe 'initialize service with no properties' do
+    let(:service) do
+      GitlabIssueTrackerService.create(
+        project: create(:project),
+        title: 'random title'
+      )
+    end
+
+    it 'does not raise error' do
+      expect { service }.not_to raise_error
+    end
+
+    it 'creates the properties' do
+      expect(service.properties).to eq({ "title" => "random title" })
+    end
+  end
+
   describe "callbacks" do
     let(:project) { create(:project) }
     let!(:service) do
diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb
index 905a7311372..b467890a403 100644
--- a/spec/requests/api/access_requests_spec.rb
+++ b/spec/requests/api/access_requests_spec.rb
@@ -195,7 +195,7 @@ describe API::AccessRequests, api: true  do
       end
 
       context 'when authenticated as the access requester' do
-        it 'returns 200' do
+        it 'deletes the access requester' do
           expect do
             delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", access_requester)
 
@@ -205,7 +205,7 @@ describe API::AccessRequests, api: true  do
       end
 
       context 'when authenticated as a master/owner' do
-        it 'returns 200' do
+        it 'deletes the access requester' do
           expect do
             delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", master)
 
@@ -213,6 +213,16 @@ describe API::AccessRequests, api: true  do
           end.to change { source.requesters.count }.by(-1)
         end
 
+        context 'user_id matches a member, not an access requester' do
+          it 'returns 404' do
+            expect do
+              delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{developer.id}", master)
+
+              expect(response).to have_http_status(404)
+            end.not_to change { source.requesters.count }
+          end
+        end
+
         context 'user_id does not match an existing access requester' do
           it 'returns 404' do
             expect do
diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb
index 2395445e7fd..9995f3488af 100644
--- a/spec/services/members/destroy_service_spec.rb
+++ b/spec/services/members/destroy_service_spec.rb
@@ -2,70 +2,111 @@ require 'spec_helper'
 
 describe Members::DestroyService, services: true do
   let(:user) { create(:user) }
-  let(:project) { create(:project) }
-  let!(:member) { create(:project_member, source: project) }
+  let(:member_user) { create(:user) }
+  let(:project) { create(:project, :public) }
+  let(:group) { create(:group, :public) }
 
-  context 'when member is nil' do
-    before do
-      project.team << [user, :developer]
+  shared_examples 'a service raising ActiveRecord::RecordNotFound' do
+    it 'raises ActiveRecord::RecordNotFound' do
+      expect { described_class.new(source, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound)
     end
+  end
 
-    it 'does not destroy the member' do
-      expect { destroy_member(nil, user) }.to raise_error(Gitlab::Access::AccessDeniedError)
+  shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do
+    it 'raises Gitlab::Access::AccessDeniedError' do
+      expect { described_class.new(source, user, params).execute }.to raise_error(Gitlab::Access::AccessDeniedError)
     end
   end
 
-  context 'when current user cannot destroy the given member' do
-    before do
-      project.team << [user, :developer]
+  shared_examples 'a service destroying a member' do
+    it 'destroys the member' do
+      expect { described_class.new(source, user, params).execute }.to change { source.members.count }.by(-1)
+    end
+
+    context 'when the given member is an access requester' do
+      before do
+        source.members.find_by(user_id: member_user).destroy
+        source.request_access(member_user)
+      end
+      let(:access_requester) { source.requesters.find_by(user_id: member_user) }
+
+      it_behaves_like 'a service raising ActiveRecord::RecordNotFound'
+
+      %i[requesters all].each do |scope|
+        context "and #{scope} scope is passed" do
+          it 'destroys the access requester' do
+            expect { described_class.new(source, user, params).execute(scope) }.to change { source.requesters.count }.by(-1)
+          end
+
+          it 'calls Member#after_decline_request' do
+            expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(access_requester)
+
+            described_class.new(source, user, params).execute(scope)
+          end
+
+          context 'when current user is the member' do
+            it 'does not call Member#after_decline_request' do
+              expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(access_requester)
+
+              described_class.new(source, member_user, params).execute(scope)
+            end
+          end
+        end
+      end
     end
+  end
+
+  context 'when no member are found' do
+    let(:params) { { user_id: 42 } }
 
-    it 'does not destroy the member' do
-      expect { destroy_member(member, user) }.to raise_error(Gitlab::Access::AccessDeniedError)
+    it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do
+      let(:source) { project }
+    end
+
+    it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do
+      let(:source) { group }
     end
   end
 
-  context 'when current user can destroy the given member' do
+  context 'when a member is found' do
     before do
-      project.team << [user, :master]
+      project.team << [member_user, :developer]
+      group.add_developer(member_user)
     end
+    let(:params) { { user_id: member_user.id } }
 
-    it 'destroys the member' do
-      destroy_member(member, user)
+    context 'when current user cannot destroy the given member' do
+      it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+        let(:source) { project }
+      end
 
-      expect(member).to be_destroyed
+      it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+        let(:source) { group }
+      end
     end
 
-    context 'when the given member is a requester' do
+    context 'when current user can destroy the given member' do
       before do
-        member.update_column(:requested_at, Time.now)
+        project.team << [user, :master]
+        group.add_owner(user)
       end
 
-      it 'calls Member#after_decline_request' do
-        expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(member)
-
-        destroy_member(member, user)
+      it_behaves_like 'a service destroying a member' do
+        let(:source) { project }
       end
 
-      context 'when current user is the member' do
-        it 'does not call Member#after_decline_request' do
-          expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member)
-
-          destroy_member(member, member.user)
-        end
+      it_behaves_like 'a service destroying a member' do
+        let(:source) { group }
       end
 
-      context 'when current user is the member and ' do
-        it 'does not call Member#after_decline_request' do
-          expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member)
+      context 'when given a :id' do
+        let(:params) { { id: project.members.find_by!(user_id: user.id).id } }
 
-          destroy_member(member, member.user)
+        it 'destroys the member' do
+          expect { described_class.new(project, user, params).execute }.
+            to change { project.members.count }.by(-1)
         end
       end
     end
   end
-
-  def destroy_member(member, user)
-    Members::DestroyService.new(member, user).execute
-  end
 end
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index 31167675d07..e49a0d5e553 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -38,6 +38,42 @@ describe MergeRequests::MergeService, services: true do
       end
     end
 
+    context 'closes related issues' do
+      let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
+
+      before do
+        allow(project).to receive(:default_branch).and_return(merge_request.target_branch)
+      end
+
+      it 'closes GitLab issue tracker issues' do
+        issue  = create :issue, project: project
+        commit = double('commit', safe_message: "Fixes #{issue.to_reference}")
+        allow(merge_request).to receive(:commits).and_return([commit])
+
+        service.execute(merge_request)
+
+        expect(issue.reload.closed?).to be_truthy
+      end
+
+      context 'with JIRA integration' do
+        include JiraServiceHelper
+
+        let(:jira_tracker) { project.create_jira_service }
+
+        before { jira_service_settings }
+
+        it 'closes issues on JIRA issue tracker' do
+          jira_issue = ExternalIssue.new('JIRA-123', project)
+          commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}")
+          allow(merge_request).to receive(:commits).and_return([commit])
+
+          expect_any_instance_of(JiraService).to receive(:close_issue).with(merge_request, jira_issue).once
+
+          service.execute(merge_request)
+        end
+      end
+    end
+
     context 'closes related todos' do
       let(:merge_request) { create(:merge_request, assignee: user, author: user) }
       let(:project) { merge_request.project }
diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb
index e876d44c166..f57c82809a6 100644
--- a/spec/support/mentionable_shared_examples.rb
+++ b/spec/support/mentionable_shared_examples.rb
@@ -9,7 +9,7 @@ shared_context 'mentionable context' do
   let(:author)  { subject.author }
 
   let(:mentioned_issue)  { create(:issue, project: project) }
-  let!(:mentioned_mr)     { create(:merge_request, :simple, source_project: project) }
+  let!(:mentioned_mr)     { create(:merge_request, source_project: project) }
   let(:mentioned_commit) { project.commit("HEAD~1") }
 
   let(:ext_proj)   { create(:project, :public) }
@@ -100,6 +100,7 @@ shared_examples 'an editable mentionable' do
 
   it 'creates new cross-reference notes when the mentionable text is edited' do
     subject.save
+    subject.create_cross_references!
 
     new_text = <<-MSG.strip_heredoc
       These references already existed:
@@ -131,6 +132,7 @@ shared_examples 'an editable mentionable' do
     end
 
     # These two issues are new and should receive reference notes
+    # In the case of MergeRequests remember that cannot mention commits included in the MergeRequest
     new_issues.each do |newref|
       expect(SystemNoteService).to receive(:cross_reference).
         with(newref, subject.local_reference, author)