Fatal errors during or after adding default values for autocomplete widgets

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?

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.

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.

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.

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.

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)

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

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.

Added summary with the above steps to reproduce.

Updated issue summary.

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

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.

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.

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

Brilliant!

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.

Probably should add a test for this as well.

Generalized file path in error.

Updated issue summary.

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?

Updated issue summary.

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...

changing the status

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

+++ 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.

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

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.

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

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.

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.

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?

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.

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.

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

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

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!

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?

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

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.

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

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!

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.

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?

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.

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

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.

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

Can you verify if this bug still occurs?

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

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

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

What is this code for?

+  $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

-  $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:

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

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

  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;
      }
    }
  }

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:

Label: Test
Name: field_test
Field type: Term reference
Widget: Autocomplete term widget (tagging)

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

Result

The node is saved with a title and no tags.

Yeah, this is still broken.

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().

Sending for test.

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

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.

#67: taxonomy_default_value_autocreate.patch queued for re-testing.

Here's another try to get this patched.

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.

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

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.

Updated issue summary.

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.

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.

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

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

Forgot to attach the patch ;)

This patch has the tests as well as the fix.

#83 patch is for Drupal 7

Patch in #83 worked for me.

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

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.

Testbot fluke.

Again RTBC as it was before the testbot fluke.

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....

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

@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.

I just experienced what David_Rothstein wrote in #98.

On HEAD, fresh install of D8. Edit field_tags on the Article content type. This is an entity reference to the Tags vocabulary. Enter a term that does not exist as the default value, and click Save settings.

Fatal error: Fatal error: Call to a member function uuid() on a non-object in /Applications/MAMP/htdocs/d8/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php on line 115

This is a fatal error on an admin screen, using default settings from the installation. If this issue is to address that bug, this should be bumped to critical. If not, let me know, and I'll open another issue.

Here's an attempt to address the issue.

The fix and the test looks good to me, thanks @jhedstrom! :)

Bad bot!

The 8.0.x patch was committed by @catch, moving back to 7.x.

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....

I think it is not desireable in D7 because of the lack of UUIDs. If we'd create the tags upfront like in D8 it would mean the content-type config is at least harder to deploy.

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're right - the comment is wrong: hook_field_presave() won't handle these cases (#1994594: hook_field_presave() not called reliably). Thus hook_field_insert() / hook_field_update() must be used. IMHO the only thing left to do is to update the comments.

Here is the patch from #91 with updated comments.

Jumping on the bandwagon, I am suddenly having this error, however it is happening in reverse (when people attempt to create taxonomy terms using the autocomplete, not when they leave the field null). In my case, the taxonomy field is inside of a field collection.

scratch this comment, found an error in code logic causing this error, but for an unrelated reason

Let's close this and move the patch to new issue for 7.x

Novice Task:

1. Create a new issue for D7.
2. Add a reference to this issue.
3. Move the existing patch to the new issue.
4. Put the issue to Needs review.
5. Mark fixed this issue.

Thank you!

I think the Drupal community has a serious issue with dealing with issues like this. The process established here on drupal.org actively prevents people from fixing bugs in D7 and that’s not since D8 was released but since 2013 in this case. This ticket has a history of:

1. A long discussion and fix for D7.
2. The fix being ignored for a while.
3. The issue being hijacked then fixed for D8.
4. The fix for D7 being ignored for 4 years again.
5. Being told to create a new issue because this is now too confusing (because it was hijacked for D8).

This is for a major issue.

So basically D7 developers get either of two treatments “patch is ignored” or “the bar is raised” — sometimes and alternatively also “it’s too late now to introduce this kind of change” (after ignoring the patch for years). This has been the case in many other issues as well.

Anyway here is the “new” D7 issue: #3091311: Fatal errors during or after adding default values for autocomplete widgets (D7). I’m taking bets on how long it is ignored.