Commit ddfdd494 authored by Vladimir Shushlin's avatar Vladimir Shushlin Committed by Kamil Trzciński

Allow maintainers to remove pages

Move remove_pages permission to maintainer
Fix before_action in pages controller to check `remove_pages`
permission
Add specs
parent c6b9ac86
......@@ -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
......
......@@ -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
......
......@@ -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
---
title: Allow maintainers to remove pages
merge_request:
author:
type: fixed
......@@ -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 | | | | ✓ | ✓ |
......
......@@ -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
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment