diff -u b/core/modules/help_topics/help_topics.api.php b/core/modules/help_topics/help_topics.api.php --- b/core/modules/help_topics/help_topics.api.php +++ b/core/modules/help_topics/help_topics.api.php @@ -14,10 +14,11 @@ * Perform alterations on help topic definitions. * * @param array $info - * Array of information exposed by help topic plugins. + * Array of help topic plugin definitions keyed by their plugin ID. */ -function hook_help_topics_info_alter(&$info) { - // @todo +function hook_help_topics_info_alter(array &$info) { + // Alter the help topic to be displayed on admin/help. + $info['example.help_topic']['top_level'] = TRUE; } /** diff -u b/core/modules/help_topics/src/Controller/HelpTopicPluginController.php b/core/modules/help_topics/src/Controller/HelpTopicPluginController.php --- b/core/modules/help_topics/src/Controller/HelpTopicPluginController.php +++ b/core/modules/help_topics/src/Controller/HelpTopicPluginController.php @@ -2,6 +2,7 @@ namespace Drupal\help_topics\Controller; +use Drupal\Component\Utility\SortArray; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Url; use Drupal\help_topics\HelpTopicPluginManagerInterface; @@ -86,9 +87,6 @@ $related = $help_topic->getRelated(); foreach ($related as $other_id) { if ($other_id !== $id) { - if (!$this->helpTopicPluginManager->hasDefinition($other_id)) { - break; - } /** @var \Drupal\help_topics\HelpTopicPluginInterface $topic */ $topic = $this->helpTopicPluginManager->createInstance($other_id); $links[$other_id] = [ @@ -100,12 +98,7 @@ } if (count($links)) { - uasort($links, function ($a, $b) { - if ($a['title'] == $b['title']) { - return 0; - } - return ($a['title'] < $b['title']) ? -1 : 1; - }); + uasort($links, [SortArray::class, 'sortByTitleElement']); $build['#related'] = [ '#theme' => 'links__related', '#heading' => [ diff -u b/core/modules/help_topics/src/HelpTopicDiscovery.php b/core/modules/help_topics/src/HelpTopicDiscovery.php --- b/core/modules/help_topics/src/HelpTopicDiscovery.php +++ b/core/modules/help_topics/src/HelpTopicDiscovery.php @@ -98,8 +98,9 @@ // The plugin ID begins with provider. list($file_name_provider,) = explode('.', $plugin_id, 2); // Only the Help Topics module can provider help for other extensions. - // @todo Remove help_topics special case once Help Topics is stable and - // core modules can provide their own help topics. + // @todo https://www.drupal.org/project/drupal/issues/3025577 Remove + // help_topics special case once Help Topics is stable and core + // modules can provide their own help topics. if ($provider !== 'help_topics' && $provider !== $file_name_provider) { throw new DiscoveryException("The $file should begin with '$provider.'"); } @@ -131,7 +132,7 @@ } } if (!isset($data['label'])) { - throw new DiscoveryException("The $file contains does not contain the required meta tag with name='label'"); + throw new DiscoveryException("The $file contains does not contain the required meta tag with name='help_topic:label'"); } $all[$provider][$data['id']] = $data; diff -u b/core/modules/help_topics/src/HelpTopicPluginManager.php b/core/modules/help_topics/src/HelpTopicPluginManager.php --- b/core/modules/help_topics/src/HelpTopicPluginManager.php +++ b/core/modules/help_topics/src/HelpTopicPluginManager.php @@ -92,9 +92,13 @@ */ protected function getDiscovery() { if (!isset($this->discovery)) { - // We want to find help topic plugins in both modules and themes in + // We want to find help topic plugins in core, modules and themes in // a sub-directory called help_topics. - $directories = array_merge($this->moduleHandler->getModuleDirectories(), $this->themeHandler->getThemeDirectories()); + $directories = array_merge( + ['core'], + $this->moduleHandler->getModuleDirectories(), + $this->themeHandler->getThemeDirectories() + ); $directories = array_map(function ($dir) { return [$dir . '/help_topics']; diff -u b/core/modules/help_topics/src/Plugin/HelpSection/HelpTopicSection.php b/core/modules/help_topics/src/Plugin/HelpSection/HelpTopicSection.php --- b/core/modules/help_topics/src/Plugin/HelpSection/HelpTopicSection.php +++ b/core/modules/help_topics/src/Plugin/HelpSection/HelpTopicSection.php @@ -122,14 +122,15 @@ } } - // Sort the top level topics by label and if they match by plugin ID. + // Sort the top level topics by label and, if the labels match, then by + // plugin ID. usort($this->topLevelPlugins, function (HelpTopicPluginInterface $a, HelpTopicPluginInterface $b) { $a_label = (string) $a->getLabel(); $b_label = (string) $b->getLabel(); if ($a_label === $b_label) { return $a->getPluginId() < $b->getPluginId() ? -1 : 1; } - return $a_label < $b_label ? -1 : 1; + return strnatcasecmp($a_label, $b_label); }); } return $this->topLevelPlugins; diff -u b/core/modules/help_topics/tests/modules/help_topics_test/help_topics_test.module b/core/modules/help_topics/tests/modules/help_topics_test/help_topics_test.module --- b/core/modules/help_topics/tests/modules/help_topics_test/help_topics_test.module +++ b/core/modules/help_topics/tests/modules/help_topics_test/help_topics_test.module @@ -18,0 +19,7 @@ + +/** + * Implements hook_help_topics_info_alter(). + */ +function help_topics_test_help_topics_info_alter(array &$info) { + $info['help_topics_test.test']['top_level'] = \Drupal::state()->get('help_topics_test.test:top_level', TRUE); +} diff -u b/core/modules/help_topics/tests/src/Functional/HelpTopicTest.php b/core/modules/help_topics/tests/src/Functional/HelpTopicTest.php --- b/core/modules/help_topics/tests/src/Functional/HelpTopicTest.php +++ b/core/modules/help_topics/tests/src/Functional/HelpTopicTest.php @@ -102,6 +102,16 @@ $pos = $new_pos; } + // Ensure the plugin manager alter hook works as expected. + $session->linkExists('ABC Help Test module'); + \Drupal::state()->set('help_topics_test.test:top_level', FALSE); + \Drupal::service('plugin.manager.help_topic')->clearCachedDefinitions(); + $this->drupalGet('admin/help'); + $session->linkNotExists('ABC Help Test module'); + \Drupal::state()->set('help_topics_test.test:top_level', TRUE); + \Drupal::service('plugin.manager.help_topic')->clearCachedDefinitions(); + $this->drupalGet('admin/help'); + // Ensure all the expected links are present before uninstalling. $session->linkExists('ABC Help Test module'); $session->linkExists('ABC Help Test'); only in patch2: unchanged: --- /dev/null +++ b/core/modules/help_topics/tests/src/Unit/HelpTopicDiscoveryTest.php @@ -0,0 +1,157 @@ + [ + 'foo' => [ + 'help_topics' => [ + // The content of the help topic does not matter. + 'test.topic.html.twig' => '', + ], + ], + ], + ]); + $discovery = new HelpTopicDiscovery(['foo' => vfsStream::url('root/modules/foo/help_topics')]); + + $this->setExpectedException(DiscoveryException::class, "The vfs://root/modules/foo/help_topics/test.topic.html.twig should begin with 'foo.'"); + $discovery->getDefinitions(); + } + + /** + * @covers ::findAll + */ + public function testDiscoveryExceptionMissingLabelMetaTag() { + vfsStream::setup('root'); + + vfsStream::create([ + 'modules' => [ + 'test' => [ + 'help_topics' => [ + // The content of the help topic does not matter. + 'test.topic.html.twig' => '', + ], + ], + ], + ]); + $discovery = new HelpTopicDiscovery(['test' => vfsStream::url('root/modules/test/help_topics')]); + + $this->setExpectedException(DiscoveryException::class, "The vfs://root/modules/test/help_topics/test.topic.html.twig contains does not contain the required meta tag with name='help_topic:label'"); + $discovery->getDefinitions(); + } + + /** + * @covers ::findAll + */ + public function testDiscoveryExceptionInvalidMetaTag() { + vfsStream::setup('root'); + // Note a blank line is required after the last meta tag otherwise the last + // meta tag is not parsed. + $topic_content = <<<'EOS' + + + +EOS; + + vfsStream::create([ + 'modules' => [ + 'test' => [ + 'help_topics' => [ + 'test.topic.html.twig' => $topic_content, + ], + ], + ], + ]); + $discovery = new HelpTopicDiscovery(['test' => vfsStream::url('root/modules/test/help_topics')]); + + $this->setExpectedException(DiscoveryException::class, "The vfs://root/modules/test/help_topics/test.topic.html.twig contains invalid meta tag with name='foo'"); + $discovery->getDefinitions(); + } + + /** + * @covers ::findAll + */ + public function testHelpTopicsExtensionProviderSpecialCase() { + vfsStream::setup('root'); + $topic_content = <<<'EOS' + +

Test

+EOS; + + vfsStream::create([ + 'modules' => [ + 'help_topics' => [ + 'help_topics' => [ + 'core.topic.html.twig' => $topic_content, + ], + ], + ], + ]); + $discovery = new HelpTopicDiscovery(['help_topics' => vfsStream::url('root/modules/help_topics/help_topics')]); + $this->assertArrayHasKey('core.topic', $discovery->getDefinitions()); + } + + /** + * @covers ::findAll + */ + public function testHelpTopicsDefinition() { + $container = new ContainerBuilder(); + $container->set('string_translation', $this->getStringTranslationStub()); + \Drupal::setContainer($container); + + vfsStream::setup('root'); + $topic_content = <<<'EOS' + + + +

Test

+EOS; + + vfsStream::create([ + 'modules' => [ + 'foo' => [ + 'help_topics' => [ + 'foo.topic.html.twig' => $topic_content, + ], + ], + ], + ]); + $discovery = new HelpTopicDiscovery(['foo' => vfsStream::url('root/modules/foo/help_topics')]); + $definition = $discovery->getDefinitions()['foo.topic']; + $this->assertEquals('Test', $definition['label']); + $this->assertInstanceOf(TranslatableMarkup::class, $definition['label']); + $this->assertSame(TRUE, $definition['top_level']); + $this->assertSame(['one', 'two', 'three'], $definition['related']); + $this->assertSame('foo', $definition['provider']); + $this->assertSame(HelpTopicTwig::class, $definition['class']); + $this->assertSame(vfsStream::url('root/modules/foo/help_topics/foo.topic.html.twig'), $definition['_discovered_file_path']); + $this->assertSame('foo.topic', $definition['id']); + } + +}