Problem/Motivation

If a user attempts to set a default value on taxonomy autocomplete widget for a term that does not yet exist, the term is not autocreated as expected. The user has no indication that the term was not created. Furthermore, for fields other than the core Tags field, the failure to autocreate the term breaks new submissions entirely.

Steps to reproduce

  1. On a fresh standard install of D7 or D8, create a new vocabulary named Test. Do not create any terms.
  2. Go to admin/structure/types/manage/page/fields.
  3. Add a new field:
    • Label: Test
    • Name: field_test
    • Field type: Term reference
    • Widget: Autocomplete term widget (tagging)
  4. Select the Test vocabulary as the vocabulary for the field when prompted.
  5. In the default value field of the widget configuration form, enter a new term Foo.
  6. Save the form.
  7. Now, go to node/add/page. Note that no default value is set in the Test field.
  8. Enter a title. Leave the Test field blank.
  9. Save the node.

Expected result

The node is saved with a title and no tags.

Actual result

You receive error:

PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: 'autocreate' for column 'tid' at row 1: INSERT INTO {taxonomy_index} (nid, tid, sticky, created) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3); Array ( [:db_insert_placeholder_0] => 1 [:db_insert_placeholder_1] => autocreate [:db_insert_placeholder_2] => 0 [:db_insert_placeholder_3] => 1312137784 ) in taxonomy_field_insert() (line 1701 of /.../modules/taxonomy/taxonomy.module).

Note: This does not seem to be reproducible using the core Tags vocabulary. In that situation, it is a usability issue, because the user sets a default value on the field, but no default value is saved or used. Nodes can still be created in that case.

Proposed resolution

The current patch uses two steps to resolve the issue:

  1. Make taxonomy_field_widget_form() handle the case where $item['tid'] == 'autocreate' so that everything is passed through correctly until taxonomy_field_presave() transforms the autocreate value into a real tid.

Remaining tasks

None.

Done

  1. The problem does not exist in D8. So this is a D7-only issue.
  2. The patch has been reviewed by at least 4 people.
  3. Automated tests are provided.

User interface changes

None.

API changes

None.

Original report by @dynamicdan

The following error started appearing when saving a NEW content item:

PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: 'autocreate' for column 'tid' at row 1: INSERT INTO {taxonomy_index} (nid, tid, sticky, created) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3); Array ( [:db_insert_placeholder_0] => 40 [:db_insert_placeholder_1] => autocreate [:db_insert_placeholder_2] => 0 [:db_insert_placeholder_3] => 1303921062 ) in taxonomy_field_insert() (line 1673 of /.../modules/taxonomy/taxonomy.module).

This was after installing a whole bunch of standard taxonomy modules (Taxonomy Manager, pathauto, Taxonomy CSV, Taxonomy menu etc.). Disabling them did not resolve the problem. Through testing, it appears that submitting a custom content type with no entry in the default taxonomy term field (using the autocomplete or any other widget) triggers the error.

I can't confirm if the client is sending an empty/null term or the server is making stuff up.

I have 3 taxonomy term references, but only the first is causing issues. Setting the term to none for the other 2 (using the select widget) works fine. It's just when not entering text for the first taxonomy autocomplete field that an error is triggered.

BTW, this bug hurts... BAD. Only temporary work around would be to ensure a tag is always entered.

Further research from this:
"taxonomy form validation creates new terms" (http://drupal.org/node/826028 or #826028)
and (http://druplicon.info/bot/log/drupal-support/2011-02-28):

Hi, im having issues implementing a custom entity in d7. i've implemented the entity but when i attempt to save an entity that has a taxonomy field, i get the error msg "366 Incorrect integer value: 'autocreate' for column 'field_taxonomy_tags_tid' at row 1"

Suggestions:
reading incorrect fields from form post?
widget malfunctioning?
default vocab giving me probs?

Files: 
CommentFileSizeAuthor
#91 1140188-taxonomy-autocreate-incorrect-integer-91.patch5.22 KBtorotil
PASSED: [[SimpleTest]]: [MySQL] 41,257 pass(es).
[ View ]
#83 1140188-taxonomy-autocreate-incorrect-integer-83.patch4.93 KBtorotil
PASSED: [[SimpleTest]]: [MySQL] 41,152 pass(es).
[ View ]
#81 1140188-taxonomy-autocreate-incorrect-integer-81-tests.patch2.66 KBtorotil
FAILED: [[SimpleTest]]: [MySQL] 40,422 pass(es), 0 fail(s), and 2 exception(s).
[ View ]
#50 taxonomy-autocomplete_widget_testcase-1140188-50.patch2.44 KBNROTC_Webmaster
FAILED: [[SimpleTest]]: [MySQL] 35,179 pass(es), 1 fail(s), and 1 exception(s).
[ View ]
#46 taxonomy-autocomplete_widget_testcase-1140188-46.patch2.45 KBcam8001
FAILED: [[SimpleTest]]: [MySQL] 33,349 pass(es), 1 fail(s), and 1 exception(es).
[ View ]
#40 taxonomy-autocomplete_widget_testcase-1140188-40.patch3.81 KBcam8001
FAILED: [[SimpleTest]]: [MySQL] 33,352 pass(es), 1 fail(s), and 1 exception(es).
[ View ]
#34 incorrect_value_autocreate-1140188-34.patch3.07 KBkathyh
PASSED: [[SimpleTest]]: [MySQL] 33,808 pass(es).
[ View ]
#27 code_review-1140188-24_0.patch3.85 KBlnunesbr
PASSED: [[SimpleTest]]: [MySQL] 33,229 pass(es).
[ View ]
#22 incorrect_value_autocreate-1140188-22.patch3.22 KBagi.novanta
PASSED: [[SimpleTest]]: [MySQL] 33,637 pass(es).
[ View ]
#19 incorrect_value_autocreate-1140188-19.patch893 bytesagi.novanta
FAILED: [[SimpleTest]]: [MySQL] 33,635 pass(es), 1 fail(s), and 1 exception(es).
[ View ]

Comments

catch’s picture

Version:7.0-rc4» 8.x-dev

This will need to be fixed in 8.x first and backported (there's no code changes in taxonomy module yet so it should be the same patch).

Can you reproduce this on a clean Drupal 7 install?

dynamicdan’s picture

I'm not to keen to install a new version and redo everything I did prior to the bug... I have no idea when it stopped working as the problem is only related to adding or not adding tags for a given taxonomy reference and a given content type.

I have been able to find out more info... The taxonomy module is attempting to create a tag named 'untagged' which is failing. It's obvious to me that the tid value of 'autocreate' should be replaced or simply removed from the query so that the MYSQL auto_increment feature is used (considering it's a new tag).

Below are the 3 taxonomy fields (tags, t_t = theme_tags and regions).

    [field_tags] => Array
        (
            [und] => Array
                (
                    [0] => Array
                        (
                            [tid] => autocreate
                            [vid] => 1
                            [name] => untagged
                            [vocabulary_machine_name] => tags
                        )
                )
        )
    [field_t_t] => Array
        (
            [und] => Array
                (
                    [0] => Array
                        (
                            [tid] => 5
                        )
                )
        )
    [field_regions] => Array
        (
            [und] => Array
                (
                )
        )

It seems clear that the vocabulary_machine_name 'tags' handles things differently from the other 2 taxonomy fields/inputs.

I've debugged further the value of $instance from the function: taxonomy_field_insert($entity_type, $entity, $field, $instance, $langcode, &$items)

$instance is: Array
(
    [label] => Tags
    [widget] => Array
        (
            [weight] => 1
            [type] => options_select
            [module] => options
            [active] => 0
            [settings] => Array
                (
                    [size] => 60
                    [autocomplete_path] => taxonomy/autocomplete
                )
        )
    [settings] => Array
        (
            [user_register_form] =>
        )
    [display] => Array
        (
            [default] => Array
                (
                    [label] => above
                    [type] => taxonomy_term_reference_link
                    [weight] => 3
                    [settings] => Array
                        (
                        )
                    [module] => taxonomy
                )
            [search_result] => Array
                (
                    [label] => above
                    [type] => taxonomy_term_reference_link
                    [weight] => 4
                    [settings] => Array
                        (
                        )
                    [module] => taxonomy
                )
            [teaser] => Array
                (
                    [type] => hidden
                    [label] => above
                    [settings] => Array
                        (
                        )

                    [weight] => 0
                )
        )
    [required] => 0
    [description] => You should enter at least 3 tags to help users find this content.
    [default_value] => Array
        (
            [0] => Array
                (
                    [tid] => autocreate
                    [vid] => 1
                    [name] => untagged
                    [vocabulary_machine_name] => tags
                )
        )
    [id] => 12
    [field_id] => 3
    [field_name] => field_tags
    [entity_type] => node
    [bundle] => interactive_display_item
    [deleted] => 0
)

Seems like this taxonomy has a default_value ?? I can't for the life of me find out where 'untagged' comes from (which php file or DB field).

Any one know why drupal is trying to set a default value?

catch’s picture

Priority:Critical» Major
Status:Active» Postponed (maintainer needs more info)

It looks like one of the contrib modules you installed may have altered the field/instance definition - and to an invalid setting. This would explain why you are still getting the error after disabling those modules - they'd have to explicitly remove that change again.

I'm downgrading to 'major' and 'needs more info' - we'd need to find out which module is setting this, which means grepping the folders of the modules you mentioned in the opening post for 'untagged' - then we can move the bug report to that module.

dynamicdan’s picture

I have tried grepping for 'untagged' (grep -R "untagged" *) with no success.. Could this be in a language file somewhere (like $tag_default = EN_UN.'tagged')? I aslo checked the database content but with no luck.

Setting a default tag for the field changed 'untagged' to my 'not tagged' phrase but I still have the same bug.
I'm more interested in this function below which is meant to replace the 'autocreate' tid with an actual id as I understand it.

(I have no clue what the event flow is in this drupal world... _presave called before db_insert() or execute() and/or after taxonomy_term_save() ?)

<?php
/**
 * Implements hook_field_presave().
 *
 * Create any new terms defined in a freetagging vocabulary.
 */
function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langcode, &$items) {
  foreach (
$items as $delta => $item) {
    if (
$item['tid'] == 'autocreate') {
     
$term = (object) $item;
      unset(
$term->tid);
     
taxonomy_term_save($term);
     
$items[$delta]['tid'] = $term->tid;
    }
  }
}
?>

With my current testing, it seems like the if statement doesn't get triggered.

BTW, auto_increment is not enabled on tid, so no quick fix for me... perhaps I should add the auto_increment setting and remove the tid insertion part of the query... or hijack the spot where it adds this 'autocreate' term construct.... = bad hacks.

Should we change the title to:
"saving tags with tid as 'autocreate' fails when no tags are entered by the user"
?

dynamicdan’s picture

Status:Postponed (maintainer needs more info)» Active

Changed status so that ppl know to reply.

dynamicdan’s picture

Title:taxonomy module tries to save tags that don't exist... because there is no user input» taxonomy module fails to 'autocreate' new (default) tags
dynamicdan’s picture

Version:8.x-dev» 7.0-rc4

Found out via IRC that ppl are ignoring this because it's marked as 8.x and not 7.x
Changing to 7.0-rc4 because that's where users currently notice the problem (regardless of if it's drupal core or not).

catch’s picture

Version:7.0-rc4» 8.x-dev

No patches are getting into 7.x that haven't been committed to 8.x first. Right now the rate of commits to D8 is ridiculously slow, but changing version to D7 only decreases further the chance of this being looked at and eventually fixed.

dynamicdan’s picture

Without doing a full install and retest of all the modules that may have caused the problem, I have noticed something useful.

I believe the problem doesn't exist when adding new content via the frontend edit/add mode. When I was editing as an admin from the admin interface, the error was showing.

I have also disabled taxonomy_menu (the think that generates a block from a list of taxonomy terms automatically). I noticed some errors in that module showing up on the admin screen. I will monitor the problem as our site develops and migrates to a production environment.

dynamicdan’s picture

Ok, still have the problem when the user doesn't enter any tags for a node.

I believe this is NOT due to 3rd party modules. My only option atm is to hack/hook into the taxonomy 'get args' function and set 'not tagged' as a valid existing tag instead of relying on the 'autocreate' default 'untagged' stuff. Otherwise, I could hook into the submit function with JS and set a value if applicable on submit.

I still don't know where the value from 'autocreate' comes from. I'm starting to think the problem lies with the database default values for a taxonomy or the autocomplete widget.

sun’s picture

Priority:Major» Normal
Issue tags:+needs backport to D7

Only one person is able to reproduce this bug, demoting to normal.

Given the comment in #9, #996240: Javascript error in node preview when there are unlimited value fields in overlay might be related.

altaso’s picture

I have the same error:
PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: 'autocreate' for column 'tid' at row 1: INSERT INTO {taxonomy_index} (nid, tid, sticky, created) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3); Array ( [:db_insert_placeholder_0] => 181 [:db_insert_placeholder_1] => autocreate [:db_insert_placeholder_2] => 0 [:db_insert_placeholder_3] => 1309204315 ) in taxonomy_field_insert() (line 1673 of /DEVEL/alpinecanada/modules/taxonomy/taxonomy.module).

Drupal 7.0
Install profile Acquia Drupal (acquia-7.x-1.1)
Taxonomy 7.0

I'll see if I can get more info, but for now I think I'm going to make taxonomies required so users would not try submit empty fields.

Note: It looks like this is happening only when trying to crate new node ... when editing existing node and taxonomy tags auto-complete field is empty everything seems to be working fine.

dsdart’s picture

Not sure if this will help with debugging, but when I deleted the taxonomy term field from my content type, and re-added it, the problem went away. (I don't have a lot of content, so I could manually add back the tags)

dynamicdan’s picture

Error only on new node might be the key here. I haven't experienced this problem lately, but I know it's still there.
I would suggest testing in that direction. Could be linked with the node not existing when a new tag needs to be added.

juahoo’s picture

I'm getting this error as well and have been unable to resolve it.

PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: 'autocreate' for column 'tid' at row 1: INSERT INTO {taxonomy_index} (nid, tid, sticky, created) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3); Array ( [:db_insert_placeholder_0] => 14 [:db_insert_placeholder_1] => autocreate [:db_insert_placeholder_2] => 0 [:db_insert_placeholder_3] => 1311777059 ) in taxonomy_field_insert() (line 1701 of /modules/taxonomy/taxonomy.module).

I've tried deleting and re-adding several fields in my content type, but it hasn't worked so far.

Drupal 7.4
Install Profile Minimal (minimal-7.4)
Taxonomy 7.4
PHP 5.2.17
MySQL 5.0.91-log
Host: 1&1 shared server

xjm’s picture

I ran into this same issue today.

Steps to reproduce

  1. On a fresh standard install of D7 or D8, create a new vocabulary named Test. Do not create any terms.
  2. Go to admin/structure/types/manage/page/fields.
  3. Add a new field:
    • Label: Test
    • Name: field_test
    • Field type: Term reference
    • Widget: Autocomplete term widget (tagging)
  4. Select the Test vocabulary as the vocabulary for the field when prompted.
  5. In the default value field of the widget configuration form, enter a new term Foo.
  6. Save the form.
  7. Now, go to node/add/page.
  8. Enter a title. Leave the Test field blank.
  9. Save the node.

Expected result

The node is saved with a title and no tags.

Actual result

You receive error:

PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: 'autocreate' for column 'tid' at row 1: INSERT INTO {taxonomy_index} (nid, tid, sticky, created) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3); Array ( [:db_insert_placeholder_0] => 1 [:db_insert_placeholder_1] => autocreate [:db_insert_placeholder_2] => 0 [:db_insert_placeholder_3] => 1312137784 ) in taxonomy_field_insert() (line 1701 of /.../modules/taxonomy/taxonomy.module).

Note: This does not seem to be reproducible using the core Tags vocabulary. In that situation, it is a usability issue, because the user sets a default value on the field, but no default value is saved or used. Nodes can still be created in that case.

xjm’s picture

Priority:Normal» Major

Added summary with the above steps to reproduce.

xjm’s picture

Issue summary:View changes

Updated issue summary.

xjm’s picture

Issue summary:View changes

Clarification: The field is empty when the form is loaded.

xjm’s picture

Title:taxonomy module fails to 'autocreate' new (default) tags» Incorrect integer value: 'autocreate' for column 'tid' (default values for autocomplete widgets are not autocreated)
agi.novanta’s picture

StatusFileSize
new893 bytes
FAILED: [[SimpleTest]]: [MySQL] 33,635 pass(es), 1 fail(s), and 1 exception(es).
[ View ]

I investigated the issue. The issue seems to be related with the merging with default value. The presave callback is currently called before merging the data with default value so the callback is not applied to the data added automatically.

taxonomy module use uid = 'automatic' for those tags which are not added yet and it actually fills in taxonomy_field_presave(), if the tag is added by default the function is not called and we turn in this issue.

My little patch simply put _field_invoke_default('insert', $entity_type, $entity);, which load default data, in field_attach_presave() before calling the actual presave. This patch, however, should break those modules which rely on this order.

xjm’s picture

Status:Active» Needs review

Status:Needs review» Needs work

The last submitted patch, incorrect_value_autocreate-1140188-19.patch, failed testing.

agi.novanta’s picture

Status:Needs work» Needs review
StatusFileSize
new3.22 KB
PASSED: [[SimpleTest]]: [MySQL] 33,637 pass(es).
[ View ]

I think this time i have got the problem.

  • Current implementation of taxonomy_field_widget_form() add tags to form with this:
    <?php
    foreach ($items as $item) {
       
    $tags[$item['tid']] = isset($item['taxonomy_term']) ? $item['taxonomy_term'] : taxonomy_term_load($item['tid']);
    }
    ?>

    but if $item come from default, $item['taxonomy_term'] is not set and $item['tid'] = 'autocreate', so we need to cast $item as an object like this:

    <?php
    foreach ($items as $item) {
       
    $tags[$item['tid']] = ($item['tid'] == 'autocreate' ? (object) $item :
           (isset(
    $item['taxonomy_term']) ? $item['taxonomy_term'] : taxonomy_term_load($item['tid'])));
    }
    ?>
  • in function taxonomy_field_insert(), again, the $item which comes from default has $item['tid'] = 'autocreate' even if that tag is already on database so we need to check like:
    <?php
    function taxonomy_check_tids(&$items) {
      foreach (
    $items as $key => $item) {
       
    // items inserted by default does not have a tid
       
    if( $item['tid'] == 'autocreate' ) {
         
    $existing = taxonomy_get_term_by_name($item['name']);
          if( isset(
    $existing[1]) ) {
           
    $items[$key]['tid'] = $existing[1]->tid;
          }
        }
      }
    }
    ?>

    and if the tag is not on database we need to insert it, so i moved insert code in a function:

    <?php
    function taxonomy_save_new_terms(&$items) {
      foreach (
    $items as $delta => $item) {
        if (
    $item['tid'] == 'autocreate') {
         
    $term = (object) $item;
          unset(
    $term->tid);
         
    taxonomy_term_save($term);
         
    $items[$delta]['tid'] = $term->tid;
        }
      }
    }
    ?>

Here comes the patch.

marblegravy’s picture

Installed the patch at #22 on a website exhibiting all of the symptoms listed here and everything now works.

Brilliant!

xjm’s picture

Status:Needs review» Needs work

Thanks for the patch! Here are a few pointers for code cleanup:

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1530,10 +1530,13 @@ function taxonomy_term_title($term) {
+    $tags[$item['tid']] = ($item['tid'] == 'autocreate' ? (object) $item :
+       (isset($item['taxonomy_term']) ? $item['taxonomy_term'] : taxonomy_term_load($item['tid'])));

I'd suggest a comment explaining these two lines. Edit: Let's expand this into a full if/else.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1577,7 +1580,7 @@ function taxonomy_autocomplete_validate($element, &$form_state) {
-          'vocabulary_machine_name' => $vocabulary->machine_name,
+          'vocabulary_machine_name' => $vocabulary->machine_name

Lost a final comma here. Let's not make this change; the final comma should be included in arrays according to the coding standards.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1705,10 +1706,37 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
+/*
+ * Checking 'autocreate' tids and replace with database tid
+ */
+function taxonomy_check_tids(&$items) {
+  foreach ($items as $key => $item) {
+    // items inserted by default does not have a tid
+    if( $item['tid'] == 'autocreate' ) {
+      $existing = taxonomy_get_term_by_name($item['name']);
+      if( isset($existing[1]) ) {
+        $items[$key]['tid'] = $existing[1]->tid;
+      } ¶
+    }
+  }
+}

Do we need to create this function? The logic is simple enough that it could be simply included in hook_field_insert(), and I don't see a strong case for reusing it.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1705,10 +1706,37 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
+  // default values have not tid value setted

I'd change this comment to: "Try to match autocreated terms to existing terms of the same name in the database." The comment needs capitalization and a period.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1705,10 +1706,37 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
+  taxonomy_check_tids($items);

Replace this with the foreach loop from the function.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1705,10 +1706,37 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
+  // default values that are not in database yet
+  // need to be inserted now
+  taxonomy_save_new_terms($items);

I'd add a line of whitespace above and below this hunk. Additionally, let's change the comment to: "Save any new terms that were not matched."

Finally, there's several places in the patch that have trailing whitespace. Let's make sure to get rid of that when we reroll it.

11 days to next Drupal core point release.

xjm’s picture

Issue tags:+Needs tests

Probably should add a test for this as well.

xjm’s picture

Issue summary:View changes

Generalized file path in error.

xjm’s picture

Issue summary:View changes

Updated issue summary.

xjm’s picture

Is it possible to instead save the term when we save the field instance itself, rather than passing around autocreate forever as the tid for terms that may or may not exist?

xjm’s picture

Issue summary:View changes

Updated issue summary.

lnunesbr’s picture

Status:Needs review» Needs work
StatusFileSize
new3.85 KB
PASSED: [[SimpleTest]]: [MySQL] 33,229 pass(es).
[ View ]

I have reviewed those points, and have applied xjm suggestions according the coding standards...

follows a patch...

actually i'm not very familiar with simpletests...

lnunesbr’s picture

Status:Needs work» Needs review

changing the status

morningtime’s picture

Status:Needs work» Reviewed & tested by the community

The patch from #27 worked.

Except I didn't even have a Default value! I just left the field empty (not required). Without the patch I could not save a node on D7.

Please backport to D7, this is very needed :)

sun’s picture

Status:Reviewed & tested by the community» Needs review

The patch needs work. I'll let someone else point out the details.

ericduran’s picture

Status:Needs review» Needs work
+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1531,10 +1531,21 @@ function taxonomy_term_title($term) {
+      $tags[$item['tid']] = (object) $item;
+    } ¶
...
-
+  ¶
   $element += array(

@@ -1543,7 +1554,7 @@ function taxonomy_field_widget_form(&$form, &$form_state, $field, $instance, $la
+  ¶
   return $element;

@@ -1706,10 +1715,32 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
+      } ¶

The white spaces needs to be cleaned out.

I didn't actual review the code, it just stood out right away.

15 days to next Drupal core point release.

pwolanin’s picture

Just ran into this - though it seemed like part of the problem was setting a fixed number of allowed values, rather than unlimited.

xjm’s picture

Issue tags:+Novice

We might want to change the approach a bit, but at a minimum it needs some coding standards cleanup. Tagging as novice to reroll according to coding standards. References: http://drupal.org/coding-standards and http://drupal.org/node/1354

The patch in #27 needs to have trailing whitespace removed and a newline added at the end of the file. Additional comments:

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1531,10 +1531,21 @@ function taxonomy_term_title($term) {
+  //Assigning default values for the term fields

Needs a space after //, a period, and should start with "Assign" rather than "Assigning." Also, the comment itself is a bit vague and unclear.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1531,10 +1531,21 @@ function taxonomy_term_title($term) {
+    if ($item['tid'] == 'autocreate') {
+      $tags[$item['tid']] = (object) $item;
+    } ¶
+    else {
+      if (isset($item['taxonomy_term'])) {
+        $tags[$item['tid']] = $item['taxonomy_term'];
+      }
+      else {
+        $tags[$item['tid']] = taxonomy_term_load($item['tid']);
+      }

Let's simplify to if/elseif/else here.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1578,7 +1589,7 @@ function taxonomy_autocomplete_validate($element, &$form_state) {
-          'vocabulary_machine_name' => $vocabulary->machine_name,
+          'vocabulary_machine_name' => $vocabulary->machine_name

Again, this comma should not be removed.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1691,11 +1702,9 @@ function taxonomy_rdf_mapping() {
+  * Create any new terms defined in a freetagging vocabulary.

Should start with "Creates." Also, I'd suggest something a bit clearer, like: "Creates new term from autocomplete field values." Also, we need the @param and @return values.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1706,10 +1715,32 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
+  // default values have not tid value setted

I'd remove this comment.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1706,10 +1715,32 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
+    // items inserted by default does not have a tid

I'd change this to: "Set tids for existing terms."

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1706,10 +1715,32 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
+      if (isset($existing[1])) {
+        $items[$key]['tid'] = $existing[1]->tid;

Why $existing[1]? At a minimum, this needs a comment. Better, we should use something more meaningful than the numeric index for better readability.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1531,10 +1531,21 @@ function taxonomy_term_title($term) {
+  //Assigning default values for the term fields

Needs a space after //, a period, and should start with "Assign..." I'd also suggest more specific comments explaining each case.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1531,10 +1531,21 @@ function taxonomy_term_title($term) {
+    if ($item['tid'] == 'autocreate') {
+      $tags[$item['tid']] = (object) $item;
+    } ¶
+    else {
+      if (isset($item['taxonomy_term'])) {
+        $tags[$item['tid']] = $item['taxonomy_term'];
+      }
+      else {
+        $tags[$item['tid']] = taxonomy_term_load($item['tid']);
+      }

We could probably simplify this to if/elseif/else.

+++ b/modules/taxonomy/taxonomy.moduleundefined
@@ -1578,7 +1589,7 @@ function taxonomy_autocomplete_validate($element, &$form_state) {
-          'vocabulary_machine_name' => $vocabulary->machine_name,
+          'vocabulary_machine_name' => $vocabulary->machine_name

Again, this is not a good change. Let's keep that comma.

We also still need a test, which is another task someone could do.

kathyh’s picture

Status:Needs work» Needs review
StatusFileSize
new3.07 KB
PASSED: [[SimpleTest]]: [MySQL] 33,808 pass(es).
[ View ]

Patches to address #33. Still not sure on why existing[1] was used - maybe someone can comment on that. In the meantime, set the 1 to $termname=1;

After botcheck - this issue still needs:
-review approach
-write a test

xjm’s picture

Thanks for the reroll and cleanup; it's easier to look closely at the code in a clean patch.

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -1718,10 +1730,32 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
+  $termname=1;
+  foreach ($items as $key => $item) {
+    // Set tids for existing terms.
+    if ($item['tid'] == 'autocreate') {
+      $existing = taxonomy_get_term_by_name($item['name']);
+      if (isset($existing[$termname])) {
+        $items[$key]['tid'] = $existing[$termname]->tid;
+      }
+    }
+  }

Mmm, I think this just hides that it's using magic numbers, so let's not do that.

xjm’s picture

Status:Needs review» Needs work

Regarding the general approach: I really think we should not save 'autocreate' as the default field value. I'm more convinced of this now than I was in August for a few reasons. One, we're adding lots of queries to look up what that term should be--lots--and secondly, we're lying to other modules that handle these form values and telling them we've got an autocreated term in the set.

The correct approach is to save the default value term before saving its value to the field settings.

xjm’s picture

Oh, another thing: Two people have now mentioned that they encountered this problem but it didn't seem to necessarily be tied to a default value. Can we get steps to reproduce for those cases as well to be sure we cover them?

xjm’s picture

The current novice task in this issue is to create a functional automated test that fails without the patch and passes with it, using the steps to reproduce from the summary. Thanks!

I'd also like to see the point in #35 fixed if we do consider the current approach, but I'd prefer a different one.

cam8001’s picture

I've written a test case that is hopefully useful. Here it is: https://github.com/cam8001/Drupal-test-cases

I couldn't get the patch to apply to the D8 trunk as it is now, I might attempt a reroll tomorrow if no one has beaten me to it.

cam8001’s picture

StatusFileSize
new3.81 KB
FAILED: [[SimpleTest]]: [MySQL] 33,352 pass(es), 1 fail(s), and 1 exception(es).
[ View ]

On the ride to work I realised this would be much more useful as a patch to taxonomy.test. Here it is :)

xjm’s picture

Status:Needs work» Needs review

Yay! Setting NR for testbot; we are expecting a failure.

Status:Needs review» Needs work

The last submitted patch, taxonomy-autocomplete_widget_testcase-1140188-40.patch, failed testing.

xjm’s picture

Excellent. This will make writing a better fix much more efficient. Thanks cam8001!

Here are some points for code style cleanup:

  1. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    +class TaxonomyBugTestCase extends TaxonomyWebTestCase {

    This class needs a docblock. Also "TaxonomyBugTestCase" is not the most descriptive.

  2. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    +  /**
    +   * Implements getInfo().
    +   */

    We omit docblocks for getInfo(), setUp(), and tearDown(). (There's an existing issue debating this somewhere, but for now we should continue to, for consistency. Also there isn't actually an interface to implement.)

  3. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    +      'name' => t('Test taxonomy http://drupal.org/node/1140188'),
    +      'description' => t('If a user attempts to set a default value on taxonomy autocomplete widget for a term that does not yet exist, the term is not autocreated as expected. The user has no indication that the term was not created. Furthermore, for fields other than the core Tags field, the failure to autocreate the term breaks new submissions entirely. @see http://drupal.org/node/1140188'),

    We don't need to link the issue in the codebase, because people will be able to look it up with git blame once the patch goes in. :) Also, the description is a bit long. But actually, now that I think about it, I think we can probably put the test method inside an existing class somewhere instead. So ignore these first three points and instead look for a different class where it makes sense to add this test method?

  4. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    +    // Enable any module that you will need in your tests.
    ...
    +   *
    +   * - Creates a Vocabulary
    +   * - Creates Taxonomy term field, with autocomplete widget. Autocomplete is
    +   *   given a default value
    +   * - Creates a node without entering any value in the term field
    +   *
    ...
    +    // Set up code taken from TaxonomyTermTestCase::setUp()
    ...
    +    // ...taken from TaxonomyVocabularyFunctionalTest::testVocabularyInterface()

    Let's remove these comments.

  5. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    +   * Test case for http://drupal.org/node/1140188

    This should start with a third person verb, something like, "Tests autocreation of default value terms." or something. Reference: http://drupal.org/node/1354#functions

  6. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    +    $this->assertRaw(t('Created new vocabulary %name.', array('%name' => $edit['name'])), t('Vocabulary created successfully'));
    ...
    +    $this->assertRaw(t('These settings apply to the %name field everywhere it is used.', array('%name' => $label)), t('Created field successfully'));
    ...
    +    $this->assertRaw(t('Updated field %name field settings.', array('%name' => $label)), t('Assigned field to vocabulary'));
    ...
    +    $this->assertRaw(t('Saved %name configuration.', array('%name' => $label)), t('Added default value to term field'));
    ...
    +    $this->assertRaw(t('Basic page %title has been created.', array('%title' => $edit['title'])), t('Node created succesfully.'));

    It's preferred not to use t() on the assertion message texts (the last argument), because no translator is going to translate them. (The first args do still need t().)

  7. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    +    // Add a new field
    ...
    +    // Assign field to vocabulary
    ...
    +    // Set non-existent default value for field

    These two comments need periods and articles (a, the, etc.) I'd suggest:
    Assign the field to the vocabulary.
    and
    Set a non-existent default value for the field.

  8. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    +    $label = 'Test of taxonomy autocompletion';
    +    $edit = array(
    +      'fields[_add_new_field][label]' => $label,

    Rather than defining the $label variable, can we just put this in the array definition for better readability?

  9. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    +    // Try to create a node. Code taken from

    I think we can just remove the words "Code taken from" here?

  10. +++ b/core/modules/taxonomy/taxonomy.testundefined
    @@ -1528,3 +1528,84 @@ class TaxonomyThemeTestCase extends TaxonomyWebTestCase {
    \ No newline at end of file

    We need a single newline at the end of the file here.

So it would be good to get a reroll that fixes these things. Thanks!

cam8001’s picture

Wow what a thorough review, I'll tidy things up per your specs a bit later on tonight!

Regarding the $label definition, I've assigned it to a variable as it is checked in an assertion later on. I think it's the best way to assure that the value in the assertion matches the value assigned. It does look a bit ugly though. What do you think?

xjm’s picture

Ah, I missed that $label was used later on. That's fine then, so disregard that point. :)

cam8001’s picture

Status:Needs work» Needs review
StatusFileSize
new2.45 KB
FAILED: [[SimpleTest]]: [MySQL] 33,349 pass(es), 1 fail(s), and 1 exception(es).
[ View ]

OK, so I:

  • moved the tests into TaxonomyTermFieldTestCase;
  • renamed the test to testTaxonomyTermFieldAutocompleteDefault;
  • cleaned up the comments.

The test creates and logs in a new user, overriding the previous tests' user setup. This means that for the test needs to be the last function defined in the class, or it may jeopardise the integrity of other tests that run as a user with different permissions. The user has the same permissions as the one used in TaxonomyTermTestCase, but as its a field widget test, I decided to put it into TaxonomyTermFieldTestCase.

::setUp() is called on every test* method call, so this point is wrong.

The final check only asserts for creation of the node, it doesn't check that the taxonomy term was created successfully. But, it should fail without the patch in this issue, and pass with it.

Setting issue to needs review, expecting fail.

The last submitted patch, taxonomy-autocomplete_widget_testcase-1140188-46.patch, failed testing.

cam8001’s picture

I will test the patch against the new test case this week.

xjm’s picture

Status:Needs review» Needs work

Excellent; this looks much better. Just a couple remaining code style nitpicks:

+++ b/core/modules/taxonomy/taxonomy.testundefined
@@ -1388,6 +1388,45 @@ class TaxonomyTermFieldTestCase extends TaxonomyWebTestCase {
+  ¶
...
+    $this->drupalLogin($this->admin_user);
+    ¶
...
+    ¶
...
+    $this->assertRaw(t('Updated field %name field settings.', array('%name' => $label)) , 'Assigned field to vocabulary');
+    ¶

Lots of trailing whitespace. I suggest configuring your editor to display this if possible. (I know I have a habit of hitting tab to indent on every line, which leads to this sometimes. Some editors also just add it automatically.)

+++ b/core/modules/taxonomy/taxonomy.testundefined
@@ -1388,6 +1388,45 @@ class TaxonomyTermFieldTestCase extends TaxonomyWebTestCase {
+   * Test autocreation of default value terms in autocomplete widgets.

"Tests" rather than "test". (Reference: http://drupal.org/node/1354#functions)

Thanks!

NROTC_Webmaster’s picture

Status:Needs work» Needs review
StatusFileSize
new2.44 KB
FAILED: [[SimpleTest]]: [MySQL] 35,179 pass(es), 1 fail(s), and 1 exception(s).
[ View ]

Here is the test only updated per #49. The code since the patch in #34 has changed significantly so I'm not sure how that should be handled.

Status:Needs review» Needs work

The last submitted patch, taxonomy-autocomplete_widget_testcase-1140188-50.patch, failed testing.

xjm’s picture

Issue tags:-Needs tests

Thanks @NROTC_Webmaster; those changes look good.

+++ b/core/modules/taxonomy/taxonomy.testundefined
@@ -1516,6 +1516,45 @@ class TaxonomyTermFieldTestCase extends TaxonomyWebTestCase {
+    $this->assertRaw(t('These settings apply to the %name field everywhere it is used.', array('%name' => $label)), 'Created field successfully');
...
+    $this->assertRaw(t('Updated field %name field settings.', array('%name' => $label)) , 'Assigned field to vocabulary');
...
+    $this->assertRaw(t('Saved %name configuration.' , array('%name' => $label)), 'Added default value to term field');
...
+    $this->assertRaw(t('Basic page %title has been created.', array('%title' => $edit['title'])), 'Node created succesfully.');

Just noticed these use assertRaw() to match strings in t(), with placeholders and everything, which seems kind of weird. Shouldn't it be assertText()? Or am I mistaken?

NROTC_Webmaster’s picture

Status:Needs work» Needs review
StatusFileSize
new2.44 KB
FAILED: [[SimpleTest]]: [MySQL] 35,332 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

xjm,

I'm really not sure the proper way to do this. I'm going to assume you are correct and change assertRaw() to assertText(). Let me know if it needs any other changes.

Status:Needs review» Needs work

The last submitted patch, taxonomy-autocomplete_widget_testcase-1140188-53.patch, failed testing.

xjm’s picture

Well, the test failures are different, which probably means I was wrong. :) I'll try running the test locally.

NROTC_Webmaster’s picture

Status:Needs work» Needs review
StatusFileSize
new5.5 KB
FAILED: [[SimpleTest]]: [MySQL] 35,912 pass(es), 16 fail(s), and 986 exception(s).
[ View ]
new2.44 KB
FAILED: [[SimpleTest]]: [MySQL] 35,936 pass(es), 16 fail(s), and 0 exception(s).
[ View ]

I'm not sure what has changed but now every part of the test is failing. Anyways here are the latest for the test and original patch. If anyone has any ideas please let me know.

Status:Needs review» Needs work

The last submitted patch, taxonomy-autocomplete_widget_testcase-1140188-56.patch, failed testing.

Berdir’s picture

The autocreate behavior has been changed with the introduction of the TaxonomyTerm entity class.

Can you verify if this bug still occurs?

catch’s picture

Status:Needs work» Postponed (maintainer needs more info)
boztek’s picture

Tested this issue on D7 and D8 with MySQL and sqlite

Drupal 7 (MySQL)

  • No page created
  • Error displayed immediately in overlay
  • PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: 'autocreate' for column 'field_test_tid' at row 1: INSERT INTO {field_data_field_test} (entity_type, entity_id, revision_id, bundle, delta, language, field_test_tid) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6); Array ( [:db_insert_placeholder_0] => node [:db_insert_placeholder_1] => 1 [:db_insert_placeholder_2] => 1 [:db_insert_placeholder_3] => page [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => und [:db_insert_placeholder_6] => autocreate ) in field_sql_storage_field_storage_write() (line 448 of .../drupal/modules/field/modules/field_sql_storage/field_sql_storage.module).
  • Repeated creation same behaviour.

Drupal 7 (sqlite)

  • Page created without tag
  • Redirected to new page
  • Notice: Undefined index: name in taxonomy_field_formatter_view() (line 1484 of .../drupal/modules/taxonomy/taxonomy.module).
  • Repeated page creations same behaviour

Drupal 8 (MySQL)

  • No page created
  • Initially redirected to node/add/page
  • PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: 'autocreate' for column 'field_test_tid' at row 1: INSERT INTO {field_data_field_test} (entity_type, entity_id, revision_id, bundle, delta, langcode, field_test_tid) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6); Array ( [:db_insert_placeholder_0] => node [:db_insert_placeholder_1] => 1 [:db_insert_placeholder_2] => 1 [:db_insert_placeholder_3] => page [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => und [:db_insert_placeholder_6] => autocreate ) in field_sql_storage_field_storage_write() (line 452 of .../drupal/core/modules/field/modules/field_sql_storage/field_sql_storage.module).
  • Repeated page creation attempts yield WSOD and same PDOException in dblog

Drupal 8 (sqlite)

  • Page created without tag
  • Redirected to new page
  • Notice: Undefined index: name in taxonomy_field_formatter_view() (line 1236 of .../drupal/core/modules/taxonomy/taxonomy.module).
  • Repeated page creations same behaviour
boztek’s picture

Status:Postponed (maintainer needs more info)» Needs work
andypost’s picture

seems related or duplicate #873722: 'autocreate' == 0

xjm’s picture

I don't believe it's a duplicate, though in both cases our "magic" value name is being treated as an integer.

Alan D.’s picture

What is this code for?

<?php
$termname=1;
+  foreach (
$items as $key => $item) {
+   
// Set tids for existing terms.
+    if ($item['tid'] == 'autocreate') {
+     
$existing = taxonomy_get_term_by_name($item['name']);
+      if (isset(
$existing[$termname])) {
+       
$items[$key]['tid'] = $existing[$termname]->tid;
+      }
+    }
+  }
?>

The formatting is wrong on this

<?php
$termname=1;
$termname = 1;
?>

#1377628: taxonomy_get_term_by_name() should be taxonomy_term_load_multiple_by_name(), you need to use taxonomy_term_load_multiple_by_name().

This returns an array of terms indexed by tid.

The first term is usually referenced similar to:

<?php
$term
= reset($terms);
// or this, but less common I think.
$term = array_shift($terms);
?>

So do you mean this? I'm really not sure...

<?php
 
foreach ($items as $delta => $item) {
   
// Check new 'autocreate' terms are not duplicates.
   
if ($item['tid'] === 'autocreate') {
     
$terms = taxonomy_term_load_multiple_by_name($item['name']);
     
$term = reset($terms);
      if (
$term) {
       
$items[$delta]['tid'] = $term->tid;
      }
    }
  }
?>
mitron’s picture

Status:Needs work» Needs review

Repeated steps to reproduce on Drupal 8 and received expected result.

Steps

  1. git pull of commit 3698e17fb2a1e549712923641193fcc23cd81951.
  2. drush si
  3. Create a new vocabulary named Test. Do not create any terms.
  4. Go to admin/structure/types/manage/page/fields.
  5. Add a new field:
  6. Label: Test
    Name: field_test
    Field type: Term reference
    Widget: Autocomplete term widget (tagging)

  7. Select the Test vocabulary as the vocabulary for the field when prompted.
  8. In the default value field of the widget configuration form, enter a new term Foo.
  9. Save the form.
  10. Now, go to node/add/page. Note that no default value is set in the Test field.
  11. Enter a title. Leave the Test field blank.
  12. Save the node.

Result

The node is saved with a title and no tags.

xjm’s picture

Status:Needs review» Needs work

Yeah, this is still broken.

torotil’s picture

StatusFileSize
new1.33 KB
FAILED: [[SimpleTest]]: [MySQL] 40,300 pass(es), 22 fail(s), and 35 exception(s).
[ View ]

I investigated the problem again for D7 and came up with a slightly smaller patch.

I've also learned quite a few things about the field api during that:

  • hook_field_presave() isn't called for default values. Default values are only added through field_attach_insert(). That's the reason for the SQL error in the first place. - This is fixed by moving the code from hook_field_presave() to hook_field_insert().
  • The only place where we could add a taxonomy term from the field settings form to the database is in the validate function. It seems that no kind of submit handler is called for the field type. I think that was the reason to use the magic value "autocreate" in the first place. Storing data in a validate callback just doesn't seem right.
  • There is a callback in the field API that's called default_value_function. This would be the natural point to transform the default values. But the only way to set this callback is through a direct call to field_update_instance().
jibran’s picture

Status:Needs work» Needs review

Sending for test.

Status:Needs review» Needs work

The last submitted patch, taxonomy_default_value_autocreate.patch, failed testing.

torotil’s picture

The patch is for 7.x - naturally it fails to apply in 8.x!

torotil’s picture

Version:8.x-dev» 7.x-dev

I've just tested this with D8:
In D8 "autocreate" is not used anymore so this bug report is not applicable to D8 anymore. Marking this as 7.x-dev and requeueing for automated tests.

torotil’s picture

Status:Needs work» Needs review
Issue tags:-Novice, -needs backport to D7

Status:Needs review» Needs work
Issue tags:+Novice, +needs backport to D7

The last submitted patch, taxonomy_default_value_autocreate.patch, failed testing.

torotil’s picture

Status:Needs work» Needs review
StatusFileSize
new889 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,343 pass(es).
[ View ]

Here's another try to get this patched.

thelee’s picture

i can second that i've run into this exact issue (and been doing the temporary work around of just making tags a required field).

would be nice to get some vague movement on the latest patch from someone more knowledgeable than i.

maartendeblock’s picture

Priority:Major» Critical

I can confirm the above patch #74 works. I have had this problem with 2 commerce websites. This really should be fixed asap.

cam8001’s picture

Issue tags:-Novice+Needs tests

The patch in #74 seems to work fine here too, nice fix! We can't put it in without tests though.

Quick review:

  1. +++ b/modules/taxonomy/taxonomy.module
    @@ -1720,8 +1720,17 @@ function taxonomy_term_title($term) {
    +  $autocreates = 0;
       foreach ($items as $item) {
    -    $tags[$item['tid']] = isset($item['taxonomy_term']) ? $item['taxonomy_term'] : taxonomy_term_load($item['tid']);
    +    if ($item['tid'] == 'autocreate') {
    +      $tags["autocreate" . $autocreates++] = (object) $item;
    +    }

    Can we comment what $autocreates is doing here?

  2. +++ b/modules/taxonomy/taxonomy.module
    @@ -1720,8 +1720,17 @@ function taxonomy_term_title($term) {
    +    elseif (isset($item['taxonomy_term'])) {
    +      $tags[$item['tid']] = $item['taxonomy_term'];
    +    }
    +    else {
    +      $tags[$item['tid']] = taxonomy_term_load($item['tid']);
    +    }

    It would be great to comment these cases too - although I recognise they were not commented previously.

cam8001’s picture

Issue summary:View changes

Updated issue summary.

ianthomas_uk’s picture

Version:7.x-dev» 8.x-dev
Priority:Critical» Major
Issue summary:View changes
Status:Needs review» Needs work

While it's useful to have a working patch for 7.x, it won't be committed unless the issue has already been fixed in 8.x. I also don't think this qualifies as critical, and has previously been downgraded to major by a core committer.

Berdir’s picture

The bug no longer occurs in 8.x, according to #71. So the only thing that would be necessary there is test coverage, if we don't have it already.

torotil’s picture

Version:8.x-dev» 7.x-dev
Issue summary:View changes
Status:Needs work» Needs review
Issue tags:-Needs tests, -needs backport to D7

This is D7 only. This bug does not exist in D8.

Here is a patch containing tests. This will fail (without the fixes).

torotil’s picture

StatusFileSize
new2.66 KB
FAILED: [[SimpleTest]]: [MySQL] 40,422 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

Forgot to attach the patch ;)

Status:Needs review» Needs work

The last submitted patch, 81: 1140188-taxonomy-autocreate-incorrect-integer-81-tests.patch, failed testing.

torotil’s picture

StatusFileSize
new4.93 KB
PASSED: [[SimpleTest]]: [MySQL] 41,152 pass(es).
[ View ]

This patch has the tests as well as the fix.

torotil’s picture

Status:Needs work» Needs review

nmeegama’s picture

#83 patch is for Drupal 7

dieuwe’s picture

Status:Needs review» Reviewed & tested by the community

Patch in #83 worked for me.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 83: 1140188-taxonomy-autocreate-incorrect-integer-83.patch, failed testing.

Marvine’s picture

#83 works for me to (it was an error with default tag taxonomy vocabulary).

Status:Needs work» Needs review
torotil’s picture

Issue summary:View changes
Status:Needs review» Reviewed & tested by the community
StatusFileSize
new5.22 KB
PASSED: [[SimpleTest]]: [MySQL] 41,257 pass(es).
[ View ]

I've added a comment above to explain the 'autocreate'-special-case as requested in comment #77.

I'm also adapting the the issue summary and set it to RTBC as per #76, #86, #89.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 91: 1140188-taxonomy-autocreate-incorrect-integer-91.patch, failed testing.

Status:Needs work» Needs review
David_Rothstein’s picture

Status:Needs review» Reviewed & tested by the community

Testbot fluke.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 91: 1140188-taxonomy-autocreate-incorrect-integer-91.patch, failed testing.

Status:Needs work» Needs review
torotil’s picture

Status:Needs review» Reviewed & tested by the community

Again RTBC as it was before the testbot fluke.

David_Rothstein’s picture

Title:Incorrect integer value: 'autocreate' for column 'tid' (default values for autocomplete widgets are not autocreated)» Fatal errors during or after adding default values for autocomplete widgets
Version:7.x-dev» 8.0.x-dev
Status:Reviewed & tested by the community» Needs work
Issue tags:+needs backport to D7

I tried to reproduce this in Drupal 8 and while it probably doesn't have the exact same bug, you can never actually get far enough to test it due to a very similar bug that kicks in slightly earlier. From a fresh installation of Drupal 8:

1. Add a taxonomy term entityreference field to the basic page content type.
2. Configure it to be unlimited and to point to the "Tags" vocabulary and to have the "Create referenced entities if they don't already exist" checkbox checked. (Don't try to add any default values yet, since it won't let you until that checkbox is checked and saved.) Save the form.
3. Now go to Manage Form Display and configure the field to be "Autocomplete (Tags Style)".
4. Now go back to edit the field and in the default value section enter a new term "Foo".
5. When you save you get this: Fatal error: Call to a member function uuid() on a non-object in .../core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php on line 115

Although the cause is presumably different in Drupal 8 than Drupal 7, the symptoms are close enough that we'd probably want a simliar test for both... and the fix could turn out to be related too. So I'm pushing this back to Drupal 8 for now.

Also, in the Drupal 7 patch it does all this:

-    $tags[$item['tid']] = isset($item['taxonomy_term']) ? $item['taxonomy_term'] : taxonomy_term_load($item['tid']);
+    // Default values from the field instance settings are not passed through
+    // taxonomy_field_presave(). So we need to deal with untransformed
+    // autocreate values. The actual transformation to terms is still handled
+    // in taxonomy_field_presave() once the field items are saved.
+    if ($item['tid'] == 'autocreate') {
+      $tags["autocreate" . $autocreates++] = (object) $item;
+    }
+    elseif (isset($item['taxonomy_term'])) {
+      $tags[$item['tid']] = $item['taxonomy_term'];
+    }
+    else {
+      $tags[$item['tid']] = taxonomy_term_load($item['tid']);
+    }

But then, contrary to the code comment, it also removes taxonomy_field_presave() and replaces it with other functions. Does it really need both changes in the same patch? Looks like maybe taxonomy_field_presave() could be left alone...

And while the patch does appear to work, it seems like it should not be waiting until the first person creates a node to actually create the term - wouldn't it make more sense to create the term right when the default value form is submitted? At least seems worth discussing if that's desirable/possible....

jibran’s picture

Component:taxonomy.module» entity_reference.module

autocomplete part of taxonomy is handled by entity reference so moving it to proper component.

torotil’s picture

@David_Rothenstein

Yeah that D8 first policy. Having that in place even for bug-fixes is one of the major nuisances for working with D7. Essentially that means that D7 is getting security fixes only. I'd just wish the drupal.org front-page would say so.

But then, contrary to the code comment, it also removes taxonomy_field_presave() and replaces it with other functions. Does it really need both changes in the same patch? Looks like maybe taxonomy_field_presave() could be left alone...

And while the patch does appear to work, it seems like it should not be waiting until the first person creates a node to actually create the term - wouldn't it make more sense to create the term right when the default value form is submitted? At least seems worth discussing if that's desirable/possible....

You might find some answers to that question in #67 … if you cared to read them.