Problem/Motivation

There's not a lot of usage taxonomy_*_save|delete()

Also taxonomy_get_tree() and taxonomy_check_vocabulary_hierarchy()

Proposed resolution

Replace usage of following functions with direct entity_() methods
taxonomy_term_(save|delete|delete_multiple)
taxonomy_vocabulary_(save|delete|delete_multiple)

API changes

taxonomy_term_delete() entity_delete_multiple('taxonomy_term', array($tid))
taxonomy_term_delete_multiple() entity_delete_multiple('taxonomy_term', array())
taxonomy_term_save() $term->save()
taxonomy_vocabulary_delete() entity_delete_multiple('taxonomy_vocabulary', array($tid))
taxonomy_vocabulary_delete_multiple() entity_delete_multiple('taxonomy_vocabulary', array())
taxonomy_vocabulary_save() $vocabulary->save()

taxonomy_check_vocabulary_hierarchy() #1976298: Move taxonomy_get_tree() and associated functions to Taxonomy storage, deprecate procedural wrappers.
taxonomy_get_tree() #1980966: Refactor TermStorage::loadTree() to properly work with Entity

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

I see no reason to drop hook definition from taxonomy.api.php
And I think better keep this patch in one issue

Patch splitted into parts to easy review

andypost’s picture

Issue tags: -API clean-up
FileSize
30.23 KB
1.41 KB

A bit more clean-up

Berdir’s picture

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/TermFormController.phpundefined
@@ -138,7 +138,7 @@ public function buildEntity(array $form, array &$form_state) {
-    // Convert text_format field into values expected by taxonomy_term_save().
+    // Convert text_format field into values expected by save() method.

Needs to use the full reference, \Drupal\Core\Entity\Entity::save()

Looks good otherwise. There's a general issue to remove them from all entities but I agree that it's better to do it in steps and per entity type. The main one that's questionable at the moment are the load() wrappers, those are IMHO the only ones that are still useful.

andypost’s picture

Issue tags: +API clean-up
FileSize
30.27 KB
861 bytes

Fixed. There's a lot of issues with tremendous size patches so I'd like to get this commited to not get the same as #1953410: [Meta] Remove field_create_*(), field_update_*() and field_delete_*() in favor of just using the ConfigEntity API

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Looks like a no-brainer, RTBC. Will conflict with some other issues that are moving forms around, not sure which one is easier to re-roll.

dawehner’s picture

Issue tags: +Needs change record

Add just a tag. The patch looks great!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Great patch... unfortunately needs a reroll..

curl https://drupal.org/files/1980982-taxoop-4.patch | git apply
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 30992  100 30992    0     0  15248      0  0:00:02  0:00:02 --:--:-- 21054
error: patch failed: core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php:321
error: core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php: patch does not apply
error: patch failed: core/modules/taxonomy/lib/Drupal/taxonomy/TermFormController.php:155
error: core/modules/taxonomy/lib/Drupal/taxonomy/TermFormController.php: patch does not apply
error: patch failed: core/modules/taxonomy/lib/Drupal/taxonomy/VocabularyFormController.php:169
error: core/modules/taxonomy/lib/Drupal/taxonomy/VocabularyFormController.php: patch does not apply
aspilicious’s picture

Status: Needs work » Needs review
FileSize
30.26 KB

Been a long time, lets see if this applies...

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Passed, so back to RTBC.

As for the change notice, the new methods were already documented in e.g. http://drupal.org/node/1400186, maybe just extend that with a list of removed functions?

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 5838ea9 and pushed to 8.x. Thanks!

alexpott’s picture

Title: Clean-up procedural wrappers for taxonomy » Change notice: Clean-up procedural wrappers for taxonomy
Priority: Normal » Critical
Status: Fixed » Active

Oops... and lets update http://drupal.org/node/1400186 as @berdir suggested

andypost’s picture

Title: Change notice: Clean-up procedural wrappers for taxonomy » Clean-up procedural wrappers for taxonomy
Priority: Critical » Normal
Status: Active » Fixed

Status: Fixed » Closed (fixed)

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

xjm’s picture

Issue tags: -Needs change record

Untagging. Please remove the "Needs change notification" tag when the change notice task is complete.

xjm’s picture

Issue summary: View changes

summary