diff --git a/core/modules/help_topics/help_topics/core.security.html.twig b/core/modules/help_topics/help_topics/core.security.html.twig index 97c0ea1079ca9530cbd979203453ad869ba58819..99e66f55e249034ceff1f08ff12fc53aa0c09c98 100644 --- a/core/modules/help_topics/help_topics/core.security.html.twig +++ b/core/modules/help_topics/help_topics/core.security.html.twig @@ -2,6 +2,6 @@ label: 'Making your site secure' top_level: true related: - - menu_ui.menu_overview + - core.menu_overview --- <p>{% trans %}The topics listed here will help you make and keep your site secure.{% endtrans %}</p> diff --git a/core/modules/help_topics/src/HelpTopicDiscovery.php b/core/modules/help_topics/src/HelpTopicDiscovery.php index 159cb00b91cfde7edee0d67d7ca45b30018b8ed9..6ff7a10148c055ce0585fa2ae149ef95a4295f54 100644 --- a/core/modules/help_topics/src/HelpTopicDiscovery.php +++ b/core/modules/help_topics/src/HelpTopicDiscovery.php @@ -99,12 +99,12 @@ public function findAll() { $plugin_id = substr(basename($file), 0, -10); // 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 https://www.drupal.org/project/drupal/issues/3025577 Remove + // Only the Help Topics module can provide help for other extensions. + // @todo https://www.drupal.org/project/drupal/issues/3072312 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("$file should begin with '$provider.'"); + throw new DiscoveryException("$file file name should begin with '$provider'"); } $data = [ // The plugin ID is derived from the filename. The extension diff --git a/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.bad_html.html.twig b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.bad_html.html.twig new file mode 100644 index 0000000000000000000000000000000000000000..897f2d5ac69b6404fb8cdf43ba532a81742edef7 --- /dev/null +++ b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.bad_html.html.twig @@ -0,0 +1,5 @@ +--- +label: 'Help topic with bad HTML syntax' +top_level: true +--- +<p>{% trans %}Body goes here{% endtrans %}</h3> diff --git a/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.empty.html.twig b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.empty.html.twig new file mode 100644 index 0000000000000000000000000000000000000000..0023b35aba2c2a8c0115992d3ecf4d6dbc58dc61 --- /dev/null +++ b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.empty.html.twig @@ -0,0 +1,4 @@ +--- +label: 'Help topic containing no body' +top_level: true +--- diff --git a/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.h1.html.twig b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.h1.html.twig new file mode 100644 index 0000000000000000000000000000000000000000..cd47ddd8beb81e6591305ef5bac79713deee9e7c --- /dev/null +++ b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.h1.html.twig @@ -0,0 +1,5 @@ +--- +label: 'Help topic with H1 header' +top_level: true +--- +<h1>{% trans %}Body goes here{% endtrans %}</h1> diff --git a/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.hierarchy.html.twig b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.hierarchy.html.twig new file mode 100644 index 0000000000000000000000000000000000000000..d2b046dd468434af7a5e2ceded0b095926ac76f9 --- /dev/null +++ b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.hierarchy.html.twig @@ -0,0 +1,5 @@ +--- +label: 'Help topic with h3 without an h2' +top_level: true +--- +<h3>{% trans %}Body goes here{% endtrans %}</h3> diff --git a/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.related.html.twig b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.related.html.twig new file mode 100644 index 0000000000000000000000000000000000000000..96f46670d93b435909d7668c2757366d810f20c0 --- /dev/null +++ b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.related.html.twig @@ -0,0 +1,7 @@ +--- +label: 'Help topic related to nonexistent topic' +top_level: true +related: + - this.is.not.a.valid.help_topic.id +--- +<p>{% trans %}Body goes here{% trans %}</p> diff --git a/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.top_level.html.twig b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.top_level.html.twig new file mode 100644 index 0000000000000000000000000000000000000000..ae19252f8581efd6fae3dc02d3a461f2a6a349b0 --- /dev/null +++ b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.top_level.html.twig @@ -0,0 +1,4 @@ +--- +label: 'Help topic not top level or related to top level' +--- +<p>{% trans %}Body goes here{% endtrans %}</p> diff --git a/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.translated.html.twig b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.translated.html.twig new file mode 100644 index 0000000000000000000000000000000000000000..5b618acde930d20cf1ea60f50f26c562f25a760b --- /dev/null +++ b/core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.translated.html.twig @@ -0,0 +1,5 @@ +--- +label: 'Help topic with untranslated text' +top_level: true +--- +<p>Body goes here</p> diff --git a/core/modules/help_topics/tests/src/Functional/HelpTopicsSyntaxTest.php b/core/modules/help_topics/tests/src/Functional/HelpTopicsSyntaxTest.php new file mode 100644 index 0000000000000000000000000000000000000000..a8ef4eb2f7b10050177067f4a07ddfcdf543956e --- /dev/null +++ b/core/modules/help_topics/tests/src/Functional/HelpTopicsSyntaxTest.php @@ -0,0 +1,252 @@ +<?php + +namespace Drupal\Tests\help_topics\Functional; + +use Drupal\Tests\BrowserTestBase; +use Drupal\help_topics\HelpTopicDiscovery; +use PHPUnit\Framework\ExpectationFailedException; + +/** + * Verifies that all core Help topics can be rendered and comply with standards. + * + * @todo This test should eventually be folded into + * Drupal\Tests\system\Functional\Module\InstallUninstallTest + * when help_topics becomes stable, so that it will test with only one module + * at a time installed and not duplicate the effort of installing. See issue + * https://www.drupal.org/project/drupal/issues/3074040 + * + * @group help_topics + */ +class HelpTopicsSyntaxTest extends BrowserTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = [ + 'help', + 'help_topics', + ]; + + /** + * Tests that all Core help topics can be rendered and have good syntax. + */ + public function testHelpTopics() { + $this->drupalLogin($this->rootUser); + + // Enable all modules and themes, so that all routes mentioned in topics + // will be defined. + $module_directories = $this->listDirectories('module'); + \Drupal::service('module_installer')->install(array_keys($module_directories)); + $theme_directories = $this->listDirectories('theme'); + \Drupal::service('theme_installer')->install(array_keys($theme_directories)); + + $directories = $module_directories + $theme_directories + + $this->listDirectories('profile'); + $directories['core'] = 'core/help_topics'; + $directories['bad_help_topics'] = \Drupal::service('extension.list.module')->getPath('help_topics_test') . '/bad_help_topics/syntax/'; + + // Filter out directories outside of core. If you want to run this test + // on a contrib/custom module, remove the next line. + $directories = array_filter($directories, function ($directory) { + return strpos($directory, 'core') === 0; + }); + + // Verify that a few key modules, themes, and profiles are listed, so that + // we can be certain our directory list is complete and we will be testing + // all existing help topics. If these lines in the test fail in the future, + // it is probably because something we chose to list here is being removed. + // Substitute another item of the same type that still exists, so that this + // test can continue. + $this->assertArrayHasKey('system', $directories, 'System module is being scanned'); + $this->assertArrayHasKey('help', $directories, 'Help module is being scanned'); + $this->assertArrayHasKey('seven', $directories, 'Seven theme is being scanned'); + $this->assertArrayHasKey('standard', $directories, 'Standard profile is being scanned'); + + $definitions = (new HelpTopicDiscovery($directories))->getDefinitions(); + $this->assertGreaterThan(0, count($definitions), 'At least 1 topic was found'); + + // Test each topic for compliance with standards, or for failing in the + // right way. + foreach (array_keys($definitions) as $id) { + if (strpos($id, 'bad_help_topics.') === 0) { + $this->verifyBadTopic($id, $definitions); + } + else { + $this->verifyTopic($id, $definitions); + } + } + } + + /** + * Verifies rendering and standards compliance of one help topic. + * + * @param string $id + * ID of the topic to verify. + * @param array $definitions + * Array of all topic definitions, keyed by ID. + * @param int $response + * Expected response from visiting the page for the topic. + */ + protected function verifyTopic($id, $definitions, $response = 200) { + $definition = $definitions[$id]; + + // Visit the URL for the topic. + $this->drupalGet('admin/help/topic/' . $id); + + // Verify the title and response. + $session = $this->assertSession(); + $session->statusCodeEquals($response); + if ($response == 200) { + $session->titleEquals($definition['label'] . ' | Drupal'); + } + + // Verify that all the related topics exist. Also check to see if any of + // them are top-level (we will need that in the next section). + $has_top_level_related = FALSE; + if (isset($definition['related'])) { + foreach ($definition['related'] as $related_id) { + $this->assertArrayHasKey($related_id, $definitions, 'Topic ' . $id . ' is only related to topics that exist (' . $related_id . ')'); + $has_top_level_related = $has_top_level_related || !empty($definitions[$related_id]['top_level']); + } + } + + // Verify this is either top-level or related to a top-level topic. + $this->assertTrue(!empty($definition['top_level']) || $has_top_level_related, 'Topic ' . $id . ' is either top-level or related to at least one other top-level topic'); + + // Verify that the label is not empty. + $this->assertNotEmpty($definition['label'], 'Topic ' . $id . ' has a non-empty label'); + + // Read in the file so we can run some tests on that. + $body = file_get_contents($definition[HelpTopicDiscovery::FILE_KEY]); + $this->assertNotEmpty($body, 'Topic ' . $id . ' has a non-empty Twig file'); + + // Remove the front matter data (already tested above), and Twig set and + // variable printouts from the file. + $body = preg_replace('|---.*---|sU', '', $body); + $body = preg_replace('|\{\{.*\}\}|sU', '', $body); + $body = preg_replace('|\{\% set.*\%\}|sU', '', $body); + $body = trim($body); + $this->assertNotEmpty($body, 'Topic ' . $id . ' Twig file contains some text outside of front matter'); + + // Verify that if we remove all the translated text, whitespace, and + // HTML tags, there is nothing left (that is, all text is translated). + $text = preg_replace('|\{\% trans \%\}.*\{\% endtrans \%\}|sU', '', $body); + $text = strip_tags($text); + $text = preg_replace('|\s+|', '', $text); + $this->assertEmpty($text, 'Topic ' . $id . ' Twig file has all of its text translated'); + + // Load the topic body as HTML and verify that it parses. + $doc = new \DOMDocument(); + $doc->strictErrorChecking = TRUE; + $doc->validateOnParse = TRUE; + libxml_use_internal_errors(TRUE); + if (!$doc->loadHTML($body)) { + foreach (libxml_get_errors() as $error) { + $this->fail($error->message); + } + + libxml_clear_errors(); + } + + // Check for headings hierarchy. + $levels = [1, 2, 3, 4, 5, 6]; + foreach ($levels as $level) { + $num_headings[$level] = $doc->getElementsByTagName('h' . $level)->length; + if ($level == 1) { + $this->assertSame(0, $num_headings[1], 'Topic ' . $id . ' has no H1 tag'); + // Set num_headings to 1 for this level, so the rest of the hierarchy + // can be tested using simpler code. + $num_headings[1] = 1; + } + else { + // We should either not have this heading, or if we do have one at this + // level, we should also have the next-smaller level. That is, if we + // have an h3, we should have also had an h2. + $this->assertTrue($num_headings[$level - 1] > 0 || $num_headings[$level] == 0, + 'Topic ' . $id . ' has the correct H2-H6 heading hierarchy'); + } + } + } + + /** + * Verifies that a bad topic fails in the expected way. + * + * @param string $id + * ID of the topic to verify. It should start with "bad_help_topics.". + * @param array $definitions + * Array of all topic definitions, keyed by ID. + */ + protected function verifyBadTopic($id, $definitions) { + $bad_topic_type = substr($id, 16); + // Topics should fail verifyTopic() in specific ways. + try { + $this->verifyTopic($id, $definitions, 404); + } + catch (ExpectationFailedException $e) { + $message = $e->getMessage(); + switch ($bad_topic_type) { + case 'related': + $this->assertContains('only related to topics that exist', $message); + break; + + case 'bad_html': + $this->assertContains('Unexpected end tag', $message); + break; + + case 'top_level': + $this->assertContains('is either top-level or related to at least one other top-level topic', $message); + break; + + case 'empty': + $this->assertContains('contains some text outside of front matter', $message); + break; + + case 'translated': + $this->assertContains('Twig file has all of its text translated', $message); + break; + + case 'h1': + $this->assertContains('has no H1 tag', $message); + break; + + case 'hierarchy': + $this->assertContains('has the correct H2-H6 heading hierarchy', $message); + break; + + default: + // This was an unexpected error. + throw $e; + } + } + } + + /** + * Lists the extension help topic directories of a certain type. + * + * @param string $type + * The type of extension to list: module, theme, or profile. + * + * @return string[] + * An array of all of the help topic directories for this type of + * extension, keyed by extension short name. + */ + protected function listDirectories($type) { + $directories = []; + + // Find the extensions of this type, even if they are not installed, but + // excluding test ones. + $lister = \Drupal::service('extension.list.' . $type); + foreach (array_keys($lister->getAllAvailableInfo()) as $name) { + $path = $lister->getPath($name); + // You can tell test modules because they are in package 'Testing', but + // test themes are only known by being found in test directories. So... + // exclude things in test directories. + if ((strpos($path, '/tests') === FALSE) && + (strpos($path, '/testing') === FALSE)) { + $directories[$name] = $path . '/help_topics'; + } + } + return $directories; + } + +} diff --git a/core/modules/help_topics/tests/src/Unit/HelpTopicDiscoveryTest.php b/core/modules/help_topics/tests/src/Unit/HelpTopicDiscoveryTest.php index 99cfb13834dc4f97da74112e6df3da2013efff1f..6457cf75b347fd4ca8678fe8bd0f52ba92623819 100644 --- a/core/modules/help_topics/tests/src/Unit/HelpTopicDiscoveryTest.php +++ b/core/modules/help_topics/tests/src/Unit/HelpTopicDiscoveryTest.php @@ -34,7 +34,7 @@ public function testDiscoveryExceptionProviderMismatch() { $discovery = new HelpTopicDiscovery(['foo' => vfsStream::url('root/modules/foo/help_topics')]); $this->expectException(DiscoveryException::class); - $this->expectExceptionMessage("vfs://root/modules/foo/help_topics/test.topic.html.twig should begin with 'foo.'"); + $this->expectExceptionMessage("vfs://root/modules/foo/help_topics/test.topic.html.twig file name should begin with 'foo'"); $discovery->getDefinitions(); }