Merge branch 'master' into 41249-clearing-the-cache

48 files changed, 180 insertions, 109 deletions

diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index f044a068938..f350641a643 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -13,7 +13,7 @@ describe Admin::UsersController do
     let!(:issue) { create(:issue, author: user) }
 
     before do
-      project.team << [user, :developer]
+      project.add_developer(user)
     end
 
     it 'deletes user and ghosts their contributions' do
diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb
index 44d504d5852..79bbc29e80d 100644
--- a/spec/controllers/boards/issues_controller_spec.rb
+++ b/spec/controllers/boards/issues_controller_spec.rb
@@ -13,8 +13,8 @@ describe Boards::IssuesController do
   let!(:list2) { create(:list, board: board, label: development, position: 1) }
 
   before do
-    project.team << [user, :master]
-    project.team << [guest, :guest]
+    project.add_master(user)
+    project.add_guest(guest)
   end
 
   describe 'GET index' do
@@ -221,7 +221,7 @@ describe Boards::IssuesController do
       let(:guest) { create(:user) }
 
       before do
-        project.team << [guest, :guest]
+        project.add_guest(guest)
       end
 
       it 'returns a forbidden 403 response' do
diff --git a/spec/controllers/boards/lists_controller_spec.rb b/spec/controllers/boards/lists_controller_spec.rb
index a2b432af23a..71d45a22d91 100644
--- a/spec/controllers/boards/lists_controller_spec.rb
+++ b/spec/controllers/boards/lists_controller_spec.rb
@@ -7,8 +7,8 @@ describe Boards::ListsController do
   let(:guest)   { create(:user) }
 
   before do
-    project.team << [user, :master]
-    project.team << [guest, :guest]
+    project.add_master(user)
+    project.add_guest(guest)
   end
 
   describe 'GET index' do
diff --git a/spec/controllers/dashboard/milestones_controller_spec.rb b/spec/controllers/dashboard/milestones_controller_spec.rb
index 2f3d7be9abe..60547db82b6 100644
--- a/spec/controllers/dashboard/milestones_controller_spec.rb
+++ b/spec/controllers/dashboard/milestones_controller_spec.rb
@@ -17,7 +17,7 @@ describe Dashboard::MilestonesController do
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   it_behaves_like 'milestone tabs'
diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb
index f9faa4fa59a..b4a731fd3a3 100644
--- a/spec/controllers/dashboard/todos_controller_spec.rb
+++ b/spec/controllers/dashboard/todos_controller_spec.rb
@@ -8,7 +8,7 @@ describe Dashboard::TodosController do
 
   before do
     sign_in(user)
-    project.team << [user, :developer]
+    project.add_developer(user)
   end
 
   describe 'GET #index' do
diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb
index 566d8515198..97c2c3fb940 100644
--- a/spec/controllers/dashboard_controller_spec.rb
+++ b/spec/controllers/dashboard_controller_spec.rb
@@ -5,7 +5,7 @@ describe DashboardController do
   let(:project) { create(:project) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
     sign_in(user)
   end
 
diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb
index c1aba46be04..733386500ca 100644
--- a/spec/controllers/groups/milestones_controller_spec.rb
+++ b/spec/controllers/groups/milestones_controller_spec.rb
@@ -28,7 +28,7 @@ describe Groups::MilestonesController do
   before do
     sign_in(user)
     group.add_owner(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   describe '#index' do
diff --git a/spec/controllers/notification_settings_controller_spec.rb b/spec/controllers/notification_settings_controller_spec.rb
index 9014b8b5084..e133950e684 100644
--- a/spec/controllers/notification_settings_controller_spec.rb
+++ b/spec/controllers/notification_settings_controller_spec.rb
@@ -6,7 +6,7 @@ describe NotificationSettingsController do
   let(:user) { create(:user) }
 
   before do
-    project.team << [user, :developer]
+    project.add_developer(user)
   end
 
   describe '#create' do
diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb
index d1051741430..12cb7b2647f 100644
--- a/spec/controllers/projects/artifacts_controller_spec.rb
+++ b/spec/controllers/projects/artifacts_controller_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Projects::ArtifactsController do
-  set(:user) { create(:user) }
+  let(:user) { project.owner }
   set(:project) { create(:project, :repository, :public) }
 
   let(:pipeline) do
@@ -15,14 +15,12 @@ describe Projects::ArtifactsController do
   let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
 
   before do
-    project.add_developer(user)
-
     sign_in(user)
   end
 
   describe 'GET download' do
     it 'sends the artifacts file' do
-      expect(controller).to receive(:send_file).with(job.artifacts_file.path, disposition: 'attachment').and_call_original
+      expect(controller).to receive(:send_file).with(job.artifacts_file.path, hash_including(disposition: 'attachment')).and_call_original
 
       get :download, namespace_id: project.namespace, project_id: project, job_id: job
     end
@@ -113,20 +111,43 @@ describe Projects::ArtifactsController do
   end
 
   describe 'GET raw' do
+    subject { get(:raw, namespace_id: project.namespace, project_id: project, job_id: job, path: path) }
+
     context 'when the file exists' do
-      it 'serves the file using workhorse' do
-        get :raw, namespace_id: project.namespace, project_id: project, job_id: job, path: 'ci_artifacts.txt'
+      let(:path) { 'ci_artifacts.txt' }
 
-        send_data = response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]
+      shared_examples 'a valid file' do
+        it 'serves the file using workhorse' do
+          subject
 
-        expect(send_data).to start_with('artifacts-entry:')
+          expect(response).to have_gitlab_http_status(200)
+          expect(send_data).to start_with('artifacts-entry:')
 
-        base64_params = send_data.sub(/\Aartifacts\-entry:/, '')
-        params = JSON.parse(Base64.urlsafe_decode64(base64_params))
+          expect(params.keys).to eq(%w(Archive Entry))
+          expect(params['Archive']).to start_with(archive_path)
+          # On object storage, the URL can end with a query string
+          expect(params['Archive']).to match(/build_artifacts.zip(\?[^?]+)?$/)
+          expect(params['Entry']).to eq(Base64.encode64('ci_artifacts.txt'))
+        end
+
+        def send_data
+          response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]
+        end
 
-        expect(params.keys).to eq(%w(Archive Entry))
-        expect(params['Archive']).to end_with('build_artifacts.zip')
-        expect(params['Entry']).to eq(Base64.encode64('ci_artifacts.txt'))
+        def params
+          @params ||= begin
+            base64_params = send_data.sub(/\Aartifacts\-entry:/, '')
+            JSON.parse(Base64.urlsafe_decode64(base64_params))
+          end
+        end
+      end
+
+      context 'when using local file storage' do
+        it_behaves_like 'a valid file' do
+          let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
+          let(:store) { ObjectStoreUploader::LOCAL_STORE }
+          let(:archive_path) { JobArtifactUploader.local_store_path }
+        end
       end
     end
   end
diff --git a/spec/controllers/projects/avatars_controller_spec.rb b/spec/controllers/projects/avatars_controller_spec.rb
index f5ea097af8b..3bbe168f6d5 100644
--- a/spec/controllers/projects/avatars_controller_spec.rb
+++ b/spec/controllers/projects/avatars_controller_spec.rb
@@ -6,7 +6,7 @@ describe Projects::AvatarsController do
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
     controller.instance_variable_set(:@project, project)
   end
 
diff --git a/spec/controllers/projects/blame_controller_spec.rb b/spec/controllers/projects/blame_controller_spec.rb
index 54282aa4001..88d4f4e9cd0 100644
--- a/spec/controllers/projects/blame_controller_spec.rb
+++ b/spec/controllers/projects/blame_controller_spec.rb
@@ -7,7 +7,7 @@ describe Projects::BlameController do
   before do
     sign_in(user)
 
-    project.team << [user, :master]
+    project.add_master(user)
     controller.instance_variable_set(:@project, project)
   end
 
diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb
index 6a1c07b4a0b..00a7df6ccc8 100644
--- a/spec/controllers/projects/blob_controller_spec.rb
+++ b/spec/controllers/projects/blob_controller_spec.rb
@@ -89,7 +89,7 @@ describe Projects::BlobController do
     end
 
     before do
-      project.team << [user, :master]
+      project.add_master(user)
 
       sign_in(user)
     end
@@ -147,7 +147,7 @@ describe Projects::BlobController do
       let(:developer) { create(:user) }
 
       before do
-        project.team << [developer, :developer]
+        project.add_developer(developer)
         sign_in(developer)
         get :edit, default_params
       end
@@ -161,7 +161,7 @@ describe Projects::BlobController do
       let(:master) { create(:user) }
 
       before do
-        project.team << [master, :master]
+        project.add_master(master)
         sign_in(master)
         get :edit, default_params
       end
@@ -190,7 +190,7 @@ describe Projects::BlobController do
     end
 
     before do
-      project.team << [user, :master]
+      project.add_master(user)
 
       sign_in(user)
     end
diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb
index d6ccb92c54b..305af289531 100644
--- a/spec/controllers/projects/boards_controller_spec.rb
+++ b/spec/controllers/projects/boards_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::BoardsController do
   let(:user)    { create(:user) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
     sign_in(user)
   end
 
diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb
index d731200f70f..734396ddf7b 100644
--- a/spec/controllers/projects/branches_controller_spec.rb
+++ b/spec/controllers/projects/branches_controller_spec.rb
@@ -6,8 +6,8 @@ describe Projects::BranchesController do
   let(:developer) { create(:user) }
 
   before do
-    project.team << [user, :master]
-    project.team << [user, :developer]
+    project.add_master(user)
+    project.add_developer(user)
 
     allow(project).to receive(:branches).and_return(['master', 'foo/bar/baz'])
     allow(project).to receive(:tags).and_return(['v1.0.0', 'v2.0.0'])
@@ -148,6 +148,20 @@ describe Projects::BranchesController do
         end
       end
 
+      context 'when create branch service fails' do
+        let(:branch) { "./invalid-branch-name" }
+
+        it "doesn't post a system note" do
+          expect(SystemNoteService).not_to receive(:new_issue_branch)
+
+          post :create,
+            namespace_id: project.namespace,
+            project_id: project,
+            branch_name: branch,
+            issue_iid: issue.iid
+        end
+      end
+
       context 'without issue feature access' do
         before do
           project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb
index c459d732507..73fb90d73ec 100644
--- a/spec/controllers/projects/commits_controller_spec.rb
+++ b/spec/controllers/projects/commits_controller_spec.rb
@@ -6,7 +6,7 @@ describe Projects::CommitsController do
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   describe "GET show" do
diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb
index fe5818da0bc..046ce027965 100644
--- a/spec/controllers/projects/compare_controller_spec.rb
+++ b/spec/controllers/projects/compare_controller_spec.rb
@@ -8,7 +8,7 @@ describe Projects::CompareController do
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   it 'compare shows some diffs' do
diff --git a/spec/controllers/projects/cycle_analytics_controller_spec.rb b/spec/controllers/projects/cycle_analytics_controller_spec.rb
index 6fae52edbad..7c708a418a7 100644
--- a/spec/controllers/projects/cycle_analytics_controller_spec.rb
+++ b/spec/controllers/projects/cycle_analytics_controller_spec.rb
@@ -6,7 +6,7 @@ describe Projects::CycleAnalyticsController do
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   describe 'cycle analytics not set up flag' do
diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb
index c3208357694..97db69427e9 100644
--- a/spec/controllers/projects/deploy_keys_controller_spec.rb
+++ b/spec/controllers/projects/deploy_keys_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::DeployKeysController do
   let(:user) { create(:user) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
 
     sign_in(user)
   end
@@ -48,7 +48,7 @@ describe Projects::DeployKeysController do
       end
 
       before do
-        project2.team << [user, :developer]
+        project2.add_developer(user)
       end
 
       it 'returns json in a correct format' do
diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb
index 3164fd5c143..73e7921fab7 100644
--- a/spec/controllers/projects/deployments_controller_spec.rb
+++ b/spec/controllers/projects/deployments_controller_spec.rb
@@ -8,7 +8,7 @@ describe Projects::DeploymentsController do
   let(:environment) { create(:environment, name: 'production', project: project) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
 
     sign_in(user)
   end
diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb
index 3bf676637a2..00328d3ea51 100644
--- a/spec/controllers/projects/discussions_controller_spec.rb
+++ b/spec/controllers/projects/discussions_controller_spec.rb
@@ -31,7 +31,7 @@ describe Projects::DiscussionsController do
 
     context "when the user is authorized to resolve the discussion" do
       before do
-        project.team << [user, :developer]
+        project.add_developer(user)
       end
 
       context "when the discussion is not resolvable" do
@@ -92,7 +92,7 @@ describe Projects::DiscussionsController do
 
     context "when the user is authorized to resolve the discussion" do
       before do
-        project.team << [user, :developer]
+        project.add_developer(user)
       end
 
       context "when the discussion is not resolvable" do
diff --git a/spec/controllers/projects/find_file_controller_spec.rb b/spec/controllers/projects/find_file_controller_spec.rb
index 6a5433bcc9c..505fe82851a 100644
--- a/spec/controllers/projects/find_file_controller_spec.rb
+++ b/spec/controllers/projects/find_file_controller_spec.rb
@@ -7,7 +7,7 @@ describe Projects::FindFileController do
   before do
     sign_in(user)
 
-    project.team << [user, :master]
+    project.add_master(user)
     controller.instance_variable_set(:@project, project)
   end
 
diff --git a/spec/controllers/projects/forks_controller_spec.rb b/spec/controllers/projects/forks_controller_spec.rb
index 1bedb8ebdff..c4b32dc3a09 100644
--- a/spec/controllers/projects/forks_controller_spec.rb
+++ b/spec/controllers/projects/forks_controller_spec.rb
@@ -51,7 +51,7 @@ describe Projects::ForksController do
 
         context 'when user is a member of the Project' do
           before do
-            forked_project.team << [project.creator, :developer]
+            forked_project.add_developer(project.creator)
           end
 
           it 'sees the project listed' do
diff --git a/spec/controllers/projects/graphs_controller_spec.rb b/spec/controllers/projects/graphs_controller_spec.rb
index 5af03ae118c..c3605555fe7 100644
--- a/spec/controllers/projects/graphs_controller_spec.rb
+++ b/spec/controllers/projects/graphs_controller_spec.rb
@@ -6,7 +6,7 @@ describe Projects::GraphsController do
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   describe 'GET languages' do
diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb
index f8c792cd0f0..5bfc3d31401 100644
--- a/spec/controllers/projects/group_links_controller_spec.rb
+++ b/spec/controllers/projects/group_links_controller_spec.rb
@@ -7,7 +7,7 @@ describe Projects::GroupLinksController do
   let(:user) { create(:user) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
     sign_in(user)
   end
 
diff --git a/spec/controllers/projects/hooks_controller_spec.rb b/spec/controllers/projects/hooks_controller_spec.rb
index 07174660f46..aba70c6d4c1 100644
--- a/spec/controllers/projects/hooks_controller_spec.rb
+++ b/spec/controllers/projects/hooks_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::HooksController do
   let(:user) { create(:user) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
     sign_in(user)
   end
 
diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb
index 2a5ec6d584b..7fb4c1b7425 100644
--- a/spec/controllers/projects/imports_controller_spec.rb
+++ b/spec/controllers/projects/imports_controller_spec.rb
@@ -9,7 +9,7 @@ describe Projects::ImportsController do
 
       before do
         sign_in(user)
-        project.team << [user, :master]
+        project.add_master(user)
       end
 
       it 'renders template' do
@@ -30,7 +30,7 @@ describe Projects::ImportsController do
 
       before do
         sign_in(user)
-        project.team << [user, :master]
+        project.add_master(user)
       end
 
       context 'when import is in progress' do
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index a2ef937609b..6b7db947216 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -37,7 +37,7 @@ describe Projects::IssuesController do
     context 'internal issue tracker' do
       before do
         sign_in(user)
-        project.team << [user, :developer]
+        project.add_developer(user)
       end
 
       it_behaves_like "issuables list meta-data", :issue
@@ -69,7 +69,7 @@ describe Projects::IssuesController do
 
       before do
         sign_in(user)
-        project.team << [user, :developer]
+        project.add_developer(user)
         allow(Kaminari.config).to receive(:default_per_page).and_return(1)
       end
 
@@ -116,7 +116,7 @@ describe Projects::IssuesController do
     context 'internal issue tracker' do
       before do
         sign_in(user)
-        project.team << [user, :developer]
+        project.add_developer(user)
       end
 
       it 'builds a new issue' do
@@ -127,7 +127,7 @@ describe Projects::IssuesController do
 
       it 'fills in an issue for a merge request' do
         project_with_repository = create(:project, :repository)
-        project_with_repository.team << [user, :developer]
+        project_with_repository.add_developer(user)
         mr = create(:merge_request_with_diff_notes, source_project: project_with_repository)
 
         get :new, namespace_id: project_with_repository.namespace, project_id: project_with_repository, merge_request_to_resolve_discussions_of: mr.iid
@@ -153,7 +153,7 @@ describe Projects::IssuesController do
 
       before do
         sign_in(user)
-        project.team << [user, :developer]
+        project.add_developer(user)
 
         external = double
         allow(project).to receive(:external_issue_tracker).and_return(external)
@@ -329,7 +329,7 @@ describe Projects::IssuesController do
 
       it 'does not list confidential issues for project members with guest role' do
         sign_in(member)
-        project.team << [member, :guest]
+        project.add_guest(member)
 
         get_issues
 
@@ -354,7 +354,7 @@ describe Projects::IssuesController do
 
       it 'lists confidential issues for project members' do
         sign_in(member)
-        project.team << [member, :developer]
+        project.add_developer(member)
 
         get_issues
 
@@ -394,7 +394,7 @@ describe Projects::IssuesController do
 
       it 'returns 404 for project members with guest role' do
         sign_in(member)
-        project.team << [member, :guest]
+        project.add_guest(member)
         go(id: unescaped_parameter_value.to_param)
 
         expect(response).to have_gitlab_http_status :not_found
@@ -416,7 +416,7 @@ describe Projects::IssuesController do
 
       it "returns #{http_status[:success]} for project members" do
         sign_in(member)
-        project.team << [member, :developer]
+        project.add_developer(member)
         go(id: unescaped_parameter_value.to_param)
 
         expect(response).to have_gitlab_http_status http_status[:success]
@@ -450,7 +450,7 @@ describe Projects::IssuesController do
 
       before do
         sign_in(user)
-        project.team << [user, :developer]
+        project.add_developer(user)
       end
 
       it_behaves_like 'restricted action', success: 200
@@ -594,7 +594,7 @@ describe Projects::IssuesController do
         let(:deleted_user) { create(:user) }
 
         before do
-          project.team << [user, :developer]
+          project.add_developer(user)
 
           issue.update!(last_edited_by: deleted_user, last_edited_at: Time.now)
 
@@ -638,7 +638,7 @@ describe Projects::IssuesController do
     def post_new_issue(issue_attrs = {}, additional_params = {})
       sign_in(user)
       project = create(:project, :public)
-      project.team << [user, :developer]
+      project.add_developer(user)
 
       post :create, {
         namespace_id: project.namespace.to_param,
@@ -655,7 +655,7 @@ describe Projects::IssuesController do
       let(:project) { merge_request.source_project }
 
       before do
-        project.team << [user, :master]
+        project.add_master(user)
         sign_in user
       end
 
@@ -829,7 +829,7 @@ describe Projects::IssuesController do
       def post_spam
         admin = create(:admin)
         create(:user_agent_detail, subject: issue)
-        project.team << [admin, :master]
+        project.add_master(admin)
         sign_in(admin)
         post :mark_as_spam, {
           namespace_id: project.namespace,
diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb
index 7490f8fefce..e6a4e7c8257 100644
--- a/spec/controllers/projects/jobs_controller_spec.rb
+++ b/spec/controllers/projects/jobs_controller_spec.rb
@@ -374,7 +374,7 @@ describe Projects::JobsController do
     let(:role) { :master }
 
     before do
-      project.team << [user, role]
+      project.add_role(user, role)
       sign_in(user)
 
       post_erase
diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb
index cf83f2f3265..452d7e23983 100644
--- a/spec/controllers/projects/labels_controller_spec.rb
+++ b/spec/controllers/projects/labels_controller_spec.rb
@@ -6,7 +6,7 @@ describe Projects::LabelsController do
   let(:user)    { create(:user) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
 
     sign_in(user)
   end
diff --git a/spec/controllers/projects/mattermosts_controller_spec.rb b/spec/controllers/projects/mattermosts_controller_spec.rb
index 33d48ff94d1..c5ac0be27bb 100644
--- a/spec/controllers/projects/mattermosts_controller_spec.rb
+++ b/spec/controllers/projects/mattermosts_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::MattermostsController do
   let!(:user) { create(:user) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
     sign_in(user)
   end
 
diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb
index 7fdddc02fd3..7e2366847f4 100644
--- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb
@@ -6,7 +6,7 @@ describe Projects::MergeRequests::CreationsController do
   let(:fork_project) { create(:forked_project_with_submodules) }
 
   before do
-    fork_project.team << [user, :master]
+    fork_project.add_master(user)
 
     sign_in(user)
   end
@@ -86,7 +86,7 @@ describe Projects::MergeRequests::CreationsController do
       let(:other_project) { create(:project, :repository) }
 
       before do
-        other_project.team << [user, :master]
+        other_project.add_master(user)
       end
 
       context 'when the path exists in the diff' do
diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb
index ba97ccfbbd4..5d297c654bf 100644
--- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb
@@ -151,7 +151,7 @@ describe Projects::MergeRequests::DiffsController do
       let(:other_project) { create(:project) }
 
       before do
-        other_project.team << [user, :master]
+        other_project.add_master(user)
         diff_for_path(old_path: existing_path, new_path: existing_path, project_id: other_project)
       end
 
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index 209979e642d..00cf464ec5b 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -11,7 +11,7 @@ describe Projects::MilestonesController do
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
     controller.instance_variable_set(:@project, project)
   end
 
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 37e9f863fc4..de132dfaa21 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -32,7 +32,7 @@ describe Projects::NotesController do
 
     before do
       sign_in(user)
-      project.team << [user, :developer]
+      project.add_developer(user)
     end
 
     it 'passes last_fetched_at from headers to NotesFinder' do
@@ -351,7 +351,7 @@ describe Projects::NotesController do
 
     before do
       sign_in(note.author)
-      project.team << [note.author, :developer]
+      project.add_developer(note.author)
     end
 
     it "updates the note" do
@@ -372,7 +372,7 @@ describe Projects::NotesController do
     context 'user is the author of a note' do
       before do
         sign_in(note.author)
-        project.team << [note.author, :developer]
+        project.add_developer(note.author)
       end
 
       it "returns status 200 for html" do
@@ -389,7 +389,7 @@ describe Projects::NotesController do
     context 'user is not the author of a note' do
       before do
         sign_in(user)
-        project.team << [user, :developer]
+        project.add_developer(user)
       end
 
       it "returns status 404" do
@@ -403,7 +403,7 @@ describe Projects::NotesController 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
@@ -445,7 +445,7 @@ describe Projects::NotesController do
 
       context "when the user is authorized to resolve the note" do
         before do
-          project.team << [user, :developer]
+          project.add_developer(user)
         end
 
         context "when the note is not resolvable" do
@@ -506,7 +506,7 @@ describe Projects::NotesController do
 
       context "when the user is authorized to resolve the note" do
         before do
-          project.team << [user, :developer]
+          project.add_developer(user)
         end
 
         context "when the note is not resolvable" do
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index 290dba0610a..46b08a03b19 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -21,7 +21,7 @@ describe Projects::ProjectMembersController do
 
     context 'when user does not have enough rights' do
       before do
-        project.team << [user, :developer]
+        project.add_developer(user)
       end
 
       it 'returns 404' do
@@ -37,7 +37,7 @@ describe Projects::ProjectMembersController do
 
     context 'when user has enough rights' do
       before do
-        project.team << [user, :master]
+        project.add_master(user)
       end
 
       it 'adds user to members' do
@@ -106,7 +106,7 @@ describe Projects::ProjectMembersController do
     context 'when member is found' do
       context 'when user does not have enough rights' do
         before do
-          project.team << [user, :developer]
+          project.add_developer(user)
         end
 
         it 'returns 404' do
@@ -121,7 +121,7 @@ describe Projects::ProjectMembersController do
 
       context 'when user has enough rights' do
         before do
-          project.team << [user, :master]
+          project.add_master(user)
         end
 
         it '[HTML] removes user from members' do
@@ -164,7 +164,7 @@ describe Projects::ProjectMembersController do
     context 'when member is found' do
       context 'and is not an owner' do
         before do
-          project.team << [user, :developer]
+          project.add_developer(user)
         end
 
         it 'removes user from members' do
@@ -181,7 +181,7 @@ describe Projects::ProjectMembersController do
         let(:project) { create(:project, namespace: user.namespace) }
 
         before do
-          project.team << [user, :master]
+          project.add_master(user)
         end
 
         it 'cannot remove himself from the project' do
@@ -248,7 +248,7 @@ describe Projects::ProjectMembersController do
     context 'when member is found' do
       context 'when user does not have enough rights' do
         before do
-          project.team << [user, :developer]
+          project.add_developer(user)
         end
 
         it 'returns 404' do
@@ -263,7 +263,7 @@ describe Projects::ProjectMembersController do
 
       context 'when user has enough rights' do
         before do
-          project.team << [user, :master]
+          project.add_master(user)
         end
 
         it 'adds user to members' do
@@ -285,8 +285,8 @@ describe Projects::ProjectMembersController do
     let(:member) { create(:user) }
 
     before do
-      project.team << [user, :master]
-      another_project.team << [member, :guest]
+      project.add_master(user)
+      another_project.add_guest(member)
       sign_in(user)
     end
 
@@ -300,7 +300,7 @@ describe Projects::ProjectMembersController do
 
     context 'when user can access source project members' do
       before do
-        another_project.team << [user, :guest]
+        another_project.add_guest(user)
       end
 
       include_context 'import applied'
@@ -332,7 +332,7 @@ describe Projects::ProjectMembersController do
 
     context 'when creating owner' do
       before do
-        project.team << [user, :master]
+        project.add_master(user)
         sign_in(user)
       end
 
@@ -348,7 +348,7 @@ describe Projects::ProjectMembersController do
 
     context 'when create master' do
       before do
-        project.team << [user, :master]
+        project.add_master(user)
         sign_in(user)
       end
 
diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb
index 748ae040928..ceaffd92623 100644
--- a/spec/controllers/projects/refs_controller_spec.rb
+++ b/spec/controllers/projects/refs_controller_spec.rb
@@ -6,7 +6,7 @@ describe Projects::RefsController do
 
   before do
     sign_in(user)
-    project.team << [user, :developer]
+    project.add_developer(user)
   end
 
   describe 'GET #logs_tree' do
diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb
index 358f26dfb02..fc1619acec6 100644
--- a/spec/controllers/projects/releases_controller_spec.rb
+++ b/spec/controllers/projects/releases_controller_spec.rb
@@ -7,7 +7,7 @@ describe Projects::ReleasesController do
   let!(:tag)     { release.tag }
 
   before do
-    project.team << [user, :developer]
+    project.add_developer(user)
     sign_in(user)
   end
 
diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb
index 8b777eb68ca..04d16e98913 100644
--- a/spec/controllers/projects/repositories_controller_spec.rb
+++ b/spec/controllers/projects/repositories_controller_spec.rb
@@ -17,7 +17,7 @@ describe Projects::RepositoriesController do
       let(:user) { create(:user) }
 
       before do
-        project.team << [user, :developer]
+        project.add_developer(user)
         sign_in(user)
       end
 
diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb
index a907da2b60f..847ac6f2be0 100644
--- a/spec/controllers/projects/services_controller_spec.rb
+++ b/spec/controllers/projects/services_controller_spec.rb
@@ -9,7 +9,7 @@ describe Projects::ServicesController do
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   describe '#test' do
@@ -114,5 +114,41 @@ describe Projects::ServicesController do
         expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.'
       end
     end
+
+    context 'with a deprecated service' do
+      let(:service) { create(:kubernetes_service, project: project) }
+
+      before do
+        put :update,
+          namespace_id: project.namespace, project_id: project, id: service.to_param, service: { namespace: 'updated_namespace' }
+      end
+
+      it 'should not update the service' do
+        service.reload
+        expect(service.namespace).not_to eq('updated_namespace')
+      end
+    end
+  end
+
+  describe "GET #edit" do
+    before do
+      get :edit, namespace_id: project.namespace, project_id: project, id: service_id
+    end
+
+    context 'with approved services' do
+      let(:service_id) { 'jira' }
+
+      it 'should render edit page' do
+        expect(response).to be_success
+      end
+    end
+
+    context 'with a deprecated service' do
+      let(:service_id) { 'kubernetes' }
+
+      it 'should render edit page' do
+        expect(response).to be_success
+      end
+    end
   end
 end
diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
index acd40f4a305..e8ba04c7f24 100644
--- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb
+++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::Settings::CiCdController do
   let(:user) { create(:user) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
     sign_in(user)
   end
 
diff --git a/spec/controllers/projects/settings/integrations_controller_spec.rb b/spec/controllers/projects/settings/integrations_controller_spec.rb
index 3068837f394..77df9a6f567 100644
--- a/spec/controllers/projects/settings/integrations_controller_spec.rb
+++ b/spec/controllers/projects/settings/integrations_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::Settings::IntegrationsController do
   let(:user) { create(:user) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
     sign_in(user)
   end
 
diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb
index 70e7f9ca96e..8fcfa3c9ecd 100644
--- a/spec/controllers/projects/templates_controller_spec.rb
+++ b/spec/controllers/projects/templates_controller_spec.rb
@@ -8,7 +8,7 @@ describe Projects::TemplatesController do
   let(:body) { JSON.parse(response.body) }
 
   before do
-    project.team << [user, :developer]
+    project.add_developer(user)
     sign_in(user)
   end
 
diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb
index 4622e27e60f..e2524be7724 100644
--- a/spec/controllers/projects/todos_controller_spec.rb
+++ b/spec/controllers/projects/todos_controller_spec.rb
@@ -20,7 +20,7 @@ describe Projects::TodosController do
       context 'when authorized' do
         before do
           sign_in(user)
-          project.team << [user, :developer]
+          project.add_developer(user)
         end
 
         it 'creates todo for issue' do
@@ -88,7 +88,7 @@ describe Projects::TodosController do
       context 'when authorized' do
         before do
           sign_in(user)
-          project.team << [user, :developer]
+          project.add_developer(user)
         end
 
         it 'creates todo for merge request' do
diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb
index 65b821c9486..d3188f054cf 100644
--- a/spec/controllers/projects/tree_controller_spec.rb
+++ b/spec/controllers/projects/tree_controller_spec.rb
@@ -7,7 +7,7 @@ describe Projects::TreeController do
   before do
     sign_in(user)
 
-    project.team << [user, :master]
+    project.add_master(user)
     controller.instance_variable_set(:@project, project)
   end
 
diff --git a/spec/controllers/projects/variables_controller_spec.rb b/spec/controllers/projects/variables_controller_spec.rb
index d065cd00d00..9fde6544215 100644
--- a/spec/controllers/projects/variables_controller_spec.rb
+++ b/spec/controllers/projects/variables_controller_spec.rb
@@ -6,7 +6,7 @@ describe Projects::VariablesController do
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   describe 'POST #create' do
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index e61187fb518..5202ffdd8bb 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -102,7 +102,7 @@ describe ProjectsController do
         render_views
 
         before do
-          project.team << [user, :developer]
+          project.add_developer(user)
           project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
         end
 
@@ -437,7 +437,7 @@ describe ProjectsController do
 
     before do
       sign_in(user)
-      project.team << [user, :developer]
+      project.add_developer(user)
       allow(Gitlab.config.incoming_email).to receive(:enabled).and_return(true)
     end
 
@@ -465,7 +465,7 @@ describe ProjectsController do
 
     before do
       sign_in(user)
-      project.team << [user, :developer]
+      project.add_developer(user)
       allow(Gitlab.config.incoming_email).to receive(:enabled).and_return(true)
     end
 
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 7e42e43345c..b1f601a19e5 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -265,13 +265,13 @@ describe UploadsController do
 
           context "when the user has access to the project" do
             before do
-              project.team << [user, :master]
+              project.add_master(user)
             end
 
             context "when the user is blocked" do
               before do
                 user.block
-                project.team << [user, :master]
+                project.add_master(user)
               end
 
               it "redirects to the sign in page" do
@@ -465,13 +465,13 @@ describe UploadsController do
 
           context "when the user has access to the project" do
             before do
-              project.team << [user, :master]
+              project.add_master(user)
             end
 
             context "when the user is blocked" do
               before do
                 user.block
-                project.team << [user, :master]
+                project.add_master(user)
               end
 
               it "redirects to the sign in page" do
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 01ab59aa363..2898c4b119e 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -91,7 +91,7 @@ describe UsersController do
 
       before do
         sign_in(user)
-        project.team << [user, :developer]
+        project.add_developer(user)
 
         push_data = Gitlab::DataBuilder::Push.build_sample(project, user)
 
@@ -117,7 +117,7 @@ describe UsersController do
       allow_any_instance_of(User).to receive(:contributed_projects_ids).and_return([project.id])
 
       sign_in(user)
-      project.team << [user, :developer]
+      project.add_developer(user)
     end
 
     it 'assigns @calendar_date' do