This is a follow-up to #1188388: Entity translation UI in core (this might be a potential novice issue or something for future core office hours)

Problem/Motivation

The are some methods of EntityTranslationControllerInterface which should be best located elsewhere:
The getAccess() method should be removed as soon as we have entity access available: #1696660: Add an entity access API for single entity access, #777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter()

Proposed resolution

Use a more generic solution.

Remaining tasks

Postponed on #1696660: Add an entity access API for single entity access
Postponed on #777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter()
Kind of postponed on #1188388: Entity translation UI in core (the changes will be made to the code that that introduces)
Once those land, post a patch to remove getAccess()
Then, do reviews, update or write tests, evaluate if documentation or a change notice is needed.

User interface changes

Most likely, *no* new or changed features/functionality in the user interface, modules added or removed, changes to URL paths, changes to user interface text.

API changes

This will probably be a small internal change and no major API changes/additions that would affect module, install profile, and theme developers. We could include an example of before/after code.

Background

Originally in #1188388-19: Entity translation UI in core
plach

The getAccess() method should be removed as soon as we have entity access available and replaced with EnttyInterface::access() calls or something along those lines.

And responded in support of making this a follow-up in #1188388-81: Entity translation UI in core
Gabor

This sounds like it would be a followup to this patch once/if entity access lands. I'm not seeing an outpouring of activity on #777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter(), so looks like this is likely a no-go for Drupal 8 anyway, and nodes will stay special.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Issue tags: +Entity Access
plach’s picture

Title: remove EntityTranslationControllerInterface getAccess() once have entity access [Follow-up to Entity Translation UI in core] » Remove EntityTranslationControllerInterface getAccess() once have entity access
Component: language system » translation_entity.module
Issue tags: +API clean-up
plach’s picture

Title: Remove EntityTranslationControllerInterface getAccess() once have entity access » Remove EntityTranslationControllerInterface::getAccess() once have entity access
Berdir’s picture

Issue tags: +Novice, +Entity Field API

Adding some tags and bumping. #1862754: Implement entity access API for comments is the last one that has to land and then we can start doing this.

#777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() isn't really a blocker, not quite sure what the goal of that issue is now that we have an entity access controller, have commented on there.

Steps to do this, once the comment issue mentioned above is in:

- Remove getAccess() from the interface mentioned in the title
- Remove the method from all still existing implementations ( I think node right now still has one for example)
- Change all calls to getAccess() on the translation controller to $entity->access().

This doesn't need a change notice or similar because this really was just an temporary, mostly entity_translation.module-internal wrapper because the entity access api (which is new on 8.x and has a change notice) did not yet exist. So maybe just list to that one, if you want to :)

Berdir’s picture

Status: Postponed » Active
Issue tags: -Novice, -Entity Field API

This should now be possible.

Gábor Hojtsy’s picture

Component: translation_entity.module » content_translation.module
Berdir’s picture

Issue tags: +Novice

Let's get a novice on this :)

alarcombe’s picture

Novice here. Think this fixes it, but would like guidance on how to test - thanks!

plach’s picture

Status: Active » Needs review

Thanks! Just set the issue status to "needs review" and the bot will pick it up and run automated tests.

Status: Needs review » Needs work

The last submitted patch, core-remove_getaccess-1810320-8.patch, failed testing.

alarcombe’s picture

Status: Needs work » Needs review
Berdir’s picture

Status: Needs review » Needs work

Patch looks good, but the translation_entity module was renamed to content_translation, that's why it doesn't apply anymore, so you need to update/re-do these parts.

alarcombe’s picture

Status: Needs work » Needs review
FileSize
6.15 KB

Ah yes, thanks. Fixed now (I hope).

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Looks great to me :)

Passed tests, I did not find any left-over getAccess call or implementation. RTBC.

plach’s picture

+1

Awesome work!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Yay!

Committed 78f93fa and pushed to 8.x. Thanks!

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