Feature flexible resource filters

[robinboening]

What is this pull request for?

This PR enhances the resource filtering feature. Models can now define multiple and more flexible filters in the alchemy_resource_filters class method. It closes #2085

Disclaimer

This PR grew to a much bigger size as I initially imagined. This is mainly caused by the number of internal models having individually implemented filters as an addition to the also used alchemy_resource_filters feature which wasn't sufficient enough to cover their needs.

I consider this PR almost done, but not 100%. Hence the WiP in the title.

What's missing: It should add some tests covering some cases with multiple filters. Though, the changes made many tests fail which I was able to fix already, so the basic feature is actually still covered quite okay. Anyhow, even after fixing many tests there are still two select2 feature tests I haven't been able to fix yet. Any help fixing those two is much appreciated.

Notable changes

The changes allow to set up multiple filters in a unified but new data structure. Filters are still utilising model scopes, but now they not only support normal scopes but scopes with an argument as well.

Introducing a new data structure also means introducing a breaking change. Existing Alchemy apps having alchemy_resource_filters defined will need to adopt the new structure.

Scope with argument

The filter name is assumed to be a scope on the model and the values are passed as the argument. The values will probably almost always dynamically determined (most likely database records).

Scope without argument

The filter values are assumed to be scopes on the model (basically as before) and are usually hard-coded as the scope names are known and don't change.

Example

In this example you'll see two filters. The by_language filter fetches its values dynamically from the database and the according scope expects an argument. The status filter has hard-coded values and those are assumed to be existing scopes without arguments.

class Mymodel < ApplicationRecord
  scope :by_language, ->(lang) { where(language: lang) }
  scope :published, -> { where(published: true) }
  scope :drafts, -> { not.published }

  def self.alchemy_resource_filters
    [
      {
        name: :by_language,
        values: distinct.pluck(:language)
      },
      {
        name: :status,
        values: %w[published drafts]
      }
    ]
  end
end

Both, the name and the values can be translated. Since filter values are passed into options_for_select they can be provided in two formats:

• Array of String/Symbol [:value1, :value2, ...]
• Array of Array [["Translated value1", :value1], ["Translated value2", :value2], ...]

In the first case Alchemy will lookup a translation in alchemy.filters.<model_name>.<name_of_filter>.values whereas in the second case the provided values will just be passed to into options_for_select directly.

Security

There are two security measures in place against filter param tempering:

1. Arriving filter values must match the values defined in the model. If the value doesn't exist, the parameter gets rejected and the value won't be send to the model.
2. Rails' strong parameters kicks in for parameter names. In case someone tempered with the parameter names and the name doesn't match one of the defined filter names in the model it won't get permitted.

Misc

While adopting the new filter feature by the internal models I was forced to refactor a few areas.

1. One notable refactoring (which I believe is an improvement) is the removal and lifting of the Picture.search_by(params, ...), Picture.next(params) and Picture.previous(params) methods into the controller level. This eliminates the need to pass params into the model and allows to reuse existing code.
2. After uploading a new picture, we auto-apply a filter to show the last uploaded picture. The headline on top of the picture changes into "n Picture from last upload". Since the filter is no individual implementation any longer, the headline can not easily change anymore. Imo this is acceptable and we could open a separate issue to discuss if/how we want to bring this back in and if Attachment upload should behave the same (it never did).
3. I am using URLSearchParams for setting/removing params in javascript. Imo this is the preferred way as its the native Browser API. Since all modern browsers have this API implemented (source: https://caniuse.com/?search=URLSearchParams) I don't see an issue using it.

Screenshots

From the screenshots you can see it plays well together with other filters and search.

Checklist

• I have followed Pull Request guidelines
• I have added a detailed description into each commit message
• I have added tests to cover this change

[tvdeyen]

@robinboening rebasing with latest main should resolve CI issues

[tvdeyen]

Wow. What a great change and improvement 👍🏻

I really like that and only have some naming nits and naive questions.

Regarding the remaining failing specs I think we can solve them by wrapping the expectations in a within block to narrow down the potential nodes.

[tvdeyen]

true

*) TIP: in order to not ever need to think of those formatting things again: https://github.com/ruby-formatter/rufo

[tvdeyen]

Sorry about all the Rubocop comments. The easiest way to get rid of them is to install rufo, and autoformat the file changes with it.

[tvdeyen]

@robinboening btw. extremely good work on the PR description 👏🏻

[robinboening]

FYI, I haven't had the time to revisit this, but I'll most likely find some time in the next days or next week.

[robinboening]

@tvdeyen I finally got back to this PR and covered all the comments with either changes or a comment, except for one: The headline change when auto filtering after a picture upload. I'd like you to check the current state first so I can start fresh on the last item.

[tvdeyen]

Wow! This is really impressive work and is an improvement to the current resourceful admin views.

I think with the recent renaming of the picture filters to misc (I agree that this is a good name btw.) we need to update some views.

Thanks again! 🚀

[tvdeyen]

I just thought can we maybe find a way to still support the old resource filter usage by checking the contents of the filters array and pass it to miscellaneous filters if they are not a Hash, then print out a deprecation warning?

[robinboening]

I just thought can we maybe find a way to still support the old resource filter usage by checking the contents of the filters array and pass it to miscellaneous filters if they are not a Hash, then print out a deprecation warning?

Sounds feasible. I'll check how that would look like.

[robinboening]

@tvdeyen I pushed another commit for supporting the legacy filter structure. Now off to bed... 😴

[robinboening]

@tvdeyen I requested another review, hopefully the last one now 🤞

[tvdeyen]

@robinboening left one comment. If this is not easy to achieve now, we can add that later in another PR. This is already is major improvement. Thank you for all this great work!

[tvdeyen]

Impressive work