Cleaning up Taxonomy hooks and updating baseline.
Closes #3502014
Merge request reports
Activity
added 1 commit
- b8689ba8 - Splitting entity hooks into group hook class.
46 * multiple node and field tables with various conditions and order by criteria, 47 * we maintain a denormalized table with all relationships between terms, 48 * published nodes and common sort criteria such as status, sticky and created. 49 * When using other field storage engines or alternative methods of 50 * denormalizing this data you should set the 51 * taxonomy.settings:maintain_index_table to '0' to avoid unnecessary writes in 52 * SQL. 53 */ 54 55 /** 56 * Implements hook_ENTITY_TYPE_insert() for node entities. 57 */ 58 #[Hook('node_insert')] 59 public function nodeInsert(EntityInterface $node) { 60 // Add taxonomy index entries for the node. 61 taxonomy_build_node_index($node); -
On the other hand, I'm unsure if dealing with these functions is in scope or not. there are only two calls in core, probably should have been an underscored internal function.
http://codcontrib.hank.vps-private.net/search?text=taxonomy_build_node_index&filename= finds a single module using it in contrib and that's no longer supported.
We want to get rid of those methods anyway to remove the .module file and the sooner we do, the more likely we can remove them in D12.
It's quite a chunk of code, which poses another question, should it be inlined into the hook service, or an API.
My suggestion: It uses mostly the same dependencies and nothing else uses it, so I'm tempted to say make it (both build and delete) protected methods on this class, do dependency injection (hook classes are autowired, add protected ConfigFactory $configFactory to constructor, that's it, no docs needed). To be nice, we can keep the old function, add a @trigger_error() but keep the implementation there, then we don't need to make it public.
Not feeling too strongly about it though.
changed this line in version 11 of the diff
These have been moved to protected methods in TaxonomyEntityHooks, ConfigFactory has been added to the constructor. I'm not super clear on the @trigger_error, specifically around versions and @see getting picked up when I run PHPCS. Would it be deprecated in 11.1.x and removed from 12.0.0? Do we need a separate issue for deprecating these two functions that can be referenced in the error/deprecation notice?
1 <?php 2 3 namespace Drupal\taxonomy\Hook; 4 5 use Drupal\Core\Url; 6 use Drupal\Core\Routing\RouteMatchInterface; 7 use Drupal\Core\Hook\Attribute\Hook; 8 9 /** 10 * Hook implementations for taxonomy. 11 */ 12 class TaxonomyHelp { -
Thank you for the review @berdir! Should I update this to TaxonomyHelpHooks then?
added 1 commit
- 03bc9728 - Adding return type to help hook, updating baseline.
added 1 commit
- 85ba0ccf - Adding return types for TaxonomyEntityHooks.
added 1 commit
- 69152e2f - Adding string translation trait to taxonomy help hook.
added 1 commit
- 8bfacd4f - Cleanup of adding return type to taxonomy help hook.
added 1 commit
- aa8e7549 - Adding build_node_index and delete_node_index to TaxonomyEntityHooks as...
added 1 commit
- 90046b46 - Adding return types for build_hook_index and delete_hook_index, updating baseline.
added 1 commit
- 129f03a0 - Reverting changes to baseline for taxonomy_delete_node_index and taxonomy_build_node_index.