Now as the new entity field API got committed, we need to convert existing entity types to make use of it. See #1346214: [meta] Unified Entity Field API and the "Entity Field API" tag.

ToDo
#1634280-24: drupal_anonymous_user() should return a User entity

Files: 
CommentFileSizeAuthor
#96 user-ng-1818570-96.patch110.78 KBeffulgentsia
PASSED: [[SimpleTest]]: [MySQL] 55,002 pass(es).
[ View ]
#96 interdiff.txt988 byteseffulgentsia
#93 user-ng-1818570-93.patch109.23 KBeffulgentsia
FAILED: [[SimpleTest]]: [MySQL] 56,602 pass(es), 6 fail(s), and 105 exception(s).
[ View ]
#94 user-ng-1818570-94.patch109.66 KBeffulgentsia
FAILED: [[SimpleTest]]: [MySQL] 56,765 pass(es), 6 fail(s), and 105 exception(s).
[ View ]
#94 interdiff.txt1.11 KBeffulgentsia
#89 user-ng-1818570-89.patch109.63 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 56,051 pass(es).
[ View ]
#89 user-ng-1818570-89-interdiff.txt508 bytesBerdir
#87 user-ng-1818570-87.patch109.58 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 57,522 pass(es), 4 fail(s), and 4 exception(s).
[ View ]
#87 user-ng-1818570-87-interdiff.txt2.32 KBBerdir
#82 interdiff.txt695 bytesandypost
#82 user-ng-1818570-82.patch110.86 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-ng-1818570-82.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#75 user-ng-1818570-75.patch110.14 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 55,806 pass(es), 0 fail(s), and 5 exception(s).
[ View ]
#75 user-ng-1818570-75-interdiff.txt2.09 KBBerdir
#73 user-ng-1818570-73.patch110.01 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 55,853 pass(es), 7 fail(s), and 4 exception(s).
[ View ]
#73 user-ng-1818570-73-interdiff.txt7.19 KBBerdir
#67 user-ng-1818570-67.patch108.37 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 55,720 pass(es).
[ View ]
#67 user-ng-1818570-66-67-interdiff-do-not-test.diff5.59 KBdas-peter
#66 user-ng-1818570-66.patch108.38 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 55,858 pass(es).
[ View ]
#66 user-ng-1818570-66-interdiff.txt1.63 KBBerdir
#64 user-ng-1818570-64.patch107.59 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 55,993 pass(es), 1 fail(s), and 16 exception(s).
[ View ]
#62 user-ng-1818570-62.patch110.7 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-ng-1818570-62_2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#62 user-ng-1818570-62-interdiff.txt4.5 KBBerdir
#61 user-ng-1818570-61.patch109.58 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 55,708 pass(es).
[ View ]
#61 interdiff-user-ng-1818570-58-61-do-not-test.diff760 bytesdas-peter
#58 user-ng-1818570-58.patch108.83 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 55,603 pass(es), 0 fail(s), and 18 exception(s).
[ View ]
#58 user-ng-1818570-58-interdiff.txt898 bytesBerdir
#56 user-ng-1818570-56.patch108.82 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 56,217 pass(es), 43 fail(s), and 4,589 exception(s).
[ View ]
#56 user-ng-1818570-56-interdiff.txt7.68 KBBerdir
#52 user-ng-phpunit-1818570-52.patch106.86 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 55,594 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#52 user-ng-phpunit-1818570-52-interdiff.txt1 KBBerdir
#50 user-ng-phpunit-1818570-50.patch105.86 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 55,292 pass(es), 118 fail(s), and 3,236 exception(s).
[ View ]
#50 user-ng-phpunit-1818570-50-interdiff.txt2.89 KBBerdir
#48 user-ng-phpunit-1818570-48.patch103.94 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
#48 user-ng-phpunit-1818570-48-interdiff.txt1 KBBerdir
#46 user-ng-context-1818570-46.patch102.94 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
#46 user-ng-context-1818570-46-interdiff.txt592 bytesBerdir
#43 user-ng-context-1818570-43.patch103.51 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
#43 user-ng-context-1818570-43-interdiff.txt2.93 KBBerdir
#41 user-ng-slightly-simplified-1818570-41.patch100.58 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 55,376 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#41 user-ng-slightly-simplified-1818570-41-interdiff.txt7.11 KBBerdir
#40 user-ng-interface-1818570-40.patch105.01 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 55,563 pass(es).
[ View ]
#40 user-ng-interface-1818570-40-interdiff.txt1.6 KBBerdir
#38 user-ng-interface-1818570-37.patch104.45 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 55,462 pass(es), 8 fail(s), and 0 exception(s).
[ View ]
#38 user-ng-interface-1818570-37-interdiff.txt2.66 KBBerdir
#35 user-ng-interface-1818570-35.patch102.64 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 55,383 pass(es), 4 fail(s), and 1 exception(s).
[ View ]
#35 user-ng-interface-1818570-35-interdiff.txt35.88 KBBerdir
#33 user-ng-1818570-33.patch74.53 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 54,637 pass(es), 550 fail(s), and 63,173 exception(s).
[ View ]
#33 user-ng-1818570-33-interdiff.txt9.99 KBBerdir
#29 user-ng-1818570-29.patch74.67 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#25 user-ng-1818570-25.patch74.18 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-ng-1818570-25.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#25 user-ng-1818570-25-interdiff.txt3.51 KBBerdir
#22 user-ng-1818570-22-interdiff.txt5.27 KBBerdir
#22 user-ng-1818570-22.patch70.67 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 54,178 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#20 user-ng-1818570-20.patch67.59 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 53,910 pass(es), 3 fail(s), and 51 exception(s).
[ View ]
#20 user-ng-1818570-20-interdiff.txt723 bytesBerdir
#19 user-ng-1818570-19.patch68.3 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#19 user-ng-1818570-19-interdiff.txt4.93 KBBerdir
#16 user-ng-more-role-fixes-1818570-16.patch70.12 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 53,276 pass(es), 50 fail(s), and 8 exception(s).
[ View ]
#16 user-ng-more-role-fixes-1818570-16-interdiff.txt11.4 KBBerdir
#13 user-ng-node-access-hook-1818570-13-interdiff.txt2.27 KBBerdir
#13 user-ng-node-access-hook-1818570-13.patch59.5 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 53,108 pass(es), 91 fail(s), and 38 exception(s).
[ View ]
#11 user-ng-roles-stuff-1818570-11.patch60.41 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 52,047 pass(es), 177 fail(s), and 38 exception(s).
[ View ]
#11 user-ng-roles-stuff-1818570-11-interdiff.txt17.48 KBBerdir
#8 user-ng-more-test-fixes-1818570-8.patch44.97 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 52,982 pass(es), 199 fail(s), and 69 exception(s).
[ View ]
#8 user-ng-more-test-fixes-1818570-8-interdiff.txt21.12 KBBerdir
#6 user-ng-some-fixes-1818570-6.patch25.53 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 52,575 pass(es), 364 fail(s), and 900 exception(s).
[ View ]
#6 user-ng-some-fixes-1818570-6-interdiff.txt3.1 KBBerdir
#4 user-ng-lets-get-started-1818570-4.patch23.3 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 51,336 pass(es), 1,142 fail(s), and 2,307 exception(s).
[ View ]

Comments

Assigned:Unassigned» chx

Assigned:chx» Unassigned

GLOBALS['user']. Nope. This is too big for me. I can take this back on but need guidance on what to do with that. Alternatively, someone else who knows what to do with it can do it.

Assigned:Unassigned» Berdir

Working a bit on this.

Status:Active» Needs review
StatusFileSize
new23.3 KB
FAILED: [[SimpleTest]]: [MySQL] 51,336 pass(es), 1,142 fail(s), and 2,307 exception(s).
[ View ]

This will probably not get through the installer but I have quite a few tests passing.

Had to add a few hacks, drupal_anonyomous_user() now returns a User entity that's now too early for something like that. Using a stdClass for global $user (it was actually even messier than before because the loaded user was a stdClass but the anon global user object is User.

Status:Needs review» Needs work

The last submitted patch, user-ng-lets-get-started-1818570-4.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.1 KB
new25.53 KB
FAILED: [[SimpleTest]]: [MySQL] 52,575 pass(es), 364 fail(s), and 900 exception(s).
[ View ]

This should fix some of the failures.

Status:Needs review» Needs work

The last submitted patch, user-ng-some-fixes-1818570-6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new21.12 KB
new44.97 KB
FAILED: [[SimpleTest]]: [MySQL] 52,982 pass(es), 199 fail(s), and 69 exception(s).
[ View ]

Ok, this should fix a lot of tests. Some parts of this are a bit ugly, e.g. due to User type hints but I really think we shouldn't replace them with EntityInterface, at least not everywhere where they are access related. We really need to replace that with something else :(

Assigned:Berdir» Unassigned

I'm done for the day. :)

Status:Needs review» Needs work

The last submitted patch, user-ng-more-test-fixes-1818570-8.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new17.48 KB
new60.41 KB
FAILED: [[SimpleTest]]: [MySQL] 52,047 pass(es), 177 fail(s), and 38 exception(s).
[ View ]

This should fix a lot of user role related tests, still not everything 100% correct there.

Status:Needs review» Needs work

The last submitted patch, user-ng-roles-stuff-1818570-11.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new59.5 KB
FAILED: [[SimpleTest]]: [MySQL] 53,108 pass(es), 91 fail(s), and 38 exception(s).
[ View ]
new2.27 KB

That node access fix was wrong.

Status:Needs review» Needs work

The last submitted patch, user-ng-node-access-hook-1818570-13.patch, failed testing.

For anyone reading here, I'm working on a UserSessionInterface that both the global $user object and User would implement, that should make a lot of the changes here less hackish: #1825332: Introduce an AccountInterface to represent the current user

Status:Needs work» Needs review
StatusFileSize
new11.4 KB
new70.12 KB
FAILED: [[SimpleTest]]: [MySQL] 53,276 pass(es), 50 fail(s), and 8 exception(s).
[ View ]

Some more user role fixes.

Status:Needs review» Needs work

The last submitted patch, user-ng-more-role-fixes-1818570-16.patch, failed testing.

Issue tags:+sprint

Status:Needs work» Needs review
StatusFileSize
new4.93 KB
new68.3 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Fixed the permission page default values and reverted drupal_anonoymous_user() return to an stdClass, removing the session.inc changes. I really need #1825332: Introduce an AccountInterface to represent the current user :(

This should fix most of the remaining fails.

StatusFileSize
new723 bytes
new67.59 KB
FAILED: [[SimpleTest]]: [MySQL] 53,910 pass(es), 3 fail(s), and 51 exception(s).
[ View ]

Removed debug message.

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-20.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new70.67 KB
FAILED: [[SimpleTest]]: [MySQL] 54,178 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
new5.27 KB

Ok, fixed most of the remaining test fails. User registration is related to the array(NULL) bug I think, for which there already is an issue. Also removed some left-over debug()'s.

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-22.patch, failed testing.

The mentioned issue is #1957888: Exception when a field is empty on programmatic creation, need to check if that fixes the registration fails.

EDIT: Yes, can confirm that this fixes the test fail.

Status:Needs work» Needs review
StatusFileSize
new3.51 KB
new74.18 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-ng-1818570-25.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Same fix as in the node type issue, needs a clear cache as something is now triggering a field info cache build before it's changed. Also added the patch from the linked issue so that I can see this getting green :)

Note that #1498664: Refactor user entity properties to multilingual is currently postponed on this one. Tagging for D8MI.

Status:Needs review» Needs work
Issue tags:+D8MI, +sprint, +language-content, +Entity Field API, +language-content-property

The last submitted patch, user-ng-1818570-25.patch, failed testing.

StatusFileSize
new74.67 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

just a merge about current head

Status:Needs work» Needs review

+++ b/core/includes/bootstrap.incundefined
@@ -2002,14 +2003,13 @@ drupal_anonymous_user() {
-  $values = array(
+  return (object) array(
...
-  return new User($values, 'user');

This still waits for #1825332: Introduce an AccountInterface to represent the current user

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-29.patch, failed testing.

Priority:Normal» Major

Raising to major because blocking #1983554: Remove BC-mode from EntityNG . Given that plus #26, let's move forward with this independent of #1825332: Introduce an AccountInterface to represent the current user if possible, and clean up hacks in a follow up. If that's not possible, please mark this issue postponed, and update the issue summary to explain why that issue is a blocker.

Status:Needs work» Needs review
StatusFileSize
new9.99 KB
new74.53 KB
FAILED: [[SimpleTest]]: [MySQL] 54,637 pass(es), 550 fail(s), and 63,173 exception(s).
[ View ]

Re-roll, getOriginalEntity() is now getNGEntity().

@effulgentsia: I'm... not sure. I can make this patch pass the tests without the other issue, but I'm not if it can get in like that, there are very ugly cases in regards to some type hints and e.g. the roles handling. The type hints can now probably be solved by changing them to UserInterface and have a subclassed UserBCDecorator that implements UserInterface as well.

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-33.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new35.88 KB
new102.64 KB
FAILED: [[SimpleTest]]: [MySQL] 55,383 pass(es), 4 fail(s), and 1 exception(s).
[ View ]

Yes, the interface allows me to do something like this. Defined UserBCDecorator that extends EntityBCDecorator and implements UserInterface, changing most type hints of User to UserInterface. Let's see how the testbot likes this.

If this works then I should be able to revert quite a few getBC/NGEntity() calls as I no longer need to sneak around type hints anymore.

Status:Needs review» Needs work

The last submitted patch, user-ng-interface-1818570-35.patch, failed testing.

the forum fail is the same as on #1957888: Exception when a field is empty on programmatic creation
it started happening when forum views integration went in

Status:Needs work» Needs review
StatusFileSize
new2.66 KB
new104.45 KB
FAILED: [[SimpleTest]]: [MySQL] 55,462 pass(es), 8 fail(s), and 0 exception(s).
[ View ]

This should be green again. Not sure why the forum integration test suddenly starts to fail here but the fix makes sense anyway.

Status:Needs review» Needs work

The last submitted patch, user-ng-interface-1818570-37.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.6 KB
new105.01 KB
PASSED: [[SimpleTest]]: [MySQL] 55,563 pass(es).
[ View ]

This, then :)

StatusFileSize
new7.11 KB
new100.58 KB
FAILED: [[SimpleTest]]: [MySQL] 55,376 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

Removed some ng/bc switches and other unecessary changes with the interface now. Let's see if this still passes.

Status:Needs review» Needs work

The last submitted patch, user-ng-slightly-simplified-1818570-41.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.93 KB
new103.51 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

This should fix the context test.

There's an unrelated hunk in there, will remove that in the next re-roll that will be required sooner or later anyway I guess ;)

Status:Needs review» Needs work

The last submitted patch, user-ng-context-1818570-43.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new592 bytes
new102.94 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

Looks like that wasn't a good idea.

Status:Needs review» Needs work

The last submitted patch, user-ng-context-1818570-46.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1 KB
new103.94 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

Hm, this is a problem.

Figured out that failure, opened #1986528: Fatal error: Call to undefined method PHPUnit_Framework_Warning::getInfo() when there is an error in tests.

But that just reveals the real problem here, as visible the fatal errors in the previous patch. As User is EntityNG, which is not yet using injected dependencies and calls Drupal::entityManager(), among many other things that will then follow. i don't really see a way to fix that other than #1825332: Introduce an AccountInterface to represent the current user and use UserSession instead of User for those tests.

Status:Needs review» Needs work

The last submitted patch, user-ng-phpunit-1818570-48.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.89 KB
new105.86 KB
FAILED: [[SimpleTest]]: [MySQL] 55,292 pass(es), 118 fail(s), and 3,236 exception(s).
[ View ]

Ah stupid me. Let's just use stdClass for those tests, that's what they are working with in case of global user anyway. We can switch to the AccountInterface/UserSessionInterface when we have it.

Status:Needs review» Needs work

The last submitted patch, user-ng-phpunit-1818570-50.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1 KB
new106.86 KB
FAILED: [[SimpleTest]]: [MySQL] 55,594 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Replace a new User type hint with UserInterface.

Status:Needs review» Needs work

The last submitted patch, user-ng-phpunit-1818570-52.patch, failed testing.

+++ b/core/lib/Drupal/Core/Entity/Entity.phpundefined
@@ -257,7 +257,7 @@ public function getIterator() {
+  public function access($operation = 'view', \Drupal\user\UserInterface $account = NULL) {

Why don't we add the UserInterface in the use statement?

+++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.phpundefined
--- a/core/modules/entity_reference/lib/Drupal/entity_reference/Tests/EntityReferenceItemTest.php
+++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Tests/EntityReferenceItemTest.phpundefined
+++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Tests/EntityReferenceItemTest.phpundefined
@@ -97,5 +97,12 @@ public function testEntityReferenceItem() {
+    // Delete terms so we have nothing to reference and try again
+    $term->delete();
+    $term2->delete();
+    $entity = entity_create('entity_test', array('name' => $this->randomName()));
+    $entity->save();

This feels out of scope.

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/FieldAccessTest.phpundefined
@@ -70,12 +70,12 @@ function testFieldAccess() {
+    $this->assertTrue($entity->field_test_text->access('view', $account->getNGEntity()), 'Access to the field was granted.');
+++ b/core/modules/user/lib/Drupal/user/Plugin/Core/Entity/User.phpundefined
@@ -142,40 +143,91 @@ class User extends Entity implements UserInterface {
+  protected function init() {

Just in general I'm wondering whether could use get_object_vars to make this generic.

+++ b/core/modules/user/lib/Drupal/user/Plugin/Core/Entity/User.phpundefined
@@ -142,40 +143,91 @@ class User extends Entity implements UserInterface {
+  public function getBCEntity() {

Needs @inheritdoc

+++ b/core/modules/user/lib/Drupal/user/Tests/UserTranslationUITest.phpundefined
@@ -39,6 +39,8 @@ function setUp() {
+    entity_get_controller('user')->resetCache();

Not important: This could also use the container to fetch the manager ...

+++ b/core/modules/user/lib/Drupal/user/UserStorageController.phpundefined
@@ -52,19 +52,25 @@ public function create(array $values) {
+    if ($entity->original instanceof \Drupal\Core\Entity\EntityBCDecorator) {

It would be also possible to use a "use" on this class?

+++ b/core/modules/user/lib/Drupal/user/UserStorageController.phpundefined
@@ -111,28 +112,28 @@ protected function postSave(EntityInterface $entity, $update) {
+        drupal_session_destroy_uid($entity->id());
+        if ($entity->uid->value == $GLOBALS['user']->uid) {

Wouldn't it be better use $entity->id() on both lines?

+++ b/core/modules/user/lib/Drupal/user/UserStorageController.phpundefined
@@ -111,28 +112,28 @@ protected function postSave(EntityInterface $entity, $update) {
       // Remove roles that are no longer enabled for the user.
-      $entity->roles = array_filter($entity->roles);
+      //$entity->roles = array_filter($entity->roles);

What happened here?

+++ b/core/modules/user/lib/Drupal/user/UserStorageController.phpundefined
@@ -177,4 +178,123 @@ protected function postDelete($entities) {
+   * Overrides \Drupal\Core\Entity\DataBaseStorageControllerNG::invokeHook().
...
+   * Overrides \Drupal\Core\Entity\DataBaseStorageControllerNG::baseFieldDefinitions().

These could use @inheritdoc

+++ b/core/modules/user/lib/Drupal/user/UserStorageController.phpundefined
@@ -177,4 +178,123 @@ protected function postDelete($entities) {
+    // @todo: field_attach_delete_revision() is named the wrong way round,
+    // consider renaming it.
+    if ($function == 'field_attach_revision_delete') {
+      $function = 'field_attach_delete_revision';
+    }

Just wondering why this fix is not part of DatabaseStorageController?

+++ b/core/modules/user/lib/Drupal/user/UserStorageController.phpundefined
@@ -177,4 +178,123 @@ protected function postDelete($entities) {
+    module_invoke_all($this->entityType . '_' . $hook, $entity->getBCEntity());
...
+    module_invoke_all('entity_' . $hook, $entity->getBCEntity(), $this->entityType);

module_invoke_all should be Drupal::moduleHandler()->invokeAll()

+++ b/core/modules/user/user.moduleundefined
@@ -2135,12 +2147,11 @@ function user_multiple_role_edit($accounts, $operation, $rid) {
+          $account->roles[count($account->roles)] = $rid;

Wouldn't it also just be possible to use ->roles[] = $rid?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/access/AccessPluginBase.phpundefined
@@ -56,7 +56,7 @@ public function summaryTitle() {
+   * @param Drupal\user\UserInterface $account

Just to note: The method doesn't have type hinting yet.

+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewUI.phpundefined
index f171580..c0d383c 100644
--- a/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php

It's good to see that this is easier to read.

Thanks for the review.

- The missing use is because it was like that before, will change it.
- "This feels out of scope." These are tests of #1957888: Exception when a field is empty on programmatic creation, which I merged into this issue. that issue is currently blocking this anyway.
- Looks like one of the review blocks ("$this->assertTrue($entity->field_test_text->access('view', $account->getNGEntity()), 'Access to the field was granted.');"), is cut of, has no comment below it. The getNGEntity() part should no longer be necessary there.
- We might remove init() as we are switching to interfaces where the public properties on he class are useless anyway. Until then, I'd like to keep this consistent with other conversions.
- The array_filter() can be removed, that happens in the form controller now. Will remove it.
- The field_attach_revision_delete stuff is there because we duplicate the whole method so that we can pass the BC decorator to the hook implementations.
- "Wouldn't it also just be possible to use ->roles[] = $rid?" Right now it's not because the Field class wants a numerical index. But I agree that it should be possible, just felt out of scope to change it here.

Status:Needs work» Needs review
StatusFileSize
new7.68 KB
new108.82 KB
FAILED: [[SimpleTest]]: [MySQL] 56,217 pass(es), 43 fail(s), and 4,589 exception(s).
[ View ]

Fixed the last test fail and the feedback from @dawehner as far as possible :)

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-56.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new898 bytes
new108.83 KB
FAILED: [[SimpleTest]]: [MySQL] 55,603 pass(es), 0 fail(s), and 18 exception(s).
[ View ]

Ok, invokeAll() wants an array... We should then actually type hint that?

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-58.patch, failed testing.

+++ b/core/modules/locale/lib/Drupal/locale/Tests/LocaleUninstallTest.php
@@ -62,6 +62,12 @@ function testUninstallProcess() {
+    // @todo: If the global user is an EntityBCDecorator, getting the roles
+    // roles from it within LocaleLookup results in a loop that invokes

"getting the roles roles from", I guess we can remove one "roles".

+++ b/core/modules/search/lib/Drupal/search/Tests/SearchCommentTest.php
@@ -125,9 +125,7 @@ function testSearchResultsComment() {
-    $this->admin_role = key($this->admin_role);
+    $this->admin_role = $this->admin_user->roles[0];

Relying on an "generic" index to fetch a certain roles is a bit odd. Before the conversion "roles" was an associative array, and thus more reliable. Similar stuff happens in other places too.

+++ b/core/modules/user/user.module
@@ -2099,12 +2111,11 @@ function user_multiple_role_edit($accounts, $operation, $rid) {
-          $account->roles = $roles;
+          $account->roles[count($account->roles)] = $rid;

Is there a reason why $account->roles[] = $rid; shouldn't work?

Further I've found some legacy code:

  • In hook_node_grants_alter():
    if (isset($account->roles[$role_id])) {
  • In _drupal_session_read():
    $user->roles[DRUPAL_AUTHENTICATED_RID] = DRUPAL_AUTHENTICATED_RID;

Status:Needs work» Needs review
StatusFileSize
new760 bytes
new109.58 KB
PASSED: [[SimpleTest]]: [MySQL] 55,708 pass(es).
[ View ]

Attempt to fix the test fails.

StatusFileSize
new4.5 KB
new110.7 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-ng-1818570-62_2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Re-roll after the clean values issue went in. Some small clean-up based on the review from @das-peter. Did not yet change the [0] thing.

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-62.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new107.59 KB
FAILED: [[SimpleTest]]: [MySQL] 55,993 pass(es), 1 fail(s), and 16 exception(s).
[ View ]

Messed up the re-roll.

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-64.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.63 KB
new108.38 KB
PASSED: [[SimpleTest]]: [MySQL] 55,858 pass(es).
[ View ]

This should fix those tests.

StatusFileSize
new5.59 KB
new108.37 KB
PASSED: [[SimpleTest]]: [MySQL] 55,720 pass(es).
[ View ]

+++ b/core/includes/bootstrap.inc
@@ -1843,14 +1844,13 @@ function drupal_set_title($title = NULL, $output = CHECK_PLAIN) {
     'roles' => array(
       DRUPAL_ANONYMOUS_RID => DRUPAL_ANONYMOUS_RID,
     ),

I think we should use the numeric index here too - just to prevent wrong expectations.

+++ b/core/modules/system/lib/Drupal/system/Tests/Plugin/PluginTestBase.php
@@ -74,7 +74,7 @@ public function setUp() {
+          'user' => array('class' => 'Drupal\user\\UserInterface')
@@ -88,7 +88,7 @@ public function setUp() {
+          'user' => array('class' => 'Drupal\user\\UserInterface'),

Two backslashes doesn't look right.

+++ b/core/modules/user/lib/Drupal/user/UserAccessController.php
@@ -32,14 +32,14 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, U
+        return (($account->uid == $entity->uid->value) || user_access('administer users', $account)) && $entity->uid->value > 0;
...
+        return ((($account->uid == $entity->uid->value) && user_access('cancel account', $account)) || user_access('administer users', $account)) && $entity->uid->value > 0;

For the sake of consistency changed to ->id()

+++ b/core/modules/user/user.module
@@ -2102,12 +2114,11 @@ function user_multiple_role_edit($accounts, $operation, $rid) {
+        if ($account !== FALSE && !array_search($rid, $account->roles)) {
@@ -2116,8 +2127,8 @@ function user_multiple_role_edit($accounts, $operation, $rid) {
+        if ($account !== FALSE && $index = array_search($rid, $account->roles)) {

This looks odd, $index is never used and the return of array_search() is compared untyped.

Let's see if the adjusted patch works.

Status:Needs review» Needs work
Issue tags:-D8MI, -sprint, -language-content, -Entity Field API, -language-content-property

The last submitted patch, user-ng-1818570-67.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+D8MI, +sprint, +language-content, +Entity Field API, +language-content-property

#67: user-ng-1818570-67.patch queued for re-testing.

Issue tags:+Needs manual testing

To test this manually, apply the patch and then verify that fields can be added to users and are correctly displayed, check different field types, e.g. images, lists, references. Make sure that you can then edit and create users and the values are correctly saved, displayed.

Make sure roles operations work, e.g. removing and re-adding roles and also additional settings like the overlay setting.

I carefully reviewed the patch and I must say this is already great! Imo this is close to being ready, just a few remarks:

+++ b/core/modules/user/lib/Drupal/user/AccountFormController.php
@@ -225,6 +225,19 @@ public function form(array $form, array &$form_state) {
+    // Change the roles array to a list of enabled roles.
+    // @todo: Move this to an value callback on the form element.
+    if (empty($this->roles_filtered)) {
+      $form_state['values']['roles'] = array_keys(array_filter($form_state['values']['roles']));
+      $this->roles_filtered = TRUE;

The regular workflow is to extract from form values and write to an entity field here. So this is what it should do here instead of changing the form values. roles_filtered should go away then also.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/access/AccessPluginBase.php
@@ -56,7 +56,7 @@ public function summaryTitle() {
-   * @param Drupal\user\User $account
+   * @param Drupal\user\UserInterface $account

Misses leading \

Status:Needs review» Needs work

Status:Needs work» Needs review
StatusFileSize
new7.19 KB
new110.01 KB
FAILED: [[SimpleTest]]: [MySQL] 55,853 pass(es), 7 fail(s), and 4 exception(s).
[ View ]

Re-roll.

- As discussed, we can't re-assign it to the entity as it blows up on the initial assignment. Changed it so that we use a copy of $form_state, which is a tiny bit less ugly
- Added the missing \
- Update the @var things. As discussed, we'll remove them when we add methods on UserInterface, I will already do that for file but it doesn't help much here anyway.

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-73.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.09 KB
new110.14 KB
FAILED: [[SimpleTest]]: [MySQL] 55,806 pass(es), 0 fail(s), and 5 exception(s).
[ View ]

Well, that doesn't work, because it looks like we need the form state changes.... Also reverted an accidental change in the access tests during re-roll.

Status:Needs review» Reviewed & tested by the community

I see, so that's the best option we have right now. We can work on cleaning this up in a follow-up while moving it to form-NG.

I think this is good to go then!

Issue tags:-Needs manual testing

To test this manually, apply the patch and then verify that fields can be added to users and are correctly displayed, check different field types, e.g. images, lists, references. Make sure that you can then edit and create users and the values are correctly saved, displayed.

Make sure roles operations work, e.g. removing and re-adding roles and also additional settings like the overlay setting.

Went through that manually as well, everything worked for me.

#75: user-ng-1818570-75.patch queued for re-testing.

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

Lets get some profiling done on this... viewing a page, viewing a list of users using views etc...

Performance will be worse, e.g. NodeNG has been 25% worse. I assume it will be about the same for this one, but let's measure.

However, as discussed during the node conversion lots of that regression comes from the use of the currently enabled BC-mode here, so we are profiling something that will go away again. So any further performance work is going to have to wait until after we drop the BC mode. See #1983554: Remove BC-mode from EntityNG

Related node discussion:
http://drupal.org/node/1818556#comment-7160984

Status:Needs work» Reviewed & tested by the community
Issue tags:-Needs profiling

ok, I've added a couple of fields to users, made a view that renders users and added 10 users that are shown on this page. With the patch applied page generation time is ~23% slower. As said, that's partly due to the BC-mode - see comment above.

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new110.86 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-ng-1818570-82.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new695 bytes

Fixed broken tests, seems left from node conversion

Status:Needs review» Needs work
Issue tags:-sprint, -language-content, -language-content-property

The last submitted patch, user-ng-1818570-82.patch, failed testing.

The last submitted patch, user-ng-1818570-84.patch, failed testing.

Re-adding tags.

Status:Needs work» Needs review
StatusFileSize
new2.32 KB
new109.58 KB
FAILED: [[SimpleTest]]: [MySQL] 57,522 pass(es), 4 fail(s), and 4 exception(s).
[ View ]

Moving the original check back into the user storage controller for now.

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-87.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new508 bytes
new109.63 KB
PASSED: [[SimpleTest]]: [MySQL] 56,051 pass(es).
[ View ]

Forgot the use, this should pass again.

Status:Needs review» Reviewed & tested by the community

Back to RTBC, there's only merge changes

#89: user-ng-1818570-89.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work

Some minor nits... plus I'm also wondering why we chose to change the structure of the roles array stored on the user here? Was it necessary for the conversion?

/**
* Generates a default anonymous $user object.
*
* @return Drupal\user\Plugin\Core\Entity\User
*   The user object.
*/
function drupal_anonymous_user() {

Need to fix up the return value here as it no longer returns a User object...

+++ b/core/modules/user/user.api.phpundefined
@@ -359,7 +359,7 @@ function hook_user_logout($account) {
-function hook_user_view(\Drupal\user\Plugin\Core\Entity\User $account, \Drupal\entity\Plugin\Core\Entity\EntityDisplay $display, $view_mode, $langcode) {
+function hook_user_view(\Drupal\user\UserInterface $account, \Drupal\entity\Plugin\Core\Entity\EntityDisplay $display, $view_mode, $langcode) {
+++ b/core/modules/user/user.moduleundefined
@@ -601,7 +610,7 @@ function user_search_execute($keys = NULL, $conditions = NULL) {
-function user_user_view(User $account, EntityDisplay $display) {
+function user_user_view(EntityInterface $account, EntityDisplay $display) {

The documented implementation seems to suggest the type hinting in user_user_view should be different...

StatusFileSize
new109.23 KB
FAILED: [[SimpleTest]]: [MySQL] 56,602 pass(es), 6 fail(s), and 105 exception(s).
[ View ]

+++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
@@ -64,7 +64,7 @@ public function form(array $form, array &$form_state) {
-    if ($message->recipient instanceof User) {
+    if ($message->recipient instanceof UserInterface) {

This hunk no longer applies due to #1856556: Convert user contact form into a contact category/bundle. This patch is just a straight reroll that removes this hunk, because the new check in HEAD is $message->isPersonal() which needs no change by this issue.

I'm also wondering why we chose to change the structure of the roles array stored on the user here

Because in EntityNG, all properties/fields are expected to follow the Field API data model, where multivalued fields are numerically indexed.

I'll post the fixes for #92 shortly.

Status:Needs work» Needs review
StatusFileSize
new1.11 KB
new109.66 KB
FAILED: [[SimpleTest]]: [MySQL] 56,765 pass(es), 6 fail(s), and 105 exception(s).
[ View ]

Fixes for #92.

Status:Needs review» Needs work

The last submitted patch, user-ng-1818570-94.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new988 bytes
new110.78 KB
PASSED: [[SimpleTest]]: [MySQL] 55,002 pass(es).
[ View ]

Status:Needs review» Needs work
Issue tags:-D8MI, -sprint, -language-content, -Entity Field API, -language-content-property

The last submitted patch, user-ng-1818570-96.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+D8MI, +sprint, +language-content, +Entity Field API, +language-content-property

#96: user-ng-1818570-96.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Re-roll/updates look good, back RTBC.

Title:Convert users to the new Entity Field APIChange notice: Convert users to the new Entity Field API
Status:Reviewed & tested by the community» Active
Issue tags:+Needs change record

Committed 88e6a76 and pushed to 8.x. Thanks!

Title:Change notice: Convert users to the new Entity Field APIConvert users to the new Entity Field API
Status:Active» Fixed
Issue tags:-Needs change record

Added to https://drupal.org/node/1806650, which will need more work, but we can complete that once all entities are converted.

Issue tags:-sprint

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

Issue summary:View changes

Updated issue summary.