Follow up for #1361228: Make the user entity a classed object

We need to revise documentation so that we can properly apply type hinting for code that deals with user entities.

Files: 
CommentFileSizeAuthor
#31 1537434_31.patch36.12 KBcosmicdreams
FAILED: [[SimpleTest]]: [MySQL] 58,527 pass(es), 107 fail(s), and 8,054 exception(s). View
#27 user_entity_type_hinting_27.patch34.23 KBgarphy
FAILED: [[SimpleTest]]: [MySQL] 58,213 pass(es), 107 fail(s), and 9,865 exception(s). View
#24 user_entity_type_hinting_24.patch34.21 KBsocketwench
FAILED: [[SimpleTest]]: [MySQL] 59,218 pass(es), 196 fail(s), and 11,132 exception(s). View
#19 user_entity_type_hinting_19.patch56.29 KBcosmicdreams
FAILED: [[SimpleTest]]: [MySQL] 39,395 pass(es), 573 fail(s), and 64,741 exception(s). View
#17 user_entity_type_hinting.patch33.69 KBcosmicdreams
FAILED: [[SimpleTest]]: [MySQL] 40,686 pass(es), 56 fail(s), and 399 exception(s). View
#14 user_entity_type_hinting-1537434-14.patch46.59 KBAlbert Volkman
FAILED: [[SimpleTest]]: [MySQL] 39,472 pass(es), 307 fail(s), and 9,081 exception(s). View
#14 interdiff.txt390 bytesAlbert Volkman
#12 user_entity_type_hinting-1537434-12.patch46.54 KBAlbert Volkman
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed. View
#6 1537434-user-entity-type-hinting-6.patch46.87 KBaspilicious
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1537434-user-entity-type-hinting-6.patch. Unable to apply patch. See the log in the details link for more information. View
#3 1537434-user-entity-type-hinting-3.patch50.47 KBaspilicious
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to enable simpletest module. View
#1 1537434-user-entity-type-hinting-1.patch50.78 KBaspilicious
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to enable simpletest module. View

Comments

aspilicious’s picture

Status: Active » Needs review
FileSize
50.78 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to enable simpletest module. View

Experimental patch... Wonna see what happens with the bot :)

Status: Needs review » Needs work

The last submitted patch, 1537434-user-entity-type-hinting-1.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
50.47 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to enable simpletest module. View

Reroll, will probably fail again. Have no idea why...

Status: Needs review » Needs work

The last submitted patch, 1537434-user-entity-type-hinting-3.patch, failed testing.

aspilicious’s picture

Ok after installing I found the nastyness...
global $user is still a std object so most functions get both instances of User and std object. Resulting into failures...

aspilicious’s picture

Status: Needs work » Needs review
FileSize
46.87 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1537434-user-entity-type-hinting-6.patch. Unable to apply patch. See the log in the details link for more information. View

Ok this one should install...

Status: Needs review » Needs work

The last submitted patch, 1537434-user-entity-type-hinting-6.patch, failed testing.

tim.plunkett’s picture

Issue tags: +PSR-0

Tagging.

cosmicdreams’s picture

Status: Needs work » Needs review
Issue tags: -PSR-0

Status: Needs review » Needs work
Issue tags: +PSR-0

The last submitted patch, 1537434-user-entity-type-hinting-6.patch, failed testing.

cosmicdreams’s picture

Issue tags: +Needs reroll

needs reroll

Albert Volkman’s picture

Status: Needs work » Needs review
FileSize
46.54 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed. View

Reroll.

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting-1537434-12.patch, failed testing.

Albert Volkman’s picture

Status: Needs work » Needs review
FileSize
390 bytes
46.59 KB
FAILED: [[SimpleTest]]: [MySQL] 39,472 pass(es), 307 fail(s), and 9,081 exception(s). View

Forgot 'use' statement in user.module.

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting-1537434-14.patch, failed testing.

Albert Volkman’s picture

Status: Needs work » Postponed

Postponing.

cosmicdreams’s picture

Status: Postponed » Needs review
FileSize
33.69 KB
FAILED: [[SimpleTest]]: [MySQL] 40,686 pass(es), 56 fail(s), and 399 exception(s). View

Rerolled, provided type hinting in user.module. Checking to see how much this breaks.

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting.patch, failed testing.

cosmicdreams’s picture

Status: Needs work » Needs review
FileSize
56.29 KB
FAILED: [[SimpleTest]]: [MySQL] 39,395 pass(es), 573 fail(s), and 64,741 exception(s). View

After reviewing the patch I found many more places where type hinting could be used.

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting_19.patch, failed testing.

aspilicious’s picture

Status: Needs work » Postponed

This was postponed because session handling isn't a true entity. This will fail all the time. Leave it postponed untill we figured out the session handling.

tim.plunkett’s picture

Status: Postponed » Needs work
socketwench’s picture

Status: Needs work » Needs review
FileSize
34.21 KB
FAILED: [[SimpleTest]]: [MySQL] 59,218 pass(es), 196 fail(s), and 11,132 exception(s). View

Rerolled. I hope.

I wonder if I can beg someone to show me how to do this in PHPstorm. Doing manually is just too error prone.

socketwench’s picture

Might need to redo the patch because of https://drupal.org/node/2053489

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting_24.patch, failed testing.

garphy’s picture

Issue tags: -Needs reroll
FileSize
34.23 KB
FAILED: [[SimpleTest]]: [MySQL] 58,213 pass(es), 107 fail(s), and 9,865 exception(s). View

Rerolled

garphy’s picture

Status: Needs work » Needs review

changing status to trigger the bot

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting_27.patch, failed testing.

cosmicdreams’s picture

+++ b/core/includes/bootstrap.inc
@@ -2848,7 +2848,7 @@ function drupal_classloader_register($name, $path) {
+ * function user_access($string, User $account = NULL) {

In general, why are we using User and not UserInterface to type these parameters?

So, I think we need to document the parameters in the function's docblock.

cosmicdreams’s picture

Status: Needs work » Needs review
FileSize
36.12 KB
FAILED: [[SimpleTest]]: [MySQL] 58,527 pass(es), 107 fail(s), and 8,054 exception(s). View

Here's a patch that explains what I'm saying.

Status: Needs review » Needs work

The last submitted patch, 1537434_31.patch, failed testing.

Berdir’s picture

  1. +++ b/core/modules/comment/comment.module
    @@ -1211,8 +1212,65 @@ function comment_user_cancel($edit, $account, $method) {
    -function comment_user_predelete($account) {
    +function comment_user_predelete(User $account) {
    

    Yes, UserInterface should be used, for example here too :)

    Lots of those below, always UserInterface, in @params and hook_*() documentations always fully qualified with a leading \, in actual implementations only UserInterface + a use.

  2. +++ b/core/modules/comment/comment.module
    @@ -1211,8 +1212,65 @@ function comment_user_cancel($edit, $account, $method) {
    +
    +/**
    + * Accepts a submission of new or changed comment content.
    + *
    + * @param Drupal\comment\Comment $comment
    + *   A comment object.
    + */
    +function comment_save(Comment $comment) {
    +  $comment->save();
    +}
    

    I have absolutely no clue where all this is coming from (not just this, all the added functions around this), but it shouldn't be here :)

cosmicdreams’s picture

Probably just how I created the patch. Clearly, there's more work to be done.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.