Problem/Motivation

Over in #2766175: Fix the Views integration for entities with string ids., a helper method (now already committed) was introduced: DynamicEntityReferenceItem::entityHasIntegerId() to determine which column should be used for joining tables, etc.

This results in code looking something like:

       'relationship field' => $target_entity_id_is_int ? $field_name . '_target_id_int' : $field_name . '_target_id' ,

If we added an additional helper method, this code could usually be simplified to something like:

    'relationship field' => DynamicEntityReferenceItem::getTargetIdColumnName($field_name, $entity_type_id),

Proposed resolution

Add the method.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#11 add_helper_method_to-2827219-11.patch1.82 KBjibran
#11 interdiff.txt2.32 KBjibran
#7 add_helper_method_to-2827219-7.patch1.3 KBjibran
#6 2827219-06.patch5.6 KBjhedstrom
#6 interdiff.txt659 bytesjhedstrom
#3 2827219-03.patch5.56 KBjhedstrom
#3 interdiff.txt4.56 KBjhedstrom
#2 2827219-02.patch1022 bytesjhedstrom

Comments

jhedstrom created an issue. See original summary.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Active » Needs review
StatusFileSize
new 2827219-02.patch1022 bytes
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
StatusFileSize
new interdiff.txt4.56 KB
new 2827219-03.patch5.56 KB

This adds a unit test that covers the 2 new helper methods.

Status: Needs review » Needs work

The last submitted patch, 3: 2827219-03.patch, failed testing.

The last submitted patch, 3: 2827219-03.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt659 bytes
new 2827219-06.patch5.6 KB

Forgot the @group annotation.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
StatusFileSize
new add_helper_method_to-2827219-7.patch1.3 KB

How about something like this?

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

Just briefly reviewing #7 in the context of #2678756: Allow config entities to be flagged, it looks like we don't always have a FieldStorageDefinition in all the places where this helper method would be called (take for instance flag_views_data_alter() or any of the number of calls in FlagService). Needing to load the storage definition before calling the helper method seems...less helpful :)

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented 
+++ b/src/Plugin/Field/FieldType/DynamicEntityReferenceItem.php
@@ -560,6 +560,24 @@ class DynamicEntityReferenceItem extends EntityReferenceItem {
+  public static function getTargetIdColumnName(FieldStorageDefinitionInterface $field_definition) {

How about passing $entity_type_id and $field_name instead?

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

How about passing $entity_type_id and $field_name instead?

I might be missing something, but that's what the patch in #6 does...

+++ b/src/Plugin/Field/FieldType/DynamicEntityReferenceItem.php
@@ -560,6 +560,20 @@ class DynamicEntityReferenceItem extends EntityReferenceItem {
+   * @param string $field_name
+   *   The DER field name.
+   * @param string $entity_type_id
+   *   The entity type being referenced.
+   * @return string
+   *   The full target ID column name for the given field and entity type.
+   */
+  public static function getTargetIdColumnName($field_name, $entity_type_id) {
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
StatusFileSize
new interdiff.txt2.32 KB
new add_helper_method_to-2827219-11.patch1.82 KB

I meant something like this.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

This looks good--I'll test it by rerolling #2678756: Allow config entities to be flagged to utilize this.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs review » Reviewed & tested by the community

This works as expected.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x-2.x branch.

  • jibran committed ab407d6 on 8.x-2.x 
    Issue #2827219 by jhedstrom, jibran: Add helper method to retrieve...

Status: Fixed » Closed (fixed)

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