Merge branch 'allow-maintainers-to-remove-pages' into 'master'

Allow maintainers to remove pages

See merge request gitlab-org/gitlab-ce!25458

app/controllers/projects/pages_controller.rb

@@ -5,7 +5,8 @@ class Projects::PagesController < Projects::ApplicationController
 
   before_action :require_pages_enabled!
   before_action :authorize_read_pages!, only: [:show]
-  before_action :authorize_update_pages!, except: [:show]
+  before_action :authorize_update_pages!, except: [:show, :destroy]
+  before_action :authorize_remove_pages!, only: [:destroy]
 
   # rubocop: disable CodeReuse/ActiveRecord
   def show

app/policies/project_policy.rb

@@ -152,7 +152,6 @@ class ProjectPolicy < BasePolicy
     enable :remove_fork_project
     enable :destroy_merge_request
     enable :destroy_issue
-    enable :remove_pages
 
     enable :set_issue_iid
     enable :set_issue_created_at
@@ -271,6 +270,7 @@ class ProjectPolicy < BasePolicy
     enable :admin_pages
     enable :read_pages
     enable :update_pages
+    enable :remove_pages
     enable :read_cluster
     enable :add_cluster
     enable :create_cluster

app/views/projects/pages/_destroy.haml

@@ -9,4 +9,4 @@
         .form-actions
           = link_to 'Remove pages', project_pages_path(@project), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove"
   - else
-    .nothing-here-block Only the project owner can remove pages
+    .nothing-here-block Only project maintainers can remove pages

changelogs/unreleased/allow-maintainers-to-remove-pages.yml

+---
+title: Allow maintainers to remove pages
+merge_request:
+author:
+type: fixed

doc/user/permissions.md

@@ -97,7 +97,7 @@ The following table depicts the various user permission levels in a project.
 | Manage variables                      |         |            |             | ✓        | ✓      |
 | Manage GitLab Pages                   |         |            |             | ✓        | ✓      |
 | Manage GitLab Pages domains and certificates |         |            |             | ✓        | ✓      |
-| Remove GitLab Pages                   |         |            |             |          | ✓      |
+| Remove GitLab Pages                   |         |            |             | ✓        | ✓      |
 | View GitLab Pages protected by [access control](project/pages/introduction.md#gitlab-pages-access-control-core-only) | ✓       | ✓          | ✓           | ✓        | ✓      |
 | Manage clusters                       |         |            |             | ✓        | ✓      |
 | Manage license policy **[ULTIMATE]**  |         |            |             | ✓        | ✓      |
@@ -107,7 +107,6 @@ The following table depicts the various user permission levels in a project.
 | Transfer project to another namespace |         |            |             |          | ✓      |
 | Remove project                        |         |            |             |          | ✓      |
 | Delete issues                         |         |            |             |          | ✓      |
-| Remove pages                          |         |            |             |          | ✓      |
 | Force push to protected branches [^4] |         |            |             |          |        |
 | Remove protected branches [^4]        |         |            |             |          |        |
 | View project Audit Events             |         |            |             | ✓        | ✓      |

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

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,13 +268,8 @@ 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)
-      end
-
     context 'when pages are deployed' do
       let(:pipeline) do
         commit_sha = project.commit('HEAD').sha
@@ -336,5 +310,4 @@ describe 'Pages' do
       end
     end
   end
-  end
 end