Automatically disable caching of views facet sources (regression)

Looking at this some more, it seems that someone has already tried to solve this problem. Check out this code starting at line 313 of FacetSettingsForm::submitForm() :

    list($type,) = explode(':', $facet_source_id);

    if ($type === 'search_api_views') {
      list(, $view_id, $display) = explode(':', $facet_source_id);
    }

    if (isset($view_id)) {
      $view = Views::getView($view_id);
      $view->setDisplay($display);
      $view->display_handler->overrideOption('cache', ['type' => 'none']);
      $view->save();

      $display_plugin = $view->getDisplay()->getPluginId();
    }

If I'm not mistaken, this code should automatically disable the caching of the view when a facet settings form is saved. Unfortunately it doesn't work because the $facet_source_id has a different format than the module expects. When testing this, the facet source ID didn't look like [type]:[view_id]:[display], but like [module]:[type]__[view_id]__[display] ("search_api:views_page__search__page_1" in my case). I guess this is an oversight from the time when facets_update_8001() was introduced.

I could of course write a patch that changes the assumptions about what a facet source ID looks like, but that would easily break again. Obviously, this code needs tests. Also, it would be nice to have a utility method on the Facet class so we can "ask" the facet about its source view, instead of having to guesstimate it from the $facet_source_id string.

Maintainers, what do you think is the best approach?

Changed the title because this is in fact a regression introduced in #2772745: Search API integration doesn't check/define feature support of backends.

Writing tests isn't really my forte, especially when it comes to modules I don't fully understand... But thanks to the guidance in comment #4 I did manage to fix the regression. Patch attached, but tests are still needed.

I just noticed that the exact same problem occurs in FacetsSummarySettingsForm::submitForm(), so if the fix from #5 works, we'll have to apply it there too. (Or... do we take code duplication as a hint, and learn that this logic should be implemented in the ViewsPageDisplay class?)

That looks great, you even tested the message. Now we have to address comment #6.

Fair enough, we shouldn't try to do everything at once. Thanks for the follow-up.

So for now, applying the same fix to FacetsSummarySettingsForm will suffice. Here is a patch that does exactly that.

No test yet, sorry. I did try to port the new test in facets\Functional\IntegrationTest::testViewsCacheDisable() to facets_summary\Functional\IntegrationTest::testViewsCacheDisable(), but I stopped when I found that there is no createFacetSummary() method yet (comparable to createFacet()).

No problem, it's a good way to learn about the internals of the Facets module, and the time investment is only a fraction of what I get out of it for free :-)

Looks great. I (re-)read every line and couldn't find a single nitpick. If part of it wasn't my own code, I would RTBC :-)

I see. So we should wrap that part of the code in an if-statement?

if (\Drupal::hasService('plugin.manager.search_api.display')) {
  $search_api_display = \Drupal::service('plugin.manager.search_api.display')
    ->createInstance($facet_source_display_id);
  // etc...
}

I don't think we can return; at that point, we do want the submitForm() method to finish its task even if there is no view, right?