SearchKit - Ensure filters work with multiple search displays on a form

[colemanw]

Overview

Followup to #22887 - ensures filters work correctly with multiple search displays.

Before

Filter selection would cross-contaminate if multiple search displays were on the same form. To reproduce:

1. Create 2 searches for the same entity
2. Place them both on the same Afform
3. Add a filter to one of them
4. View the afform and make a selection in the filter. It will affect both searches, not just the one.

After

Tests lock in the fix.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@colemanw I tried testing this but I couldn't - I was actually a bit disappointed because the behaviour you were trying to get rid of seemed really useful (eg. I tried to have have 2 search-dashes side by side with one filter limiting both the contact to show and the contributions to list)

Unfortunately they won't 'stay' side by side. The side by side layout is 'too responsive' and even when you want them side by side with a scroll bar it drops down once I remove that second filter

[colemanw]

@eileenmcnaughton I think it's important to merge this and I don't think it should get hung up on layout issues. The layout doesn't matter as long as both displays are on the same form the issue will be triggered.

And I agree, it would be nice to have one set of filters control multiple displays, and we could implement that but we need to do it the right way and not the messy accident that this PR fixes because that causes unpredictable side-effects.

[eileenmcnaughton]

@colemanw from my testing this PR doens't make any difference? So I guess I'm OK merging based on that - but I failed to replicate the issue.

I tried some more to create a useful page but got stymied by the layout - here is what I would up with - it is SOOOO hard to drag those boxes into containers

<p class="af-text">My custom page</p>
<div class="af-container af-layout-cols">
  <div class="af-container af-layout-cols" af-title="Basic details">
    <div af-fieldset="">
      <af-field name="id" />
      <crm-search-display-table search-name="Basic_details" display-name="Basic_details_Table_1"></crm-search-display-table>
    </div>
  </div>
  <div af-fieldset="" af-title="Contributions">
    <div class="af-container af-layout-cols">
      <af-field name="id" />
      <af-field name="Contact_Contribution_contact_id_01.financial_type_id" defn="{input_attrs: {multiple: true}}" />
    </div>
    <crm-search-display-table search-name="Contributions" display-name="Contributions_Table_1"></crm-search-display-table>
  </div>
</div>

[colemanw]

@eileenmcnaughton I agree the drag-n-drop is frustrating to use. It's also frustrating to this developer trying to fix it (the api for angular-jquery-ui sortable widget is not well documented), but I'll give it another try.

As for this PR, if your tests showed that it didn't break anything, that's good enough for me. Because the unit test added in this PR is very thorough in demonstrating the bug and ensuring it is fixed.

[colemanw]

Aside, I think we could streamline our review process for PRs with has-test to 2 questions:

1. Is the test coverage thorough?
2. Does the test pass?
   And then merge without thinking too hard about it. Just a suggestion as our reviewer resources are pretty thin at the moment, and devs like you and I are doing double-duty on that front.

[eileenmcnaughton]

I'm not sure we can get away with that much simplification - but I have merged this one

[colemanw]

Thanks @eileenmcnaughton - I agree it's still a judgement call, but some PRs I approach purely from unit testing, and for those I think the tests and the test results speak for themselves.