Skip to content

Commit

Permalink
Move 'Request Deletion' modal to its own page
Browse files Browse the repository at this point in the history
This page is shared between projects and packages. Links to the page now
use the same icon and have the same capitalization.

There's a new controller which uses strong parameters, form builders and
nested attributes for the bs_request_actions association of the
BsRequest model.
  • Loading branch information
Dany Marcoux committed May 26, 2020
1 parent 9cbfc3a commit afcbf5d
Show file tree
Hide file tree
Showing 17 changed files with 112 additions and 117 deletions.
20 changes: 0 additions & 20 deletions src/api/app/controllers/webui/request_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,26 +147,6 @@ def list_small
render partial: 'requests_small', locals: { requests: requests }
end

def delete_request
request = nil
begin
request = BsRequest.create!(
description: params[:delete_description], bs_request_actions: [BsRequestAction.new(request_action_attributes(:delete))]
)
request_link = ActionController::Base.helpers.link_to("delete request #{request.number}", request_show_path(request.number))
flash[:success] = "Created #{request_link}"
rescue APIError => e
flash[:error] = e.message
if params[:package]
redirect_to package_show_path(project: params[:project], package: params[:package])
else
redirect_to project_show_path(project: params[:project])
end
return
end
redirect_to request_show_path(number: request.number)
end

def set_bugowner_request
required_parameters :project
request = nil
Expand Down
43 changes: 43 additions & 0 deletions src/api/app/controllers/webui/requests/deletions_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
module Webui
module Requests
class DeletionsController < WebuiController
before_action :require_login
before_action :lockout_spiders
before_action :set_package, only: [:new]
before_action :set_project, only: [:new]

after_action :verify_authorized

def new
bs_request_action = BsRequestAction.new(target_package: @package, target_project: @project, type: 'delete')
@bs_request = BsRequest.new(bs_request_actions: [bs_request_action])
authorize @bs_request, :create?
end

def create
request = BsRequest.new(bs_request_params)
authorize request, :create?
begin
request.save!
rescue APIError => e
flash[:error] = e.message
redirect_to(controller: :package, action: :show, package: params[:package_name], project: params[:project_name]) && return if @package
redirect_to(controller: :project, action: :show, project: params[:project_name]) && return
end
redirect_to request_show_path(request.number)
end

private

def bs_request_params
params.require(:bs_request).permit(:description, bs_request_actions_attributes: [:target_project, :target_package, :type])
end

def set_package
return unless params.key?(:package_name)

@package = Package.find_by(name: params[:package_name])
end
end
end
end
2 changes: 1 addition & 1 deletion src/api/app/views/webui/package/_bottom_actions.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

- else
= render partial: 'webui/package/bottom_actions/request_role_addition', locals: { package: package, project: project }
= render partial: 'webui/package/bottom_actions/request_deletion'
= render partial: 'webui/package/bottom_actions/request_deletion', locals: { package: package, project: project }

//TODO: only users w/o rights should see this, maintainers should get a different dialog:
- if package.develpackage
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
%li.list-inline-item
= link_to('#', class: 'nav-link', data: { toggle: 'modal', target: '#delete-request-modal' }) do
%i.fas.fa-list-alt.text-danger
Request deletion
= link_to(new_project_package_deletion_path(project, package), class: 'nav-link') do
%i.fas.fa-times-circle.text-danger
Request Deletion
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
= render partial: 'webui/package/responsive_ux/show_actions/trigger_services', locals: { package: package, project: project }
- else
= render partial: 'webui/package/responsive_ux/show_actions/request_role_addition', locals: { package: package, project: project }
= render partial: 'webui/package/responsive_ux/show_actions/request_deletion'
= render partial: 'webui/package/responsive_ux/show_actions/request_deletion', locals: { package: package, project: project }

//TODO: only users w/o rights should see this, maintainers should get a different dialog:
- if package.develpackage
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
%li.nav-item
= link_to('#', class: 'nav-link', data: { toggle: 'modal', target: '#delete-request-modal' }) do
%i.fas.fa-list-alt.fa-lg.mr-2
Request deletion
= link_to(new_project_package_deletion_path(project, package), class: 'nav-link') do
%i.fas.fa-times-circle.fa-lg.mr-2
Request Deletion
2 changes: 0 additions & 2 deletions src/api/app/views/webui/package/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@
- if User.possibly_nobody.can_modify?(@package)
= render partial: 'edit_dialog', locals: { project: @project, package: @package }
= render partial: 'delete_dialog', locals: { project: @project, package: @package, cleanup_source: @cleanup_source }
- else
= render partial: 'webui/request/delete_request_dialog', locals: { project: @project, package: @package }
- if @package.develpackage
= render partial: 'webui/request/change_devel_request_dialog', locals: { project: @project, package: @package }
-# haml-lint:enable ViewLength
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
%i.fas.fa-plus-circle.text-primary
Request Role Addition
%li.list-inline-item
= link_to('#', class: 'nav-link', data: { toggle: 'modal', target: '#delete-request-modal' }) do
= link_to(new_project_deletion_path(project), class: 'nav-link') do
%i.fas.fa-times-circle.text-danger
Request Deletion
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
%i.fas.fa-plus-circle.fa-lg.mr-2
Request Role Addition
%li.nav-item
= link_to('#', class: 'nav-link', data: { toggle: 'modal', target: '#delete-request-modal' }) do
= link_to(new_project_deletion_path(project), class: 'nav-link') do
%i.fas.fa-times-circle.fa-lg.mr-2
Request Deletion
2 changes: 0 additions & 2 deletions src/api/app/views/webui/project/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@
- elsif User.session!.can_modify?(@project)
= render partial: 'edit_project_dialog', locals: { project: @project }
= render partial: 'delete_project_dialog', locals: { project: @project }
- elsif !@project.is_locked?
= render partial: 'webui/request/delete_request_dialog', locals: { project: @project }

- content_for :ready_function do
initializeDataTable('#inherited-packages-table');
Expand Down
30 changes: 0 additions & 30 deletions src/api/app/views/webui/request/_delete_request_dialog.html.haml

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- if @package
= render partial: 'webui/package/breadcrumb_items'
- else
= render partial: 'webui/project/breadcrumb_items'

%li.breadcrumb-item.active{ 'aria-current' => 'page' }
Request Deletion
36 changes: 36 additions & 0 deletions src/api/app/views/webui/requests/deletions/new.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
- @pagetitle = 'Request Deletion'

.row
.col
.card
- if @package
= render partial: 'webui/package/tabs', locals: { project: @project, package: @package }
- else
= render partial: 'webui/project/tabs', locals: { project: @project }
.card-body
.row
.col-12
%h3= @pagetitle
.col-12.col-md-9.col-lg-8
%p
-# FIXME: get rid of the helper project_or_package_link
Do you really want to request the deletion of
= project_or_package_link(project: @project.name, package: @package.try(:name))
?
.col-12.col-md-9.col-lg-6
- form_path = @package ? project_package_role_additions_path(@project, @package) : project_role_additions_path(@project)
= form_for(@bs_request, url: form_path) do |f|
= f.fields_for(:bs_request_actions) do |bs_request_actions|
= bs_request_actions.hidden_field(:target_project)
= bs_request_actions.hidden_field(:type)
- if @package
.form-group
= bs_request_actions.hidden_field(:target_package)
= bs_request_actions.label(:target_project, 'In target project:')
= bs_request_actions.text_field(:target_project, disabled: true, class: 'form-control')
.form-group
= f.label(:description, "Please describe your reasons to request the deletion of this #{@package ? 'package' : 'project'}:")
%abbr.text-danger{ title: 'required' } *
= f.text_area(:description, row: 3, class: 'form-control', required: true)

= submit_tag('Request', class: 'btn btn-primary px-4')
3 changes: 2 additions & 1 deletion src/api/config/routes/webui_routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,11 @@
resources :maintenance_incident_requests, controller: 'webui/projects/maintenance_incident_requests', only: [:new, :create], constraints: cons
resources :packages, only: [], param: :name do
resources :role_additions, controller: 'webui/requests/role_additions', only: [:new, :create], constraints: cons
resources :deletions, controller: 'webui/requests/deletions', only: [:new, :create], constraints: cons
end

resources :role_additions, controller: 'webui/requests/role_additions', only: [:new, :create], constraints: cons
resources :deletions, controller: 'webui/requests/deletions', only: [:new, :create], constraints: cons
end

controller 'webui/request' do
Expand All @@ -279,7 +281,6 @@
post 'request/changerequest' => :changerequest
get 'request/diff/:number' => :diff
get 'request/list_small' => :list_small, as: 'request_list_small'
post 'request/delete_request/:project' => :delete_request, constraints: cons, as: 'delete_request'
post 'request/set_bugowner_request' => :set_bugowner_request
post 'request/change_devel_request/:project/:package' => :change_devel_request, constraints: cons, as: 'change_devel_request'
end
Expand Down
36 changes: 0 additions & 36 deletions src/api/spec/controllers/webui/request_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,42 +195,6 @@
end
end

describe 'POST #delete_request' do
before do
login(submitter)
end

context 'a valid request' do
before do
post :delete_request, params: { project: target_project, package: target_package, delete_description: 'delete it!' }
end

subject do
BsRequest.joins(:bs_request_actions).
where('bs_request_actions.target_project=? AND bs_request_actions.target_package=? AND type=?',
target_project.to_s, target_package.to_s, 'delete').first
end

it { expect(response).to redirect_to(request_show_path(number: subject)) }
it { expect(flash[:success]).to match("Created .+delete request #{subject.number}") }
it { expect(subject.description).to eq('delete it!') }
end

context 'a request causing a APIError' do
before do
allow_any_instance_of(BsRequest).to receive(:save!).and_raise(APIError, 'something happened')
post :delete_request, params: { project: target_project, package: target_package, description: 'delete it!' }
end

it { expect(flash[:error]).to eq('something happened') }
it { expect(response).to redirect_to(package_show_path(project: target_project, package: target_package)) }

it 'does not create a delete request' do
expect(BsRequest.count).to eq(0)
end
end
end

describe 'POST #modify_review' do
RSpec.shared_examples 'a valid review' do |new_state|
let(:params_hash) do
Expand Down
15 changes: 7 additions & 8 deletions src/api/spec/features/beta/webui/packages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,16 @@
scenario 'requesting package deletion' do
login user
visit package_show_path(package: other_users_package, project: other_user.home_project)
click_menu_link('Actions', 'Request deletion')
click_menu_link('Actions', 'Request Deletion')

expect(page).to have_text('Do you really want to request the deletion of package ')
within('#delete-request-modal') do
fill_in('delete_description', with: 'Hey, why not?')
click_button('Create')
end
fill_in('bs_request_description', with: 'Hey, why not?')
expect { click_button('Request') }.to change(BsRequest, :count).by(1)

expect(page).to have_text('Created delete request')
find('a', text: /delete request \d+/).click
expect(page).to have_current_path(/\/request\/show\/\d+/)
# The project name can be ellipsed when it's too long, so this explains why it's hardcoded in the spec
expect(page).to have_text("Delete package home:othe...test_user / #{other_users_package}")
expect(page).to have_css('#description-text', text: 'Hey, why not?')
expect(page).to have_text('In state new')
end

scenario "changing the package's devel project" do
Expand Down
15 changes: 7 additions & 8 deletions src/api/spec/features/webui/packages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -245,17 +245,16 @@
scenario 'requesting package deletion' do
login user
visit package_show_path(package: other_users_package, project: other_user.home_project)
click_link('Request deletion')
click_link('Request Deletion')

expect(page).to have_text('Do you really want to request the deletion of package ')
within('#delete-request-modal') do
fill_in('delete_description', with: 'Hey, why not?')
click_button('Create')
end
fill_in('bs_request_description', with: 'Hey, why not?')
expect { click_button('Request') }.to change(BsRequest, :count).by(1)

expect(page).to have_text('Created delete request')
find('a', text: /delete request \d+/).click
expect(page).to have_current_path(/\/request\/show\/\d+/)
# The project name can be ellipsed when it's too long, so this explains why it's hardcoded in the spec
expect(page).to have_text("Delete package home:othe...test_user / #{other_users_package}")
expect(page).to have_css('#description-text', text: 'Hey, why not?')
expect(page).to have_text('In state new')
end

scenario "changing the package's devel project" do
Expand Down

0 comments on commit afcbf5d

Please sign in to comment.