Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #37987 - Filters dont inherit taxonomy on creation #10370

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

MariaAga
Copy link
Member

@MariaAga MariaAga commented Nov 6, 2024

To test:

  1. Create a role
  2. Assign an organization and a location to the role
  3. Create a filter under the role with host permissions
    before the pr: no taxonomy_search in the DB, and the filter is marked as unlimited

Added a migration to regenerate the taxonomy_search field of all filters which have override set to false to make them honor the organizations and locations set on the role.

Added a ui block on the unlimited checkbox if there are taxonomies in place.

Fixed "build_taxonomy_search" as it relayed on locations, but on creation there are only location_ids. Before this pr the workaround would be to edit and submit the filter (with no changes) since on edit locations are already populated

@github-actions github-actions bot added the UI label Nov 6, 2024
@MariaAga MariaAga force-pushed the roles-taxonomy-on-create branch from baf4134 to f46f2a5 Compare November 6, 2024 16:26
app/models/filter.rb Outdated Show resolved Hide resolved
@MariaAga MariaAga force-pushed the roles-taxonomy-on-create branch 2 times, most recently from 319787a to e2698dc Compare November 7, 2024 14:20
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
filter.build_taxonomy_search_from_ids
filter.build_taxonomy_search

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}))"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert_equal f.taxonomy_search(), "(organization_id ^ (#{organization_test.id})) and (location_id ^ (#{location_test.id}))"
assert_equal f.taxonomy_search, "(organization_id ^ (#{organization_test.id})) and (location_id ^ (#{location_test.id}))"

@@ -29,7 +29,7 @@ const NewFiltersFormPage = ({ location: { search }, history }) => {
isNew
roleName={roleName || ''}
roleId={urlRoleId}
data={{}}
data={{ locations, organizations }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does

data.organizations?.map(o => o.id) || []

mean it would always be [] since taxonomies weren't passed within data before your patch?..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the new page, in the edit page we have data={response}

@MariaAga MariaAga force-pushed the roles-taxonomy-on-create branch from e2698dc to 1579997 Compare November 20, 2024 11:56
@MariaAga
Copy link
Member Author

thanks @ofedoren, updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants