Skip to content

Commit

Permalink
Fixes #37946 - Add 'allow_other_types' option in errata filters
Browse files Browse the repository at this point in the history
  • Loading branch information
qcjames53 committed Nov 25, 2024
1 parent 9c98533 commit 41195d9
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def resource_class
param :types, Array, :desc => N_("erratum: types (enhancement, bugfix, security)")
param :date_type, String, :desc => N_("erratum: search using the 'Issued On' or 'Updated On' column of the errata. Values are 'issued'/'updated'")
param :module_stream_ids, Array, :desc => N_("module stream ids")
param :allow_other_types, :bool, :desc => N_("erratum: allow types not matching a valid errata type")
def create
rule_clazz = ContentViewFilter.rule_class_for(@filter)

Expand Down Expand Up @@ -89,6 +90,7 @@ def show
param :start_date, String, :desc => N_("erratum: start date (YYYY-MM-DD)")
param :end_date, String, :desc => N_("erratum: end date (YYYY-MM-DD)")
param :types, Array, :desc => N_("erratum: types (enhancement, bugfix, security)")
param :allow_other_types, :bool, :desc => N_("erratum: allow types not matching a valid errata type")
def update
update_params = rule_params
update_params[:name] = update_params[:name].first if update_params[:name]
Expand Down Expand Up @@ -136,7 +138,7 @@ def rule_params

@rule_params ||= params.fetch(:content_view_filter_rule, {}).
permit(:uuid, :version, :min_version, :max_version, :architecture,
:errata_id, :start_date, :end_date, :date_type,
:errata_id, :start_date, :end_date, :date_type, :allow_other_types,
:types => [], :module_stream_ids => [], :errata_ids => [], name: [])
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ module Katello
module Validators
class ContentViewErratumFilterRuleValidator < ActiveModel::Validator
def validate(record)
if record.errata_id.blank? && record.start_date.blank? && record.end_date.blank? && record.types.blank?
if record.errata_id.blank? && record.start_date.blank? && record.end_date.blank? && record.types.blank? && record.allow_other_types == false
invalid_parameters = _("Invalid erratum filter rule specified, Must specify at least one of the following:" \
" 'errata_id', 'start_date', 'end_date' or 'types'")
" 'errata_id', 'start_date', 'end_date', 'types', or 'allow_other_types'")
record.errors.add(:base, invalid_parameters)
return
end
Expand Down
20 changes: 18 additions & 2 deletions app/models/katello/content_view_erratum_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,21 @@ def erratum_arel
end

def types_clause
# Create an array to store output clauses for quick type filtering later
conditions = []

# Add clauses for types in the filter
types = erratum_rules.first.types
return if types.blank?
errata_types_in(types)
conditions << errata_types_in(types) unless types.blank?

# Add clauses for 'other' types
conditions << errata_types_not_in(Erratum::TYPES) if erratum_rules.first.allow_other_types?

# Reduce the array of clauses to a single clause and return
return if conditions.empty?
conditions.reduce(nil) do |combined_clause, condition|
combined_clause ? combined_clause.or(condition) : condition
end
end

def filter_by_id?
Expand All @@ -105,6 +117,10 @@ def errata_types_in(types)
erratum_arel[:errata_type].in(types)
end

def errata_types_not_in(types)
erratum_arel[:errata_type].not_in(types)
end

def errata_in(ids)
erratum_arel[:errata_id].in(ids)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ attributes :start_date, :if => lambda { |rule| rule.respond_to?(:start_date) &&
attributes :end_date, :if => lambda { |rule| rule.respond_to?(:end_date) && !rule.end_date.blank? }
attributes :architecture, :if => lambda { |rule| rule.respond_to?(:architecture) && !rule.architecture.blank? }
attributes :types, :if => lambda { |rule| rule.respond_to?(:types) && !rule.types.blank? }
attributes :allow_other_types, :if => lambda { |rule| rule.respond_to?(:allow_other_types) }
attributes :date_type, :if => lambda { |rule| rule.respond_to?(:date_type) }
attributes :module_stream_id, :if => lambda { |rule| rule.respond_to?(:module_stream_id) && !rule.module_stream_id.blank? }
if @resource&.try(:module_stream)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddAllowOtherTypesToContentViewErratumFilterRules < ActiveRecord::Migration[6.1]
def change
add_column :katello_content_view_erratum_filter_rules, :allow_other_types, :boolean,
:default => false, :null => false
end
end
51 changes: 45 additions & 6 deletions test/models/content_view_erratum_filter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ def setup
@repo = katello_repositories(:fedora_17_x86_64)
end

TYPICAL_TYPES_RESPONSE =
" AND \"katello_errata\".\"errata_type\" IN ('bugfix', 'enhancement', 'security')".freeze

def test_erratum_by_id_returns_arel_for_specified_errata_id
erratum = katello_errata(:security)
@repo.errata = [erratum]
Expand All @@ -24,7 +27,7 @@ def test_errata_by_start_date_returns_arel_for_errata_by_updated_date_and_errata
filter = id_rule.filter
filter.reload

assert_equal "\"katello_errata\".\"updated\" >= '#{start_date}' AND \"katello_errata\".\"errata_type\" IN ('bugfix', 'enhancement', 'security')",
assert_equal "\"katello_errata\".\"updated\" >= '#{start_date}'" + TYPICAL_TYPES_RESPONSE,
filter.generate_clauses(@repo).to_sql
end

Expand All @@ -35,7 +38,7 @@ def test_errata_by_start_date_returns_arel_for_errata_by_issued_date_and_errata_
filter = id_rule.filter
filter.reload

assert_equal "\"katello_errata\".\"issued\" >= '#{start_date}' AND \"katello_errata\".\"errata_type\" IN ('bugfix', 'enhancement', 'security')",
assert_equal "\"katello_errata\".\"issued\" >= '#{start_date}'" + TYPICAL_TYPES_RESPONSE,
filter.generate_clauses(@repo).to_sql
end

Expand All @@ -45,7 +48,7 @@ def test_errata_by_end_date_returns_arel_for_errata_by_updated_date_and_errata_t
filter = id_rule.filter
filter.reload

assert_equal "\"katello_errata\".\"updated\" <= '#{end_date}' AND \"katello_errata\".\"errata_type\" IN ('bugfix', 'enhancement', 'security')",
assert_equal "\"katello_errata\".\"updated\" <= '#{end_date}'" + TYPICAL_TYPES_RESPONSE,
filter.generate_clauses(@repo).to_sql
end

Expand All @@ -56,7 +59,7 @@ def test_errata_by_end_date_returns_arel_for_errata_by_issued_date_and_errata_ty
filter = id_rule.filter
filter.reload

assert_equal "\"katello_errata\".\"issued\" <= '#{end_date}' AND \"katello_errata\".\"errata_type\" IN ('bugfix', 'enhancement', 'security')",
assert_equal "\"katello_errata\".\"issued\" <= '#{end_date}'" + TYPICAL_TYPES_RESPONSE,
filter.generate_clauses(@repo).to_sql
end

Expand All @@ -69,6 +72,16 @@ def test_errata_by_type_returns_arel_by_errata_type
filter.generate_clauses(@repo).to_sql
end

def test_errata_by_type_returns_arel_by_errata_type_other
id_rule = FactoryBot.create(:katello_content_view_erratum_filter_rule, :allow_other_types => true)
id_rule.update!(types: [])
filter = id_rule.filter
filter.reload

assert_equal "\"katello_errata\".\"errata_type\" NOT IN ('security', 'bugfix', 'recommended', 'enhancement', 'optional')",
filter.generate_clauses(@repo).to_sql
end

def test_content_unit_pulp_ids_with_empty_errata_list_returns_empty_result
rpm1 = @repo.rpms.first
rpm2 = @repo.rpms.last
Expand Down Expand Up @@ -209,13 +222,16 @@ def test_content_unit_pulp_ids_by_issued_end_date_returns_pulp_hrefs
end

def test_content_unit_pulp_ids_by_errata_type
rpm1 = @repo.rpms.first
rpm2 = @repo.rpms.last
rpm1 = @repo.rpms[0]
rpm2 = @repo.rpms[1]
rpm3 = @repo.rpms[2]

erratum1 = Katello::Erratum.new(:pulp_id => "one", :errata_id => "ERRATA1", :errata_type => 'bugfix')
erratum1.packages << Katello::ErratumPackage.new(:filename => rpm1.filename, :name => "e1", :nvrea => "e1")
erratum2 = Katello::Erratum.new(:pulp_id => "two", :errata_id => "ERRATA2", :errata_type => 'security')
erratum2.packages << Katello::ErratumPackage.new(:filename => rpm2.filename, :name => "e2", :nvrea => "e2")
erratum3 = Katello::Erratum.new(:pulp_id => "three", :errata_id => "ERRATA3", :errata_type => 'not_recognized')
erratum3.packages << Katello::ErratumPackage.new(:filename => rpm3.filename, :name => "e3", :nvrea => "e3")

@repo.errata = [erratum2]
@repo.save!
Expand All @@ -226,5 +242,28 @@ def test_content_unit_pulp_ids_by_errata_type

assert_equal [rpm2.pulp_id], filter.content_unit_pulp_ids(@repo)
end

def test_content_unit_pulp_ids_by_errata_type_other
rpm1 = @repo.rpms[0]
rpm2 = @repo.rpms[1]
rpm3 = @repo.rpms[2]

erratum1 = Katello::Erratum.new(:pulp_id => "one", :errata_id => "ERRATA1", :errata_type => 'bugfix')
erratum1.packages << Katello::ErratumPackage.new(:filename => rpm1.filename, :name => "e1", :nvrea => "e1")
erratum2 = Katello::Erratum.new(:pulp_id => "two", :errata_id => "ERRATA2", :errata_type => 'security')
erratum2.packages << Katello::ErratumPackage.new(:filename => rpm2.filename, :name => "e2", :nvrea => "e2")
erratum3 = Katello::Erratum.new(:pulp_id => "three", :errata_id => "ERRATA3", :errata_type => 'not_recognized')
erratum3.packages << Katello::ErratumPackage.new(:filename => rpm3.filename, :name => "e3", :nvrea => "e3")

@repo.errata = [erratum3]
@repo.save!

id_rule = FactoryBot.create(:katello_content_view_erratum_filter_rule, :allow_other_types => true)
id_rule.update!(types: [])
filter = id_rule.filter
filter.reload

assert_equal [rpm3.pulp_id], filter.content_unit_pulp_ids(@repo)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,33 @@ const CVErrataDateFilterContent = ({
selectCVFilterDetails(state, cvId, filterId), shallowEqual);
const { repositories = [], rules } = filterDetails;
const [{
id, types, start_date: ruleStartDate, end_date: ruleEndDate, date_type: ruleDateType,
id,

Check failure on line 49 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 14

Trailing spaces not allowed

Check failure on line 49 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 18

Trailing spaces not allowed
types,

Check failure on line 50 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 14

Trailing spaces not allowed

Check failure on line 50 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 18

Trailing spaces not allowed
allow_other_types: ruleAllowOtherTypes,

Check failure on line 51 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 14

Trailing spaces not allowed

Check failure on line 51 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 18

Trailing spaces not allowed
start_date: ruleStartDate,

Check failure on line 52 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 14

Trailing spaces not allowed

Check failure on line 52 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 18

Trailing spaces not allowed
end_date: ruleEndDate,

Check failure on line 53 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 14

Trailing spaces not allowed

Check failure on line 53 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 18

Trailing spaces not allowed
date_type: ruleDateType,
} = {}] = rules;
const { permissions } = details;
const [startDate, setStartDate] = useState(convertAPIDateToUIFormat(ruleStartDate));
const [endDate, setEndDate] = useState(convertAPIDateToUIFormat(ruleEndDate));
const [dateType, setDateType] = useState(ruleDateType);
const [dateTypeSelectOpen, setDateTypeSelectOpen] = useState(false);
const [typeSelectOpen, setTypeSelectOpen] = useState(false);
const [selectedTypes, setSelectedTypes] = useState(types);
const dispatch = useDispatch();
const [activeTabKey, setActiveTabKey] = useState(0);
const [startEntry, setStartEntry] = useState(false);
const [endEntry, setEndEntry] = useState(false);

const getInitialSelectedTypes = () => {
if (ruleAllowOtherTypes) {
return [...types, 'other'];
}

Check failure on line 70 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 14

Trailing spaces not allowed

Check failure on line 70 in webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

View workflow job for this annotation

GitHub Actions / react-tests / Foreman develop Ruby 2.7 and Node 18

Trailing spaces not allowed
return types;
};

const [selectedTypes, setSelectedTypes] = useState(getInitialSelectedTypes());

const onSave = () => {
dispatch(editCVFilterRule(
filterId,
Expand All @@ -68,8 +81,9 @@ const CVErrataDateFilterContent = ({
content_view_filter_id: filterId,
start_date: startDate && startDate !== '' ? dateParse(startDate) : null,
end_date: endDate && endDate !== '' ? dateParse(endDate) : null,
types: selectedTypes,
types: selectedTypes.filter(e => e !== 'other'),
date_type: dateType,
allow_other_types: selectedTypes.includes('other'),
},
() => {
dispatch({ type: CONTENT_VIEW_NEEDS_PUBLISH });
Expand All @@ -81,15 +95,21 @@ const CVErrataDateFilterContent = ({
const resetFilters = () => {
setStartDate(convertAPIDateToUIFormat(ruleStartDate));
setEndDate(convertAPIDateToUIFormat(ruleEndDate));
setSelectedTypes(types);
setDateType(ruleDateType);
setSelectedTypes(getInitialSelectedTypes());
};

const onTypeSelect = (selection) => {
if (selectedTypes.includes(selection)) {
// If the selection is the only selection remaining, do not allow it to be removed
if (selectedTypes.length === 1) return;

// Filter out the current selection to deselect it
setSelectedTypes(selectedTypes.filter(e => e !== selection));
} else setSelectedTypes([...selectedTypes, selection]);
} else {
// Add the selection to the selected types
setSelectedTypes([...selectedTypes, selection]);
}
};

const singleSelection = selection => (selectedTypes.length === 1
Expand All @@ -99,7 +119,7 @@ const CVErrataDateFilterContent = ({
(
isEqual(convertAPIDateToUIFormat(ruleStartDate), startDate) &&
isEqual(convertAPIDateToUIFormat(ruleEndDate), endDate) &&
isEqual(sortBy(types), sortBy(selectedTypes)) &&
isEqual(sortBy(getInitialSelectedTypes()), sortBy(selectedTypes)) &&
isEqual(ruleDateType, dateType)
);

Expand Down Expand Up @@ -171,14 +191,23 @@ const CVErrataDateFilterContent = ({
{__('Bugfix')}
</p>
</SelectOption>
<SelectOption
isDisabled={singleSelection('other') || !hasPermission(permissions, 'edit_content_views')}
key="other"
value="other"
>
<p style={{ marginTop: '4px' }}>
{__('Other')}
</p>
</SelectOption>
</Select>
</FlexItem>
<FlexItem span={1} spacer={{ default: 'spacerNone' }}>
{(selectedTypes.length === 1) &&
<Tooltip
position="top"
content={
__('Atleast one errata type needs to be selected.')
__('At least one errata type option needs to be selected.')
}
>
<OutlinedQuestionCircleIcon />
Expand Down

0 comments on commit 41195d9

Please sign in to comment.