diff --git a/core/modules/help_topics/help_topics.module b/core/modules/help_topics/help_topics.module index 2ae3081b19..92e9c7c066 100644 --- a/core/modules/help_topics/help_topics.module +++ b/core/modules/help_topics/help_topics.module @@ -7,7 +7,6 @@ use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Url; -use Drupal\search\SearchPageInterface; /** * Implements hook_help(). @@ -57,29 +56,27 @@ function help_topics_theme() { ]; } -/** - * Implements hook_rebuild(). - */ -function help_topics_rebuild() { - // If the Search module is also enabled, we need to trigger a search reindex - // when a module or theme is installed, uninstalled, or updated; and when - // languages are added, translations are changed, or string overrides are - // changed. These situations can cause an indexed help item to have different - // text, or for help items to be added or removed. We cannot detect all of - // these cases, so instead at least trigger a reindex on cache rebuild. - \Drupal::service('plugin.manager.help_topic')->updateSearchIndex('reindex'); -} - /** * Implements hook_modules_uninstalled(). */ function help_topics_modules_uninstalled(array $modules) { - \Drupal::service('plugin.manager.help_topic')->updateSearchIndex('list'); + // Early return if search is not installed. + if (!\Drupal::hasService('plugin.manager.search')) { + return; + } + $search_plugin_manager = \Drupal::service('plugin.manager.search'); + if ($search_plugin_manager->hasDefinition('help_search')) { + // Ensure that topics for extensions that have been uninstalled are removed. + $help_search = $search_plugin_manager->createInstance('help_search'); + $help_search->updateTopicList(); + } } /** * Implements hook_themes_uninstalled(). */ function help_topics_themes_uninstalled(array $themes) { - \Drupal::service('plugin.manager.help_topic')->updateSearchIndex('list'); + // Use the same code as module uninstall to ensure that theme help is removed + // when a theme is uninstalled. + help_topics_modules_uninstalled([]); } diff --git a/core/modules/help_topics/help_topics.services.yml b/core/modules/help_topics/help_topics.services.yml index ba83fee278..47fe581b05 100644 --- a/core/modules/help_topics/help_topics.services.yml +++ b/core/modules/help_topics/help_topics.services.yml @@ -7,7 +7,7 @@ services: public: false plugin.manager.help_topic: class: Drupal\help_topics\HelpTopicPluginManager - arguments: ['@module_handler', '@theme_handler', '@cache.discovery', '@entity_type.manager'] + arguments: ['@module_handler', '@theme_handler', '@cache.discovery'] help_topics.twig.loader: class: Drupal\help_topics\HelpTopicTwigLoader arguments: ['@app.root', '@module_handler', '@theme_handler'] @@ -15,3 +15,11 @@ services: tags: - { name: twig.loader, priority: -200 } public: false + help_topics.plugin.manager.help_section: + class: Drupal\help_topics\HelpSectionManager + decorates: plugin.manager.help_section + parent: plugin.manager.help_section + calls: + - [setSearchManager, ['@?plugin.manager.search']] + tags: + - { name: plugin_manager_cache_clear } diff --git a/core/modules/help_topics/src/HelpSectionManager.php b/core/modules/help_topics/src/HelpSectionManager.php new file mode 100644 index 0000000000..1d74d5ee77 --- /dev/null +++ b/core/modules/help_topics/src/HelpSectionManager.php @@ -0,0 +1,44 @@ +searchManager = $search_manager; + } + + /** + * {@inheritdoc} + */ + public function clearCachedDefinitions() { + parent::clearCachedDefinitions(); + if ($this->searchManager && $this->searchManager->hasDefinition('help_search')) { + // Rebuild the index on cache clear so that new help topics are indexed + // and any changes due to help topics edits or translation changes are + // picked up. + $help_search = $this->searchManager->createInstance('help_search'); + $help_search->markForReindex(); + } + } + +} diff --git a/core/modules/help_topics/src/HelpTopicPluginManager.php b/core/modules/help_topics/src/HelpTopicPluginManager.php index 2fb3e0a17a..636bdda0a4 100644 --- a/core/modules/help_topics/src/HelpTopicPluginManager.php +++ b/core/modules/help_topics/src/HelpTopicPluginManager.php @@ -3,7 +3,6 @@ namespace Drupal\help_topics; use Drupal\Core\Cache\CacheBackendInterface; -use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Extension\ThemeHandlerInterface; use Drupal\Core\Plugin\DefaultPluginManager; @@ -94,13 +93,6 @@ class HelpTopicPluginManager extends DefaultPluginManager implements HelpTopicPl */ protected $themeHandler; - /** - * The entity type manager service. - * - * @var \Drupal\Core\Entity\EntityTypeManagerInterface - */ - protected $entityTypeManager; - /** * Constructs a new HelpTopicManager object. * @@ -110,10 +102,8 @@ class HelpTopicPluginManager extends DefaultPluginManager implements HelpTopicPl * The theme handler. * @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend * Cache backend instance to use. - * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager - * The entity type manager. */ - public function __construct(ModuleHandlerInterface $module_handler, ThemeHandlerInterface $theme_handler, CacheBackendInterface $cache_backend, EntityTypeManagerInterface $entity_type_manager) { + public function __construct(ModuleHandlerInterface $module_handler, ThemeHandlerInterface $theme_handler, CacheBackendInterface $cache_backend) { // Note that the parent construct is not called because this not use // annotated class discovery. $this->moduleHandler = $module_handler; @@ -122,7 +112,6 @@ public function __construct(ModuleHandlerInterface $module_handler, ThemeHandler // Use the 'config:core.extension' cache tag so the plugin cache is // invalidated on theme install and uninstall. $this->setCacheBackend($cache_backend, 'help_topics', ['config:core.extension']); - $this->entityTypeManager = $entity_type_manager; } /** @@ -189,34 +178,4 @@ protected function findDefinitions() { return $definitions; } - /** - * {@inheritdoc} - */ - public function updateSearchIndex($action = 'list') { - if (!$this->entityTypeManager->hasDefinition('search_page')) { - // Early return because no search module's page entity defined. - return; - } - - $storage = $this->entityTypeManager->getStorage('search_page'); - /** @var \Drupal\search\SearchPageInterface[] $pages */ - $pages = $storage->loadMultiple(); - foreach ($pages as $page) { - if ($page->status() && $page->getPlugin()->getPluginId() === 'help_search') { - // Only update the index if there is an enabled help search plugin. - $plugin = $page->getPlugin(); - break; - } - } - - if (isset($plugin)) { - if ($action === 'list') { - $plugin->updateTopicList(); - } - elseif ($action === 'reindex') { - $plugin->markForReindex(); - } - } - } - } diff --git a/core/modules/help_topics/src/HelpTopicPluginManagerInterface.php b/core/modules/help_topics/src/HelpTopicPluginManagerInterface.php index 9aedb7451c..2c33d5226f 100644 --- a/core/modules/help_topics/src/HelpTopicPluginManagerInterface.php +++ b/core/modules/help_topics/src/HelpTopicPluginManagerInterface.php @@ -13,18 +13,4 @@ * See https://www.drupal.org/core/experimental for more information. */ interface HelpTopicPluginManagerInterface extends PluginManagerInterface { - - /** - * Updates the full search index or the topic list for help search. - * - * If there is currently an active help search page on the site, either the - * help topics that have been indexed for searching are marked for reindexing, - * or the list of topics is updated. - * - * @param string $action - * Set to 'reindex' to mark all topics for reindexing, or 'list' (default) - * to only update the topic list. - */ - public function updateSearchIndex($action = 'list'); - } diff --git a/core/modules/help_topics/src/Plugin/HelpSection/HelpTopicSection.php b/core/modules/help_topics/src/Plugin/HelpSection/HelpTopicSection.php index 0c5ad6012d..c604d650ae 100644 --- a/core/modules/help_topics/src/Plugin/HelpSection/HelpTopicSection.php +++ b/core/modules/help_topics/src/Plugin/HelpSection/HelpTopicSection.php @@ -6,7 +6,6 @@ use Drupal\help_topics\SearchableHelpInterface; use Drupal\help_topics\HelpTopicPluginInterface; use Drupal\help_topics\HelpTopicPluginManagerInterface; -use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Language\LanguageDefault; use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Language\LanguageManagerInterface; @@ -151,16 +150,6 @@ public function getCacheMaxAge() { return $this->getCacheMetadata()->getCacheMaxAge(); } - /** - * {@inheritdoc} - */ - public function clearCachedDefinitions() { - parent::clearCachedDefinitions(); - // Mark the help search index for reindexing, if there is an active - // help search page. - $this->pluginManager->updateSearchIndex('reindex'); - } - /** * {@inheritdoc} */ diff --git a/core/modules/help_topics/src/SearchableHelpInterface.php b/core/modules/help_topics/src/SearchableHelpInterface.php index 64ed3749e4..06fdb939c1 100644 --- a/core/modules/help_topics/src/SearchableHelpInterface.php +++ b/core/modules/help_topics/src/SearchableHelpInterface.php @@ -7,11 +7,6 @@ /** * Provides an interface for a HelpSection plugin that also supports search. * - * Note that in addition to these methods, plugins that support search should - * also call help_topics_update_help_search_index('reindex') in their - * clearCachedDefinitions() method (or an equivalent method using dependency - * injection). - * * @see \Drupal\help\HelpSectionPluginInterface * * @internal diff --git a/core/modules/help_topics/tests/modules/help_topics_test/src/Plugin/HelpSection/TestHelpSection.php b/core/modules/help_topics/tests/modules/help_topics_test/src/Plugin/HelpSection/TestHelpSection.php index 7c3ccdd48d..f4d2a741e0 100644 --- a/core/modules/help_topics/tests/modules/help_topics_test/src/Plugin/HelpSection/TestHelpSection.php +++ b/core/modules/help_topics/tests/modules/help_topics_test/src/Plugin/HelpSection/TestHelpSection.php @@ -66,7 +66,7 @@ public function renderTopicForSearch($topic_id, LanguageInterface $language) { ]; } return [ - 'title' => 'Barmm Foreign sdeeeee', + 'title' => \Drupal::state()->get('help_topics_test:translated_title', 'Barmm Foreign sdeeeee'), 'text' => 'Fake foreign barmm anotherwordgerman sqruct', 'url' => Url::fromUri('https://mm.bar.com'), ]; diff --git a/core/modules/help_topics/tests/src/Functional/HelpTopicSearchTest.php b/core/modules/help_topics/tests/src/Functional/HelpTopicSearchTest.php index 1aeafad973..986c5af7b8 100644 --- a/core/modules/help_topics/tests/src/Functional/HelpTopicSearchTest.php +++ b/core/modules/help_topics/tests/src/Functional/HelpTopicSearchTest.php @@ -166,6 +166,35 @@ public function testHelpSearch() { $this->assertSearchResultsCount(1); $session->linkExists('Barmm Foreign sdeeeee'); + // Just changing the title and running cron is not enough to reindex so + // 'sdeeeee' still hits a match. The content is updated because the help + // topic is rendered each time. + \Drupal::state()->set('help_topics_test:translated_title', 'Updated translated title'); + $this->cronRun(); + $this->drupalPostForm('admin/help', ['keys' => 'sdeeeee'], 'Search', [ + 'language' => $german, + ]); + $this->assertSearchResultsCount(1); + $session->linkExists('Updated translated title'); + // Searching for the updated test shouldn't produce a match. + $this->drupalPostForm('admin/help', ['keys' => 'translated title'], 'Search', [ + 'language' => $german, + ]); + $this->assertSearchResultsCount(0); + + // Clear the caches and re-run cron - this should re-index the help. + $this->rebuildAll(); + $this->cronRun(); + $this->drupalPostForm('admin/help', ['keys' => 'sdeeeee'], 'Search', [ + 'language' => $german, + ]); + $this->assertSearchResultsCount(0); + $this->drupalPostForm('admin/help', ['keys' => 'translated title'], 'Search', [ + 'language' => $german, + ]); + $this->assertSearchResultsCount(1); + $session->linkExists('Updated translated title'); + // Verify the cache tags and contexts. $session->responseHeaderContains('X-Drupal-Cache-Tags', 'config:search.page.help_search'); $session->responseHeaderContains('X-Drupal-Cache-Tags', 'search_index:help_search');