Issue #1918768 by larowlan, tim.plunkett: Refactor tour module to use routes instead of paths.

core/modules/system/config/schema/system.data_types.schema.yml

@@ -84,3 +84,18 @@ filter:
     status:
       type: boolean
       label: 'Enabled'
+
+# Array of routes with route_name and route_params keys.
+route:
+  type: mapping
+  label: 'Route'
+  mapping:
+    route_name:
+      type: text
+      label: 'Route Name'
+    route_params:
+      type: sequence
+      label: 'Route Params'
+      sequence:
+        - type: string
+          label: 'Param'

core/modules/tour/config/schema/tour.schema.yml

@@ -22,12 +22,12 @@ tour.tour.*:
     langcode:
       type: string
       label: 'Default language'
-    paths:
+    routes:
       type: sequence
-      label: 'Path settings'
+      label: 'Route settings'
       sequence:
-        - type: path
-          label: 'Path'
+        - type: route
+          label: 'Route'
     tips:
       type: sequence
       label: 'Tips'
@@ -68,5 +68,3 @@ tour.tip.text:
     body:
       type: text
       label: 'Body'
-
-

core/modules/tour/lib/Drupal/tour/Entity/Tour.php

@@ -53,11 +53,18 @@ class Tour extends ConfigEntityBase implements TourInterface {
   public $label;
 
   /**
-   * The paths in which this tip can be displayed.
+   * The routes on which this tour should be displayed.
    *
    * @var array
    */
-  protected $paths = array();
+  protected $routes = array();
+
+  /**
+   * The routes on which this tour should be displayed, keyed by route id.
+   *
+   * @var array
+   */
+  protected $keyedRoutes;
 
   /**
    * Holds the collection of tips that are attached to this tour.
@@ -85,8 +92,8 @@ class Tour extends ConfigEntityBase implements TourInterface {
   /**
    * {@inheritdoc}
    */
-  public function getPaths() {
-    return $this->paths;
+  public function getRoutes() {
+    return $this->routes;
   }
 
   /**
@@ -121,7 +128,7 @@ class Tour extends ConfigEntityBase implements TourInterface {
   public function getExportProperties() {
     $properties = parent::getExportProperties();
     $names = array(
-      'paths',
+      'routes',
       'tips',
     );
     foreach ($names as $name) {
@@ -130,4 +137,38 @@ class Tour extends ConfigEntityBase implements TourInterface {
     return $properties;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function hasMatchingRoute($route_name, $route_params) {
+    if (!isset($this->keyedRoutes)) {
+      $this->keyedRoutes = array();
+      foreach ($this->getRoutes() as $route) {
+        $this->keyedRoutes[$route['route_name']] = isset($route['route_params']) ? $route['route_params'] : array();
+      }
+    }
+    if (!isset($this->keyedRoutes[$route_name])) {
+      // We don't know about this route.
+      return FALSE;
+    }
+    if (empty($this->keyedRoutes[$route_name])) {
+      // We don't need to worry about route params, the route name is enough.
+      return TRUE;
+    }
+    foreach ($this->keyedRoutes[$route_name] as $key => $value) {
+      // If a required param is missing or doesn't match, return FALSE.
+      if (empty($route_params[$key]) || $route_params[$key] !== $value) {
+        return FALSE;
+      }
+    }
+    return TRUE;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function resetKeyedRoutes() {
+    unset($this->keyedRoutes);
+  }
+
 }

core/modules/tour/lib/Drupal/tour/Tests/TourTest.php

@@ -128,8 +128,8 @@ class TourTest extends TourTestBasic {
       'id' => 'tour-entity-create-test-en',
       'label' => 'Tour test english',
       'langcode' => 'en',
-      'paths' => array(
-        'tour-test-1',
+      'routes' => array(
+        array('route_name' => 'tour_test.1'),
       ),
       'tips' => array(
         'tour-test-1' => array(
@@ -172,5 +172,25 @@ class TourTest extends TourTestBasic {
 
     // Test hook_tour_alter().
     $this->assertText('Altered by hook_tour_tips_alter');
+
+    // Navigate to tour-test-3 and verify the tour_test_1 tip is found with
+    // appropriate classes.
+    $this->drupalGet('tour-test-3/foo');
+    $elements = $this->xpath('//li[@data-id=:data_id and @class=:classes and ./h2[contains(., :text)]]', array(
+      ':classes' => 'tip-module-tour-test tip-type-text tip-tour-test-1',
+      ':data_id' => 'tour-test-1',
+      ':text' => 'The first tip',
+    ));
+    $this->assertEqual(count($elements), 1, 'Found English variant of tip 1.');
+
+    // Navigate to tour-test-3 and verify the tour_test_1 tip is not found with
+    // appropriate classes.
+    $this->drupalGet('tour-test-3/bar');
+    $elements = $this->xpath('//li[@data-id=:data_id and @class=:classes and ./h2[contains(., :text)]]', array(
+      ':classes' => 'tip-module-tour-test tip-type-text tip-tour-test-1',
+      ':data_id' => 'tour-test-1',
+      ':text' => 'The first tip',
+    ));
+    $this->assertEqual(count($elements), 0, 'Found English variant of tip 1.');
   }
 }

core/modules/tour/lib/Drupal/tour/TourInterface.php

@@ -15,12 +15,25 @@ use Drupal\Core\Config\Entity\ConfigEntityInterface;
 interface TourInterface extends ConfigEntityInterface {
 
   /**
-   * The paths that this tour will appear on.
+   * The routes that this tour will appear on.
    *
    * @return array
-   *   Returns array of paths for the tour.
+   *   Returns array of routes for the tour.
    */
-  public function getPaths();
+  public function getRoutes();
+
+  /**
+   * Whether the tour matches a given set of route parameters.
+   *
+   * @param string $route_name
+   *   The route name the parameters are for.
+   * @param array $route_params
+   *   Associative array of raw route params.
+   *
+   * @return bool
+   *   TRUE if the tour matches the route parameters.
+   */
+  public function hasMatchingRoute($route_name, $route_params);
 
   /**
    * Returns tip plugin.
@@ -41,4 +54,9 @@ interface TourInterface extends ConfigEntityInterface {
    */
   public function getTips();
 
+  /**
+   * Resets the statically cached keyed routes.
+   */
+  public function resetKeyedRoutes();
+
 }

core/modules/tour/tests/Drupal/tour/Tests/Entity/TourTest.php

@@ -0,0 +1,156 @@
+<?php
+/**
+ * @file
+ * Contains \Drupal\tour\Tests\Entity\TourTest.
+ */
+
+namespace Drupal\tour\Tests\Entity\TourTest;
+
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * Tests the Tour entity.
+ *
+ * @group Tour
+ *
+ * @coversDefaultClass \Drupal\tour\Entity\Tour
+ */
+class TourTest extends UnitTestCase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getInfo() {
+    return array(
+      'name' => 'Tour entity tests',
+      'description' => 'Test \Drupal\tour\Entity\Tour.',
+      'group' => 'Tour',
+    );
+  }
+
+  /**
+   * Tests \Drupal\tour\Entity\Tour::hasMatchingRoute().
+   *
+   * @param array $routes
+   *   Array of routes as per the Tour::routes property.
+   * @param string $route_name
+   *   The route name to match.
+   * @param array $route_params
+   *   Array of route params.
+   * @param bool $result
+   *   Expected result.
+   *
+   * @covers ::hasMatchingRoute()
+   *
+   * @dataProvider routeProvider
+   */
+  public function testHasMatchingRoute($routes, $route_name, $route_params, $result) {
+    $tour = $this->getMockBuilder('\Drupal\tour\Entity\Tour')
+      ->disableOriginalConstructor()
+      ->setMethods(array('getRoutes'))
+      ->getMock();
+
+    $tour->expects($this->any())
+      ->method('getRoutes')
+      ->will($this->returnValue($routes));
+
+    $this->assertSame($result, $tour->hasMatchingRoute($route_name, $route_params));
+
+    $tour->resetKeyedRoutes();
+  }
+
+  /*
+   * Provides sample routes for testing.
+   */
+  public function routeProvider() {
+    return array(
+      // Simple match.
+      array(
+        array(
+          array('route_name' => 'some.route'),
+        ),
+        'some.route',
+        array(),
+        TRUE,
+      ),
+      // Simple non-match.
+      array(
+        array(
+          array('route_name' => 'another.route'),
+        ),
+        'some.route',
+        array(),
+        FALSE,
+      ),
+      // Empty params.
+      array(
+        array(
+          array(
+            'route_name' => 'some.route',
+            'route_params' => array('foo' => 'bar'),
+          ),
+        ),
+        'some.route',
+        array(),
+        FALSE,
+      ),
+      // Match on params.
+      array(
+        array(
+          array(
+            'route_name' => 'some.route',
+            'route_params' => array('foo' => 'bar'),
+          ),
+        ),
+        'some.route',
+        array('foo' => 'bar'),
+        TRUE,
+      ),
+      // Non-matching params.
+      array(
+        array(
+          array(
+            'route_name' => 'some.route',
+            'route_params' => array('foo' => 'bar'),
+          ),
+        ),
+        'some.route',
+        array('bar' => 'foo'),
+        FALSE,
+      ),
+      // One matching, one not.
+      array(
+        array(
+          array(
+            'route_name' => 'some.route',
+            'route_params' => array('foo' => 'bar'),
+          ),
+          array(
+            'route_name' => 'some.route',
+            'route_params' => array('bar' => 'foo'),
+          ),
+        ),
+        'some.route',
+        array('bar' => 'foo'),
+        TRUE,
+      ),
+      // One matching, one not.
+      array(
+        array(
+          array(
+            'route_name' => 'some.route',
+            'route_params' => array('foo' => 'bar'),
+          ),
+          array(
+            'route_name' => 'some.route',
+            'route_params' => array('foo' => 'baz'),
+          ),
+        ),
+        'some.route',
+        array('foo' => 'baz'),
+        TRUE,
+      ),
+    );
+  }
+
+}

core/modules/tour/tests/tour_test/config/tour.tour.tour-test-2.yml

@@ -2,8 +2,8 @@ id: tour-test-2
 module: tour_test
 label: Tour test english
 langcode: en
-paths:
-  - tour-test-2/*
+routes:
+  - route_name: tour_test.2
 tips:
   tour-test-2:
     id: tour-test-2

core/modules/tour/tests/tour_test/config/tour.tour.tour-test.yml

@@ -2,8 +2,11 @@ id: tour-test
 module: tour_test
 label: Tour test english
 langcode: en
-paths:
-  - tour-test-1
+routes:
+  - route_name: tour_test.1
+  - route_name: tour_test.3
+    route_params:
+      locale: foo
 tips:
   tour-test-1:
     id: tour-test-1

core/modules/tour/tests/tour_test/lib/Drupal/tour_test/Controller/TourTestController.php

@@ -23,8 +23,15 @@ class TourTestController implements ContainerInjectionInterface {
 
   /**
    * Outputs some content for testing tours.
+   *
+   * @param string $locale
+   *  (optional) Dummy locale variable for testing routing parameters. Defaults
+   *   to 'foo'.
+   *
+   * @return array
+   *   Array of markup.
    */
-  public function tourTest1() {
+  public function tourTest1($locale = 'foo') {
     return array(
       'tip-1' => array(
         '#type' => 'container',

core/modules/tour/tests/tour_test/tour_test.routing.yml

@@ -18,3 +18,12 @@ tour_test.2:
     _content: '\Drupal\tour_test\Controller\TourTestController::tourTest2'
   requirements:
     _access: 'TRUE'
+
+tour_test.3:
+  path: '/tour-test-3/{locale}'
+  defaults:
+    locale: 'foo'
+    _content: '\Drupal\tour_test\Controller\TourTestController::tourTest1'
+  requirements:
+    _access: 'TRUE'
+

core/modules/tour/tour.module

@@ -5,6 +5,7 @@
  * Main functions of the module.
  */
 use Drupal\Core\Cache\CacheBackendInterface;
+use Symfony\Cmf\Component\Routing\RouteObjectInterface;
 
 /**
  * Implements hook_permission().
@@ -108,23 +109,23 @@ function tour_preprocess_page(&$variables) {
     return;
   }
 
-  // @todo replace this with http://drupal.org/node/1918768 once it is committed.
-  $path = current_path();
-  // Load all of the items and match on path.
-  $tours = entity_load_multiple('tour');
+  // Load all of the items and match on route name.
+  $request = \Drupal::request();
+  $route_name = $request->attributes->get(RouteObjectInterface::ROUTE_NAME);
 
-  $path_alias = drupal_strtolower(\Drupal::service('path.alias_manager')->getPathAlias($path));
-  foreach ($tours as $tour_id => $tour) {
-    // @todo Replace this with an entity query that does path matching when
-    // http://drupal.org/node/1918768 lands.
-    $pages = implode("\n", $tour->getPaths());
-    if (!drupal_match_path($path_alias, $pages) && (($path == $path_alias) || drupal_match_path($path, $pages))) {
-      unset($tours[$tour_id]);
+  $results = \Drupal::entityQuery('tour')
+    ->condition('routes.*.route_name', $route_name)
+    ->execute();
+  if (!empty($results) && $tours = entity_load_multiple('tour', array_keys($results))) {
+    foreach ($tours as $id => $tour) {
+      // Match on params.
+      if (!$tour->hasMatchingRoute($route_name, $request->attributes->get('_raw_variables')->all())) {
+        unset($tours[$id]);
+      }
+    }
+    if (!empty($tours)) {
+      $variables['page']['help']['tour'] = entity_view_multiple($tours, 'full');
     }
-  }
-
-  if ($tours) {
-    $variables['page']['help']['tour'] = entity_view_multiple($tours, 'full');
   }
 }
 

core/modules/views_ui/config/tour.tour.views-ui.yml

@@ -3,8 +3,9 @@ uuid: 261db4f0-603c-440d-8211-17a095614851
 module: views_ui
 label: 'Views ui'
 langcode: en
-paths:
-  - 'admin/structure/views/view/*'
+routes:
+  - route_name: views_ui.edit
+  - route_name: views_ui.edit_display
 tips:
   views-ui-active-display:
     id: views-ui-active-display