Issue #2906665 by borisson_: Fix parereview issues

README.txt

@@ -23,7 +23,6 @@ INSTALLATION
    https://drupal.org/documentation/install/modules-themes/modules-7
    for further information.
 
-
 CONFIGURATION
 -------------
 Before adding a facet, there should be a facet source. Facet sources can be:
@@ -138,7 +137,7 @@ FAQ
 Q: Why do the facets disappear after a refresh.
 A: We don't support cached views, change the view to disable caching.
 
-Q: Why doesn't chosen (or similiar javascript dropdown replacement) not work
+Q: Why doesn't chosen (or similar javascript dropdown replacement) not work
 with the dropdown widget.
 A: Because the dropdown we create for the widget is created trough javascript,
 the chosen module (and others, probably) doesn't find the select element.

facets.module

@@ -33,7 +33,7 @@ function facets_help($route_name, RouteMatchInterface $route_match) {
     case 'entity.facets_facet.collection':
       $output = '';
       $output .= '<p>' . t('Below is a list of facets grouped by facetsources they are associated with. A facetsource is the instance where the facet does the actual filtering, for example a View on a Search API index.') . '</p>';
-      $output .= '<p>' . t('The facets weight can be changed with drag and drop withing the same facet source. Although you can drag and drop a facet under any facet source, this change will not be performed on save.') . '</p>';
+      $output .= '<p>' . t('The facets weight can be changed with drag and drop within the same facet source. Although you can drag and drop a facet under any facet source, this change will not be performed on save.') . '</p>';
       return $output;
   }
 }
@@ -281,7 +281,7 @@ function facets_system_breadcrumb_alter(Breadcrumb &$breadcrumb, RouteMatchInter
     $active_results = [];
     $active_facets = [];
     foreach ($facets as $facet) {
-      if ($active_items = $facet->getActiveItems()) {
+      if (count($facet->getActiveItems()) > 0) {
         // Add the facet as a cacheable dependency.
         $breadcrumb->addCacheableDependency($facet);
         /** @var \Drupal\facets\UrlProcessor\UrlProcessorInterface $url_processor */

js/soft-limit.js

@@ -9,7 +9,7 @@
 
   Drupal.behaviors.facetSoftLimit = {
     attach: function (context, settings) {
-      if (settings.facets.softLimit !== undefined) {
+      if (settings.facets.softLimit !== 'undefined') {
         $.each(settings.facets.softLimit, function (facet, limit) {
           Drupal.facets.applySoftLimit(facet, limit);
         });

modules/core_search_facets/src/Plugin/facets/facet_source/CoreNodeSearchFacetSource.php

@@ -240,7 +240,7 @@ class CoreNodeSearchFacetSource extends FacetSourcePluginBase implements CoreSea
     $facet_fields = $this->getDefaultFields();
 
     // Get the allowed field types.
-    $allowed_field_types = $this->moduleHandler->invokeAll('facets_core_allowed_field_types', [$field_types = []]);
+    $allowed_field_types = $this->moduleHandler->invokeAll('facets_core_allowed_field_types', [[]]);
 
     // Get the current field instances and detect if the field type is allowed.
     $fields = FieldConfig::loadMultiple();

modules/facets_range_widget/js/slider.js

@@ -11,7 +11,7 @@
 
   Drupal.behaviors.facet_slider = {
     attach: function (context, settings) {
-      if (settings.facets !== undefined && settings.facets.sliders !== undefined) {
+      if (settings.facets !== 'undefined' && settings.facets.sliders !== 'undefined') {
         $.each(settings.facets.sliders, function (facet, settings) {
           Drupal.facets.addSlider(facet, settings);
         });

modules/facets_range_widget/src/Plugin/facets/processor/RangeSliderProcessor.php

@@ -12,7 +12,7 @@ use Drupal\facets\Processor\PreQueryProcessorInterface;
  * @FacetsProcessor(
  *   id = "range_slider",
  *   label = @Translation("Range slider"),
- *   description = @Translation("Add range results for all the steps beteen min and max range."),
+ *   description = @Translation("Add range results for all the steps between min and max range."),
  *   stages = {
  *     "pre_query" = 5,
  *     "post_query" = 5,

modules/facets_range_widget/src/Plugin/facets/processor/SliderProcessor.php

@@ -13,7 +13,7 @@ use Drupal\facets\Result\Result;
  * @FacetsProcessor(
  *   id = "slider",
  *   label = @Translation("Slider"),
- *   description = @Translation("Add results for all the steps beteen min and max range."),
+ *   description = @Translation("Add results for all the steps between min and max range."),
  *   stages = {
  *     "post_query" = 5
  *   }

modules/facets_summary/src/Plugin/Block/FacetsSummaryBlockDeriver.php

@@ -64,8 +64,6 @@ class FacetsSummaryBlockDeriver implements ContainerDeriverInterface {
           'admin_label' => $facets_summary->getName(),
           'description' => $this->t('Facets Summary'),
         ] + $base_plugin_definition;
-
-        $sources[] = $this->t('Facets Summary: :facets_summary', [':facets_summary' => $facets_summary->getName()]);
       }
     }
     $this->derivatives[$base_plugin_id] = $plugin_derivatives;

src/Controller/FacetController.php

@@ -21,7 +21,9 @@ class FacetController extends ControllerBase {
    *   The facet edit form.
    */
   public function editForm(FacetInterface $facets_facet) {
-    $facet = \Drupal::service('entity_type.manager')->getStorage('facets_facet')->load($facets_facet->id());
+    $facet = $this->entityTypeManager()
+      ->getStorage('facets_facet')
+      ->load($facets_facet->id());
     return $this->entityFormBuilder()->getForm($facet, 'default');
   }
 

src/Entity/Facet.php

@@ -381,7 +381,6 @@ class Facet extends ConfigEntityBase implements FacetInterface {
     // TODO: do not hardcode on taxonomy, make this configurable (or better,
     // autoselected depending field type).
     return ['type' => 'taxonomy', 'config' => []];
-    return $this->hierarchy;
   }
 
   /**

src/FacetListBuilder.php

@@ -255,7 +255,7 @@ class FacetListBuilder extends DraggableListBuilder {
         'subgroup' => $subgroup_class,
       ];
       $form['facets'][$facet_source_group['facet_source']['id']] = $this->buildFacetSourceRow($facet_source_group['facet_source']);
-      foreach ($facet_source_group['facets'] as $i => $facet) {
+      foreach ($facet_source_group['facets'] as $facet) {
         if ($facet instanceof FacetInterface) {
           $form['facets'][$facet->id()] = $this->buildRow($facet);
           $form['facets'][$facet->id()]['weight']['#attributes']['class'][] = $subgroup_class;

src/FacetSource/SearchApiFacetSourceInterface.php

@@ -31,7 +31,7 @@ interface SearchApiFacetSourceInterface extends FacetSourcePluginInterface {
    *
    * @return \Drupal\views\ViewExecutable|null
    *   NULL when the view can't be found or loaded, the view with preset display
-   * otherwise.
+   *   otherwise.
    */
   public function getViewsDisplay();
 

src/Form/FacetCloneForm.php

@@ -71,7 +71,7 @@ class FacetCloneForm extends EntityForm {
 
     if (strpos($facet->getFacetSourceId(), 'search_api:') === FALSE) {
       // We don't know how to clone other kinds of facets.
-      drupal_set_message('We can only clone Search API based facets.');
+      drupal_set_message($this->t('We can only clone Search API based facets.'));
       return [];
     }
 

src/Form/FacetForm.php

@@ -78,16 +78,6 @@ class FacetForm extends EntityForm {
     return NULL;
   }
 
-  /**
-   * Returns the widget plugin manager.
-   *
-   * @return \Drupal\facets\Widget\WidgetPluginManager
-   *   The widget plugin manager.
-   */
-  protected function getWidgetPluginManager() {
-    return $this->widgetPluginManager ?: \Drupal::service('plugin.manager.facets.widget');
-  }
-
   /**
    * Builds the configuration forms for all selected widgets.
    *
@@ -110,7 +100,7 @@ class FacetForm extends EntityForm {
     $widget = $facet->getWidgetInstance();
 
     $arguments = ['%widget' => $widget->getPluginDefinition()['label']];
-    if (!$config_form = $widget->buildConfigurationForm([], $form_state, $this->getEntity())) {
+    if (!$config_form = $widget->buildConfigurationForm([], $form_state, $facet)) {
       $type = 'details';
       $config_form = ['#markup' => $this->t('%widget widget needs no configuration.', $arguments)];
     }
@@ -136,7 +126,7 @@ class FacetForm extends EntityForm {
     $widget = $facet->getWidgetInstance();
 
     $widget_options = [];
-    foreach ($this->getWidgetPluginManager()->getDefinitions() as $widget_id => $definition) {
+    foreach ($this->widgetPluginManager->getDefinitions() as $widget_id => $definition) {
       $widget_options[$widget_id] = !empty($definition['label']) ? $definition['label'] : $widget_id;
     }
     $form['widget'] = [

src/Form/FacetSettingsForm.php

@@ -11,6 +11,7 @@ use Drupal\facets\FacetInterface;
 use Drupal\facets\FacetSource\FacetSourcePluginManager;
 use Drupal\facets\FacetSource\SearchApiFacetSourceInterface;
 use Drupal\facets\Processor\ProcessorPluginManager;
+use Drupal\views\Plugin\views\display\Block;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -291,10 +292,6 @@ class FacetSettingsForm extends EntityForm {
         drupal_set_message($message);
         $form_state->setRedirect('entity.facets_facet.edit_form', ['facets_facet' => $facet->id()]);
       }
-
-      if (isset($view_id, $display_plugin) && $display_plugin === 'block') {
-        $facet->setOnlyVisibleWhenFacetSourceIsVisible(FALSE);
-      }
     }
     else {
       drupal_set_message($this->t('Facet %name has been updated.', ['%name' => $facet->getName()]));
@@ -310,6 +307,9 @@ class FacetSettingsForm extends EntityForm {
     if (isset($facet_source) && $facet_source instanceof SearchApiFacetSourceInterface) {
       $view = $facet_source->getViewsDisplay();
       if ($view !== NULL) {
+        if ($view->display_handler instanceof Block) {
+          $facet->setOnlyVisibleWhenFacetSourceIsVisible(FALSE);
+        }
         $view->display_handler->overrideOption('cache', ['type' => 'none']);
         $view->save();
         drupal_set_message($this->t('Caching of view %view has been disabled.', ['%view' => $view->storage->label()]));

src/Form/FacetSourceEditForm.php

@@ -4,6 +4,7 @@ namespace Drupal\facets\Form;
 
 use Drupal\Core\Entity\EntityForm;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
+use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\facets\Entity\FacetSource;
 use Drupal\facets\UrlProcessor\UrlProcessorPluginManager;
@@ -30,7 +31,8 @@ class FacetSourceEditForm extends EntityForm {
   public static function create(ContainerInterface $container) {
     return new static(
       $container->get('entity_type.manager'),
-      $container->get('plugin.manager.facets.url_processor')
+      $container->get('plugin.manager.facets.url_processor'),
+      $container->get('module_handler')
     );
   }
 
@@ -42,7 +44,7 @@ class FacetSourceEditForm extends EntityForm {
    * @param \Drupal\facets\UrlProcessor\UrlProcessorPluginManager $url_processor_plugin_manager
    *   The url processor plugin manager.
    */
-  public function __construct(EntityTypeManagerInterface $entity_type_manager, UrlProcessorPluginManager $url_processor_plugin_manager) {
+  public function __construct(EntityTypeManagerInterface $entity_type_manager, UrlProcessorPluginManager $url_processor_plugin_manager, ModuleHandlerInterface $moduleHandler) {
     $facet_source_storage = $entity_type_manager->getStorage('facets_facet_source');
 
     $this->urlProcessorPluginManager = $url_processor_plugin_manager;
@@ -71,7 +73,7 @@ class FacetSourceEditForm extends EntityForm {
       $this->setEntity($facet_source);
     }
 
-    $this->setModuleHandler(\Drupal::moduleHandler());
+    $this->setModuleHandler($moduleHandler);
   }
 
   /**

src/Plugin/Block/FacetBlockDeriver.php

@@ -67,8 +67,6 @@ class FacetBlockDeriver implements ContainerDeriverInterface {
           'admin_label' => $facet->getName(),
           'description' => $this->t('Facet'),
         ] + $base_plugin_definition;
-
-        $sources[] = $this->t('Facet: :facet', [':facet' => $facet->getName()]);
       }
 
       $this->derivatives[$base_plugin_id] = $plugin_derivatives;

src/Plugin/facets/processor/BooleanItemProcessor.php

@@ -28,7 +28,7 @@ class BooleanItemProcessor extends ProcessorPluginBase implements BuildProcessor
     $config = $this->getConfiguration();
 
     /** @var \Drupal\facets\Result\Result $result */
-    foreach ($results as $id => $result) {
+    foreach ($results as $result) {
       if ($result->getRawValue() == 0) {
         $result->setDisplayValue($config['off_value']);
       }

src/Plugin/facets/processor/DependentFacetProcessor.php

@@ -79,7 +79,6 @@ class DependentFacetProcessor extends ProcessorPluginBase implements BuildProces
   public function buildConfigurationForm(array $form, FormStateInterface $form_state, FacetInterface $current_facet) {
     $build = [];
 
-    $processors = $current_facet->getProcessors();
     $config = $this->getConfiguration();
 
     // Loop over all defined blocks and filter them by provider, this builds an

src/Plugin/facets/processor/ListItemProcessor.php

@@ -35,21 +35,21 @@ class ListItemProcessor extends ProcessorPluginBase implements BuildProcessorInt
    *
    * @var \Drupal\Core\Config\ConfigManagerInterface
    */
-  private $configManager;
+  protected $configManager;
 
   /**
    * The entity field manager.
    *
    * @var \Drupal\Core\Entity\EntityFieldManagerInterface
    */
-  private $entityFieldManager;
+  protected $entityFieldManager;
 
   /**
    * The entity_type bundle info service.
    *
    * @var \Drupal\Core\Entity\EntityTypeBundleInfoInterface
    */
-  private $entityTypeBundleInfo;
+  protected $entityTypeBundleInfo;
 
   /**
    * Constructs a Drupal\Component\Plugin\PluginBase object.
@@ -123,7 +123,7 @@ class ListItemProcessor extends ProcessorPluginBase implements BuildProcessorInt
     elseif ($this->configManager->loadConfigEntityByName($config_entity_name) !== NULL) {
       $field = $this->configManager->loadConfigEntityByName($config_entity_name);
     }
-    // Fields defined in code don't cant be loaded from storage so check the
+    // Fields defined in code don't can't be loaded from storage so check the
     // fields property path and see if its part of the base fields.
     elseif ($field->getDataDefinition() instanceof FieldItemDataDefinition) {
       $fieldDefinition = $field->getDataDefinition()

src/Plugin/facets/query_type/SearchApiRange.php

@@ -67,7 +67,7 @@ class SearchApiRange extends QueryTypePluginBase {
 
     if (!empty($this->results)) {
       $facet_results = [];
-      foreach ($this->results as $key => $result) {
+      foreach ($this->results as $result) {
         if ($result['count'] || $query_operator == 'or') {
           $count = $result['count'];
           $result_filter = trim($result['filter'], '"');

src/Plugin/facets/query_type/SearchApiString.php

@@ -72,7 +72,7 @@ class SearchApiString extends QueryTypePluginBase {
 
     if (!empty($this->results)) {
       $facet_results = [];
-      foreach ($this->results as $key => $result) {
+      foreach ($this->results as $result) {
         if ($result['count'] || $query_operator == 'or') {
           $count = $result['count'];
           $result_filter = trim($result['filter'], '"');

src/Plugin/facets/widget/ArrayWidget.php

@@ -17,13 +17,6 @@ use Drupal\facets\Widget\WidgetPluginBase;
  */
 class ArrayWidget extends WidgetPluginBase {
 
-  /**
-   * A flag that indicates if we should display the numbers.
-   *
-   * @var bool
-   */
-  protected $showNumbers = FALSE;
-
   /**
    * {@inheritdoc}
    */
@@ -51,7 +44,7 @@ class ArrayWidget extends WidgetPluginBase {
   /**
    * {@inheritdoc}
    */
-  protected function buildListItems($facet, ResultInterface $result) {
+  protected function buildListItems(FacetInterface $facet, ResultInterface $result) {
     if ($children = $result->getChildren()) {
       $items = $this->prepare($result);
 

src/QueryType/QueryTypeRangeBase.php

@@ -72,7 +72,7 @@ abstract class QueryTypeRangeBase extends QueryTypePluginBase {
     // defined by self::calculateResultFilter().
     if (!empty($this->results)) {
       $facet_results = [];
-      foreach ($this->results as $key => $result) {
+      foreach ($this->results as $result) {
         if ($result['count'] || $query_operator == 'or') {
           $count = $result['count'];
           $result_filter = $this->calculateResultFilter(trim($result['filter'], '"'));

src/Widget/WidgetPluginBase.php

@@ -83,7 +83,7 @@ abstract class WidgetPluginBase extends PluginBase implements WidgetPluginInterf
    * This allows the following template suggestions:
    *  - facets-item-list--WIDGET_TYPE--FACET_ID
    *  - facets-item-list--WIDGET_TYPE
-   *  - facets-item-list
+   *  - facets-item-list.
    *
    * @param \Drupal\facets\FacetInterface $facet
    *   The facet whose output is being generated.
@@ -156,7 +156,7 @@ abstract class WidgetPluginBase extends PluginBase implements WidgetPluginInterf
    * @return array
    *   A renderable array of the result.
    */
-  protected function buildListItems($facet, ResultInterface $result) {
+  protected function buildListItems(FacetInterface $facet, ResultInterface $result) {
     $classes = ['facet-item'];
     $items = $this->prepareLink($result);
 

tests/src/Functional/IntegrationTest.php

@@ -134,11 +134,13 @@ class IntegrationTest extends FacetsTestBase {
    * Tests that a block view also works.
    */
   public function testBlockView() {
-    $facet_id = 'bvf';
+    $facet_id = 'block_view_facet';
 
-    $this->createFacet('Block view facet', $facet_id, 'type', 'block_1', 'views_block__search_api_test_view');
+    $webAssert = $this->assertSession();
+    $this->addFacet('Block view facet', 'type', 'search_api:views_block__search_api_test_view__block_1');
+    $this->createBlock($facet_id);
     $this->drupalGet('admin/config/search/facets/' . $facet_id . '/edit');
-    $this->drupalPostForm(NULL, ['facet_settings[only_visible_when_facet_source_is_visible]' => FALSE], 'Save');
+    $webAssert->checkboxNotChecked('facet_settings[only_visible_when_facet_source_is_visible]');
 
     // Place the views block in the footer of all pages.
     $block_settings = [
@@ -149,14 +151,14 @@ class IntegrationTest extends FacetsTestBase {
 
     // By default, the view should show all entities.
     $this->drupalGet('<front>');
-    $this->assertSession()->pageTextContains('Fulltext test index');
-    $this->assertSession()->pageTextContains('Displaying 5 search results');
-    $this->assertSession()->pageTextContains('item');
-    $this->assertSession()->pageTextContains('article');
+    $webAssert->pageTextContains('Fulltext test index');
+    $webAssert->pageTextContains('Displaying 5 search results');
+    $webAssert->pageTextContains('item');
+    $webAssert->pageTextContains('article');
 
     // Click the item link, and test that filtering of results actually works.
     $this->clickLink('item');
-    $this->assertSession()->pageTextContains('Displaying 3 search results');
+    $webAssert->pageTextContains('Displaying 3 search results');
   }
 
   /**
@@ -925,8 +927,10 @@ class IntegrationTest extends FacetsTestBase {
    *   The name of the facet.
    * @param string $facet_type
    *   The field of the facet.
+   * @param string $source_id
+   *   The facet source id.
    */
-  protected function addFacet($facet_name, $facet_type = 'type') {
+  protected function addFacet($facet_name, $facet_type = 'type', $source_id = 'search_api:views_page__search_api_test_view__page_1') {
     $facet_id = $this->convertNameToMachineName($facet_name);
 
     // Go to the Add facet page and make sure that returns a 200.
@@ -952,7 +956,7 @@ class IntegrationTest extends FacetsTestBase {
 
     // Configure the facet source by selecting one of the Search API views.
     $this->drupalGet($facet_add_page);
-    $this->drupalPostForm(NULL, ['facet_source_id' => 'search_api:views_page__search_api_test_view__page_1'], 'Configure facet source');
+    $this->drupalPostForm(NULL, ['facet_source_id' => '' . $source_id . ''], 'Configure facet source');
 
     // The field is still required.
     $this->drupalPostForm(NULL, $form_values, 'Save');
@@ -961,8 +965,8 @@ class IntegrationTest extends FacetsTestBase {
     // Fill in all fields and make sure the 'field is required' message is no
     // longer shown.
     $facet_source_form = [
-      'facet_source_id' => 'search_api:views_page__search_api_test_view__page_1',
-      'facet_source_configs[search_api:views_page__search_api_test_view__page_1][field_identifier]' => $facet_type,
+      'facet_source_id' => $source_id,
+      'facet_source_configs[' . $source_id . '][field_identifier]' => $facet_type,
     ];
     $this->drupalPostForm(NULL, $form_values + $facet_source_form, 'Save');
     $this->assertSession()->pageTextNotContains('field is required.');