Fixes #37946 - Add 'allow_other_types' option in errata filters

Author: qcjames53

app/controllers/katello/api/v2/content_view_filter_rules_controller.rb

@@ -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)
 
@@ -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]
@@ -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
 

app/lib/katello/validators/content_view_erratum_filter_rule_validator.rb

@@ -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

app/models/katello/content_view_erratum_filter.rb

@@ -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?
@@ -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

app/views/katello/api/v2/content_view_filter_rules/show.json.rabl

@@ -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)

db/migrate/20241120213713_add_allow_other_types_to_content_view_erratum_filter_rules.rb

@@ -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

test/actions/katello/repository_test.rb

@@ -235,6 +235,7 @@ class DestroyTest < TestBase
     let(:in_use_repository) { katello_repositories(:fedora_17_no_arch) }
     let(:published_repository) { katello_repositories(:rhel_6_x86_64) }
     let(:published_fedora_repository) { katello_repositories(:fedora_17_x86_64) }
+    let(:published_rhel7_repository) { katello_repositories(:rhel_7_no_arch) }
     let(:simplified_acs) { katello_alternate_content_sources(:yum_alternate_content_source) }
     def setup
       simplified_acs.products << published_repository.product
@@ -378,22 +379,24 @@ def setup
       assert_not_equal(0, simplified_acs.products.length)
     end
 
-    it 'plans ACS product removal when removing the deleting the last repo with URL' do
-      ::Katello::SmartProxyAlternateContentSource.create!(alternate_content_source_id: simplified_acs.id, smart_proxy_id: proxy.id, repository_id: published_repository.id)
+    it 'plans ACS product removal when deleting the last repo with URL' do
+      ::Katello::SmartProxyAlternateContentSource.create!(alternate_content_source_id: simplified_acs.id, smart_proxy_id: proxy.id, repository_id: published_rhel7_repository.id)
       action = create_action action_class
-      action.stubs(:action_subject).with(published_repository)
+      action.stubs(:action_subject).with(published_rhel7_repository)
 
       # manually remove the URLs from all repos in product except repository
-      repository.product.repositories.each do |repo|
-        repo.root.url = nil unless repo.id == repository.id
+      testing_repo = repository.product.repositories[4]
+      testing_repo.product.repositories.each do |repo|
+        repo.root.url = nil unless repo.id == testing_repo.id || repo.root.id == testing_repo.root.id
+        repo.root.save!(validate: false)
       end
-      url_sum = repository.product.repositories.count do |repo|
+      url_sum = testing_repo.product.repositories.count do |repo|
         repo.root.url.present?
       end
       assert_equal(1, url_sum) # double check there's only one URL left
 
       # Since there is only one URL remaining, the product should be removed
-      plan_action action, published_repository, remove_from_content_view_versions: true
+      plan_action action, published_rhel7_repository, remove_from_content_view_versions: true
       simplified_acs.reload
       assert_equal(0, simplified_acs.products.length)
     end

test/models/content_view_erratum_filter_test.rb

@@ -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]
@@ -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
 
@@ -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
 
@@ -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
 
@@ -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
 
@@ -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
@@ -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!
@@ -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

webpack/scenes/ContentViews/Details/Filters/CVErrataDateFilterContent.js

@@ -1,6 +1,6 @@
 import React, { useState, useEffect } from 'react';
 import PropTypes from 'prop-types';
-import { isEqual, sortBy, capitalize } from 'lodash';
+import { isEqual, sortBy, capitalize, initial } from 'lodash';
 import { shallowEqual, useSelector, useDispatch } from 'react-redux';
 import { Link, useHistory } from 'react-router-dom';
 import {
@@ -46,30 +46,37 @@ const CVErrataDateFilterContent = ({
     selectCVFilterDetails(state, cvId, filterId), shallowEqual);
   const { repositories = [], rules } = filterDetails;
   const [{
-    id, types, start_date: ruleStartDate, end_date: ruleEndDate, date_type: ruleDateType,
+    id, types, allow_other_types, start_date: ruleStartDate, end_date: ruleEndDate, 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 [selectedTypes, setSelectedTypes] = useState(() => {
+    if (allow_other_types) {
+      return[...types, 'other'];
+    }
+    return types;
+  });
   const dispatch = useDispatch();
   const [activeTabKey, setActiveTabKey] = useState(0);
   const [startEntry, setStartEntry] = useState(false);
   const [endEntry, setEndEntry] = useState(false);
 
   const onSave = () => {
+    console.log('contains other', selectedTypes.includes('other'));
     dispatch(editCVFilterRule(
       filterId,
       {
         id,
         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 });
@@ -81,15 +88,31 @@ const CVErrataDateFilterContent = ({
   const resetFilters = () => {
     setStartDate(convertAPIDateToUIFormat(ruleStartDate));
     setEndDate(convertAPIDateToUIFormat(ruleEndDate));
-    setSelectedTypes(types);
     setDateType(ruleDateType);
+    setSelectedTypes(getInitialSelectedTypes());
+  };
+
+  const getInitialSelectedTypes = () => {
+    if (allow_other_types) {
+      return[...types, 'other'];
+    } else {
+      return types;
+    }
   };
 
   const onTypeSelect = (selection) => {
+    let updatedSelectedTypes;
     if (selectedTypes.includes(selection)) {
+      // If the selection is the only selection remaining, do not allow it to be removed
       if (selectedTypes.length === 1) return;
-      setSelectedTypes(selectedTypes.filter(e => e !== selection));
-    } else setSelectedTypes([...selectedTypes, selection]);
+
+      // Filter out the current selection from the selected types to update
+      updatedSelectedTypes = selectedTypes.filter(e => e !== selection);
+    } else {
+      updatedSelectedTypes = [...selectedTypes, selection];
+    }
+
+    setSelectedTypes(updatedSelectedTypes);
   };
 
   const singleSelection = selection => (selectedTypes.length === 1
@@ -99,7 +122,7 @@ const CVErrataDateFilterContent = ({
     (
       isEqual(convertAPIDateToUIFormat(ruleStartDate), startDate) &&
       isEqual(convertAPIDateToUIFormat(ruleEndDate), endDate) &&
-      isEqual(sortBy(types), sortBy(selectedTypes)) &&
+      isEqual(sortBy(getInitialSelectedTypes()), sortBy(selectedTypes)) &&
       isEqual(ruleDateType, dateType)
     );
 
@@ -171,14 +194,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 />