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?

CommentFileSizeAuthor
#8 drupal-forum-hook-help-use-route-2250335-8.patch950 bytespingwin4eg
#6 cleanup-forum_hook_help-2250335-6.patch1.04 KBdrupaledmonk
#3 cleanup-forum_hook_help-2250335-2.patch1.12 KBdrupaledmonk
#1 cleanup-forum_hook_help-2250335-1.patch596 bytesdrupaledmonk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drupaledmonk’s picture

drupaledmonk CreditAttribution: drupaledmonk commented
Status: Active » Needs review
FileSize
cleanup-forum_hook_help-2250335-1.patch596 bytes

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

drupaledmonk CreditAttribution: drupaledmonk commented
FileSize
cleanup-forum_hook_help-2250335-2.patch1.12 KB
drupaledmonk’s picture

drupaledmonk CreditAttribution: drupaledmonk commented
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

drupaledmonk CreditAttribution: drupaledmonk commented
Status: Needs work » Needs review
FileSize
cleanup-forum_hook_help-2250335-6.patch1.04 KB
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
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

pingwin4eg
Location Zaporizhia 🇺🇦
CreditAttribution: pingwin4eg commented
Status: Needs work » Needs review
FileSize
drupal-forum-hook-help-use-route-2250335-8.patch950 bytes

Patch made @ #dcdn code sprint. Please review.

sanchiz’s picture

sanchiz CreditAttribution: sanchiz commented
Issue tags: +dcdn-sprint
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Perfect!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

Can we please wait for #2183113: Update hook_help signature to use route_name instead of path?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

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

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
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.