Ensure consistency between $langcode parameter and entity language in the Entity Access API [#2581721]

Problem/Motivation

Alternative approach to fix #2072945: Remove the $langcode parameter in EntityAccessControllerInterface::access() and friends.

Proposed resolution

@berdir in #2072945-56: Remove the $langcode parameter in EntityAccessControllerInterface::access() and friends:

Aren't there easier ways to prevent that, for example by making sure that $langcode is consistent with $entity, for content entities? If it's default, then replace it with the active langcode of the entity, if not, then make sure the entity is using that translation?

@plach in #2072945-58: Remove the $langcode parameter in EntityAccessControllerInterface::access() and friends:

That would be a possible alternative, but it would still require an API break: we'd need to change the default value of $langcode to NULL, because LanguageInterface::LANGCODE_DEFAULT has a very precise meaning, which is not active language, but entity default language. Only when $langcode is not provided, we can safely rely on the active language, otherwise a developer explicitly passing LanguageInterface::LANGCODE_DEFAULT would get an unexpected behavior. Additionally, ensuring that active language and $langcode match would either require us to make $langcode prevail, if they don't, or throw an exception. In the first case we'd end-up ignoring the active language most of the time, unless we changed the $langcode default to NULL. In both cases we'd have a BC break.

Remaining tasks

Decide whether we want to do #2072945: Remove the $langcode parameter in EntityAccessControllerInterface::access() and friends instead.
Reviews

User interface changes

None

API changes

The default value for the $langcode parameter of EntityAccessControlHandlerInterface:access() is NULL.
NodeGrantDatabaseStorageInterface::access() no longer has a $langcode parameter.
NodeInterface::prepareLangcode() is removed.

Data model changes

None

Comment	File	Size	Author
#2	et-fix_access_langcode-2581721-2.patch	13.15 KB	plach

Comments

Comment #1

7 October 2015 at 11:42

plach created an issue. See original summary.

Comment #2

plach

he/him

Italian

Venezia

commented 7 October 2015 at 11:46

Status:

Active

» Needs review

Status	File	Size
new	et-fix_access_langcode-2581721-2.patch	13.15 KB

Note that this is the same patch posted at #2496337-75: [plach] Testing issue, so it's expected to be green.

Comment #3

wim leers

Ghent 🇧🇪🇪🇺

commented 7 October 2015 at 11:57

I wrote my rationale in favor of the other solution at #2072945-71: Remove the $langcode parameter in EntityAccessControllerInterface::access() and friends, quoting it in full here:

I personally would prefer this solution.

This is similar to the EntityViewBuilder $langcode thing from a few days ago: #2073217: Remove the $langcode parameter from the entity view/render system.

This is IMO about Entity API completion/DX/understandability. This helps make it clear that you want $entity->language(), not some separate $langcode variable. i.e. it makes it clear that Entities are self-contained value objects that you interact with, containing all info. Which contrasts with D7, where some metadata had to be passed around and kept in sync.

(Which ironically is exactly what #2581721: Ensure consistency between $langcode parameter and entity language in the Entity Access API demonstrates: $entity and $langcode are not always in sync in HEAD.)

The strongest reason not to do this IMO is the fact that very few people will be confused by this, because, really, how many people actually write entity access code? OTOH, that's also a reason to do this: few people are affected, so disruption is small. (Berdir with his 30 implementations of this, described in #56, is clearly the big exception. Sorry Berdir.)