Fix up topics to use new help_topic_link function

re-parented as previous parent closed

added CR to summary https://www.drupal.org/node/3192582

First part - conversion for help_topic_link() (few places require URL so just renamed variable)

Also patch trying to use consistent naming

I'm reviewing #3094482: Convert action module hook_help() to topic(s), including views bulk operations and also working on a new patch for it. One of the things I'm doing is adding links that use the new help_topic_link() function for the topics in that patch. What I noticed is that if the module that provides the help topic being linked isn't installed, the link text is replaced by plain text, Missing help topic. Is there any sane way to display plain text with the title of the help topic instead of "Missing help topic"? Is there a parameter available? I have only been using the Help Topics Standards and this CR as a reference.

I think the situation I was running into was a bit of a chicken and egg situation. I've attached a screenshot of the page before I went with another approach (adding an "Additional resources" section). You can see how in this case "Missing help topic" isn't really helpful in this context.

Anyway, I don't want to derail this issue -- I just ran into this when using help_topic_link() and wondered if I was missing something here.

Interesting to check if there's something in watchdog when topic is missing...

The failed message could provide a topic ID to help find what is missing

I think this was the error message. Copied the traced until it hit the HelpTopicPluginController.

TypeError: Argument 1 passed to Drupal\help_topics\HelpTwigExtension::getTopicLink() must be of the type string, null given, called in /var/www/html/sites/default/files/php/twig/607653b04a1f0_action.overview.html.twig_bplZe_pZ85OIjtOM9gco-RuGq/Rgnx-chqgVZgDEjmK4YrDtlSVCOA8INuApJqwV3fxMc.php on line 47 in Drupal\help_topics\HelpTwigExtension->getTopicLink() (line 135 of /var/www/html/core/modules/help_topics/src/HelpTwigExtension.php)

#0 /var/www/html/sites/default/files/php/twig/607653b04a1f0_action.overview.html.twig_bplZe_pZ85OIjtOM9gco-RuGq/Rgnx-chqgVZgDEjmK4YrDtlSVCOA8INuApJqwV3fxMc.php(47): Drupal\help_topics\HelpTwigExtension->getTopicLink(NULL)
#1 /var/www/html/vendor/twig/twig/src/Template.php(405): __TwigTemplate_1821e9c34d40db56e588fa52216b2ab2518a3cc6db0b38db0af3f7223d082b30->doDisplay(Array, Array)
#2 /var/www/html/vendor/twig/twig/src/Template.php(378): Twig\Template->displayWithErrorHandling(Array, Array)
#3 /var/www/html/vendor/twig/twig/src/Template.php(390): Twig\Template->display(Array)
#4 /var/www/html/vendor/twig/twig/src/TemplateWrapper.php(45): Twig\Template->render(Array, Array)
#5 /var/www/html/core/modules/help_topics/src/HelpTopicTwig.php(65): Twig\TemplateWrapper->render()
#6 /var/www/html/core/modules/help_topics/src/Controller/HelpTopicPluginController.php(79): Drupal\help_topics\HelpTopicTwig->getBody()
#7 [internal function]: Drupal\help_topics\Controller\HelpTopicPluginController->viewHelpTopic('action.overview')

I don't think that's the correct watchdog message. I will recreate the problem on a fresh install and try again with the error message.

@Amber I mean kinda this patch, probably makes sense to display the missing topic ID

Filed follow-up for #15 #3213022: When generating link to non-existent help topic, put topic ID in fallback text

Fix #7 and checked all topics - all links to topics now using help_topic_link

Also used to change "see {{ topic }} as suggested so needs review for interdiff

Filed follow-up to convert url to help_route_link #3215784: [META] Fix up topics to use new help_route_link function

Looking pretty good! A few must-fix grammar issues. Because of the way some topic link tokens are embedded in sentences, the resulting sentence ends up being unclear or grammatically incorrect/confusing. Also, some grammar nit-picks which aren't as critical.

1. +++ b/core/modules/help_topics/help_topics/book.adding.html.twig
   @@ -7,12 +7,12 @@ related:
   +  <li>{% trans %}In the <em>Manage</em> administrative menu, navigate to <em>Content</em> &gt; <a href="{{ node_add }}"><em>Add content</em></a> &gt; <em>Book page</em>. If you have configured additional content types that can be added to books, you can substitute a different content type for <em>Book page</em> (see {{ configuring_topic }} for more information).{% endtrans %}</li>
   

   Grammar nit. Suggestion:

   <li>{% trans %}In the <em>Manage</em> administrative menu, navigate to <em>Content</em> &gt; <a href="{{ node_add }}"><em>Add content</em></a> &gt; <em>Book page</em>. If you have configured additional content types that can be added to books, you can substitute a different content type for <em>Book page</em>; see {{ configuring_topic }} for more information.{% endtrans %}</li>
   

2. +++ b/core/modules/help_topics/help_topics/search.overview.html.twig
   @@ -28,7 +28,7 @@ related:
   +<p>{% trans %}In order to configure site search using the core Search module, you will need to configure one or more search pages. You will also need to verify or alter permissions so that the desired user roles can search the site (see {{ user_overview_topic }} for more information about roles and permissions). For content search, you will also need to make sure that the search index is configured and that the site is fully indexed. Finally, you may wish to place the <em>Search form</em> block on pages of your site, or add the search page to a navigation menu, to give users easy access to search. See the related topics listed below for specific tasks.{% endtrans %}</p>
   

   Grammar nit. Suggestion:

   <p>{% trans %}In order to configure site search using the core Search module, you will need to configure one or more search pages. You will also need to verify or alter permissions so that the desired user roles can search the site. (See {{ user_overview_topic }} for more information about roles and permissions.) For content search, you will also need to make sure that the search index is configured and that the site is fully indexed. Finally, you may wish to place the <em>Search form</em> block on pages of your site, or add the search page to a navigation menu, to give users easy access to search. See the related topics listed below for specific tasks.{% endtrans %}</p>
   

3. +++ b/core/modules/help_topics/help_topics/views_ui.edit.html.twig
   @@ -5,14 +5,14 @@ related:
   +  <li>{% trans %}Find the section whose settings you want to change, such as <em>Format</em> or <em>Filter criteria</em> (see {{ views_overview_topic }} for more information).{% endtrans %}</li>
   

   Grammar nit. Suggestion:

   <li>{% trans %}Find the section whose settings you want to change, such as <em>Format</em> or <em>Filter criteria</em>. (See {{ views_overview_topic }} for more information.){% endtrans %}</li>
   

4. +++ b/core/modules/help_topics/help_topics/workflows.overview.html.twig
   @@ -4,12 +4,12 @@ top_level: true
   +<p>{% trans %}The core software allows you to {{ configuring_workflows_topic }} in which each transition has an associated permission that can be granted to a particular role.{% endtrans %}</p>
   

   The grammar doesn't work out with this particular topic link token. It renders as "The core software allows you to Configuring workflows in which each transition has an associated permission that can be granted to a particular role."

   I suggest changing to:

   <p>{% trans %}The core software allows you to configure workflows in which each transition has an associated permission that can be granted to a particular role. See {{ configuring_workflows_topic }} for more information.{% endtrans %}</p>
   

5. +++ b/core/modules/help_topics/help_topics/workflows.overview.html.twig
   @@ -4,12 +4,12 @@ top_level: true
   +<p>{% trans %}Users with sufficient permissions can use {{ changing_states_topic }}.{% endtrans %}</p>
   

   Same with this line. The resulting grammar ("Users with sufficient permissions can use Moving content between workflow states.") makes the statement unclear. I suggest changing to:

   <p>{% trans %}Users with sufficient permissions can change the workflow state of a particular entity. See {{ changing_states_topic }} for more information.{% endtrans %}</p>
   

Updated patch in #19 addressing suggestions made in #20, thanks!

Thank you @ankithashetty! I've reviewed the interdiff in #21 and all my suggestions were addressed. Great! Patch in #21 applies cleanly to a fresh clone of 9.3.x. I also checked to make sure no help topic links were missed. This looks ready to go.

The proposed resolution has 3 action items, but it looks like item 3 is not yet complete?

The only question left here is should we add a check to render related topics into https://git.drupalcode.org/project/drupal/-/blob/9.3.x/core/modules/help...

I mean this tests also should change the controller to prevent display of related topics which will be not accessible when related help topics will point to disabled modules (when topics will live outside of help module - in every core module)

Applied this locally and compared the changes with git word diff.

Committed d596833 and pushed to 9.3.x. Thanks!

As help topics is experimental, backported to 9.2.x

Thanks folks, this is a nice cleanup

Thank you! the last one is #3215784-23: [META] Fix up topics to use new help_route_link function