EckEntity not implementing EntityOwnerInterface correctly: getOwner() should return \Drupal\user\UserInterface.

The problem is not the entity reference field, but rather the way the owner of your content is retrieved. When the author of your entity type is configured to be multilingual, Drupal will call the getOwner function of the EckEntity class. Instead of returning a User, a List is presented because of the multilingualism.

If you look at the getOwnerId function inside that same class, you will see the correct implementation. The patch attached fixes this issue for the getOwner function.

Fixed the failing test.

+++ b/src/Tests/AccessTest.php
@@ -152,11 +152,11 @@ class AccessTest extends TestBase {
-    $this->assertResponse(200, 'The user can see content which is his own.');
+    $this->assertResponse(403, 'The user can see content which is his own.');
     $this->drupalGet("admin/structure/eck/entity/{$this->entityType['id']}/2/edit");
-    $this->assertResponse(200, 'The user can edit content which is his own.');
+    $this->assertResponse(403, 'The user can edit content which is his own.');
     $this->drupalGet("admin/structure/eck/entity/{$this->entityType['id']}/2/delete");
-    $this->assertResponse(200, 'The user can delete content which not his own.');
+    $this->assertResponse(403, 'The user can delete content which not his own.');

I believe the tests are right and this "fix" is wrong. The tests clearly indicate that the user is allowed to view the content. However, the tests are now changed to indicate that the user now is no longer allowed to view that content?

Can confirm this bug. It appears the fix is grabbing the original author of the entity. But is that desired behaviour? Shouldn't it get the author of the translation being edited?

UPDATE: was looking at the first patch. The second patch fixed my objection above :)

Re the tests, I agree @legolasbo, it appears the tests were correct, except the last one: and it's the text that is wrong (the entity does belong to the user, should assert 200).

The patch from #7 fixed my error as well, but the part of the patch that changed the Tests/AccessTest.php failed patching. From the comments from #8-#10 the tests didn't not need to be changed except for a word. I made a new patch that should now completely work and changes `$this->assertResponse(200, 'The user can delete content which not his own.');` to `$this->assertResponse(200, 'The user can delete content which is his own.');`

I believe this is what needed to be done.

Not positive, but I think the reason the tests are failing is that getOwnerId() is returning the entity key of the UID field, instead of the actual UID value. I could be wrong about that, but it seems like changing the return value in that method to something like return $this->get('uid')->target_id (is it an entity reference? If not maybe return $this->get('uid')->value instead) might resolve the test failures and result in the expected behavior when calling that method.

I think the change to the tests in kamkejj's last patch are correct, but now we just need to make the test pass again.

New patch to test from #14 comments.

Re-worked number #7 to change the wording in the assert response text to match the other text for when not the owner.

Created a new patch, works as expected.

The errors reported above are present with or without my patch on the 8.5 branch, so they should not hold back the review of this patch.

When you say the new patch works as expected, what did not work as expected using patch #18? That one has less test failures, and it fixes the issue for me.

I'm just curious what the purpose of the different approach is, and if the additional code is doing anything useful.

I don't recall why the patch from #18 didn't work, but it didn't. Based on the comments from my code it looks like it has something to do with a multi-lingual setup and $this->get('uid') returning a list rather than a single value.

I've added the tests for the 8.4 branch and they all passed. As I stated in comment #22 the errors in the 8.5 branch exist with or without my patch and are unrelated to it.

I'm confident that my patch will fix your issue as well.

The patch in #18 already fixed the issue you mentioned by changing $this->get('uid') into $this->get('uid')->entity so I'm not sure why you would be running into that issue still using that patch. It's still working properly on Drupal 8.5 for us on a multilingual site and seems sufficient to correct this issue.

#18 also works for me.

I don't recall why the patch from #18 didn't work, but it didn't.

I think it should be confirmed somehow.

...it looks like it has something to do with a multi-lingual setup and $this->get('uid') returning a list rather than a single value.

May the "authored by" field was configured as translatable (at /admin/config/regional/content-language)?

Patch from #22 worked for me with Drupal 8.5. Thanks!

This is also causes an issue in Feeds, which tries to call the method isAnonymous() on the result of $entity->getOwner(). Closed #2979832: Call to undefined method Drupal\Core\Field\EntityReferenceFieldItemList::isAnonymous() in EntityProcessorBase->entitySaveAccess() as a duplicate.

Changing title cause the issue is wider than just a translation issue.

Thanks, patch from #22 works well and solve problem with importing Feeds into ECK entities.

Patch from #22 works also for me when editing a translation of an eck with uid field.

This is a #18 review. ECK entity types do not add a key for the "uid", so the patched method "getOwnerId" fails, since it expects that such entity key exists.

Having an entity key for the uid (author) is adequate, core entity types that have an owner base field define this key. The reviewed patch adds it, making getOwnerId work correctly. I also expect that it solves the failed "access own entity type" tests.

Can we get #22 re-rolled for 8.5 and submitted? Thanks!!!

@abasso: The patch #22 is still valid for the latest commit of the dev branch and works also for the alpha3 version.

Is #22 actually needed considering #34 fixes an issue with the patch in #18, which I think is a simpler way to solve the problem that is more in line with how other entity types tend to handle it? I haven't run into any issues with it yet, but if there is some reason for the more complex uid handling in #22 instead, I'd be curious what that reason is. Thanks for the clarification!

Let's give this a go.

Here are the steps I used to recreate the error:

1. Fresh install of Drupal
2. Enable ECK and Content Translation
3. Go to admin/configuration/regional and language/languages
4. Click on Add language, select a language and click Add language
5. Go to structure/ECK Entity Types and click on Add entity type
6. Enter a label for the entity type and select Author checkbox from Available Base Fields and click Create Entity Type
7. Click on Add bundle for the entity type you created and enter a Name and click Save bundle
8. Click on Manage Fields and then Add field
9. Select Reference - Content as the new field and give it a label and click Save and continue
10. Click Save field settings
11. Under Reference Type, checkmark Create referenced entities if they don't already exist and Basic page and click Save settings
12. Go to admin/configuration/regional and language/Content language and translation
13. Checkmark the entity type you just created and click Save configuration
14. Checkmark the bundle you created and all fields should already be check marked
15. Click on Save configuration
16. Go back to the entity type you created and click on Add content and then Save
17. In the tabs, click on Translate
18. The language you added from earlier should be in the list, click Add for the new language then click Save on the next page
19. Now click on Edit and you should get the drupal white screen of death

I then started over with a fresh install of drupal and applied the patch to ECK and followed the same steps. No error! Great work!

Committed and pushed to 8.x-1.x. Thanks for your help everyone!