Allow maintainers to remove pages

Move remove_pages permission to maintainer
Fix before_action in pages controller to check `remove_pages`
permission
Add specs

2 files changed, 50 insertions, 65 deletions

diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb
index 4b742a5d427..d6eece47804 100644
--- a/spec/controllers/projects/pages_controller_spec.rb
+++ b/spec/controllers/projects/pages_controller_spec.rb
@@ -42,6 +42,18 @@ describe Projects::PagesController do
 
       expect(response).to have_gitlab_http_status(302)
     end
+
+    context 'when user is developer' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'returns 404 status' do
+        delete :destroy, params: request_params
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
   end
 
   context 'pages disabled' do
diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb
index 435fb229b69..72faeba92ee 100644
--- a/spec/features/projects/pages_spec.rb
+++ b/spec/features/projects/pages_spec.rb
@@ -13,16 +13,6 @@ describe 'Pages' do
     sign_in(user)
   end
 
-  shared_examples 'no pages deployed' do
-    it 'does not see anything to destroy' do
-      visit project_pages_path(project)
-
-      expect(page).to have_content('Configure pages')
-      expect(page).not_to have_link('Remove pages')
-      expect(page).not_to have_text('Only the project owner can remove pages')
-    end
-  end
-
   context 'when user is the owner' do
     before do
       project.namespace.update(owner: user)
@@ -181,7 +171,12 @@ describe 'Pages' do
       end
     end
 
-    it_behaves_like 'no pages deployed'
+    it 'does not see anything to destroy' do
+      visit project_pages_path(project)
+
+      expect(page).to have_content('Configure pages')
+      expect(page).not_to have_link('Remove pages')
+    end
 
     describe 'project settings page' do
       it 'renders "Pages" tab' do
@@ -208,22 +203,6 @@ describe 'Pages' do
     end
   end
 
-  context 'when the user is not the owner' do
-    context 'when pages deployed' do
-      before do
-        allow_any_instance_of(Project).to receive(:pages_deployed?) { true }
-      end
-
-      it 'sees "Only the project owner can remove pages" text' do
-        visit project_pages_path(project)
-
-        expect(page).to have_text('Only the project owner can remove pages')
-      end
-    end
-
-    it_behaves_like 'no pages deployed'
-  end
-
   describe 'HTTPS settings', :js, :https_pages_enabled do
     before do
       project.namespace.update(owner: user)
@@ -289,51 +268,45 @@ describe 'Pages' do
   end
 
   describe 'Remove page' do
-    context 'when user is the owner' do
-      let(:project) { create :project, :repository }
-
-      before do
-        project.namespace.update(owner: user)
+    let(:project) { create :project, :repository }
+
+    context 'when pages are deployed' do
+      let(:pipeline) do
+        commit_sha = project.commit('HEAD').sha
+
+        project.ci_pipelines.create(
+          ref: 'HEAD',
+          sha: commit_sha,
+          source: :push,
+          protected: false
+        )
       end
 
-      context 'when pages are deployed' do
-        let(:pipeline) do
-          commit_sha = project.commit('HEAD').sha
-
-          project.ci_pipelines.create(
-            ref: 'HEAD',
-            sha: commit_sha,
-            source: :push,
-            protected: false
-          )
-        end
-
-        let(:ci_build) do
-          create(
-            :ci_build,
-            project: project,
-            pipeline: pipeline,
-            ref: 'HEAD',
-            legacy_artifacts_file: fixture_file_upload(File.join('spec/fixtures/pages.zip')),
-            legacy_artifacts_metadata: fixture_file_upload(File.join('spec/fixtures/pages.zip.meta'))
-          )
-        end
+      let(:ci_build) do
+        create(
+          :ci_build,
+          project: project,
+          pipeline: pipeline,
+          ref: 'HEAD',
+          legacy_artifacts_file: fixture_file_upload(File.join('spec/fixtures/pages.zip')),
+          legacy_artifacts_metadata: fixture_file_upload(File.join('spec/fixtures/pages.zip.meta'))
+        )
+      end
 
-        before do
-          result = Projects::UpdatePagesService.new(project, ci_build).execute
-          expect(result[:status]).to eq(:success)
-          expect(project).to be_pages_deployed
-        end
+      before do
+        result = Projects::UpdatePagesService.new(project, ci_build).execute
+        expect(result[:status]).to eq(:success)
+        expect(project).to be_pages_deployed
+      end
 
-        it 'removes the pages' do
-          visit project_pages_path(project)
+      it 'removes the pages' do
+        visit project_pages_path(project)
 
-          expect(page).to have_link('Remove pages')
+        expect(page).to have_link('Remove pages')
 
-          click_link 'Remove pages'
+        click_link 'Remove pages'
 
-          expect(project.pages_deployed?).to be_falsey
-        end
+        expect(project.pages_deployed?).to be_falsey
       end
     end
   end