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

SearchKit - Support implied operators in exposed search forms #19959

Merged
merged 6 commits into from
Apr 5, 2021

Conversation

colemanw
Copy link
Member

Overview

This opens up a lot more possibilities for exposed filters with searchKit displays.

Before

Only single-value filters accepted.

After

New "Multi-Select" and "Search by range" options will allow searches like IN(...) or BETWEEN [...] respectively.

image

The most complex one is Date fields, which allows you to change the widget to Select and enable "Search by range". If you do both then you get the familiar dropdown of relative dates plus a "Choose Date Range" option:

image

Technical Details

Adds a basic unit test to ensure operators are functioning.

Comments

This doesn't implement exposed filters, but it doesn't box us out of that possibility either. The filter syntax will accept operators.

Previously, default help_pre and help_post would not show if set for a custom field.
@civibot
Copy link

civibot bot commented Mar 31, 2021

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@colemanw style issue

};
defn.input_attrs = _.isEmpty(defn.input_attrs) ? {} : defn.input_attrs;
Copy link
Contributor

Choose a reason for hiding this comment

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

are we switching from an empty array to an empty object here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes the former was a mistake made by my php-centric brain.

@eileenmcnaughton
Copy link
Contributor

I gave this a quick spin & it seems to work fine although I note that

  1. the date range I selected doesn't seem to show once selected - here I have 'this calendar month selected' on the test site here http://core-19959-95lp4.test-1.civicrm.org:8001/civicrm/post

image

  1. the cache clearing is really tricky - when you save the form it does not clear the cache out so if you add a field to the form it doesn't show up until you take a separate cache-clearing action

  2. I was able to select 'search by range' for 'prefix_id' which seems a bit odd but I suppose it permits people to do the sorts of weird and wonderful things people always seem to come up with

  3. I wasn't able to do search by range on postal code - which makes sense from a metadata POV as it is a string but we have handling on advanced search to permit it

@colemanw I don't consider the above to be blocking although the first is perhaps 'near blocking'. I'm happy to merge & leave those things out of scope if you want

@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label Apr 5, 2021
colemanw added 3 commits April 5, 2021 15:14
This adds support for filter operators in SearchKit. It does not expose an operator selector to Afform
but allows an operator to be implied through the type of field configured.
e.g. a multiselect implies the IN operator & a range select implies BETWEEN.
Search display forms only allow numeric fields to be a search range,
but postal code is an exception (it's stored as a string but is numeric in some locales)
@colemanw
Copy link
Member Author

colemanw commented Apr 5, 2021

@eileenmcnaughton I've fixed #1 and #4. I think #3 is fine.
I reproduced #2 by enabling asset caching & if I can find the cause I'll put up a separate PR for that one.
This should be good to merge.

@eileenmcnaughton
Copy link
Contributor

I've retested & 1 is fixed

3 is quasi fixed - see how 900 is included here - I don't think addressing that needs to be in scope so I'm merging

image

@eileenmcnaughton eileenmcnaughton merged commit b07e0ca into civicrm:master Apr 5, 2021
@eileenmcnaughton eileenmcnaughton deleted the afFilterRange branch April 5, 2021 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:search-kit has-test master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants