Skip to content

SearchKit - Auto-apply filters passed in from Afform markup #20758

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

Merged
merged 1 commit into from
Jul 8, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Jul 3, 2021

Overview

Ensures that any scalar value passed as a filter via Afform markup will be auto-applied.

Adds unit test for a search display embedded in an Afform to lock in filter behavior.

Before

When embedding a search display in an Afform, filters passed in through the markup could be overridden.

After

If the filter contains a scalar value (not a dynamic value like a javascript variable) then it will be "force" applied by the SearchDisplay::run api and cannot be overridden.

e.g. <crm-search-display-table filters="{last_name: 'AfformTest'}" ... will have the last_name filter permanently set to the value "AfformTest".

@civibot
Copy link

civibot bot commented Jul 3, 2021

(Standard links)

@civibot civibot bot added the master label Jul 3, 2021
@colemanw colemanw force-pushed the addSearchAfformTest branch 3 times, most recently from 0d6f994 to 879f611 Compare July 3, 2021 04:49
@colemanw colemanw changed the title SearchKit - Add unit test for a search display embedded in an afform SearchKit - Auto-apply filters passed in from Afform markup Jul 3, 2021
Ensures that any scalar value passed as a filter via Afform markup will be auto-applied.
Adds unit test for a search display embedded in an afform to lock in filter behavior.
@colemanw colemanw force-pushed the addSearchAfformTest branch from 879f611 to 6bff386 Compare July 3, 2021 10:26
@eileenmcnaughton
Copy link
Contributor

@colemanw I tried testing this by pulling it down & enabling 'mock forms'. Unfortunately there is no path to this form so I tried to edit it from the form builder UI but I got an error (note that I have this PR and your event location search PR locally on top of master)

image

@colemanw
Copy link
Member Author

colemanw commented Jul 7, 2021

@eileenmcnaughton I didn't do any manual testing, just relied on the existing test cover plus the new test added to lock this in.

@eileenmcnaughton
Copy link
Contributor

@colemanw yeah - the error might be unrelated - but I do want to test this manually before merging

@colemanw
Copy link
Member Author

colemanw commented Jul 7, 2021

I think the reason you can't test that afform manually is because it references a search display that doesn't exist in your database. The unit test creates it.

@eileenmcnaughton
Copy link
Contributor

@colemanw hmm - could we create on install for that mock ext?

@colemanw
Copy link
Member Author

colemanw commented Jul 8, 2021

I suppose we could, if we declared SearchKit a dependency of the AfformMock extension.

@eileenmcnaughton
Copy link
Contributor

yeah we chatted about this on the PR review call - ideally we would. It might be installed conditionally rather than a dependency. But @seamuslee001 said he would test this PR

@seamuslee001
Copy link
Contributor

I was able to get this to work but had to do it in a round about way, I created a basic contact table search view, then created a form. Then had to edit the afform html using the afform_html extension to add in the filter of last_name of buckman and confirmed it directly applied to the search. This works for me merging

@seamuslee001 seamuslee001 merged commit a82068b into civicrm:master Jul 8, 2021
@seamuslee001 seamuslee001 deleted the addSearchAfformTest branch July 8, 2021 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants