Merge commit 'b39d0c318921bae2e3a11df9ee6828291dad9864' into object-storage-ee-to-ce-backport

11 files changed, 228 insertions, 606 deletions

diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb
index d2c613a2423..caa63e7bd22 100644
--- a/spec/controllers/projects/artifacts_controller_spec.rb
+++ b/spec/controllers/projects/artifacts_controller_spec.rb
@@ -81,14 +81,6 @@ describe Projects::ArtifactsController do
         expect(params['Entry']).to eq(Base64.encode64('ci_artifacts.txt'))
       end
     end
-
-    context 'when the file does not exist' do
-      it 'responds Not Found' do
-        get :raw, namespace_id: project.namespace, project_id: project, job_id: job, path: 'unknown'
-
-        expect(response).to be_not_found
-      end
-    end
   end
 
   describe 'GET latest_succeeded' do
diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb
deleted file mode 100644
index 3f6c1092163..00000000000
--- a/spec/controllers/projects/boards/issues_controller_spec.rb
+++ /dev/null
@@ -1,221 +0,0 @@
-require 'spec_helper'
-
-describe Projects::Boards::IssuesController do
-  let(:project) { create(:project) }
-  let(:board)   { create(:board, project: project) }
-  let(:user)    { create(:user) }
-  let(:guest)   { create(:user) }
-
-  let(:planning)    { create(:label, project: project, name: 'Planning') }
-  let(:development) { create(:label, project: project, name: 'Development') }
-
-  let!(:list1) { create(:list, board: board, label: planning, position: 0) }
-  let!(:list2) { create(:list, board: board, label: development, position: 1) }
-
-  before do
-    project.team << [user, :master]
-    project.team << [guest, :guest]
-  end
-
-  describe 'GET index' do
-    let(:johndoe) { create(:user, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) }
-
-    context 'with invalid board id' do
-      it 'returns a not found 404 response' do
-        list_issues user: user, board: 999, list: list2
-
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    context 'when list id is present' do
-      context 'with valid list id' do
-        it 'returns issues that have the list label applied' do
-          issue = create(:labeled_issue, project: project, labels: [planning])
-          create(:labeled_issue, project: project, labels: [planning])
-          create(:labeled_issue, project: project, labels: [development], due_date: Date.tomorrow)
-          create(:labeled_issue, project: project, labels: [development], assignees: [johndoe])
-          issue.subscribe(johndoe, project)
-
-          list_issues user: user, board: board, list: list2
-
-          parsed_response = JSON.parse(response.body)
-
-          expect(response).to match_response_schema('issues')
-          expect(parsed_response.length).to eq 2
-          expect(development.issues.map(&:relative_position)).not_to include(nil)
-        end
-      end
-
-      context 'with invalid list id' do
-        it 'returns a not found 404 response' do
-          list_issues user: user, board: board, list: 999
-
-          expect(response).to have_http_status(404)
-        end
-      end
-    end
-
-    context 'when list id is missing' do
-      it 'returns opened issues without board labels applied' do
-        bug = create(:label, project: project, name: 'Bug')
-        create(:issue, project: project)
-        create(:labeled_issue, project: project, labels: [planning])
-        create(:labeled_issue, project: project, labels: [development])
-        create(:labeled_issue, project: project, labels: [bug])
-
-        list_issues user: user, board: board
-
-        parsed_response = JSON.parse(response.body)
-
-        expect(response).to match_response_schema('issues')
-        expect(parsed_response.length).to eq 2
-      end
-    end
-
-    context 'with unauthorized user' do
-      before do
-        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
-        allow(Ability).to receive(:allowed?).with(user, :read_issue, project).and_return(false)
-      end
-
-      it 'returns a forbidden 403 response' do
-        list_issues user: user, board: board, list: list2
-
-        expect(response).to have_http_status(403)
-      end
-    end
-
-    def list_issues(user:, board:, list: nil)
-      sign_in(user)
-
-      params = {
-        namespace_id: project.namespace.to_param,
-        project_id: project,
-        board_id: board.to_param,
-        list_id: list.try(:to_param)
-      }
-
-      get :index, params.compact
-    end
-  end
-
-  describe 'POST create' do
-    context 'with valid params' do
-      it 'returns a successful 200 response' do
-        create_issue user: user, board: board, list: list1, title: 'New issue'
-
-        expect(response).to have_http_status(200)
-      end
-
-      it 'returns the created issue' do
-        create_issue user: user, board: board, list: list1, title: 'New issue'
-
-        expect(response).to match_response_schema('issue')
-      end
-    end
-
-    context 'with invalid params' do
-      context 'when title is nil' do
-        it 'returns an unprocessable entity 422 response' do
-          create_issue user: user, board: board, list: list1, title: nil
-
-          expect(response).to have_http_status(422)
-        end
-      end
-
-      context 'when list does not belongs to project board' do
-        it 'returns a not found 404 response' do
-          list = create(:list)
-
-          create_issue user: user, board: board, list: list, title: 'New issue'
-
-          expect(response).to have_http_status(404)
-        end
-      end
-    end
-
-    context 'with unauthorized user' do
-      it 'returns a forbidden 403 response' do
-        create_issue user: guest, board: board, list: list1, title: 'New issue'
-
-        expect(response).to have_http_status(403)
-      end
-    end
-
-    def create_issue(user:, board:, list:, title:)
-      sign_in(user)
-
-      post :create, namespace_id: project.namespace.to_param,
-                    project_id: project,
-                    board_id: board.to_param,
-                    list_id: list.to_param,
-                    issue: { title: title },
-                    format: :json
-    end
-  end
-
-  describe 'PATCH update' do
-    let(:issue) { create(:labeled_issue, project: project, labels: [planning]) }
-
-    context 'with valid params' do
-      it 'returns a successful 200 response' do
-        move user: user, board: board, issue: issue, from_list_id: list1.id, to_list_id: list2.id
-
-        expect(response).to have_http_status(200)
-      end
-
-      it 'moves issue to the desired list' do
-        move user: user, board: board, issue: issue, from_list_id: list1.id, to_list_id: list2.id
-
-        expect(issue.reload.labels).to contain_exactly(development)
-      end
-    end
-
-    context 'with invalid params' do
-      it 'returns a unprocessable entity 422 response for invalid lists' do
-        move user: user, board: board, issue: issue, from_list_id: nil, to_list_id: nil
-
-        expect(response).to have_http_status(422)
-      end
-
-      it 'returns a not found 404 response for invalid board id' do
-        move user: user, board: 999, issue: issue, from_list_id: list1.id, to_list_id: list2.id
-
-        expect(response).to have_http_status(404)
-      end
-
-      it 'returns a not found 404 response for invalid issue id' do
-        move user: user, board: board, issue: 999, from_list_id: list1.id, to_list_id: list2.id
-
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    context 'with unauthorized user' do
-      let(:guest) { create(:user) }
-
-      before do
-        project.team << [guest, :guest]
-      end
-
-      it 'returns a forbidden 403 response' do
-        move user: guest, board: board, issue: issue, from_list_id: list1.id, to_list_id: list2.id
-
-        expect(response).to have_http_status(403)
-      end
-    end
-
-    def move(user:, board:, issue:, from_list_id:, to_list_id:)
-      sign_in(user)
-
-      patch :update, namespace_id: project.namespace.to_param,
-                     project_id: project,
-                     board_id: board.to_param,
-                     id: issue.to_param,
-                     from_list_id: from_list_id,
-                     to_list_id: to_list_id,
-                     format: :json
-    end
-  end
-end
diff --git a/spec/controllers/projects/boards/lists_controller_spec.rb b/spec/controllers/projects/boards/lists_controller_spec.rb
deleted file mode 100644
index 65beec16307..00000000000
--- a/spec/controllers/projects/boards/lists_controller_spec.rb
+++ /dev/null
@@ -1,252 +0,0 @@
-require 'spec_helper'
-
-describe Projects::Boards::ListsController do
-  let(:project) { create(:project) }
-  let(:board)   { create(:board, project: project) }
-  let(:user)    { create(:user) }
-  let(:guest)   { create(:user) }
-
-  before do
-    project.team << [user, :master]
-    project.team << [guest, :guest]
-  end
-
-  describe 'GET index' do
-    it 'returns a successful 200 response' do
-      read_board_list user: user, board: board
-
-      expect(response).to have_http_status(200)
-      expect(response.content_type).to eq 'application/json'
-    end
-
-    it 'returns a list of board lists' do
-      create(:list, board: board)
-
-      read_board_list user: user, board: board
-
-      parsed_response = JSON.parse(response.body)
-
-      expect(response).to match_response_schema('lists')
-      expect(parsed_response.length).to eq 3
-    end
-
-    context 'with unauthorized user' do
-      before do
-        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
-        allow(Ability).to receive(:allowed?).with(user, :read_list, project).and_return(false)
-      end
-
-      it 'returns a forbidden 403 response' do
-        read_board_list user: user, board: board
-
-        expect(response).to have_http_status(403)
-      end
-    end
-
-    def read_board_list(user:, board:)
-      sign_in(user)
-
-      get :index, namespace_id: project.namespace.to_param,
-                  project_id: project,
-                  board_id: board.to_param,
-                  format: :json
-    end
-  end
-
-  describe 'POST create' do
-    context 'with valid params' do
-      let(:label) { create(:label, project: project, name: 'Development') }
-
-      it 'returns a successful 200 response' do
-        create_board_list user: user, board: board, label_id: label.id
-
-        expect(response).to have_http_status(200)
-      end
-
-      it 'returns the created list' do
-        create_board_list user: user, board: board, label_id: label.id
-
-        expect(response).to match_response_schema('list')
-      end
-    end
-
-    context 'with invalid params' do
-      context 'when label is nil' do
-        it 'returns a not found 404 response' do
-          create_board_list user: user, board: board, label_id: nil
-
-          expect(response).to have_http_status(404)
-        end
-      end
-
-      context 'when label that does not belongs to project' do
-        it 'returns a not found 404 response' do
-          label = create(:label, name: 'Development')
-
-          create_board_list user: user, board: board, label_id: label.id
-
-          expect(response).to have_http_status(404)
-        end
-      end
-    end
-
-    context 'with unauthorized user' do
-      it 'returns a forbidden 403 response' do
-        label = create(:label, project: project, name: 'Development')
-
-        create_board_list user: guest, board: board, label_id: label.id
-
-        expect(response).to have_http_status(403)
-      end
-    end
-
-    def create_board_list(user:, board:, label_id:)
-      sign_in(user)
-
-      post :create, namespace_id: project.namespace.to_param,
-                    project_id: project,
-                    board_id: board.to_param,
-                    list: { label_id: label_id },
-                    format: :json
-    end
-  end
-
-  describe 'PATCH update' do
-    let!(:planning)    { create(:list, board: board, position: 0) }
-    let!(:development) { create(:list, board: board, position: 1) }
-
-    context 'with valid position' do
-      it 'returns a successful 200 response' do
-        move user: user, board: board, list: planning, position: 1
-
-        expect(response).to have_http_status(200)
-      end
-
-      it 'moves the list to the desired position' do
-        move user: user, board: board, list: planning, position: 1
-
-        expect(planning.reload.position).to eq 1
-      end
-    end
-
-    context 'with invalid position' do
-      it 'returns an unprocessable entity 422 response' do
-        move user: user, board: board, list: planning, position: 6
-
-        expect(response).to have_http_status(422)
-      end
-    end
-
-    context 'with invalid list id' do
-      it 'returns a not found 404 response' do
-        move user: user, board: board, list: 999, position: 1
-
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    context 'with unauthorized user' do
-      it 'returns a forbidden 403 response' do
-        move user: guest, board: board, list: planning, position: 6
-
-        expect(response).to have_http_status(403)
-      end
-    end
-
-    def move(user:, board:, list:, position:)
-      sign_in(user)
-
-      patch :update, namespace_id: project.namespace.to_param,
-                     project_id: project,
-                     board_id: board.to_param,
-                     id: list.to_param,
-                     list: { position: position },
-                     format: :json
-    end
-  end
-
-  describe 'DELETE destroy' do
-    let!(:planning) { create(:list, board: board, position: 0) }
-
-    context 'with valid list id' do
-      it 'returns a successful 200 response' do
-        remove_board_list user: user, board: board, list: planning
-
-        expect(response).to have_http_status(200)
-      end
-
-      it 'removes list from board' do
-        expect { remove_board_list user: user, board: board, list: planning }.to change(board.lists, :size).by(-1)
-      end
-    end
-
-    context 'with invalid list id' do
-      it 'returns a not found 404 response' do
-        remove_board_list user: user, board: board, list: 999
-
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    context 'with unauthorized user' do
-      it 'returns a forbidden 403 response' do
-        remove_board_list user: guest, board: board, list: planning
-
-        expect(response).to have_http_status(403)
-      end
-    end
-
-    def remove_board_list(user:, board:, list:)
-      sign_in(user)
-
-      delete :destroy, namespace_id: project.namespace.to_param,
-                       project_id: project,
-                       board_id: board.to_param,
-                       id: list.to_param,
-                       format: :json
-    end
-  end
-
-  describe 'POST generate' do
-    context 'when board lists is empty' do
-      it 'returns a successful 200 response' do
-        generate_default_lists user: user, board: board
-
-        expect(response).to have_http_status(200)
-      end
-
-      it 'returns the defaults lists' do
-        generate_default_lists user: user, board: board
-
-        expect(response).to match_response_schema('lists')
-      end
-    end
-
-    context 'when board lists is not empty' do
-      it 'returns an unprocessable entity 422 response' do
-        create(:list, board: board)
-
-        generate_default_lists user: user, board: board
-
-        expect(response).to have_http_status(422)
-      end
-    end
-
-    context 'with unauthorized user' do
-      it 'returns a forbidden 403 response' do
-        generate_default_lists user: guest, board: board
-
-        expect(response).to have_http_status(403)
-      end
-    end
-
-    def generate_default_lists(user:, board:)
-      sign_in(user)
-
-      post :generate, namespace_id: project.namespace.to_param,
-                      project_id: project,
-                      board_id: board.to_param,
-                      format: :json
-    end
-  end
-end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index bdee3894a13..5d9403c23ac 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1,7 +1,7 @@
 require('spec_helper')
 
 describe Projects::IssuesController do
-  let(:project) { create(:project_empty_repo) }
+  let(:project) { create(:project) }
   let(:user)    { create(:user) }
   let(:issue)   { create(:issue, project: project) }
 
@@ -233,144 +233,119 @@ describe Projects::IssuesController do
       end
     end
 
-    context 'when moving issue to another private project' do
-      let(:another_project) { create(:project, :private) }
-
-      context 'when user has access to move issue' do
-        before do
-          another_project.team << [user, :reporter]
-        end
-
-        it 'moves issue to another project' do
-          move_issue
+    context 'Akismet is enabled' do
+      let(:project) { create(:project_empty_repo, :public) }
 
-          expect(response).to have_http_status :found
-          expect(another_project.issues).not_to be_empty
-        end
+      before do
+        stub_application_setting(recaptcha_enabled: true)
+        allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
       end
 
-      context 'when user does not have access to move issue' do
-        it 'responds with 404' do
-          move_issue
+      context 'when an issue is not identified as spam' do
+        before do
+          allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false)
+          allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false)
+        end
 
-          expect(response).to have_http_status :not_found
+        it 'normally updates the issue' do
+          expect { update_issue(title: 'Foo') }.to change { issue.reload.title }.to('Foo')
         end
       end
 
-      context 'Akismet is enabled' do
-        let(:project) { create(:project_empty_repo, :public) }
-
+      context 'when an issue is identified as spam' do
         before do
-          stub_application_setting(recaptcha_enabled: true)
-          allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
+          allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true)
         end
 
-        context 'when an issue is not identified as spam' do
-          before do
-            allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false)
-            allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false)
+        context 'when captcha is not verified' do
+          def update_spam_issue
+            update_issue(title: 'Spam Title', description: 'Spam lives here')
           end
 
-          it 'normally updates the issue' do
-            expect { update_issue(title: 'Foo') }.to change { issue.reload.title }.to('Foo')
+          before do
+            allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false)
           end
-        end
 
-        context 'when an issue is identified as spam' do
-          before do
-            allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
+          it 'rejects an issue recognized as a spam' do
+            expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true)
+            expect { update_spam_issue }.not_to change { issue.reload.title }
           end
 
-          context 'when captcha is not verified' do
-            def update_spam_issue
-              update_issue(title: 'Spam Title', description: 'Spam lives here')
-            end
+          it 'rejects an issue recognized as a spam when recaptcha disabled' do
+            stub_application_setting(recaptcha_enabled: false)
 
-            before do
-              allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false)
-            end
+            expect { update_spam_issue }.not_to change { issue.reload.title }
+          end
 
-            it 'rejects an issue recognized as a spam' do
-              expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true)
-              expect { update_spam_issue }.not_to change{ issue.reload.title }
-            end
+          it 'creates a spam log' do
+            update_spam_issue
 
-            it 'rejects an issue recognized as a spam when recaptcha disabled' do
-              stub_application_setting(recaptcha_enabled: false)
+            spam_logs = SpamLog.all
 
-              expect { update_spam_issue }.not_to change{ issue.reload.title }
-            end
+            expect(spam_logs.count).to eq(1)
+            expect(spam_logs.first.title).to eq('Spam Title')
+            expect(spam_logs.first.recaptcha_verified).to be_falsey
+          end
 
-            it 'creates a spam log' do
+          context 'as HTML' do
+            it 'renders verify template' do
               update_spam_issue
 
-              spam_logs = SpamLog.all
-
-              expect(spam_logs.count).to eq(1)
-              expect(spam_logs.first.title).to eq('Spam Title')
-              expect(spam_logs.first.recaptcha_verified).to be_falsey
+              expect(response).to render_template(:verify)
             end
+          end
 
-            context 'as HTML' do
-              it 'renders verify template' do
-                update_spam_issue
-
-                expect(response).to render_template(:verify)
-              end
+          context 'as JSON' do
+            before do
+              update_issue({ title: 'Spam Title', description: 'Spam lives here' }, format: :json)
             end
 
-            context 'as JSON' do
-              before do
-                update_issue({ title: 'Spam Title', description: 'Spam lives here' }, format: :json)
-              end
-
-              it 'renders json errors' do
-                expect(json_response)
-                  .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."])
-              end
+            it 'renders json errors' do
+              expect(json_response)
+                .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."])
+            end
 
-              it 'returns 422 status' do
-                expect(response).to have_http_status(422)
-              end
+            it 'returns 422 status' do
+              expect(response).to have_http_status(422)
             end
           end
+        end
 
-          context 'when captcha is verified' do
-            let(:spammy_title) { 'Whatever' }
-            let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) }
+        context 'when captcha is verified' do
+          let(:spammy_title) { 'Whatever' }
+          let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) }
 
-            def update_verified_issue
-              update_issue({ title: spammy_title },
-                           { spam_log_id: spam_logs.last.id,
-                             recaptcha_verification: true })
-            end
+          def update_verified_issue
+            update_issue({ title: spammy_title },
+                         { spam_log_id: spam_logs.last.id,
+                           recaptcha_verification: true })
+          end
 
-            before do
-              allow_any_instance_of(described_class).to receive(:verify_recaptcha)
-                .and_return(true)
-            end
+          before do
+            allow_any_instance_of(described_class).to receive(:verify_recaptcha)
+              .and_return(true)
+          end
 
-            it 'redirect to issue page' do
-              update_verified_issue
+          it 'redirect to issue page' do
+            update_verified_issue
 
-              expect(response)
-                .to redirect_to(project_issue_path(project, issue))
-            end
+            expect(response)
+              .to redirect_to(project_issue_path(project, issue))
+          end
 
-            it 'accepts an issue after recaptcha is verified' do
-              expect{ update_verified_issue }.to change{ issue.reload.title }.to(spammy_title)
-            end
+          it 'accepts an issue after recaptcha is verified' do
+            expect { update_verified_issue }.to change { issue.reload.title }.to(spammy_title)
+          end
 
-            it 'marks spam log as recaptcha_verified' do
-              expect { update_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true)
-            end
+          it 'marks spam log as recaptcha_verified' do
+            expect { update_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true)
+          end
 
-            it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do
-              spam_log = create(:spam_log)
+          it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do
+            spam_log = create(:spam_log)
 
-              expect { update_issue(spam_log_id: spam_log.id, recaptcha_verification: true) }
-                .not_to change { SpamLog.last.recaptcha_verified }
-            end
+            expect { update_issue(spam_log_id: spam_log.id, recaptcha_verification: true) }
+              .not_to change { SpamLog.last.recaptcha_verified }
           end
         end
       end
@@ -385,13 +360,45 @@ describe Projects::IssuesController do
 
         put :update, params
       end
+    end
+  end
+
+  describe 'POST #move' do
+    before do
+      sign_in(user)
+      project.add_developer(user)
+    end
+
+    context 'when moving issue to another private project' do
+      let(:another_project) { create(:project, :private) }
+
+      context 'when user has access to move issue' do
+        before do
+          another_project.add_reporter(user)
+        end
+
+        it 'moves issue to another project' do
+          move_issue
+
+          expect(response).to have_http_status :ok
+          expect(another_project.issues).not_to be_empty
+        end
+      end
+
+      context 'when user does not have access to move issue' do
+        it 'responds with 404' do
+          move_issue
+
+          expect(response).to have_http_status :not_found
+        end
+      end
 
       def move_issue
-        put :update,
+        post :move,
+          format: :json,
           namespace_id: project.namespace.to_param,
           project_id: project,
           id: issue.iid,
-          issue: { title: 'New title' },
           move_to_project_id: another_project.id
       end
     end
@@ -672,7 +679,7 @@ describe Projects::IssuesController do
       context 'when an issue is not identified as spam' do
         before do
           allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false)
-          allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false)
+          allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false)
         end
 
         it 'does not create an issue' do
@@ -682,7 +689,7 @@ describe Projects::IssuesController do
 
       context 'when an issue is identified as spam' do
         before do
-          allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
+          allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true)
         end
 
         context 'when captcha is not verified' do
@@ -841,7 +848,7 @@ describe Projects::IssuesController do
   describe 'POST #toggle_award_emoji' do
     before do
       sign_in(user)
-      project.team << [user, :developer]
+      project.add_developer(user)
     end
 
     it "toggles the award emoji" do
@@ -855,6 +862,8 @@ describe Projects::IssuesController do
   end
 
   describe 'POST create_merge_request' do
+    let(:project) { create(:project, :repository) }
+
     before do
       project.add_developer(user)
       sign_in(user)
@@ -877,4 +886,19 @@ describe Projects::IssuesController do
                                   format: :json
     end
   end
+
+  describe 'GET #discussions' do
+    let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
+
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    it 'returns discussion json' do
+      get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid
+
+      expect(JSON.parse(response.body).first.keys).to match_array(%w[id reply_id expanded notes individual_note])
+    end
+  end
 end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index bb67db268fa..6775012bab5 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -56,6 +56,28 @@ describe Projects::MergeRequestsController do
 
         expect(response).to be_success
       end
+
+      context "loads notes" do
+        let(:first_contributor) { create(:user) }
+        let(:contributor) { create(:user) }
+        let(:merge_request) { create(:merge_request, author: first_contributor, target_project: project, source_project: project) }
+        let(:contributor_merge_request) { create(:merge_request, :merged, author: contributor, target_project: project, source_project: project) }
+        # the order here is important
+        # as the controller reloads these from DB, references doesn't correspond after
+        let!(:first_contributor_note) { create(:note, author: first_contributor, noteable: merge_request, project: project) }
+        let!(:contributor_note) { create(:note, author: contributor, noteable: merge_request, project: project) }
+        let!(:owner_note) { create(:note, author: user, noteable: merge_request, project: project) }
+
+        it "with special_role FIRST_TIME_CONTRIBUTOR" do
+          go(format: :html)
+
+          notes = assigns(:notes)
+          expect(notes).to match(a_collection_containing_exactly(an_object_having_attributes(special_role: Note::SpecialRole::FIRST_TIME_CONTRIBUTOR),
+                                                                 an_object_having_attributes(special_role: nil),
+                                                                 an_object_having_attributes(special_role: nil)
+                                                                ))
+        end
+      end
     end
 
     describe 'as json' do
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index f280c55059c..6ffe41b8608 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -46,10 +46,13 @@ describe Projects::NotesController do
     end
 
     context 'for a discussion note' do
-      let!(:note) { create(:discussion_note_on_issue, noteable: issue, project: project) }
+      let(:project) { create(:project, :repository) }
+      let!(:note) { create(:discussion_note_on_merge_request, project: project) }
+
+      let(:params) { request_params.merge(target_type: 'merge_request', target_id: note.noteable_id) }
 
       it 'responds with the expected attributes' do
-        get :index, request_params
+        get :index, params
 
         expect(note_json[:id]).to eq(note.id)
         expect(note_json[:discussion_html]).not_to be_nil
@@ -104,10 +107,12 @@ describe Projects::NotesController do
     end
 
     context 'for a regular note' do
-      let!(:note) { create(:note, noteable: issue, project: project) }
+      let!(:note) { create(:note_on_merge_request, project: project) }
+
+      let(:params) { request_params.merge(target_type: 'merge_request', target_id: note.noteable_id) }
 
       it 'responds with the expected attributes' do
-        get :index, request_params
+        get :index, params
 
         expect(note_json[:id]).to eq(note.id)
         expect(note_json[:html]).not_to be_nil
@@ -125,7 +130,9 @@ describe Projects::NotesController do
         note: { note: 'some note', noteable_id: merge_request.id, noteable_type: 'MergeRequest' },
         namespace_id: project.namespace,
         project_id: project,
-        merge_request_diff_head_sha: 'sha'
+        merge_request_diff_head_sha: 'sha',
+        target_type: 'merge_request',
+        target_id: merge_request.id
       }
     end
 
diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb
index 4d0111302f3..83c7744a231 100644
--- a/spec/controllers/projects/pages_controller_spec.rb
+++ b/spec/controllers/projects/pages_controller_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Projects::PagesController do
   let(:user) { create(:user) }
-  let(:project) { create(:project, :public, :access_requestable) }
+  let(:project) { create(:project, :public) }
 
   let(:request_params) do
     {
@@ -23,6 +23,17 @@ describe Projects::PagesController do
 
       expect(response).to have_http_status(200)
     end
+
+    context 'when the project is in a subgroup' do
+      let(:group) { create(:group, :nested) }
+      let(:project) { create(:project, namespace: group) }
+
+      it 'returns a 404 status code' do
+        get :show, request_params
+
+        expect(response).to have_http_status(404)
+      end
+    end
   end
 
   describe 'DELETE destroy' do
diff --git a/spec/controllers/projects/protected_tags_controller_spec.rb b/spec/controllers/projects/protected_tags_controller_spec.rb
index 64658988b3f..b6de90039f3 100644
--- a/spec/controllers/projects/protected_tags_controller_spec.rb
+++ b/spec/controllers/projects/protected_tags_controller_spec.rb
@@ -8,4 +8,21 @@ describe Projects::ProtectedTagsController do
       get(:index, namespace_id: project.namespace.to_param, project_id: project)
     end
   end
+
+  describe "DELETE #destroy" do
+    let(:project) { create(:project, :repository) }
+    let(:protected_tag) { create(:protected_tag, :developers_can_create, project: project) }
+    let(:user) { create(:user) }
+
+    before do
+      project.add_master(user)
+      sign_in(user)
+    end
+
+    it "deletes the protected tag" do
+      delete(:destroy, namespace_id: project.namespace.to_param, project_id: project, id: protected_tag.id)
+
+      expect { ProtectedTag.find(protected_tag.id) }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
 end
diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb
index 4e9b0c09ff2..efba9cc7306 100644
--- a/spec/controllers/projects/services_controller_spec.rb
+++ b/spec/controllers/projects/services_controller_spec.rb
@@ -10,9 +10,6 @@ describe Projects::ServicesController do
   before do
     sign_in(user)
     project.team << [user, :master]
-
-    controller.instance_variable_set(:@project, project)
-    controller.instance_variable_set(:@service, service)
   end
 
   describe '#test' do
@@ -20,7 +17,7 @@ describe Projects::ServicesController do
       it 'renders 404' do
         allow_any_instance_of(Service).to receive(:can_test?).and_return(false)
 
-        put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id
+        put :test, namespace_id: project.namespace, project_id: project, id: service.to_param
 
         expect(response).to have_http_status(404)
       end
@@ -36,7 +33,7 @@ describe Projects::ServicesController do
           it 'returns success' do
             allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true)
 
-            put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id
+            put :test, namespace_id: project.namespace, project_id: project, id: service.to_param
 
             expect(response.status).to eq(200)
           end
@@ -45,7 +42,7 @@ describe Projects::ServicesController do
         it 'returns success' do
           expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client)
 
-          put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
+          put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params
 
           expect(response.status).to eq(200)
         end
@@ -54,17 +51,42 @@ describe Projects::ServicesController do
       it 'returns success' do
         expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client)
 
-        put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
+        put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params
 
         expect(response.status).to eq(200)
       end
+
+      context 'when service is configured for the first time' do
+        before do
+          allow_any_instance_of(ServiceHook).to receive(:execute).and_return(true)
+        end
+
+        it 'persist the object' do
+          do_put
+
+          expect(BuildkiteService.first).to be_present
+        end
+
+        it 'creates the ServiceHook object' do
+          do_put
+
+          expect(BuildkiteService.first.service_hook).to be_present
+        end
+
+        def do_put
+          put :test, namespace_id: project.namespace,
+                     project_id: project,
+                     id: 'buildkite',
+                     service: { 'active' => '1', 'push_events' => '1', token: 'token', 'project_url' => 'http://test.com' }
+        end
+      end
     end
 
     context 'failure' do
       it 'returns success status code and the error message' do
         expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_raise('Bad test')
 
-        put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
+        put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params
 
         expect(response.status).to eq(200)
         expect(JSON.parse(response.body))
@@ -77,7 +99,7 @@ describe Projects::ServicesController do
     context 'when param `active` is set to true' do
       it 'activates the service and redirects to integrations paths' do
         put :update,
-          namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: true }
+          namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: true }
 
         expect(response).to redirect_to(project_settings_integrations_path(project))
         expect(flash[:notice]).to eq 'HipChat activated.'
@@ -87,7 +109,7 @@ describe Projects::ServicesController do
     context 'when param `active` is set to false' do
       it 'does not  activate the service but saves the settings' do
         put :update,
-          namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: false }
+          namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: false }
 
         expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.'
       end
diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb
index cc444f31797..3a1550aa730 100644
--- a/spec/controllers/projects/snippets_controller_spec.rb
+++ b/spec/controllers/projects/snippets_controller_spec.rb
@@ -98,7 +98,7 @@ describe Projects::SnippetsController do
 
     context 'when the snippet is spam' do
       before do
-        allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
+        allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true)
       end
 
       context 'when the snippet is private' do
@@ -176,7 +176,7 @@ describe Projects::SnippetsController do
 
     context 'when the snippet is spam' do
       before do
-        allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
+        allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true)
       end
 
       context 'when the snippet is private' do
diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb
index 974330e2bbd..41d211ed1bb 100644
--- a/spec/controllers/projects/todos_controller_spec.rb
+++ b/spec/controllers/projects/todos_controller_spec.rb
@@ -67,7 +67,7 @@ describe Projects::TodosController do
         end
 
         it "doesn't create todo" do
-          expect{ go }.not_to change { user.todos.count }
+          expect { go }.not_to change { user.todos.count }
           expect(response).to have_http_status(404)
         end
       end
@@ -135,7 +135,7 @@ describe Projects::TodosController do
         end
 
         it "doesn't create todo" do
-          expect{ go }.not_to change { user.todos.count }
+          expect { go }.not_to change { user.todos.count }
           expect(response).to have_http_status(404)
         end
       end