From f46f2a5a5c67605504945db2552a1c7fef72f248 Mon Sep 17 00:00:00 2001 From: MariaAga Date: Wed, 6 Nov 2024 15:43:22 +0000 Subject: [PATCH] Fixes #37987 - Filters dont inherit taxonomy on creation --- app/models/filter.rb | 17 +++++++++- ..._search_for_filters_with_override_false.rb | 10 ++++++ test/models/filter_test.rb | 11 ++++++- .../routes/FiltersForm/FiltersForm.js | 31 +++++++++++++++++-- .../routes/FiltersForm/NewFiltersFormPage.js | 4 +-- 5 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20241105143353_regenerate_taxonomy_search_for_filters_with_override_false.rb diff --git a/app/models/filter.rb b/app/models/filter.rb index d96ae6e9175..1c4ff7b4d69 100644 --- a/app/models/filter.rb +++ b/app/models/filter.rb @@ -165,7 +165,22 @@ def enforce_inherited_taxonomies def inherit_taxonomies! self.organization_ids = role.organization_ids if resource_taxable_by_organization? self.location_ids = role.location_ids if resource_taxable_by_location? - build_taxonomy_search + build_taxonomy_search_from_ids + end + + def build_taxonomy_search_from_ids + orgs = build_taxonomy_search_string_from_ids('organization') + locs = build_taxonomy_search_string_from_ids('location') + + taxonomies = [orgs, locs].reject { |t| t.blank? } + self.taxonomy_search = taxonomies.join(' and ').presence + end + + def build_taxonomy_search_string_from_ids(name) + return '' unless send("resource_taxable_by_#{name}?") + relation = send("#{name}_ids") + return '' if relation.empty? + parenthesize("#{name}_id ^ (#{relation.join(',')})") end private diff --git a/db/migrate/20241105143353_regenerate_taxonomy_search_for_filters_with_override_false.rb b/db/migrate/20241105143353_regenerate_taxonomy_search_for_filters_with_override_false.rb new file mode 100644 index 00000000000..9b3931ffeb4 --- /dev/null +++ b/db/migrate/20241105143353_regenerate_taxonomy_search_for_filters_with_override_false.rb @@ -0,0 +1,10 @@ +class RegenerateTaxonomySearchForFiltersWithOverrideFalse < ActiveRecord::Migration[6.1] + def change + filters = Filter.where(role_id: Role.where(origin: nil).or(Role.where(builtin: 2))).where(override: false).where(taxonomy_search: nil) + + filters.each do |filter| + filter.build_taxonomy_search_from_ids + filter.update_column(:taxonomy_search, filter.taxonomy_search) + end + end +end diff --git a/test/models/filter_test.rb b/test/models/filter_test.rb index ade9b53f924..bca1965a77d 100644 --- a/test/models/filter_test.rb +++ b/test/models/filter_test.rb @@ -245,8 +245,17 @@ class FilterTest < ActiveSupport::TestCase test 'saving nilifies empty taxonomy search' do f = FactoryBot.build(:filter, :resource_type => 'Domain') - f.role = FactoryBot.build(:role, :organizations => [FactoryBot.build(:organization)]) + f.role = FactoryBot.build(:role) f.save assert_nil f.taxonomy_search end + + test 'creating a filter inherits taxonomies from the role' do + f = FactoryBot.create(:filter, :resource_type => 'Domain') + organization_test = FactoryBot.create(:organization) + location_test = FactoryBot.create(:location) + f.role = FactoryBot.create(:role, :organizations => [organization_test], :locations => [location_test]) + f.save + assert_equal f.taxonomy_search(), "(organization_id ^ (#{organization_test.id})) and (location_id ^ (#{location_test.id}))" + end end diff --git a/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.js b/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.js index 765b71a91a2..65c2fa598a4 100644 --- a/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.js +++ b/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.js @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useState, useEffect, useMemo } from 'react'; import PropTypes from 'prop-types'; import { useDispatch } from 'react-redux'; import { @@ -39,6 +39,17 @@ export const FiltersForm = ({ roleName, roleId, isNew, data, history }) => { show_organizations: showOrgs = false, show_locations: showLocations = false, } = type; + + const isTaxonomySearch = useMemo( + () => chosenOrgs.length || chosenLocations.length, + [chosenOrgs.length, chosenLocations.length] + ); + + useEffect(() => { + if (isTaxonomySearch) { + setIsUnlimited(false); + } + }, [isTaxonomySearch]); const dispatch = useDispatch(); const [autocompleteQuery, setAutocompleteQuery] = useState(data.search || ''); const submit = async () => { @@ -46,7 +57,8 @@ export const FiltersForm = ({ roleName, roleId, isNew, data, history }) => { filter: { role_id: role, search: isUnlimited ? null : autocompleteQuery, - unlimited: isUnlimited, + unlimited: + isUnlimited || (!autocompleteQuery.length && !isTaxonomySearch), override: isOverride, permission_ids: chosenPermissions, organization_ids: chosenOrgs, @@ -163,8 +175,20 @@ export const FiltersForm = ({ roleName, roleId, isNew, data, history }) => { defaultLocations={data.locations} /> )} + {isGranular ? ( <> + {!!isTaxonomySearch && ( + <> + + + )} { > { setAutocompleteQuery(''); setIsUnlimited(checked); }} + isDisabled={isTaxonomySearch} aria-label="is unlimited" id="unlimited-check" name="unlimited" diff --git a/webpack/assets/javascripts/react_app/routes/FiltersForm/NewFiltersFormPage.js b/webpack/assets/javascripts/react_app/routes/FiltersForm/NewFiltersFormPage.js index e059164ba1c..500f952b218 100644 --- a/webpack/assets/javascripts/react_app/routes/FiltersForm/NewFiltersFormPage.js +++ b/webpack/assets/javascripts/react_app/routes/FiltersForm/NewFiltersFormPage.js @@ -9,7 +9,7 @@ import { FiltersForm } from './FiltersForm'; const NewFiltersFormPage = ({ location: { search }, history }) => { const { role_id: urlRoleId } = URI.parseQuery(search); const { - response: { name: roleName, id: roleId }, + response: { name: roleName, id: roleId, locations, organizations }, } = useAPI('get', `/api/v2/roles/${urlRoleId}`); const breadcrumbOptions = { breadcrumbItems: [ @@ -29,7 +29,7 @@ const NewFiltersFormPage = ({ location: { search }, history }) => { isNew roleName={roleName || ''} roleId={urlRoleId} - data={{}} + data={{ locations, organizations }} history={history} /> ) : (