Updated: Comment #0

Problem/Motivation

\Drupal\Core\Entity\EntityAccessControllerInterface::access has arguments as follows

 @param \Drupal\Core\Entity\EntityInterface $entity
   *   The entity for which to check access.
   * @param string $operation
   *   The operation access should be checked for.
   *   Usually one of "view", "update" or "delete".
   * @param string $langcode
   *   (optional) The language code for which to check access. Defaults to
   *   Language::LANGCODE_DEFAULT.
   * @param \Drupal\Core\Session\AccountInterface $account
   *   (optional) The user session for which to check access, or NULL to check
   *   access for the current user. Defaults to NULL.

Whereas ::createAccess has

@param string $entity_bundle
   *   (optional) The bundle of the entity. Required if the entity supports
   *   bundles, defaults to NULL otherwise.
   * @param \Drupal\Core\Session\AccountInterface $account
   *   (optional) The user session for which to check access, or NULL to check
   *   access for the current user. Defaults to NULL.
   * @param array $context
   *   (optional) An array of key-value pairs to pass additional context when
   *   needed.

We should make them consistent

Proposed resolution

Make them consistent where/if possible

Remaining tasks

Patch
Review

User interface changes

None

API changes

Argument order changes

Follow-up from #2100397: [meta] Ensure that DX issues identified by a recent review are covered with individual issues.

Comments

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

The only common argument is $account. AccessibleInterface has $operation and immediately after that $account, however, $account is optional and cannot be moved to the beginning of the argument list just like that. #2072945: Remove the $langcode parameter in EntityAccessControllerInterface::access() and friends wants to remove $langcode, so $operation and $account end up being in the same order as AccessibleInterface::access() anyway.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Yes, consistent how?

That they don't get the same arguments is by design, createAccess() has no $entity. Initially create() when through access() too.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

see #2100397: [meta] Ensure that DX issues identified by a recent review are covered with individual issues patch at 1 for where this came from.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

Could very well be closed (won't fix).

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Active » Closed (won't fix)

Well, sun is unfortunately quite wrong about this ;)

As mentioned above, $langcode will go away if we manage to do that. $entity_bundle is last because it's conceptually optionally (although a value will always be passed to the internal checkCreateAccess() method, not the outer one, but changing it here would be inconsistent with createAccess().

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Status: Closed (won't fix) » Active

$context is really the only argument that has bad DX, as far as I can see. It can used to pass on arbitrary data, but only for checking create access. Also, in practice it's only used for passing on the language, and then only by node_access(), which looks like it may die soon anyway (#1947880: Replace node_access() by $entity->access() and #2019677: Remove node_access).

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Status: Active » Closed (won't fix)

Crosspost.