Since #1847596: Deprecate Taxonomy term reference field in favor of Entity-reference is not going anywhere, we need another way of grouping reference-like field types together. Let's start with the taxonomy term reference field and follow-up with file and image.

Files: 
CommentFileSizeAuthor
#53 taxonomy-target-id-1965208-53.patch27.71 KBamateescu
PASSED: [[SimpleTest]]: [MySQL] 58,058 pass(es).
[ View ]
#51 taxonomy-target-id-1965208-51.patch27.38 KBamateescu
PASSED: [[SimpleTest]]: [MySQL] 57,749 pass(es).
[ View ]
#46 taxonomy-target-id-1965208-46.patch27.28 KBamateescu
PASSED: [[SimpleTest]]: [MySQL] 57,238 pass(es).
[ View ]
#38 taxonomy-target-id-1965208-37.patch27.35 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-target-id-1965208-37.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#33 taxonomy-target-id-1965208-33.patch27.35 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-target-id-1965208-33.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#27 taxonomy-target-id-1965208-27.patch27.21 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-target-id-1965208-27.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#27 interdiff.txt634 bytesamateescu
#26 taxonomy-target-id-1965208-26.patch27.11 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#26 interdiff.txt1012 bytesamateescu
#25 taxonomy-target-id-1965208-25.patch26.95 KBamateescu
PASSED: [[SimpleTest]]: [MySQL] 55,781 pass(es).
[ View ]
#25 interdiff.txt1.82 KBamateescu
#22 taxonomy-target-id-1965208-22.patch26.98 KBamateescu
PASSED: [[SimpleTest]]: [MySQL] 55,967 pass(es).
[ View ]
#22 interdiff.txt2.57 KBamateescu
#20 taxonomy-target-id-1965208-20.patch28.15 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 55,572 pass(es).
[ View ]
#18 taxonomy-target-id-1965208-18.patch28.2 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 55,632 pass(es).
[ View ]
#18 taxonomy-target-id-1965208-18-interdiff.txt442 bytesBerdir
#16 taxonomy-target-id-1965208-16.patch27.77 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#16 taxonomy-target-id-1965208-16-interdiff.txt711 bytesBerdir
#14 taxonomy-target-id-1965208-14.patch27.77 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/lib/Drupal/system/Tests/Theme/EntityFilteringThemeTest.php.
[ View ]
#12 1965208-12.patch28.01 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#12 interdiff.txt723 bytesamateescu
#8 1965208-8.patch25.47 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#8 interdiff.txt700 bytesamateescu
#5 1965208-5.patch25.37 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] 54,015 pass(es), 24 fail(s), and 24 exception(s).
[ View ]
#5 interdiff.txt619 bytesamateescu
#3 1965208-3.patch25.39 KBamateescu
PASSED: [[SimpleTest]]: [MySQL] 54,480 pass(es).
[ View ]
#3 interdiff.txt752 bytesamateescu
#1 1965208.patch24.27 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] 54,437 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new24.27 KB
FAILED: [[SimpleTest]]: [MySQL] 54,437 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

And here's the patch.

Status:Needs review» Needs work

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

Status:Needs work» Needs review
StatusFileSize
new752 bytes
new25.39 KB
PASSED: [[SimpleTest]]: [MySQL] 54,480 pass(es).
[ View ]

This should fix it.

+++ b/core/modules/taxonomy/taxonomy.installundefined
@@ -335,3 +335,37 @@ function taxonomy_update_8006() {
+  if (!$fields = field_read_fields(array('type' => 'taxonomy_term_reference'), array('include_inactive' => 1))) {

This should probably use the versioned, update-safe field api functions.

+++ b/core/modules/taxonomy/taxonomy.installundefined
@@ -335,3 +335,37 @@ function taxonomy_update_8006() {
+    if ($field['storage']['type'] != 'field_sql_storage') {
+      // Field doesn't use SQL storage, we cannot modify the schema.
+      continue;

That's a tricky one, if not we, then who's going to do it? :)

The changes themself are simple enough and make sense I think. We'll save more code as we'll start to move additional things into there (validation, schema possibly, field API stuff as it will get merged with Entity Field API).

We should probably do the same for image and file items.

StatusFileSize
new619 bytes
new25.37 KB
FAILED: [[SimpleTest]]: [MySQL] 54,015 pass(es), 24 fail(s), and 24 exception(s).
[ View ]

This should probably use the versioned, update-safe field api functions.

Yeah, I see that the Field API to CMI conversion didn't remove the data from {field_config} so I guess _update_7000_field_read_fields() should work at least until we get a 8000 version of it.

That's a tricky one, if not we, then who's going to do it? :)

The module that provides the alternate field storage? :/ Core can only do updates for the storage it provides and knows how to deal with..

We should probably do the same for image and file items.

Those are next, as I mentioned in the OP :)

Yes, there are some follow-up field api issues about providing new upgrade path helpers and deciding when the upgrade should exactly happen. Fun stuff ;)

/me reads OP

Oh. ;)

Status:Needs review» Needs work

The last submitted patch, 1965208-5.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new700 bytes
new25.47 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Ok, so that won't fly. How about just a @todo for now?

Status:Needs review» Needs work
Issue tags:-sprint, -Entity Field API

The last submitted patch, 1965208-8.patch, failed testing.

Status:Needs work» Needs review

#8: 1965208-8.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+sprint, +Entity Field API

The last submitted patch, 1965208-8.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new723 bytes
new28.01 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Rerolled and added a update requirement to be sure that we're running after field api has been converted to config.

Status:Needs review» Needs work

The last submitted patch, 1965208-12.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new27.77 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/lib/Drupal/system/Tests/Theme/EntityFilteringThemeTest.php.
[ View ]

Most of that conflicted due the term ng patch. Will probably still fail installation.

Status:Needs review» Needs work

The last submitted patch, taxonomy-target-id-1965208-14.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new711 bytes
new27.77 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

This should fix that.

Status:Needs review» Needs work

The last submitted patch, taxonomy-target-id-1965208-16.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new442 bytes
new28.2 KB
PASSED: [[SimpleTest]]: [MySQL] 55,632 pass(es).
[ View ]

Weird. See interdiff. When those indexes were still there, the whole installation process just hang up within config_import_invoke_owner(), it comes until field.field.field_tags and then it just stops and doesn't continue.

@yched/@swentel, any clues as to what's going on there in terms of validation logic and pre/post save? This is.. interesting to debug ;)

Discussed with @yched and it turns out that field api does throw an exception, I'm just not seeing it. When checking the ajax response with developer tools, it says "SQLSTATE[42000]: Syntax error or access violation: 1072 Key column 'field_tags_tid' doesn't exist in table: CREATE TABLE {field_data_field_tags} (" which is perfect.

StatusFileSize
new28.15 KB
PASSED: [[SimpleTest]]: [MySQL] 55,572 pass(es).
[ View ]

Re-roll, conflicted in taxonomy_field_presave()

+++ b/core/modules/taxonomy/taxonomy.installundefined
@@ -335,3 +347,38 @@ function taxonomy_update_8006() {
+  // @todo Switch to the update-safe version of this function when we have one.
+  if (!$fields = field_read_fields(array('type' => 'taxonomy_term_reference'), array('include_inactive' => 1))) {
+    return;

So, the conclusion in #1969662: Provide new _update_8000_*() CRUD helpers for Field and Instance definitions was that a specialized helper function for "read fields" would be of little interest as opposed to just iterating over raw config() data.

So the current plan is to *not* include helpers for "read existing field/instance definitions", since (unlike the "create" ops, that are a little more involved since they require pinging the storage backends) they would not really bring functionality specific to those Field API structures. If anything, a generic update-safe "EFQ on config records" could be useful for all config entities, but that's outside the scope of field API.

In short - maybe try to write this on top of raw config reads ? It doesn't seem it shouldn't be too hard.

StatusFileSize
new2.57 KB
new26.98 KB
PASSED: [[SimpleTest]]: [MySQL] 55,967 pass(es).
[ View ]

Something like this then?

Also rerolled for #1985138: New autocomplete taxonomy terms not previewable.

Can't RTBC the upgrade path part but the other part of the patch looks fine to me. I did a few re-rolls but just fixed some tests. So if @yched or maybe @swentel can confirm the last interdiff, then this can be RTBC'd by them.

+++ b/core/modules/taxonomy/taxonomy.installundefined
@@ -352,33 +352,36 @@ function taxonomy_update_8006() {
+  foreach (config_get_storage_names_with_prefix('field.field.') as $config_name) {
+    $field_config = config($config_name);
+    // Only update taxonomy reference fields that use the default SQL storage.
+    if ($field_config->get('type') == 'taxonomy_term_reference' && $field_config->get('storage.type') == 'field_sql_storage') {
+      $field_id = $field_config->get('id');
+      $field = array(
+        // _field_sql_storage_tablename() still relies on the 'field_name'
+        // property.
+        'field_name' => $field_id,
+        'deleted' => FALSE,
+      );

I would do something like this:

$field = new Field(config($config_name)->get());

It's a pattern we're using in other places as well see http://drupalcode.org/project/drupal.git/blob/refs/heads/8.x:/core/modul...

This will also guarantee we don't have to touch this again when removing the BC layer.

StatusFileSize
new1.82 KB
new26.95 KB
PASSED: [[SimpleTest]]: [MySQL] 55,781 pass(es).
[ View ]

That looks much better indeed.

StatusFileSize
new1012 bytes
new27.11 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

@swentel pointed out in IRC that we need to update the indexes value in the field config too.

StatusFileSize
new634 bytes
new27.21 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-target-id-1965208-27.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

@swentel also wants to play nice with other modules that might have overriden those indexes :)

Status:Needs review» Reviewed & tested by the community

Sweet, thanks :)

Issue tags:-sprint, -Entity Field API

#27: taxonomy-target-id-1965208-27.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, taxonomy-target-id-1965208-27.patch, failed testing.

Status:Needs work» Needs review

#27: taxonomy-target-id-1965208-27.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+sprint, +Entity Field API

The last submitted patch, taxonomy-target-id-1965208-27.patch, failed testing.

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new27.35 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-target-id-1965208-33.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Issue tags:-sprint, -Entity Field API

#33: taxonomy-target-id-1965208-33.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work
Issue tags:+sprint, +Entity Field API

The last submitted patch, taxonomy-target-id-1965208-33.patch, failed testing.

Status:Needs work» Reviewed & tested by the community

I thought I linked this here before, but apparently not. Here's the FileItem/ImageItem equivalent of this patch: #1996714: Convert FileItem and ImageItem to extend EntityReferenceItem

Status:Reviewed & tested by the community» Needs work

The last submitted patch, taxonomy-target-id-1965208-33.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new27.35 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-target-id-1965208-37.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled.

Looks pretty good to me, couldn't find anything to nag about.
However, I'm not sure what's ok to change in the views code and what's not.
As far as I understand the following changes only affect "meta data" and are backward compatible because the 'base field' still is'tid', right?

+++ b/core/modules/taxonomy/taxonomy.views.inc
@@ -346,7 +346,7 @@ function taxonomy_field_views_data($field) {
-    $data[$table_name][$field['field_name'] . '_tid']['relationship'] = array(
+    $data[$table_name][$field['field_name'] . '_target_id']['relationship'] = array(
       'id' => 'standard',
       'base' => 'taxonomy_term_data',
       'base field' => 'tid',
@@ -380,7 +380,7 @@ function taxonomy_field_views_data_views_data_alter(&$data, $field) {
@@ -380,7 +380,7 @@ function taxonomy_field_views_data_views_data_alter(&$data, $field) {
       'id' => 'entity_reverse',
       'field_name' => $field['field_name'],
       'field table' => _field_sql_storage_tablename($field),
-      'field field' => $field['field_name'] . '_tid',
+      'field field' => $field['field_name'] . '_target_id',

Status:Needs review» Reviewed & tested by the community

'base field' is the taxonomy id property, and yes, that remains unchanged. The only thing this patch changes is the name of the taxonomy reference field column.

Back to rtbc per #28.

No need to wait for it, but when #1969728: Implement Field API "field types" as TypedData Plugins lands, this change will in practice mean "the 'taxonomy field type' plugin extends the "entity referrence field type' plugin", which makes very much sense.

In other words, +1.

Issue tags:-sprint, -Entity Field API

#38: taxonomy-target-id-1965208-37.patch queued for re-testing.

#38: taxonomy-target-id-1965208-37.patch queued for re-testing.

#38: taxonomy-target-id-1965208-37.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work
Issue tags:+sprint, +Entity Field API

The last submitted patch, taxonomy-target-id-1965208-37.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new27.28 KB
PASSED: [[SimpleTest]]: [MySQL] 57,238 pass(es).
[ View ]

YAR. (yet another reroll)

Status:Needs review» Needs work
Issue tags:-sprint, -Entity Field API

The last submitted patch, taxonomy-target-id-1965208-46.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+sprint, +Entity Field API

#46: taxonomy-target-id-1965208-46.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Finally!

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Needs another YAR...

curl https://drupal.org/files/taxonomy-target-id-1965208-46.patch | git apply
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 27937  100 27937    0     0  16583      0  0:00:01  0:00:01 --:--:-- 18863
error: patch failed: core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/field/widget/TaxonomyAutocompleteWidget.php:52
error: core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/field/widget/TaxonomyAutocompleteWidget.php: patch does not apply
error: patch failed: core/modules/taxonomy/taxonomy.module:994
error: core/modules/taxonomy/taxonomy.module: patch does not apply

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new27.38 KB
PASSED: [[SimpleTest]]: [MySQL] 57,749 pass(es).
[ View ]

YARRR!

Priority:Normal» Major

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Type/TaxonomyTermReferenceItem.php
@@ -9,11 +9,12 @@
-class TaxonomyTermReferenceItem extends FieldItemBase {
+class TaxonomyTermReferenceItem extends EntityReferenceItem {

I think this is crucial for rest.module to be able to handle exporting and importing of content with term reference fields (without writing new custom code for them), so raising to major.

Issue tags:-Needs reroll
StatusFileSize
new27.71 KB
PASSED: [[SimpleTest]]: [MySQL] 58,058 pass(es).
[ View ]

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks!

Issue tags:-sprint

Removing sprint tag.

Status:Fixed» Closed (fixed)

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