Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request accept buttons into dropdown #16987

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
126 changes: 95 additions & 31 deletions src/api/app/components/request_decision_component.html.haml
Original file line number Diff line number Diff line change
@@ -1,33 +1,97 @@
.request-decision
- if @show_hint
.alert.alert-warning.mb-1
You are a <strong>project maintainer</strong> but not a <strong>package maintainer</strong>. This package
has <strong>#{pluralize(@package_maintainers.size, 'package maintainer')}</strong> assigned. Please keep
in mind that also package maintainers would like to review this request.
= form_tag({ action: 'changerequest' }, id: 'request_handle_form') do
= form_tag({ action: 'changerequest' }, id: 'request_handle_form', class: 'write-and-preview',
data: { preview_message_url: preview_comments_path, message_body_param: 'comment[body]' }) do
= hidden_field_tag(:number, @bs_request.number)
= text_area_tag(:reason, nil, placeholder: 'Please explain your decision...', rows: 4, class: 'w-100 form-control mb-2')
- if single_action_request && @is_target_maintainer && @bs_request.state.in?([:new, :review])
- if show_add_submitter_as_maintainer_option?
.form-check.mb-2
= check_box_tag('add_submitter_as_maintainer_0', "#{@action.target_project}_#_#{@action.target_package}", false, class: 'form-check-input')
%label.form-check-label{ for: 'add_submitter_as_maintainer_0' }
Add #{link_to(@bs_request.creator, user_path(@bs_request.creator))} as maintainer for
#{helpers.project_or_package_link(project: @action.target_project, package: @action.target_package, short: true)}
- if @action.type == 'submit'
- @action.forward.each_with_index do |forward, index|
.form-check.mb-2
= check_box_tag("forward-#{index}", "#{forward[:project]}_#_#{forward[:package]}", true, class: 'form-check-input')
%label.form-check-label{ for: "forward-#{index}" }
Forward submit request to
#{helpers.project_or_package_link(project: forward[:project], package: forward[:package], short: true)}
- if policy(@bs_request).revoke_request?
= submit_tag 'Revoke request', name: 'revoked', class: 'btn btn-danger me-2'
- if policy(@bs_request).reopen_request?
= submit_tag 'Reopen request', name: 'new', class: 'btn btn-warning me-2'
- if policy(@bs_request).accept_request?
- if policy(@bs_request).decline_request?
= submit_tag('Decline request', name: 'declined', title: 'Decline the request, as the author of the request you can reopen it again.',
class: 'btn btn-danger me-2')
= submit_tag('Accept request', name: 'accepted', title: 'Accept the request, this will apply the changes on the target.',
class: 'btn btn-primary', data: confirmation)
.px-3.py-1.border.border-1.pb-2{ 'data-canned-controller': '' }
- if policy(Comment.new(commentable: @bs_request)).locked?
.alert.alert-warning.mt-2{ role: 'alert' }
Commenting on this is locked.
- if CommentLockPolicy.new(User.session, @bs_request).create?
= helpers.comment_lock_alert(@bs_request)
.card
%ul.card-header.nav.nav-tabs.px-3.pt-2.pb-0.disable-link-generation{ role: 'tablist' }
%li.nav-item
= link_to('Write', "#write_new_comment", class: 'nav-link active', data: { 'bs-toggle': 'tab' },
role: 'tab', aria: { controls: 'write-comment-tab', selected: 'true'})
%li.nav-item
= link_to('Preview', '#preview_new_comment', class: 'nav-link preview-message-tab', data: { 'bs-toggle': 'tab' },
role: 'tab', aria: { controls: 'preview-message-tab', selected: 'false' })
.tab-content.px-3
.tab-pane.fade.show.active.my-3{ id: 'write_new_comment', role: 'tabpanel',
'aria-labelledby': 'write-comment-tab', 'data-canned-controller': '' }
- if Flipper.enabled?(:content_moderation, User.session)
.d-flex.justify-content-end
= render CannedResponsesDropdownComponent.new(User.session.canned_responses.where(decision_type: nil).order(:title))
= text_area_tag(:reason, nil,
placeholder: 'Write your comment or decision...(markdown is only supported for comments, not for decisions)',
rows: 4, class: 'w-100 form-control mb-2 message-field')
.tab-pane.fade{ id: 'preview_new_comment', role: 'tabpanel', 'aria-labelledby': 'preview-message-tab' }
.comment-preview.message-preview.border-bottom.border-gray-300.my-3
.mt-2
- if policy(Comment.new(commentable:@bs_request)).create?
= submit_tag 'Add comment', class: 'btn btn-primary me-2', data: { disable_with: 'Creating comment...' }, name: 'commented'
- if policy(@bs_request).revoke_request?
= submit_tag 'Revoke request', name: 'revoked', class: 'btn btn-danger me-2', data: other_decision_confirmation('revoke')
- if policy(@bs_request).decline_request?
= submit_tag('Decline request', name: 'declined', class: 'btn btn-danger me-2', data: other_decision_confirmation('decline'),
title: 'Decline the request, as the author of the request you can reopen it again.')
- if policy(@bs_request).accept_request?
-# Hidden checkbox input to flag make_maintainer option
- if accept_with_options_allowed? && show_add_submitter_as_maintainer_option?
.form-check.mb-2.mt-2.d-none
= check_box_tag('add_submitter_as_maintainer_0', "#{@action.target_project}_#_#{@action.target_package}",
false, class: 'form-check-input')
-# Accept buttons
%span.dropdown.me-2
= button_tag('Accept', type: 'button', role: 'button', id: 'decision-buttons-group', class: 'btn btn-secondary dropdown-toggle',
data: { 'bs-toggle': 'dropdown'}, aria: { 'haspopup': 'true', 'expanded': 'false' })
.dropdown-menu{ aria: { labelledby: 'decision-buttons-group' } }
-# Simple Accept request submit
= submit_tag('Accept request', name: 'accepted', class: 'btn-link dropdown-item', data: confirmation,
title: 'Accept the request, this will apply the changes on the target.')
-# Accept and make maintainer submit
- if accept_with_options_allowed? && show_add_submitter_as_maintainer_option?
= submit_tag("Accept and make #{@bs_request.creator} maintainer of #{make_maintainer_of}",
name: 'accepted',
class: 'btn-link dropdown-item', data: confirmation,
title: 'Accept the request. this will apply the changes on the target.',
id: 'accept-and-make-maintainer')
-# Accept and forward buttons
- if forward_allowed?
%span.dropdown.me-2
= button_tag('Accept and Forward', type: 'button', role: 'button', id: 'decision-forward-buttons-group',
class: 'btn btn-secondary dropdown-toggle',
data: { 'bs-toggle': 'dropdown'}, aria: { 'haspopup': 'true', 'expanded': 'false' })
.dropdown-menu{ aria: { labelledby: 'decision-forward-buttons-group' } }
- @action.forward.each_with_index do |forward, index|
.form-check.m-2
= check_box_tag("forward-#{index}", "#{forward[:project]}_#_#{forward[:package]}", true,
class: 'form-check-input forward-check-inputs')
%label.form-check-label{ for: "forward-#{index}" }
Forward submit request to
#{helpers.project_or_package_link(project: forward[:project], package: forward[:package], short: true)}
%hr.dropdown-divider
-# Accept and forward submit
= submit_tag('Accept and forward request to the selected projects ', name: 'accepted', class: 'btn-link dropdown-item',
data: confirmation, title: 'Accept the request. this will apply the changes on the target.',
id: 'accept-and-forward')
-# Accept and make maintainer and forward submit
- if show_add_submitter_as_maintainer_option?
= submit_tag("Accept and make #{@bs_request.creator} maintainer of #{make_maintainer_of} and forward request to the selected projects",
name: 'accepted', class: 'btn-link dropdown-item', data: confirmation,
title: 'Accept the request. this will apply the changes on the target.',
id: 'accept-and-make-maintainer-and-forward')

- if policy(@bs_request).reopen_request?
= submit_tag 'Reopen request', name: 'new', class: 'btn btn-warning me-2', data: other_decision_confirmation('reopen')

:javascript
attachPreviewMessageOnCommentBoxes();

// Check the proper checkbox behind the scene on different type of accept submit
$('#accept-and-make-maintainer').on('click', function(){
$('input[name=add_submitter_as_maintainer_0]').prop('checked', 'checked');
});
$('#accept-and-make-maintainer-and-forward').on('click', function(){
$('input[name=add_submitter_as_maintainer_0]').prop('checked', 'checked');
});
26 changes: 24 additions & 2 deletions src/api/app/components/request_decision_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ def initialize(bs_request:, action:, is_target_maintainer:, package_maintainers:
@is_target_maintainer = is_target_maintainer
@action = action
@package_maintainers = package_maintainers
@show_hint = render? && show_project_maintainer_hint

return unless render? && show_project_maintainer_hint

@package_maintainers_hint = "Note\n" \
'You are a project maintainer but not a package maintainer. This package ' \
"has #{pluralize(@package_maintainers.size, 'package maintainer')} assigned. Please keep " \
'in mind that also package maintainers would like to review this request.'.freeze
end

def render?
Expand All @@ -19,13 +25,29 @@ def single_action_request

def confirmation
if @bs_request.state == :review
{ confirm: 'Do you really want to approve this request, despite of open review requests?' }
{ confirm: "Do you really want to approve this request, despite of open review requests?\n\n#{@package_maintainers_hint}" }
else
{}
end
end

def other_decision_confirmation(decision_text)
{ confirm: "Do you really want to #{decision_text} this request?\n\n#{@package_maintainers_hint}" }
end

def show_add_submitter_as_maintainer_option?
@action.type == 'submit' && [email protected]_is_target_maintainer
end

def accept_with_options_allowed?
single_action_request && @is_target_maintainer && @bs_request.state.in?(%i[new review])
end

def forward_allowed?
@action.type == 'submit' && @action.forward.any?
end

def make_maintainer_of
@action.target_project + ("/#{@action.target_package}" if @action.target_package)
end
end
6 changes: 3 additions & 3 deletions src/api/app/controllers/webui/comments_controller.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
class Webui::CommentsController < Webui::WebuiController
include BuildNewComment

before_action :require_login
before_action :set_commented, only: :create
before_action :set_comment, only: %i[moderate history]

def create
return commented_unavailable if @commented.nil?

@comment = @commented.comments.new(permitted_params)
authorize @comment, :create?
User.session.comments << @comment
build_new_comment(@commented, permitted_params)
@commentable = @comment.commentable

status = if @comment.save
Expand Down
10 changes: 8 additions & 2 deletions src/api/app/controllers/webui/request_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class Webui::RequestController < Webui::WebuiController
include Webui::NotificationsHandler
include Webui::RequestsFilter
include BuildNewComment

ALLOWED_INVOLVEMENTS = %w[all incoming outgoing].freeze

Expand Down Expand Up @@ -204,9 +205,14 @@ def sourcediff
end

def changerequest
changestate = (%w[accepted declined revoked new] & params.keys).last
changestate = (%w[accepted commented declined revoked new] & params.keys).last

if change_state(changestate, params)
if changestate == 'commented'

build_new_comment(BsRequest.find_by(number: params[:number]),
body: params[:reason])

elsif change_state(changestate, params)
# TODO: Make this work for each submit action individually
if params[:add_submitter_as_maintainer_0] # rubocop:disable Naming/VariableNumber
if changestate == 'accepted'
Expand Down
9 changes: 9 additions & 0 deletions src/api/app/models/concerns/build_new_comment.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module BuildNewComment
extend ActiveSupport::Concern

def build_new_comment(commented, permitted_params)
@comment = commented.comments.new(permitted_params)
authorize @comment, :create?
User.session.comments << @comment
end
end
5 changes: 3 additions & 2 deletions src/api/app/views/webui/request/beta_show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@
history_elements: @history_elements,
request_reviews_for_non_staging_projects: @request_reviews)

.comment_new
= render partial: 'webui/comment/new', locals: { commentable: @bs_request }
- if !policy(@bs_request).revoke_request? && !policy(@bs_request).reopen_request? && !policy(@bs_request).accept_request?
.comment_new
= render partial: 'webui/comment/new', locals: { commentable: @bs_request }
%hr

- if @bs_request.accept_at.present?
Expand Down
Loading