Taxonomy terms are incorrectly affected

Here's a patch for what I think the bug is.

To test this, looks like we need the following:

Vocabulary with single cardinality.
Node with at least two terms, the one that would be processed last needs to be the one in that single cardinality vocabulary.
After update, load the node and confirm that all the terms are in the right fields.

I won't be able to work on those tests for at least the next couple of hours, but may be able to look after that.

Hi all,

on a purely philosophical standpoint (the '+1' looks weird and brings a tiny additional calculation), I would write:

  if ($field['cardinality'] != FIELD_CARDINALITY_UNLIMITED && $deltas[$field_name] >= $field['cardinality']) {

rather than:

  if ($field['cardinality'] != FIELD_CARDINALITY_UNLIMITED && ($deltas[$field_name] + 1) > $field['cardinality']) {

#3 didn't fail.

OK, going to test these locally and see.

At least the troubled line in the update needs more comments, "$delta[$field_name] +1" is the next delta and if that's more than the field cardinality........" or somesuch.

Added an extra comment there.

whitespace.

+++ modules/simpletest/tests/upgrade/upgrade.taxonomy.test
@@ -98,22 +100,56 @@ class UpgradePathTaxonomyTestCase extends UpgradePathTestCase {
+        // Moreover, only vocabularies 13 to 24 are properly associated with
...
+        // More over, odd vocabularies are single, so any additional term will

Nitpick: inconsistent spelling of "moreover", also seems a little repetitive.

Powered by Dreditor.

Er... #22 is identical to #21 :)

+++ modules/simpletest/tests/upgrade/upgrade.taxonomy.test
@@ -98,22 +100,56 @@ class UpgradePathTaxonomyTestCase extends UpgradePathTestCase {
+        // In our test database, each node is arbitrary associated with all
+        // the terms, except two: one which tid is ($nid) and the other which
+        // tid is (49 - $nid).

Should be "In our test database, each node is arbitrarily associated with all terms except two: one whose tid is ($nid), and one whose tid is (49 - $nid)."

+++ modules/simpletest/tests/upgrade/upgrade.taxonomy.test
@@ -98,22 +100,56 @@ class UpgradePathTaxonomyTestCase extends UpgradePathTestCase {
+        // with the taxonomy extra field.

"to the taxonomy extra field."

+++ modules/simpletest/tests/upgrade/upgrade.taxonomy.test
@@ -98,22 +100,56 @@ class UpgradePathTaxonomyTestCase extends UpgradePathTestCase {
+        if (!$should_be_displayed) {

Ideally if ($should_be_displayed) would go first but I won't quibble.

With just the doc changes (no need to move the if clause) this is RTBC.

Powered by Dreditor.

Looks good. I verified that only comments have changed since the last testbot run (interdiff attached), so RTBC.

Wow, nice work, folks!

Committed to HEAD. Thanks!