andypost reviewed this module (in its core patch format) on #2920309-13: Add experimental module for Help Topics and suggested:

+++ b/core/modules/config_help/src/Entity/HelpTopic.php
@@ -0,0 +1,331 @@
+ *   id = "help_topic",
+ *   label = @Translation("Help topic"),
+ *   config_prefix = "topic",
+ *   handlers = {

Probably it could use route_provider to generate routes

CommentFileSizeAuthor
#6 2920839-6.patch3.07 KBandypost
#3 2920839-4.patch12.24 KBjhodgdon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon created an issue. See original summary.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

On #2920843: See if we can use EntityDeleteForm instead of HelpDeleteForm I concluded that we could probably use the default entity delete confirm form for help topics if we defined a collection link and a collection route.

I had patched the HelpTopic entity class in the heading to look like:

  *   links = {
  *     "canonical" = "/admin/help-topic/{help_topic}",
+ *     "collection" = "/admin/config/development/help",
  *     "add-form" = "/admin/config/development/help/add",

but that collection route didn't work and I got errors like this:

Symfony\Component\Routing\Exception\RouteNotFoundException: Route "entity.help_topic.collection" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 190 of /home/jhodgdon/gitrepos/drupal_msq8/core/lib/Drupal/Core/Routing/RouteProvider.php).

That's because admin/config/development/help is currently defined in config_help.routing.yml as

config_help.topic_admin:
  path: '/admin/config/development/help'
  defaults:
    _entity_list: 'help_topic'
    _title: 'Help topics'
  requirements:
    _permission: 'administer help topics'

not as a collection entity route.

So it seems sensible to add the task of defining the collection entity route to this issue, then go back and see about the delete confirm form.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Assigned: Unassigned » jhodgdon
Status: Active » Fixed
FileSize
2920839-4.patch12.24 KB

This turned out to be fairly straightforward. Here's a patch. Tests pass locally.

Note: To apply the patch if the module is in core/modules, the info.yml line has to be applied manually, because the info file is slightly different there. Everything else should apply fine.

Actually... this is pretty straightforward and boilerplate. The only difficulty is that it changes one route name from a custom route (config_help.topic_admin) to a standarized one (entity.help_topic.collection). But this is a sandbox module so I think no other contrib modules will be depending on that route name staying the same...

I think I'll go ahead and commit it to the sandbox, and update the patch on the core issue.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented 
+++ b/src/Entity/HelpTopicRouteProvider.php
@@ -0,0 +1,60 @@
+namespace Drupal\config_help\Entity;

The Drupal\<module_name>\Entity namespace is meant only for actual entity classes, so I would suggest to put the route provider one level above (i.e. directly in the src/ folder).

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Fixed » Needs work

Sure, can do that.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Needs review
FileSize
2920839-6.patch3.07 KB

Moved class + fixed coding standards in changed files

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Fixed

Perfect, thanks! Yeah, my stupid editor always wants to indent twice in situations where you have nested parens, though Drupal standards only indent once. Serves me right for being stuck on Emacs since the dark ages. :)

Committed this to the Sandbox and updating the Core patch as well.

  • jhodgdon committed 2ba4684 on 8.x-1.x 
    Issue #2920839 by jhodgdon: Use a route provider

  • jhodgdon committed 569527d on 8.x-1.x 
    Issue #2920839 by andypost, amateescu: Fix coding standards for route...

Status: Fixed » Closed (fixed)

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