Arancaytar found this bug on #251595: Add taxonomy_term_load_descendents(). Seems like the old db layer used to cast empty strings to 0/NULL on inserts, and db_insert() doesn't.

I've fixed this in the _submit() function and added a test.

CommentFileSizeAuthor
#7 taxonomy_blank_0-332145-6.patch2.24 KBcatch
#6 taxonomy_blank_0-332145-6.patch1.43 KBcburschka
#4 taxonomy_blank_0-332145-4.patch578 bytescburschka
empty_string.patch1.53 KBcatch

Comments

cburschka’s picture

cburschka
they
commented

I posted #332054: Selecting <root> as parent causes PDOException already, but since you uploaded a patch, we'll continue here.

cburschka’s picture

cburschka
they
commented

Still, I wonder if it would just make sense to give "root" a key of 0 rather than '', since that avoids additional logic and 0 is not a valid tid either.

catch’s picture

catch
he/him
Primary language English
commented

Arancaytar, that makes sense. Although there's already some logic in taxonomy_term_save() which relies on the empty string, and it looks like that might get passed onto taxonomy_check_vocabulary_hierarchy() too. With a bit of luck, using '0' we'd be able to strip some of that logic out though.

cburschka’s picture

cburschka
they
commented
StatusFileSize
new taxonomy_blank_0-332145-4.patch578 bytes

I don't see any relevant code in taxonomy_term_save except for this:

  if (!isset($term->parent) || empty($term->parent)) {
    $term->parent = array(0);
  }

empty() returns TRUE for both '' and 0, so that is no problem. taxonomy_check_vocabulary_hierarchy also checks only the count() of this value, which is the same for '' and 0. I'll still test it just to be sure...

Edit: This fixes the problem for me.

catch’s picture

catch
he/him
Primary language English
commented

Sorry, I mis-typed, meant in taxonomy_form_term_submit (after taxonomy_term_save() is called):

if (!$form['#vocabulary']->tags) {
    $current_parent_count = count($form_state['values']['parent']);
    $previous_parent_count = count($form['#term']['parent']);
    // Root doesn't count if it's the only parent.
    if ($current_parent_count == 1 && isset($form_state['values']['parent'][''])) {
      $current_parent_count = 0;
      $form_state['values']['parent'] = array();
    }
cburschka’s picture

cburschka
they
commented
StatusFileSize
new taxonomy_blank_0-332145-6.patch1.43 KB

Oh, in that case the key in the condition needs to be changed as well.

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new taxonomy_blank_0-332145-6.patch2.24 KB

Much better, added the test back in with this one.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: taxonomy_form_term_submit passes empty string as parent [dbtng conversion regression] » UNSTABLE-3 blocker: taxonomy_form_term_submit passes empty string as parent [dbtng conversion regression]

Will look at this a bit later. Tagging as a blocker for now.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Fixed

Ok, great. Code looks good (much better), and tests continue to pass. Committed. Thanks! :)

Status: Fixed » Closed (fixed)

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