Revealed via #1361232: Make the taxonomy entities classed objects

http://api.drupal.org/api/drupal/core--modules--forum--forum.module/func...

cannot work, because an TaxonomyTerm entity is passed into the hook, not a $tid.

Tasks:

- Change the argument to $term.

- For D8, add TaxonomyTerm type-hinting for $term.

- Add a test to Forum module, as this seems to be completely untested.

- Backport to D7.

CommentFileSizeAuthor
#8 forum_taxonomy_term_delete_broken-1401496-d7-8.patch1.72 KBAlbert Volkman
#1 1401496.patch1.77 KBbleen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bleen’s picture

bleen CreditAttribution: bleen commented
Status: Active » Needs review
FileSize
1401496.patch1.77 KB

I think this should do it...

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: -Needs tests +Needs backport to D7

Looks like the patch includes a test.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

#1: 1401496.patch queued for re-testing.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
Status: Needs review » Reviewed & tested by the community

I gave the patch a test - it works, code looks good and comes with a test. -> RTBC

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Looks good. Committed/pushed to 8.x, moving to 7.x for backport.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented 
+++ b/core/modules/forum/forum.module
@@ -468,10 +468,10 @@ function forum_permission() {
+function forum_taxonomy_term_delete(stdClass $term) {

The stdClass type-hinting was bogus here. No need to fix in D8, since #1361232: Make the taxonomy entities classed objects will touch it either way.

However, should be removed for the D7 backport.

bleen’s picture

bleen CreditAttribution: bleen commented

sun ... just curious why I shouldn't have used type-hinting here?

Albert Volkman’s picture

Albert Volkman CreditAttribution: Albert Volkman commented
Status: Patch (to be ported) » Needs review
FileSize
forum_taxonomy_term_delete_broken-1401496-d7-8.patch1.72 KB

D7 backport without type-hinting.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs review » Reviewed & tested by the community

The backport looks correct. Thanks @Albert Volkman!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

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