taxonomy form validation creates new terms

Subscribing.

missed that one. subscribe

what happens with the patch when the user enters several non-preexistent terms before previewing ?

The role of options_array_transpose() was to transform an array of $tids into the
array(... $delta => array('tid' => $tid) ... )
format expected for field values.

Definite +1 on removing it. I guess this was a remnant from when the 'taxo as field' widget code was split from optionwidgets, but here it's possible and indeed much better to directly build the array in the right format, which the patch does as a side effect of the general fix.

Patch looks good, so if bangpound is happy, then so am I.

Hm, patch #14 only touches rdf.module ?

Up priority due to FBI connection. And this is DB corruption IMO.

yup, patch #18 works for me

+++ modules/taxonomy/taxonomy.module	4 Jul 2010 22:58:46 -0000
@@ -7,6 +7,18 @@
 /**
+ * The 'tid' for a free-tagging term created during entity field
+ * submission.
+ *
+ * Users can create new terms in a free-tagging vocabulary when
+ * submitting a taxonomy_autocomplete_widget. We store a term object
+ * whose tid is TAXONOMY_TERM_AUTOCREATE as a field data item
+ * during widget validation and then actually create the term if/when
+ * that field data item makes it to taxonomy_field_insert/update().
+ */
+define('TAXONOMY_TERM_AUTOCREATE', 'TAXONOMY_TERM_AUTOCREATE');

@@ -1142,21 +1154,21 @@ function taxonomy_field_schema($field) {
   foreach ($items as $delta => $item) {
-    if (!empty($item['tid'])) {
+    if (!empty($item['tid']) && $item['tid'] != TAXONOMY_TERM_AUTOCREATE) {
       $tids[] = $item['tid'];

um. That's a weird use of constants. Why don't we simply use a string 'new' or 'autocreate' or whatever, instead of this *heavy* indirection? Makes no sense to me.

At the very least, this could be a negative integer, since tid cannot be < 0...

Anything, else, please.

Powered by Dreditor.

+++ modules/taxonomy/taxonomy.module	4 Jul 2010 22:58:46 -0000
@@ -1221,25 +1233,36 @@ function taxonomy_field_formatter_info()
 
+  // Terms whose tid is TAXONOMY_TERM_AUTOCREATE do not exist
+  // yet and $item['taxonomy_term'] is not set. Theme such terms as
+  // just their name.

Minor : in order to have previews look accurate (i.e with links where there will be links), we should IMO generate a link to a new 'taxonomy/temporary' page, which only displays some text saying 'No content, this term does not exist yet'.
I guess few people click on taxo term links in previews anyway (since it reloads the page and kills you edits, unless you use a middle-button click), while the overall appearance matters more.

+++ modules/taxonomy/taxonomy.module	4 Jul 2010 22:58:46 -0000
@@ -1298,6 +1323,10 @@ function taxonomy_field_formatter_prepar
+        // Terms to be created are not in $terms, but are still legitimate.
+        else if ($item['tid'] == TAXONOMY_TERM_AUTOCREATE) {
+          // Leave the item in place.
+        }
         // Otherwise, unset the instance value, since the term does not exist.
         else {
           unset($items[$id][$delta]);

empty else if clause looks odd ?

+++ modules/taxonomy/taxonomy.module	4 Jul 2010 22:58:46 -0000
@@ -1490,9 +1515,25 @@ function taxonomy_rdf_mapping() {
 function taxonomy_field_insert($entity_type, $entity, $field, $instance, $langcode, &$items) {
+  _taxonomy_term_autocreate($items);
+

+++ modules/taxonomy/taxonomy.module	4 Jul 2010 22:58:46 -0000
@@ -1513,6 +1554,8 @@ function taxonomy_field_insert($entity_t
 function taxonomy_field_update($entity_type, $entity, $field, $instance, $langcode, &$items) {
+  _taxonomy_term_autocreate($items);
+

If something needs to be done in both 'insert' and 'update', then hook_field_presave() sounds like a better candidate - it's called before any of the two others, so execution order should be the same as in the current patch.
In which case, not sure this deserves a separate _taxonomy_term_autocreate() helper func ?

Other than that, the patch and the approach look good to me.

Powered by Dreditor.

True. Looks good, then !

Committed to CVS HEAD.