Skip to content

Commit

Permalink
Move can_add_reviews logic to a pundit policy and use in request be…
Browse files Browse the repository at this point in the history
…ta views

This code is for authorization and therefore should be in a
pundit policy.
  • Loading branch information
krauselukas committed May 30, 2023
1 parent 109a5d5 commit 8217048
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 14 deletions.
5 changes: 2 additions & 3 deletions src/api/app/components/add_review_dropdown_component.rb
@@ -1,15 +1,14 @@
class AddReviewDropdownComponent < ApplicationComponent
def initialize(bs_request:, user:, can_add_reviews:, my_open_reviews:)
def initialize(bs_request:, user:, my_open_reviews:)
super

@bs_request = bs_request
@user = user
@can_add_reviews = can_add_reviews
@my_open_reviews = my_open_reviews
end

def render?
@can_add_reviews && @my_open_reviews.present?
policy(@bs_request).add_reviews? && @my_open_reviews.present?
end

def reviewer_icon_and_text(review:)
Expand Down
1 change: 0 additions & 1 deletion src/api/app/controllers/webui/request_controller.rb
Expand Up @@ -528,7 +528,6 @@ def prepare_request_data
@is_target_maintainer = @bs_request.is_target_maintainer?(User.session)
reviews = @bs_request.reviews.where(state: 'new')
@my_open_reviews = reviews.select { |review| review.matches_user?(User.session) }
@can_add_reviews = @bs_request.state.in?([:new, :review]) && (@is_author || @is_target_maintainer || @my_open_reviews.present?)

@diff_limit = params[:full_diff] ? 0 : nil
@diff_to_superseded_id = params[:diff_to_superseded]
Expand Down
7 changes: 7 additions & 0 deletions src/api/app/policies/bs_request_policy.rb
Expand Up @@ -13,4 +13,11 @@ def handle_request?
is_author = record.creator == user.login
record.state.in?([:new, :review, :declined]) && (is_target_maintainer || is_author)
end

def add_reviews?
is_target_maintainer = record.is_target_maintainer?(user)
is_author = record.creator == user.login
has_open_reviews = record.reviews.where(state: 'new').select { |review| review.matches_user?(user) }.present?
record.state.in?([:new, :review]) && (is_author || is_target_maintainer || has_open_reviews)
end
end
1 change: 0 additions & 1 deletion src/api/app/views/webui/request/beta_show.html.haml
Expand Up @@ -26,7 +26,6 @@
.row
= render partial: 'webui/request/beta_show_tabs/conversation_aside',
locals: { bs_request: @bs_request,
can_add_reviews: @can_add_reviews,
my_open_reviews: @my_open_reviews,
request_reviews: @request_reviews,
package_maintainers: @package_maintainers,
Expand Down
@@ -1,5 +1,4 @@
- if can_add_reviews
%button.btn.btn-outline-secondary.btn-sm{ data: { 'bs-toggle': 'modal', 'bs-target': '#add-reviewer-modal' }, title: 'Add Reviewer' }
%i.fas.fa-plus-circle.me-1.text-outline-secondary
%span.nav-item-name Add Reviewer
= render partial: 'add_reviewer_dialog'
%button.btn.btn-outline-secondary.btn-sm{ data: { 'bs-toggle': 'modal', 'bs-target': '#add-reviewer-modal' }, title: 'Add Reviewer' }
%i.fas.fa-plus-circle.me-1.text-outline-secondary
%span.nav-item-name Add Reviewer
= render partial: 'add_reviewer_dialog'
Expand Up @@ -3,16 +3,15 @@
- if request_reviews.present?
.mt-4
.text-end
= render AddReviewDropdownComponent.new(bs_request: bs_request, user: User.session,
can_add_reviews: can_add_reviews,
my_open_reviews: my_open_reviews)
= render AddReviewDropdownComponent.new(bs_request: bs_request, user: User.session, my_open_reviews: my_open_reviews)
= render AddReviewCollapsibleComponent.new
.mb-2
%h6
Reviewers
.mb-4
= render partial: 'webui/request/beta_show_tabs/review_summary', collection: request_reviews, as: :review
= render partial: 'webui/request/beta_show_tabs/ask_for_review', locals: { can_add_reviews: can_add_reviews }
- if policy(bs_request).add_reviews?
= render partial: 'webui/request/beta_show_tabs/ask_for_review'

-# PACKAGE MAINTAINERS
- unless package_maintainers.empty?
Expand Down

0 comments on commit 8217048

Please sign in to comment.