Skip to content

Commit 839459c

Browse files
committed
Fixes #39246 - Add sorting and searching to the new Leapp preupgrade report
1 parent 8c56131 commit 839459c

13 files changed

Lines changed: 643 additions & 208 deletions

File tree

app/controllers/api/v2/preupgrade_reports_controller.rb

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,16 @@ def index
1515

1616
api :GET, '/preupgrade_reports/:id', N_('Show Preupgrade report')
1717
param :id, :identifier, required: true
18+
param :search, String, required: false, desc: N_('Filter entries')
19+
param :order, String, required: false, desc: N_('Sort entries, e.g. "title ASC"')
20+
param :page, :number, required: false, desc: N_('Page number')
21+
param :per_page, :number, required: false, desc: N_('Items per page')
1822
def show
1923
@preupgrade_report = resource_scope.find(params[:id])
24+
scope = @preupgrade_report.preupgrade_report_entries
25+
@entries = scope.search_for(params[:search], order: params[:order]).paginate(paginate_options)
26+
@total = scope.count
27+
@subtotal = @entries.total_entries
2028
end
2129

2230
api :GET, '/job_invocations/:id/preupgrade_reports', N_('List Preupgrade reports for Job invocation')
@@ -30,8 +38,8 @@ def job_invocation
3038
# By overriding path_to_authenticate we can require REX's permission view_job_invocations
3139
def path_to_authenticate
3240
params['action'] = 'show' if params['action'] == 'job_invocation'
33-
Foreman::AccessControl.normalize_path_hash params.slice(:action, :id, :user_id)
34-
.merge({ controller: 'api/v2/job_invocations' })
41+
path_params = params.slice(:action, :id, :user_id).merge(controller: 'api/v2/job_invocations')
42+
Foreman::AccessControl.normalize_path_hash(path_params)
3543
end
3644
end
3745
end
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# frozen_string_literal: true
2+
3+
class PreupgradeReportEntriesController < ApplicationController
4+
include Foreman::Controller::AutoCompleteSearch
5+
6+
# rubocop:disable Rails/LexicallyScopedActionFilter
7+
skip_before_action :authorize, only: %i[auto_complete_search]
8+
before_action :authorize_autocomplete, only: %i[auto_complete_search]
9+
# rubocop:enable Rails/LexicallyScopedActionFilter
10+
11+
def index
12+
@preupgrade_report_entries = resource_scope
13+
.search_for(params[:search], order: params[:order])
14+
.paginate(paginate_options)
15+
render 'api/v2/preupgrade_report_entries/index'
16+
end
17+
18+
protected
19+
20+
def model_of_controller
21+
PreupgradeReportEntry.joins(:host).merge(Host.authorized(:view_hosts, Host))
22+
end
23+
24+
def resource_scope
25+
model_of_controller
26+
end
27+
28+
private
29+
30+
def authorize_autocomplete
31+
unless User.current.can?('view_job_invocations')
32+
render json: {
33+
error: {
34+
message: _('Missing one of the required permissions: view_job_invocations'),
35+
missing_permissions: ['view_job_invocations']
36+
}
37+
}, status: :forbidden
38+
return
39+
end
40+
41+
return if User.current.can?('view_hosts')
42+
43+
render json: {
44+
error: {
45+
message: _('Missing one of the required permissions: view_hosts'),
46+
missing_permissions: ['view_hosts']
47+
}
48+
}, status: :forbidden
49+
end
50+
51+
def action_permission
52+
'view'
53+
end
54+
55+
def controller_permission
56+
'job_invocations'
57+
end
58+
end

app/models/preupgrade_report_entry.rb

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,43 @@ class PreupgradeReportEntry < ApplicationRecord
88
serialize :flags, Array
99
serialize :detail, JSON
1010

11-
validates :preupgrade_report, :host, :hostname, :title, :actor, :audience, :severity, :leapp_run_id, presence: true
11+
validates :preupgrade_report, :host, :hostname, :title, :actor,
12+
:audience, :severity, :leapp_run_id, presence: true
13+
14+
scoped_search on: :title, complete_value: true
15+
scoped_search on: :hostname, complete_value: true
16+
scoped_search on: :severity, complete_value: { high: 'high', medium: 'medium', low: 'low', info: 'info' }
17+
scoped_search on: :summary
18+
19+
scoped_search on: :flags,
20+
rename: :inhibitor,
21+
only_explicit: true,
22+
operators: ['=', '!='],
23+
complete_value: { yes: 'yes', no: 'no' },
24+
ext_method: :search_yes_no_fields
25+
26+
scoped_search on: :detail,
27+
rename: :has_remediation,
28+
only_explicit: true,
29+
operators: ['=', '!='],
30+
complete_value: { yes: 'yes', no: 'no' },
31+
ext_method: :search_yes_no_fields
1232

1333
def self.remediation_details(remediation_ids, host)
1434
where(id: remediation_ids, host: host).where.not(detail: nil).pluck(:detail)
1535
end
36+
37+
def self.search_yes_no_fields(key, operator, value)
38+
column, term = case key.to_sym
39+
when :inhibitor then %w[flags inhibitor]
40+
when :has_remediation then %w[detail remediations]
41+
end
42+
43+
if (operator == '=' && value == 'yes') || (operator == '!=' && value == 'no')
44+
{ conditions: "preupgrade_report_entries.#{column} LIKE '%%#{term}%%'" }
45+
else
46+
{ conditions: "(preupgrade_report_entries.#{column} NOT LIKE '%%#{term}%%' OR " \
47+
"preupgrade_report_entries.#{column} IS NULL)" }
48+
end
49+
end
1650
end
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# frozen_string_literal: true
2+
3+
object false
4+
5+
child(@preupgrade_report_entries => :results) do
6+
extends 'api/v2/preupgrade_report_entries/main'
7+
end
8+
9+
node(:total) { @preupgrade_report_entries.total_entries }
10+
node(:subtotal) { @preupgrade_report_entries.total_entries }
11+
node(:page) { params[:page].to_i.positive? ? params[:page].to_i : 1 }
12+
node(:per_page) { params[:per_page].to_i.positive? ? params[:per_page].to_i : Setting[:entries_per_page] }
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# frozen_string_literal: true
2+
3+
extends 'api/v2/preupgrade_report_entries/base'
Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
1-
object @preupgrade_report
1+
# frozen_string_literal: true
22

3-
extends 'api/v2/preupgrade_reports/base'
3+
object @preupgrade_report
4+
attributes :id, :status, :reported_at
45

5-
child :preupgrade_report_entries do
6-
extends 'api/v2/preupgrade_report_entries/base'
6+
child(@entries => :results) do
7+
extends 'api/v2/preupgrade_report_entries/main'
78
end
9+
10+
node(:total) { @total }
11+
node(:subtotal) { @subtotal }
12+
node(:page) { params[:page].to_i.positive? ? params[:page].to_i : 1 }
13+
node(:per_page) { params[:per_page].to_i.positive? ? params[:per_page].to_i : Setting[:entries_per_page] }

config/routes.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
# frozen_string_literal: true
22

33
Rails.application.routes.draw do
4-
resources :preupgrade_reports, :only => %i[index]
4+
resources :preupgrade_reports, only: %i[index]
5+
6+
resources :preupgrade_report_entries, only: [] do
7+
collection do
8+
get :auto_complete_search
9+
end
10+
end
511

612
namespace :api, defaults: { format: 'json' } do
713
scope '(:apiv)', module: :v2, defaults: { apiv: 'v2' }, apiv: /v2/,
8-
constraints: ApiConstraints.new( version: 2, default: true) do
14+
constraints: ApiConstraints.new(version: 2, default: true) do
915
resources :preupgrade_reports, only: %i[index show]
1016
get 'job_invocations/:id/preupgrade_reports', to: 'preupgrade_reports#job_invocation'
1117
end

test/functional/api/v2/preupgrade_reports_controller_test.rb

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
module Api
66
module V2
7+
# rubocop:disable Metrics/ClassLength
78
class PreupgradeReportsControllerTest < ActionController::TestCase
89
setup do
910
@host = FactoryBot.create(:host)
@@ -22,9 +23,11 @@ class PreupgradeReportsControllerTest < ActionController::TestCase
2223
get :show, params: { id: @report.id }
2324
assert_response :success
2425

25-
response = JSON.parse(@response.body)
26-
assert_equal response['id'], @report.id
27-
assert_not_empty response['preupgrade_report_entries']
26+
body = JSON.parse(@response.body)
27+
assert_equal @report.id, body['id']
28+
assert_not_empty body['results']
29+
assert_not_nil body['total']
30+
assert_not_nil body['subtotal']
2831
end
2932

3033
test 'should get :job_invocation' do
@@ -96,7 +99,7 @@ class PreupgradeReportsControllerTest < ActionController::TestCase
9699
assert_includes JSON.parse(@response.body)['error']['missing_permissions'], 'view_hosts'
97100
end
98101

99-
test 'should not get :job_invocation' do
102+
test 'should not get :show' do
100103
get :show, params: { id: @report.id }, session: set_session_user(@user)
101104
assert_response :forbidden
102105
assert_includes JSON.parse(@response.body)['error']['missing_permissions'], 'view_hosts'
@@ -108,6 +111,34 @@ class PreupgradeReportsControllerTest < ActionController::TestCase
108111
assert_includes JSON.parse(@response.body)['error']['missing_permissions'], 'view_hosts'
109112
end
110113
end
114+
115+
test 'should filter entries by search on :show' do
116+
get :show, params: { id: @report.id, search: "title = \"#{@entry.title}\"" }
117+
assert_response :success
118+
body = JSON.parse(@response.body)
119+
assert_equal 1, body['results'].length
120+
assert_equal @entry.id, body['results'].first['id']
121+
end
122+
123+
test 'should sort entries on :show' do
124+
FactoryBot.create(:preupgrade_report_entry, host: @host, preupgrade_report: @report,
125+
title: 'Aaa first alphabetically')
126+
get :show, params: { id: @report.id, order: 'title ASC' }
127+
assert_response :success
128+
titles = JSON.parse(@response.body)['results'].map { |e| e['title'] }
129+
assert_equal titles.sort, titles
130+
end
131+
132+
test ':show always returns paginated results with total and subtotal' do
133+
get :show, params: { id: @report.id }
134+
assert_response :success
135+
body = JSON.parse(@response.body)
136+
assert_not_empty body['results']
137+
assert_not_nil body['total']
138+
assert_not_nil body['subtotal']
139+
assert_equal body['total'], body['subtotal']
140+
end
111141
end
142+
# rubocop:enable Metrics/ClassLength
112143
end
113144
end
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# frozen_string_literal: true
2+
3+
require 'test_plugin_helper'
4+
5+
class PreupgradeReportEntriesControllerTest < ActionController::TestCase
6+
setup do
7+
@user = FactoryBot.create(:user, admin: false)
8+
@host = FactoryBot.create(:host)
9+
@report = FactoryBot.create(:preupgrade_report, host: @host)
10+
@entry = FactoryBot.create(:preupgrade_report_entry, host: @host, preupgrade_report: @report)
11+
end
12+
13+
test 'should get auto_complete_search with required permissions' do
14+
User.any_instance.stubs(:can?).with('view_job_invocations').returns(true)
15+
User.any_instance.stubs(:can?).with('view_hosts').returns(true)
16+
17+
get :auto_complete_search, params: { format: :json }, session: set_session_user(@user)
18+
assert_response :success
19+
end
20+
21+
test 'should not get auto_complete_search without :view_hosts' do
22+
User.any_instance.stubs(:can?).with('view_job_invocations').returns(true)
23+
User.any_instance.stubs(:can?).with('view_hosts').returns(false)
24+
25+
get :auto_complete_search, params: { format: :json }, session: set_session_user(@user)
26+
assert_response :forbidden
27+
assert_includes JSON.parse(@response.body)['error']['missing_permissions'], 'view_hosts'
28+
end
29+
30+
test 'should not get auto_complete_search without :view_job_invocations' do
31+
User.any_instance.stubs(:can?).with('view_job_invocations').returns(false)
32+
33+
get :auto_complete_search, params: { format: :json }, session: set_session_user(@user)
34+
assert_response :forbidden
35+
assert_includes JSON.parse(@response.body)['error']['missing_permissions'], 'view_job_invocations'
36+
end
37+
end

test/models/preupgrade_report_entry_test.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,23 @@ class PreupgradeReportEntryTest < ActiveSupport::TestCase
3333
assert_equal details.size, 0
3434
end
3535
end
36+
37+
describe 'scoped_search' do
38+
it 'filters correctly by has_remediation and inhibitor' do
39+
host = FactoryBot.create(:host)
40+
report = FactoryBot.create(:preupgrade_report)
41+
42+
entry1 = FactoryBot.create(:preupgrade_report_entry, preupgrade_report: report, host: host,
43+
flags: ['inhibitor'],
44+
detail: { 'remediations' => [{ 'type' => 'hint' }] })
45+
entry2 = FactoryBot.create(:preupgrade_report_entry, preupgrade_report: report, host: host,
46+
flags: [],
47+
detail: nil)
48+
49+
assert_includes PreupgradeReportEntry.search_for('inhibitor = yes'), entry1
50+
assert_not_includes PreupgradeReportEntry.search_for('inhibitor = yes'), entry2
51+
assert_includes PreupgradeReportEntry.search_for('has_remediation = no'), entry2
52+
assert_not_includes PreupgradeReportEntry.search_for('has_remediation = no'), entry1
53+
end
54+
end
3655
end

0 commit comments

Comments
 (0)