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

Using multi-attribute Ransack filter not properly creating/pulling label for active filters sidebar content display #7702

Closed
corlissc opened this issue Oct 26, 2022 · 6 comments · May be fixed by #7717

Comments

@corlissc
Copy link

When utilizing Ransack's DSL to create an index filter for multiple attributes, ActiveAdmin::Views::ActiveFiltersSidebarContent doesn't properly create or pull the label to display within the active filters sidebar. Instead it pulls the first attribute specified to do a lookup within the ActiveFilter#resource.filters hash. This leads to misleading active filter data within the UI.

For example with the following Rails model:

class Address < ActiveRecord::Base
  #Contains the following attributes:
  # id
  # street_line_1
  # street_line_2
  # city
  # state
  # zip
  # county
end

And a corresponding ActiveAdmin object with a filter searching across city and county:

ActiveAdmin.register Address do
  filter :city_or_county, as: :string, label: 'City or county'
end

Expected behavior

When the city_or_county filter is utilized, I expect the active filters sidebar content to include an item with the label configured for the city_or_county filter (i.e. City or county) by utilizing the entirety of the filter (each attribute, any association, etc...) in order to build or properly lookup the correct filter metadata

Actual behavior

The active filters sidebar content includes an item with only the name of the first attribute accounted for "City". Furthermore, if there was a custom label for the "city" filter, it would pull that filter's label.

How to reproduce

./app/models/address.rb

class Address < ActiveRecord::Base
  #Contains the following attributes:
  # id
  # street_line_1
  # street_line_2
  # city
  # state
  # zip
  # county
end

./app/admin/addresses.rb

ActiveAdmin.register Address do
  filter :city_or_county, as: :string, label: 'City or county'
end

Start a rails server and utilize the filter... If the server started at http://localhost:3000, hitting the following URL would exercise the filter and show the output http://localhost:3000/admin/addresses?q[city_or_county_contains]=ma&commit=Filter&order=id_desc

@ray-curran
Copy link
Contributor

ray-curran commented Nov 9, 2022

This looks similar to #5885. The fix added the ability for the custom label when the condition has a predicate. So if you only need the single filter, including the predicate might help in the short term:

./app/admin/addresses.rb

ActiveAdmin.register Address do
  filter :city_or_county_contains, as: :string, label: 'City or county'
end

The second issue you mentioned about the multi-attribute filter using the first attribute, 'city', would still be present if you were using both city_and_county_contains and city filters:

./app/admin/addresses.rb

ActiveAdmin.register Address do
  filter :city_or_county_contains, as: :string, label: 'City or county'
  filter :city, as: :string, label: 'Cities'
end

Screen Shot 2022-11-09 at 2 27 42 PM

One solution might be to change this line and swap the order to prioritize the filter matching the condition key:

resource.filters[name.to_sym] || resource.filters[condition.key.to_sym]

ray-curran added a commit to ray-curran/activeadmin that referenced this issue Nov 9, 2022
…in#7702)

When there are filters for a single field and a multi-field ransack
search with predicate, the multi-field label on the active filter
should use the custom label from the filter input box.

Reproduction steps:

./app/admin/posts.rb
```
ActiveAdmin.register Post do
  filter :title_or_body_contains, as: :string, label: 'Title or Body'
  filter :title, as: :string

  ...
end
```

Navigate to the posts path for a user like
`localhost:3000/admin/users/4/posts`

Complete search in both fields, note that both active filters use the
label "Title contains". The `title_or_body_contains` should use the
label from the filter "Title or Body contains".
@corlissc
Copy link
Author

corlissc commented Nov 11, 2022

Thank you very much for your support and assistance!

The issue still persists as the condition.key contains the predicate (e.g. _has_any_term, _has_every_term, etc...) which will not match the keys within resource.filters (in ActiveAdmin::Filters::ActiveFilter#filter.) The condition.key would first have to be stripped of the predicate.

@ray-curran
Copy link
Contributor

@corlissc Can you provide some code snippets to replicate the issue fully?

@corlissc
Copy link
Author

corlissc commented Dec 5, 2022

@ray-curran, thanks for your response. I'm busy with a project at this current time, I will try to get you something in the next couple of weeks if I can find a free moment. Let me know if there is anything specific that you are looking for (format, etc...) that my original post didn't provide.

In the meantime, I can provide a quick breakdown of my troubleshooting with a more complex filter that provides the issue at hand. Let me know if this provides you enough information or if you still need additional code, etc... Thank you again for your time and support.

The Rails models:

class Facility < ActiveRecord::Base
  belongs_to :primary_contact, class_name: 'Contact', optional: true
end

class Contact < ActiveRecord::Base
  # first_name
  # last_name
  # email_address
  # phone_number
end

ActiveAdmin Facilities config:

ActiveAdmin.register Facility do
# Filter(s)
  :primary_contact_first_name_or_primary_contact_last_name_or_primary_contact_email_address_or_primary_contact_phone_number,
  as: :string,
  filters: [:has_any_term, :has_every_term],
  label: "POC Search (name, email, phone)"
end

Given the above, while troubleshooting this is what within ActiveAdmin::Views::ActiveFiltersSidebarContent provides when requesting the resources filters

pry(#<ActiveAdmin::Views::ActiveFiltersSidebarContent>)> filter.send(:resource).filters
=> { :primary_contact_first_name_or_primary_contact_last_name_or_primary_contact_email_address_or_primary_contact_phone_number=>
  {:as=>:string,
   :filters=>[:has_any_term, :has_every_term],
   :label=>"POC Search (name, email, phone)",
   :name=>"primary_contact_first_name_or_primary_contact_last_name_or_primary_contact_email_address_or_primary_contact_phone_number"},
 :created_at=>{},
 :updated_at=>{}}

The code within ActiveAdmin::Filters::ActiveFilter#filter, as you have in your example/solution above, is looking for (in the case of mine) the resource.filters[condition.key.to_sym] which won't match as it contains the selected filter option (i.e. one of has_any_term or has_every_term) which will not match without removing or ignoring the selected filter option, troubleshooting output:

pry(#<ActiveAdmin::Views::ActiveFiltersSidebarContent>)> filter.send(:condition).key.to_sym
=> :primary_contact_first_name_or_primary_contact_last_name_or_primary_contact_email_address_or_primary_contact_phone_number_has_any_term

# Will default to the name, which is the first term.
pry(#<ActiveAdmin::Views::ActiveFiltersSidebarContent>)> filter.send(:name).to_sym
=> :first_name

ray-curran added a commit to ray-curran/activeadmin that referenced this issue Feb 6, 2023
…in#7702)

When there are filters for a single field and a multi-field ransack
search with predicate, the multi-field label on the active filter
should use the custom label from the filter input box.

Reproduction steps:

./app/admin/posts.rb
```
ActiveAdmin.register Post do
  filter :title_or_body_contains, as: :string, label: 'Title or Body'
  filter :title, as: :string

  ...
end
```

Navigate to the posts path for a user like
`localhost:3000/admin/users/4/posts`

Complete search in both fields, note that both active filters use the
label "Title contains". The `title_or_body_contains` should use the
label from the filter "Title or Body contains".
@ray-curran
Copy link
Contributor

@corlissc - Thanks for the additional information. I've made an update to the pull request. Can you take a look and see if it would fix your issue? #7717

@corlissc
Copy link
Author

@ray-curran - The pull request's code will provide the fix needed for the issue. Thanks!

javierjulio pushed a commit to ray-curran/activeadmin that referenced this issue Mar 13, 2023
…in#7702)

When there are filters for a single field and a multi-field ransack
search with predicate, the multi-field label on the active filter
should use the custom label from the filter input box.

Reproduction steps:

./app/admin/posts.rb
```
ActiveAdmin.register Post do
  filter :title_or_body_contains, as: :string, label: 'Title or Body'
  filter :title, as: :string

  ...
end
```

Navigate to the posts path for a user like
`localhost:3000/admin/users/4/posts`

Complete search in both fields, note that both active filters use the
label "Title contains". The `title_or_body_contains` should use the
label from the filter "Title or Body contains".
ray-curran added a commit to ray-curran/activeadmin that referenced this issue Nov 3, 2023
…in#7702)

When there are filters for a single field and a multi-field ransack
search with predicate, the multi-field label on the active filter
should use the custom label from the filter input box.

Reproduction steps:

./app/admin/posts.rb
```
ActiveAdmin.register Post do
  filter :title_or_body_contains, as: :string, label: 'Title or Body'
  filter :title, as: :string

  ...
end
```

Navigate to the posts path for a user like
`localhost:3000/admin/users/4/posts`

Complete search in both fields, note that both active filters use the
label "Title contains". The `title_or_body_contains` should use the
label from the filter "Title or Body contains".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants