Skip to content

Commit

Permalink
Simplify User/Project/Package BsRequestsController
Browse files Browse the repository at this point in the history
- Use User/Project/Package.bs_requests to set initial ActiveRecord::Relation
  of BsRequests
- filter_involvement filters the ActiveRecord::Relation like all the other
  filters in Webui::RequestsFilter
- drop filter_by_involvement_staging_project, this is taken care of in
  Webui::RequestsFilter now
  • Loading branch information
hennevogel committed Feb 25, 2025
1 parent 109d78a commit 1a110f2
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 100 deletions.
48 changes: 19 additions & 29 deletions src/api/app/controllers/webui/packages/bs_requests_controller.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
module Webui
module Packages
class BsRequestsController < Webui::WebuiController
include Webui::RequestsFilter

before_action :set_project
before_action :require_package
before_action :redirect_legacy
include Webui::RequestsFilter
before_action :set_bs_requests

def index
if Flipper.enabled?(:request_index, User.session)
# FIXME: Once we roll out filter_requests should become a before_action
filter_requests
@bs_requests = @bs_requests.page(params[:page])

@url = packages_requests_path(@project, @package)
@bs_requests = @bs_requests.order('number DESC').page(params[:page])
@bs_requests = @bs_requests.includes(:bs_request_actions, :comments, :reviews)
@bs_requests = @bs_requests.includes(:labels) if Flipper.enabled?(:labels, User.session)
else
parsed_params = BsRequest::DataTable::ParamsParserWithStateAndType.new(params).parsed_params
requests_query = BsRequest::DataTable::FindForPackageQuery.new(@project, @package, parsed_params)
Expand All @@ -27,34 +28,23 @@ def index

private

def filter_by_involvement(involvement)
return filter_by_involvement_staging_project(involvement) if staging_projects.present?
def set_bs_requests
return unless Flipper.enabled?(:request_index, User.session)

case involvement
when 'all'
target = BsRequest.with_actions_and_reviews.where(bs_request_actions: { target_project: @project.name, target_package: @package.name })
source = BsRequest.with_actions_and_reviews.where(bs_request_actions: { source_project: @project.name, source_package: @package.name })
reviews = BsRequest.with_actions_and_reviews.where(reviews: { package: @package })
target.or(source).or(reviews)
when 'incoming'
BsRequest.with_actions.where(bs_request_actions: { target_project: @project.name, target_package: @package.name })
when 'outgoing'
BsRequest.with_actions.where(bs_request_actions: { source_project: @project.name, source_package: @package.name })
end
@bs_requests = @package.bs_requests
end

def filter_by_involvement_staging_project(involvement)
case involvement
when 'all'
target = BsRequest.with_actions_and_reviews.where(staging_project: staging_projects, bs_request_actions: { target_project: @project.name, target_package: @package.name })
source = BsRequest.with_actions_and_reviews.where(staging_project: staging_projects, bs_request_actions: { source_project: @project.name, source_package: @package.name })
reviews = BsRequest.with_actions_and_reviews.where(staging_project: staging_projects, reviews: { package: @package })
target.or(source).or(reviews)
when 'incoming'
BsRequest.with_actions.where(staging_project: staging_projects, bs_request_actions: { target_project: @project.name, target_package: @package.name })
when 'outgoing'
BsRequest.with_actions.where(staging_project: staging_projects, bs_request_actions: { source_project: @project.name, source_package: @package.name })
end
def filter_involvement
return if params[:involvement]&.compact_blank.blank?

@selected_filter['involvement'] = params[:involvement]
@bs_requests = case
when @selected_filter['involvement'].include?('incoming')
@bs_requests.where(bs_request_actions: { target_package_id: @package.id })
.or(@bs_requests.where(reviews: { by_project: @project.name, by_package: @package.name }))
when @selected_filter['involvement'].include?('outgoing')
@bs_requests.where(bs_request_actions: { source_package_id: @package.id })
end
end

def redirect_legacy
Expand Down
46 changes: 17 additions & 29 deletions src/api/app/controllers/webui/projects/bs_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ class BsRequestsController < WebuiController

before_action :set_project
before_action :redirect_legacy
before_action :set_bs_requests

def index
if Flipper.enabled?(:request_index, User.session)
# FIXME: Once we roll out filter_requests should become a before_action
filter_requests

@bs_requests = @bs_requests.order('number DESC').page(params[:page])
@bs_requests = @bs_requests.includes(:bs_request_actions, :comments, :reviews)
@bs_requests = @bs_requests.includes(:labels) if Flipper.enabled?(:labels, User.session)
@bs_requests = @bs_requests.page(params[:page])
else
parsed_params = BsRequest::DataTable::ParamsParserWithStateAndType.new(params).parsed_params
requests_query = BsRequest::DataTable::FindForProjectQuery.new(@project, parsed_params)
Expand All @@ -26,34 +25,23 @@ def index

private

def filter_by_involvement(involvement)
return filter_by_involvement_staging_project(involvement) if staging_projects.present?
def set_bs_requests
return unless Flipper.enabled?(:request_index, User.session)

case involvement
when 'all'
target = BsRequest.with_actions_and_reviews.where(bs_request_actions: { target_project: @project.name })
source = BsRequest.with_actions_and_reviews.where(bs_request_actions: { source_project: @project.name })
reviews = BsRequest.with_actions_and_reviews.where(reviews: { project: @project, package: nil })
target.or(source).or(reviews)
when 'incoming'
BsRequest.with_actions.where(bs_request_actions: { target_project: @project.name })
when 'outgoing'
BsRequest.with_actions.where(bs_request_actions: { source_project: @project.name })
end
@bs_requests = @project.bs_requests
end

def filter_by_involvement_staging_project(involvement)
case involvement
when 'all'
target = BsRequest.with_actions_and_reviews.where(staging_project: staging_projects, bs_request_actions: { target_project: @project.name })
source = BsRequest.with_actions_and_reviews.where(staging_project: staging_projects, bs_request_actions: { source_project: @project.name })
reviews = BsRequest.with_actions_and_reviews.where(staging_project: staging_projects, reviews: { project: @project, package: nil })
target.or(source).or(reviews)
when 'incoming'
BsRequest.with_actions.where(staging_project: staging_projects, bs_request_actions: { target_project: @project.name })
when 'outgoing'
BsRequest.with_actions.where(staging_project: staging_projects, bs_request_actions: { source_project: @project.name })
end
def filter_involvement
return if params[:involvement]&.compact_blank.blank?

@selected_filter['involvement'] = params[:involvement]
@bs_requests = case
when @selected_filter['involvement'].include?('incoming')
@bs_requests.where(bs_request_actions: { target_project_id: @project.id })
.or(@bs_requests.where(reviews: { by_project: @project.name }))
when @selected_filter['involvement'].include?('outgoing')
@bs_requests.where(bs_request_actions: { source_project_id: @project.id })
end
end

def redirect_legacy
Expand Down
46 changes: 23 additions & 23 deletions src/api/app/controllers/webui/users/bs_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class BsRequestsController < WebuiController
before_action :redirect_legacy
before_action :require_login
before_action :set_user
before_action :set_bs_requests

include Webui::RequestsFilter

Expand All @@ -18,11 +19,9 @@ class BsRequestsController < WebuiController
def index
respond_to do |format|
format.html do
# FIXME: Once we roll out request_index filter_requests should become a before_action
filter_requests

@bs_requests = @bs_requests.order('number DESC').page(params[:page])
@bs_requests = @bs_requests.includes(:bs_request_actions, :comments, :reviews)
@bs_requests = @bs_requests.includes(:labels) if Flipper.enabled?(:labels, User.session)
@bs_requests = @bs_requests.page(params[:page])
end
# TODO: Remove this old index action when request_index feature is rolled-over
format.json do
Expand All @@ -37,28 +36,29 @@ def index

private

def filter_by_involvement(involvement)
return filter_by_involvement_staging_project(involvement) if staging_projects.present?
def set_bs_requests
return unless Flipper.enabled?(:request_index, User.session)

case involvement
when 'all'
User.session.requests
when 'incoming'
User.session.incoming_requests.unscope(where: :state)
when 'outgoing'
User.session.outgoing_requests.unscope(where: :state)
end
@bs_requests = User.session.bs_requests
end

def filter_by_involvement_staging_project(involvement)
case involvement
when 'all'
User.session.requests.where(staging_project: staging_projects)
when 'incoming'
User.session.incoming_requests.where(staging_project: staging_projects)
when 'outgoing'
User.session.outgoing_requests.where(staging_project: staging_projects)
end
def filter_involvement
return if params[:involvement]&.compact_blank.blank?

@selected_filter['involvement'] = params[:involvement]
@bs_requests = case
when @selected_filter['involvement'].include?('incoming')
@bs_requests.where(bs_request_actions: { target_project_id: User.session.involved_projects })
.or(@bs_requests.where(bs_request_actions: { target_package_id: User.session.involved_packages }))
.or(@bs_requests.where(reviews: { user: User.session }))
.or(@bs_requests.where(reviews: { group: User.session.groups }))
.or(@bs_requests.where(reviews: { project: User.session.involved_projects }))
.or(@bs_requests.where(reviews: { package: User.session.involved_packages }))
when @selected_filter['involvement'].include?('outgoing')
@bs_requests.where(creator: User.session)
.or(@bs_requests.where(bs_request_actions: { source_project_id: User.session.involved_projects }))
.or(@bs_requests.where(bs_request_actions: { source_package_id: User.session.involved_packages }))
end
end

def set_user
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
-# haml-lint:disable ViewLength
.accordion.accordion-flush
.mt-2.mb-2.accordion-item.border-0
.mt-2.mb-2.accordion-item.border-0.auto-submit-on-change
.px-3.py-2.accordion-button.no-style{ data: { 'bs-toggle': 'collapse', 'bs-target': '#request-filter-involvement' },
aria: { expanded: 'true', controls: 'request-filter-involvement' } }
%b Involvement
.px-4.pb-2.accordion-collapse.collapse.show#request-filter-involvement
= render partial: 'webui/shared/check_box', locals: { label: 'Incoming',
key: 'involvement[incoming]', name: 'involvement', value: 'incoming',
checked: selected_filter[:involvement] == 'incoming' }
key: 'involvement[incoming]', name: 'involvement[]', value: 'incoming',
checked: selected_filter[:involvement]&.include?('incoming') }
= render partial: 'webui/shared/check_box', locals: { label: 'Outgoing',
key: 'involvement[outgoing]', name: 'involvement', value: 'outgoing',
checked: selected_filter[:involvement] == 'outgoing' }
key: 'involvement[outgoing]', name: 'involvement[]', value: 'outgoing',
checked: selected_filter[:involvement]&.include?('outgoing') }

.mt-2.mb-2.accordion-item.border-0.auto-submit-on-change
.px-3.py-2.accordion-button.no-style{ data: { 'bs-toggle': 'collapse', 'bs-target': '#request-filter-state' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@
it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request, outgoing_request, request_with_review) }

context 'and the involvement parameters is "incoming"' do
let(:context_params) { { involvement: 'incoming' } }
let(:context_params) { { involvement: ['incoming'] } }

it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request) }
it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request, request_with_review) }
end

context 'and the involvement parameters is "outgoing"' do
let(:context_params) { { involvement: 'outgoing' } }
let(:context_params) { { involvement: ['outgoing'] } }

it { expect(assigns[:bs_requests]).to contain_exactly(outgoing_request) }
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@
it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request, outgoing_request, request_with_review) }

context 'and the involvement parameters is "incoming"' do
let(:context_params) { { involvement: 'incoming' } }
let(:context_params) { { involvement: ['incoming'] } }

it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request) }
it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request, request_with_review) }
end

context 'and the involvement parameters is "outgoing"' do
let(:context_params) { { involvement: 'outgoing' } }
let(:context_params) { { involvement: ['outgoing'] } }

it { expect(assigns[:bs_requests]).to contain_exactly(outgoing_request) }
end
Expand Down Expand Up @@ -101,13 +101,13 @@
end

context 'and the involvement parameters is "incoming"' do
let(:context_params) { { involvement: 'incoming' } }
let(:context_params) { { involvement: ['incoming'] } }

it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request) }
it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request, request_with_review) }
end

context 'and the involvement parameters is "outgoing"' do
let(:context_params) { { involvement: 'outgoing' } }
let(:context_params) { { involvement: ['outgoing'] } }

it { expect(assigns[:bs_requests]).to contain_exactly(outgoing_request) }
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@
it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request, outgoing_request, request_with_review) }

context 'and the involvement parameter is incoming' do
let(:context_params) { { involvement: 'incoming' } }
let(:context_params) { { involvement: ['incoming'] } }

it { expect(assigns[:bs_requests]).to contain_exactly(incoming_request, request_with_review) }
end

context 'and the involvement parameter is outgoing' do
let(:context_params) { { involvement: 'outgoing' } }
let(:context_params) { { involvement: ['outgoing'] } }

it { expect(assigns[:bs_requests]).to contain_exactly(outgoing_request) }
end
Expand Down
2 changes: 1 addition & 1 deletion src/api/spec/features/beta/webui/my_requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

it 'filter requests' do
find_by_id('requests-dropdown-trigger').click if mobile?
choose('Incoming', allow_label_click: true)
check('Incoming', allow_label_click: true)
execute_script('$("#content-selector-filters-form").submit()')

expect(page).to have_link(href: "/request/show/#{incoming_request.number}")
Expand Down
2 changes: 1 addition & 1 deletion src/api/spec/features/beta/webui/package_requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

it 'filters requests' do
find_by_id('requests-dropdown-trigger').click if mobile? # open the filter dropdown
choose('Incoming')
check('Incoming', allow_label_click: true)
execute_script('$("#content-selector-filters-form").submit()')

expect(page).to have_link(href: "/request/show/#{incoming_request.number}")
Expand Down
2 changes: 1 addition & 1 deletion src/api/spec/features/beta/webui/project_requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

it 'filter requests' do
find_by_id('requests-dropdown-trigger').click if mobile?
choose('Incoming', allow_label_click: true)
check('Incoming', allow_label_click: true)
execute_script('$("#content-selector-filters-form").submit()')

expect(page).to have_link(href: "/request/show/#{incoming_request.number}")
Expand Down

0 comments on commit 1a110f2

Please sign in to comment.