Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix d8 User Entity Reference fields #117

Merged
merged 1 commit into from
Nov 20, 2017

Conversation

Sut3kh
Copy link
Contributor

@Sut3kh Sut3kh commented Jan 2, 2017

Creating a node with an user entity reference field value throws:

'Drupal\Core\Entity\Query\QueryException' with message ''' not found' in web/core/lib/Drupal/Core/Entity/Query/Sql/Tables.php:316

It would seem that the User entity does not define the label column, an issue that the d7 EntityReferenceHandler appears to work around:

// For users set label to username.
if ($entity_type == 'user') {
  $entity_info['entity keys']['label'] = 'name';
}

So i have used the same approach to fix it, manually setting the label_key for user entities.

@jonathanjfshaw
Copy link
Contributor

https://www.drupal.org/node/2448729 is the relevant Drupal issue, which holds no help at present.

Hardcoding a reference to User is horrible but easy. The only alternative I can see without rearchitecting the whole thing is to iterate over the entity base fields and choose the first one that is required but not the id or uuid key.

However, that seems like not much of an improvement, so I suggest we do it your way for now.

@pfrenssen
Copy link
Collaborator

I don't like the solution either, but we are already doing the same thing for D7 so this will do for now :)

@jonathanjfshaw
Copy link
Contributor

Agreed it's good enough for now. I think it is something that Robust Entity handling (jhedstrom/drupalextension#337) might be able to help with. Basically if we had something like our own Behat Drupal User class (extending a Behat Drupal Entity class) that offered the properties/methods we needed. Then we could query the labelField() method of the class and get the field name we needed.

@jonathanjfshaw
Copy link
Contributor

The CS issues that cause the test fails are unrelated, and I think have been fixed since the last tests ran here.

@pwolanin
Copy link

I just ran into this exact bug an applied the same fix - any ETA?

Looking at https://www.drupal.org/node/2448729 there are several possible problem entity types - maybe need a switch statement to handle the ones in core at least?

@pfrenssen
Copy link
Collaborator

Oh yeah we should fix this. I wish github had some kind of way to follow or flag issues, I have no time to look at it now, and when I will have time I won't be able to find it any more :(

@jonathanjfshaw
Copy link
Contributor

jonathanjfshaw commented Mar 10, 2017 via email

@pwolanin
Copy link

The other entity types are less important than User - would be great to get this in.

@pwolanin
Copy link

Note that this is working for me (applied as a patch in composer.json and testing a node type with a user entity reference field)

@jonathanjfshaw
Copy link
Contributor

Working for me too.

@jonathanjfshaw
Copy link
Contributor

See also #94 and #133 for discussions addressing this problem

@Berdir
Copy link
Contributor

Berdir commented Nov 9, 2017

Bump :)

Wouldn't hurt to at least add a check to make sure there really is a label but agreed, user is by far the most common use case.

@jhedstrom
Copy link
Owner

I'm fine with this once #139 is resolved so we can verify tests are passing here.

@jhedstrom jhedstrom added the bug label Nov 10, 2017
@jhedstrom
Copy link
Owner

Tests are green again. I've kicked off a rebuild on travis.

@jhedstrom
Copy link
Owner

I think this needs to be rebased off of the latest master. I can't tell why the hhvm tests are failing here but not there.

@Sut3kh Sut3kh force-pushed the fix-d8-user-entity-ref-field branch from d1e077c to 3e8515f Compare November 20, 2017 09:12
@Sut3kh
Copy link
Contributor Author

Sut3kh commented Nov 20, 2017

done, sorry for the delay

@jhedstrom jhedstrom merged commit f2f10cc into jhedstrom:master Nov 20, 2017
@jhedstrom
Copy link
Owner

Thanks everybody!

jhedstrom added a commit that referenced this pull request Nov 20, 2017
Fix d8 User Entity Reference fields

Signed-off-by: Jonathan Hedstrom <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants