This feature was added out of the blue and without anyone giving it much thought as part of #1801304: Add Entity reference field (see #73 and #111 over there, for the only mentions of this feature). It has remained half-baked and unattended since then.

#2209981: ConfigurableEntityReferenceItem should not mess with the field schema promotes it from entity_reference.module to the base EntityReference field type used for all core reference fields.

- The behavior (field display - if the user doesn't have access to previous/unpulblished revisions, or views relationships) is still largely unclear / TBD
- No core widget supports referencing a specific revision anyway.
- Core has no use for it
- This really doesn't feel like a "80%" (nor 95%, for that matter) use case in terms of "data modeling toolbox for end users.
- The concept was never actually battle tested since it wasn't part of entityreference D7.
- I see no technical reason for which it *has* to be done in core native's reference field type.

This is an unfinished piece of wishful thinking, we should remove it from core and leave it to contrib to build a solid "reference a specific revision" field type.

CommentFileSizeAuthor
#2 2356609-2-remove-entity-reference-revision.patch5.09 KBtstoeckler

Comments

damien tournoud’s picture

damien tournoud commented

No opposition. @yched description is fair and balanced. This is very half-backed, and core has no use for it. Feel free to kill it.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Needs review
StatusFileSize
new 2356609-2-remove-entity-reference-revision.patch5.09 KB

OK, let's see.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

+1 to remove, that is not 80% case and looking through patch I don't think that changing a field schema depending on target entity definition needs a lot more work.

Also #1801304-111: Add Entity reference field

Added revision ID column. There's no UI for it, but if you set the value programmatically, and use the "Render entity" formatter you can see the difference. No tests yet.

still no tests...

+++ b/core/modules/entity_reference/src/ConfigurableEntityReferenceItem.php
@@ -100,28 +100,6 @@ public function getSettableOptions(AccountInterface $account = NULL) {
-    if ($target_type_info->hasKey('revision') && $target_type_info->getRevisionTable()) {

@@ -139,27 +117,6 @@ public function getConstraints() {
-    if ($target_type_info->isSubclassOf('\Drupal\Core\Entity\FieldableEntityInterface') && $field_definition instanceof FieldStorageConfigInterface) {

+++ b/core/modules/entity_reference/src/Plugin/Field/FieldFormatter/EntityReferenceFormatterBase.php
@@ -79,38 +75,21 @@ public function prepareView(array $entities_items) {
-        // If we have a revision ID, the key uses it as well.
-        $identifier = !empty($item->revision_id) ? $item->target_id . ':' . $item->revision_id : $item->target_id;

looks very fragile

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Nice removing untested and unproved functionality is definitely the right call at this point in the release cycle - the work in #2209981: ConfigurableEntityReferenceItem should not mess with the field schema will make the basis of a great contrib module.

Committed b2e8f08 and pushed to 8.0.x. Thanks!

  • alexpott committed b2e8f08 on 8.0.x 
    Issue #2356609 by tstoeckler | yched: Remove support for "reference a...

Status: Fixed » Closed (fixed)

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