Updated: Comment #0

Problem/Motivation

Routing system is a new big thing so it is time to convert info file configure link to route name.

Proposed resolution

Convert info file configure link to route name.

Remaining tasks

Discussion and patch
Related code in \Drupal\system\Form\ModulesListForm

<?php
   
// Generate link for module's configuration page, if it has one.
   
$row['links']['configure'] = array();
    if (
$module->status && isset($module->info['configure'])) {
      if ((
$configure = menu_get_item($module->info['configure'])) && $configure['access']) {
       
$row['links']['configure'] = array(
         
'#type' => 'link',
         
'#title' => $this->t('Configure'),
         
'#href' => $configure['href'],
         
'#options' => array('attributes' => array('class' => array('module-link', 'module-link-configure'), 'title' => $configure['description'])),
        );
      }
    }
?>

User interface changes

None

API changes

I don't think it is an api change.

https://drupal.org/project/issues/search/drupal?issue_tags=WSCCI-conversion

Files: 
CommentFileSizeAuthor
#15 configure-route-2089511-15.patch14.11 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 58,739 pass(es).
[ View ]
#15 interdiff.txt1.14 KBtim.plunkett
#15 Screen Shot 2013-10-03 at 3.58.49 PM.png17.43 KBtim.plunkett
#12 drupal8.routing-system.2089511-12.patch14.04 KBRoSk0
PASSED: [[SimpleTest]]: [MySQL] 59,283 pass(es).
[ View ]
#12 interdiff-2089511-8-12.txt323 bytesRoSk0
#9 drupal8.routing-system.2089511-8.patch14.04 KBRoSk0
FAILED: [[SimpleTest]]: [MySQL] 58,982 pass(es), 31 fail(s), and 7 exception(s).
[ View ]
#9 interdiff-2089511-2-6.txt1.5 KBRoSk0
#6 drupal8.routing-system.2089511-6.patch14.04 KBRoSk0
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.routing-system.2089511-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#6 interdiff-2089511-2-6.txt1.5 KBRoSk0
#2 drupal8.routing-system.2089511-2.patch14.83 KBRoSk0
FAILED: [[SimpleTest]]: [MySQL] 58,560 pass(es), 6 fail(s), and 3 exception(s).
[ View ]

Comments

Assigned:Unassigned» RoSk0

Lets rock!

Status:Active» Needs review
StatusFileSize
new14.83 KB
FAILED: [[SimpleTest]]: [MySQL] 58,560 pass(es), 6 fail(s), and 3 exception(s).
[ View ]

Initial patch.

Forget to mention - I removed 'configure' link for toolbar module, there is no route for it and looks like this was added accidentally.

Status:Needs review» Needs work

The last submitted patch, drupal8.routing-system.2089511-2.patch, failed testing.

+++ b/core/includes/common.inc
@@ -3521,7 +3521,7 @@ function drupal_pre_render_link($element) {
-    $element['#markup'] = \Drupal::linkGenerator()->generate($element['#title'], $element['#route_name'], $element['#route_parameters'], $element['#options']);
+    $element['#markup'] = \Drupal::linkGenerator()->generate(t('Configure'), $element['#route_name'], $element['#route_parameters'], $element['#options']);

We should not change the title

Status:Needs work» Needs review
StatusFileSize
new1.5 KB
new14.04 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.routing-system.2089511-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Fixed review comments.

Status:Needs review» Needs work

The last submitted patch, drupal8.routing-system.2089511-6.patch, failed testing.

Status:Needs work» Needs review
  1. +++ b/core/modules/toolbar/toolbar.info.yml
    @@ -7,4 +7,3 @@ version: VERSION
       - menu_link
    -configure: admin/structure/toolbar

    What happened here? Oh I see, there is no configuration available for toolbar.

  2. +++ b/core/modules/user/user.info.yml
    @@ -5,4 +5,4 @@ package: Core
    -configure: admin/config/people
    +configure: user.account_settings

    user.admin_index:
      path: '/admin/config/people'

    You seem to have changed the used route name...

StatusFileSize
new1.5 KB
new14.04 KB
FAILED: [[SimpleTest]]: [MySQL] 58,982 pass(es), 31 fail(s), and 7 exception(s).
[ View ]

Reroll.

Status:Needs review» Needs work

The last submitted patch, drupal8.routing-system.2089511-8.patch, failed testing.

Have a look at my review...

Status:Needs work» Needs review
StatusFileSize
new323 bytes
new14.04 KB
PASSED: [[SimpleTest]]: [MySQL] 59,283 pass(es).
[ View ]

Fixed review comments and rerolled.

Status:Needs review» Reviewed & tested by the community

I manually reviewed each path/route_name replacement, looks good!

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/system/lib/Drupal/system/Form/ModulesListForm.php
@@ -193,12 +198,17 @@ protected function buildRow(array $modules, $module, $distribution) {
+          '#route_name' => $module->info['configure'],
+          '#options' => array(
+            'attributes' => array(
+              'class' => array('module-link', 'module-link-configure'),
...
+            ),
+          ),

Are $configure['description'] and $module->info['description'] really equivalent? I don't think so.

Status:Needs work» Needs review
StatusFileSize
new17.43 KB
new1.14 KB
new14.11 KB
PASSED: [[SimpleTest]]: [MySQL] 58,739 pass(es).
[ View ]

You're 100% right. HEAD uses the description of the menu link it points to, this patch uses the module description (which is already visible on the page).

I think we could chose to skip this completely, since its just a title text on a link in a collapsed details element, but if we want to keep it this is how.

Screen Shot 2013-10-03 at 3.58.49 PM.png

Status:Needs review» Reviewed & tested by the community

Awesome catch by @alexpott and nice fix by @tim.plunkett back to RTBC.

Title:Convert info file configure link to route nameChange notice: Convert info file configure link to route name
Priority:Normal» Major
Status:Reviewed & tested by the community» Active
Issue tags:+Needs change record

This seems consistent with our use of routes over paths elsewhere.

Committed and pushed to 8.x. Thanks!

This'll need a change notice.

Title:Change notice: Convert info file configure link to route nameConvert info file configure link to route name
Assigned:RoSk0» Unassigned
Priority:Major» Normal
Status:Active» Fixed
Issue tags:-Needs change record

I moved @jibran's change notice into the main one:
https://drupal.org/node/1935708#configure

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

#2147689: [regression] Support route parameters for *info.yml files' "configure" route name tries to fix a small regression that was introduced by this issue.