Problem/Motivation

Since \Drupal::currentUser() returns AccountProxyInterface, it is a very common misunderstanding to type hint to that interface. However, doing so precludes using loaded User entities or UserSessions in many cases where there is no reason to do so.

Proposed resolution

Add a line or two of clarifying comments.

CommentFileSizeAuthor
#2 2936253-2-accountproxyinterface-documentation.patch680 bytesgabesullice
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gabesullice created an issue. See original summary.

gabesullice’s picture

gabesullice
Primary language English
CreditAttribution: gabesullice at Acquia commented
Status: Active » Needs review
FileSize
2936253-2-accountproxyinterface-documentation.patch680 bytes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: Clarify usage of AccountProxyInterface » Clarify usage of AccountProxyInterface: recommend AccountInterface for most use cases
Component: documentation » base system
Priority: Normal » Minor
Status: Needs review » Reviewed & tested by the community
Issue tags: +DX (Developer Experience), +Documentation

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2936253-2-accountproxyinterface-documentation.patch, failed testing. View results

gabesullice’s picture

gabesullice
Primary language English
CreditAttribution: gabesullice at Acquia commented
Status: Needs work » Reviewed & tested by the community

Hm, doubt that...

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/lib/Drupal/Core/Session/AccountProxyInterface.php
@@ -5,6 +5,11 @@
+ * It is generally more useful to use \Drupal\Core\Session\AccountInterface
+ * unless one specifically needs the proxying features of this interface.
+ *
+ * @see \Drupal\Core\Session\AccountInterface
+ *
  * @ingroup user_api

Should we mark this as @internal as well then - to further communicate that - discuss/thoughts?

Would also require a change record.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

I don't think we need to mark this @internal. Nor can we, because there are contrib modules (e.g. https://www.drupal.org/project/masquerade) using it.

Leaving NR so @gabesullice can agree/disagree and if agree, re-RTBC.

gabesullice’s picture

gabesullice
Primary language English
CreditAttribution: gabesullice at Acquia commented
Status: Needs review » Reviewed & tested by the community

I agree w/ @Wim Leers. The AccountProxyInterface is useful if you need to do any kind of account substitution, which might be useful for all kinds of things (masquerade is just one example, creating email digests might be another).

  • larowlan committed 4453f44 on 8.6.x 
    Issue #2936253 by gabesullice: Clarify usage of AccountProxyInterface:...

  • larowlan committed db92bf5 on 8.5.x 
    Issue #2936253 by gabesullice: Clarify usage of AccountProxyInterface:...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Reviewed & tested by the community » Fixed

Thanks, looks like d.o ate my comment, - committed this to 8.6.x and cherry-picked to 8.5.x

Status: Fixed » Closed (fixed)

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