Allow facets to be accessed over REST

Yes, I'm interested, please post that patch! ;)

It's 'relatively' easy to trigger facets on rest resources if you create your own facetResource and play with the isRenderedInCurrentRequest() method. Some code that's actually in production right now.

  /**
   * {@inheritdoc}
   */
  public function isRenderedInCurrentRequest() {
    $request = \Drupal::requestStack()->getMasterRequest();

    $routes = [
      'rest.score_resource_json.GET.json',
      'rest.score_resource_xml.GET.xml',
    ];

    $route = $request->get('_route');
    if (in_array($route, $routes)) {
      return TRUE;
    }

    return FALSE;
  }

Might serve as inspiration for a more global rest facet resource.
Moving to task. It's not because it isn't supported that something is a bug.

And no patch either

Working on it. Thanks @borisson_ and @swentel for the great help :)

1) Creating a new Style plugin for views.

2) Creating a new widget.

Here is the result:

Needs tests and feedback please.

Thanks!!!

I found a couple of smaller things that should be changed in the patch.

1. +++ b/src/FacetManager/DefaultFacetManager.php
   @@ -270,7 +270,7 @@ class DefaultFacetManager {
   -        return [];
   +        //return [];
   

   Was this change needed?

2. +++ b/src/Plugin/facets/facet_source/SearchApiViewsDeriver.php
   @@ -44,7 +44,7 @@ class SearchApiViewsDeriver extends FacetSourceDeriverBase {
   -            if (in_array($display_info['display_plugin'], ['page', 'block'])) {
   +            if (in_array($display_info['display_plugin'], ['page', 'block', 'rest_export'])) {
   

   so easy, nice!

3. +++ b/src/Plugin/facets/widget/RestWidget.php
   @@ -0,0 +1,184 @@
   + * The REST widget.
   

   We might be able to change this from REST widget to ArrayWidget? That makes more sense and it makes it easier for people that they can use it for other custom purposes as well.

4. +++ b/src/Plugin/facets/widget/RestWidget.php
   @@ -0,0 +1,184 @@
   +    //$item['#wrapper_attributes'] = ['class' => ['leaf']];
   

   let's just remove this line.

5. +++ b/src/Plugin/facets/widget/RestWidget.php
   @@ -0,0 +1,184 @@
   +    $form['soft_limit'] = [
   +      '#type' => 'select',
   +      '#title' => $this->t('Soft limit'),
   +      '#default_value' => $widget_configs['soft_limit'],
   +      '#options' => [0 => $this->t('No limit')] + array_combine($options, $options),
   +      '#description' => $this->t('Limits the number of displayed facets via JavaScript.'),
   +    ];
   

   I don't think soft limit is useful for this widget, so let's remove it.

6. +++ b/src/Plugin/views/style/FacetsSerializer.php
   @@ -0,0 +1,69 @@
   +    /** @var \Drupal\facets\FacetManager\DefaultFacetManager $facet_manager */
   +    $facet_manager = \Drupal::service('facets.manager');
   

   Is it possible to add this in the __construct by adding a create method?

1) Oops Sorry Done.

3) Sure. Makes sense. Done

4) Done

5) Done

6) Sure DI++

This will need a reroll, I just committed a bunch of issues.

Done. Needs tests and probably improve comments(and maybe refactor).

I only found one thing:

+++ b/src/Plugin/views/style/FacetsSerializer.php
@@ -0,0 +1,93 @@
+    $facets = $this->facetsManager->getFacetsByFacetSourceId("search_api_views:{$this->view->id()}:{$this->view->getDisplay()->display['id']}");
+    $this->facetsManager->updateResults("search_api_views:{$this->view->id()}:{$this->view->getDisplay()->display['id']}");

Maybe we can extract this into a variable for easier readability.

I started reviewing this because I thought this was adding a @RestResource plugin. But it's extending Views' REST export support. I don't know enough about that. Posting what I had so far.

1. +++ b/src/Plugin/facets/widget/ArrayWidget.php
   @@ -0,0 +1,173 @@
   + * The ArrayWidget widget.
   

   This sounds super abstract :D

2. +++ b/src/Plugin/facets/widget/ArrayWidget.php
   @@ -0,0 +1,173 @@
   +   * Prepares the url and values for the facet.
   

   s/url/URL/

3. +++ b/src/Plugin/facets/widget/ArrayWidget.php
   @@ -0,0 +1,173 @@
   +      '#title' => $this->t('Show the amount of results'),
   

   s/amount/number/, I think?

4. +++ b/src/Plugin/views/style/FacetsSerializer.php
   @@ -0,0 +1,93 @@
   +   * Constructs a Plugin object.
   

   FacetsSerializer

5. +++ b/src/Plugin/views/style/FacetsSerializer.php
   @@ -0,0 +1,93 @@
   +    if ((empty($this->view->live_preview))) {
   

   Unnecessary parenthesis-wrapping.

Thanks for the review Wim! I fixed all your remarks and added a unit test for the array widget. I'm adding a webtest now.

Adding integration test.

1. +++ b/src/Tests/RestIntegrationTest.php
   @@ -0,0 +1,72 @@
   +    $this->assertEqual($json_decoded->facets[0][0]->url, 'http://d81/facets-rest?f[0]=tawny_browed_owl%3Aarticle');
   ...
   +    $this->assertEqual($json_decoded->facets[0][1]->url, 'http://d81/facets-rest?f[0]=tawny_browed_owl%3Aitem');
   

   This will fail on the d.o testbots, you'll probably have to do a strpos($url, 'facets-rest?f[0]=tawny_browed_owl%3Aarticle') !== FALSE instead.

2. +++ b/src/Tests/RestIntegrationTest.php
   @@ -0,0 +1,72 @@
   +    $actual_json = $this->drupalGet('http://d81/facets-rest?f[0]=tawny_browed_owl%3Aitem');
   

   Same here, this won't work either, can we extract this from the json? so let's do $this->drupalGet($json_decoded->facets[0][0]->url instead.

3. +++ b/src/Tests/RestIntegrationTest.php
   @@ -0,0 +1,72 @@
   +    $this->assertEqual($json_decoded->facets[0][0]->url, 'http://d81/facets-rest?f[0]=tawny_browed_owl%3Aitem&f[1]=tawny_browed_owl%3Aarticle');
   

   same here.

4. +++ b/src/Tests/RestIntegrationTest.php
   @@ -0,0 +1,72 @@
   +    $this->assertEqual($json_decoded->facets[0][1]->url, 'http://d81/facets-rest');
   +    $this->assertEqual($json_decoded->facets[0][1]->values->value, 'item');
   +    $this->assertEqual($json_decoded->facets[0][1]->values->count, 3);
   +    $this->assertEqual($json_decoded->facets[0][1]->values->active, 'active');
   

   I'm not sure about 'active' as a way to define if it's active, a boolean might be better. Url is not correct either.

Sorry it was a copy/paste :D

Well, we can use $base_url?

Let's change active to use a bool instead.

Locally the integration test looks correct. Let's try to fix the problem with the order.

Fixing the unit tests

Just some small changes - nothing really changes here.

I made a booboo.

Removing tag.

Awesome! I think we can commit this new feature. As discussed we can open new follow-ups later if needed.

1. +++ b/src/Plugin/views/style/FacetsSerializer.php
   @@ -0,0 +1,94 @@
   +      $this->view->row_index = $row_index;
   

   What is this doing and why is this necessary? Can we add comments to this?

2. +++ b/src/Plugin/views/style/FacetsSerializer.php
   @@ -0,0 +1,94 @@
   +      $content_type = !empty($this->options['formats']) ? reset($this->options['formats']) : 'json';
   

   Why use reset? Reset "resets" the pointer to the first element of the array. We probably want to use "current" because that implies we haven't iterated of the options array yet and it will give us the first element. Why do we default to json? Is there constant or function we should use to find out what the default is?

3. +++ b/src/Plugin/views/style/FacetsSerializer.php
   @@ -0,0 +1,94 @@
   +      $processed_facets = $this->facetsManager->build($facet);
   ...
   +
   

   Aren't we overwriting the processed facets over and over here? Unless it's passed along. I would imagine we need array_merge here.

4. +++ b/src/Tests/RestIntegrationTest.php
   @@ -0,0 +1,81 @@
   +    // Get the output from the rest view and decde it into an array.
   

   typo: decode

5. +++ b/src/Tests/RestIntegrationTest.php
   @@ -0,0 +1,81 @@
   +    $actual_json = $this->drupalGet('facets-rest');
   

   Just call it $json and $page_output or keep it at $json_decoded.

6. +++ b/src/Tests/RestIntegrationTest.php
   @@ -0,0 +1,81 @@
   +    $actual_json = $this->drupalGet($json_decoded->facets[0][1]->url);
   

   same here

7. +++ b/src/Tests/RestIntegrationTest.php
   @@ -0,0 +1,81 @@
   +    $this->assertEqual($json_decoded->facets[0][0]->values->value, 'article');
   

   How come we do not show the processed facet value field as well? Eg Term ID that is processed in to a term name? Or am I missing something here? Is this perhaps present in a property "title"? And if so, can we add it in the test? :)

36.1) Done

36.2) For this endpoint we don't need to add the _format to the url. So we want to select the first one and by default the json. As discussed in Milano this is a copy paste from views :)

36.3) Done. To cover this part I'm adding 2 facets to the Facet Source.

36.4) Done.

36.5) Done.

36.6) Done.

36.7) AFAIK from the UI we use the term name by default... To be honest not sure what to do here... maybe it is correct as it is :) We should ask @borisson_!

1) Adding the field identifier per facet.

2) From Integration tests I need to use drupal_flush_all_caches(); when adding 2 facets. Not really sure why.

3) Looks like the "or" operator is not working. I'll try to take a look later or tomorrow.

>

Thanks Nick!

#36.7, this is correct behavior. We're only showing the value as it would be outputted by another facet, meaning that all processors will be executed as for other facets.

@borisson_ Perfect thanks!

So I'll continue later.

Fixing integration tests. Locally it looks ok.

Fixing unit tests.

Show numbers config option now works.

I think the sorting here isn't working as expected in php7, there's a difference between how php7 and php5 handle sorting, so we'll have to add more explicit sorting configuration here, or remove the order of the items from the test.

Yeahh really interesting. When the result is the same, the order is different depending on the php version...

      'banana' => [
        'url' => $base_url . '/facets-rest?f[0]=type%3Aitem&f[1]=keywords%3Abanana',
        'count' => 0
      ],
      'strawberry' => [
        'url' => $base_url . '/facets-rest?f[0]=type%3Aitem&f[1]=keywords%3Astrawberry',
        'count' => 0
      ],

@borisson_ Not sure if "banana" and "strawberry" should disappear filtering by item here because the result is 0. The OR operator works as expected for the Type facet but not sure about the behavior of a second facet. Perhaps create a followup?

We can create a followup if we need, I feel we can commit this but I'd love to discuss this a little more.

Thank you Joris

Thank you for the hard work for this patch.

I have the problem that a view with a "REST export" display based upon a Search API indexed content type ignores the facet filters. The main view display supports facet filtering. I have added both paths (main view display and REST export display) to the facet (in the blocks overview).

Which steps did I forget to enable facet filters for the "REST export" view display?

@Kartoffelsalat: next time, please create a new issue instead of commenting on a 8month old issue ;)

You need to create the facets to the correct facet source. This is not possible trough the block UI, as blocks don't show up on rest pages. You will have to create facets for every display of the view.

First of all, make sure the rest view uses the correct serializer.

Create your facets for the correct facet source.