In forum.module in function forum_help(), there is the following code:

      $output = '<p>' . t('Forums contain forum topics. Use containers to group related forums.') . '</p>';
      $more_help_link = array(
        '#type' => 'link',
        '#href' => 'admin/help/forum',
        '#title' => t('More help'),
      );
      $container = array(
        '#theme' => 'container',
        '#children' => drupal_render($more_help_link),
        '#attributes' => array(
          'class' => array('more-help-link'),
        ),
      );
      $output .= drupal_render($container);

This shouldn't be hard-wiring the admin/help/forum link. It should be using

\Drupal::url('help.page', array('name' => 'forum'))

Should be a good Novice task to fix this, I think?

Files: 
CommentFileSizeAuthor
#8 drupal-forum-hook-help-use-route-2250335-8.patch950 bytespingwin4eg
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,321 pass(es).
[ View ]
#6 cleanup-forum_hook_help-2250335-6.patch1.04 KBdrupaledmonk
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 69,692 pass(es).
[ View ]
#3 cleanup-forum_hook_help-2250335-2.patch1.12 KBdrupaledmonk
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/forum/forum.module.
[ View ]
#1 cleanup-forum_hook_help-2250335-1.patch596 bytesdrupaledmonk
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/forum/forum.module.
[ View ]

Comments

drupaledmonk’s picture

Status:Active» Needs review
StatusFileSize
new596 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/forum/forum.module.
[ View ]

Helped me understand routes. :)

Status:Needs review» Needs work

The last submitted patch, 1: cleanup-forum_hook_help-2250335-1.patch, failed testing.

drupaledmonk’s picture

StatusFileSize
new1.12 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/forum/forum.module.
[ View ]
drupaledmonk’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 3: cleanup-forum_hook_help-2250335-2.patch, failed testing.

drupaledmonk’s picture

Status:Needs work» Needs review
StatusFileSize
new1.04 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 69,692 pass(es).
[ View ]
jhodgdon’s picture

Status:Needs review» Needs work

Thanks for taking this on!

So... I don't actually think we want to get rid of the render array structure that was being used here. I just think we want to get rid of:

        '#href' => 'admin/help/forum',

and use a route instead.

Take a look at:
https://api.drupal.org/api/drupal/core!includes!common.inc/function/drup...
It looks like instead of #href, you can use #route_name in a '#type' => 'link' render array.

Thanks!

pingwin4eg’s picture

Status:Needs work» Needs review
StatusFileSize
new950 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,321 pass(es).
[ View ]

Patch made @ #dcdn code sprint. Please review.

sanchiz’s picture

Issue tags:+dcdn-sprint
dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Perfect!

tim.plunkett’s picture

jhodgdon’s picture

I do not think this patch conflicts with #22183113, does it? That patch in this area has:

+
+    case 'forum.overview':
       $output = '<p>' . t('Forums contain forum topics. Use containers to group related forums.') . '</p>';
       $more_help_link = array(
         '#type' => 'link',
@@ -60,11 +62,14 @@ function forum_help($path, $arg) {
       );
       $output .= drupal_render($container);
       return $output;
-    case 'admin/structure/forum/add/container':

and this one has:

      $output = '<p>' . t('Forums contain forum topics. Use containers to group related forums.') . '</p>';
       $more_help_link = array(
         '#type' => 'link',
-        '#href' => 'admin/help/forum',
+        '#route_name' => 'help.page',
+        '#route_parameters' => array('name' => 'forum'),
         '#title' => t('More help'),
       );
       $container = array(

so it looks like they neatly manage to avoid hitting each others' context.

And you might want to tag the other issue "avoid commit conflicts".

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 04d9f33 and pushed to 8.x. Thanks!

  • Commit 04d9f33 on 8.x by alexpott:
    Issue #2250335 by drupaledmonk, pingwin4eg | jhodgdon: Forum hook_help...

Status:Fixed» Closed (fixed)

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