Problem/Motivation

The UI displays Translate operation links for untranslatable entities, which results in a HTTP 404.

Steps to reproduce

Proposed resolution

Check if an entity is translatable before displaying the Translate link.

Remaining tasks

Determine if this is still a problem.

User interface changes

Links may be hidden if they are not applicable.

API changes

None

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xano’s picture

Assigned: Xano » Unassigned
Status: Active » Needs review
FileSize
1015 bytes

Status: Needs review » Needs work

The last submitted patch, 1: drupal_2140925_1.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review

1: drupal_2140925_1.patch queued for re-testing.

tstoeckler’s picture

This makes a lot of sense. Instead of checking for the existence of 'config_prefix', however, I think we should simply check for $entity instanceof ConfigEntityInterface. Thoughts?

Xano’s picture

As it has been decided that config entity storage isn't swappable after all, every entity type needs to implement ConfigEntityInterface and have a config prefix, so either solution should work. Checking for ConfigEntityInterface is cleaner, but may not communicate how the code works as much as checking for the prefix does.

Xano’s picture

Wait, no. We can't just do either. Not all config entities are translatable. We need the config prefix to construct the filename of the config entity, which is needed to check if the entity itself is translatable.

I would very much like to put this piece of code in a method somewhere, because #1839516: Introduce entity operation providers needs it too.

Status: Needs review » Needs work

The last submitted patch, 1: drupal_2140925_1.patch, failed testing.

tstoeckler’s picture

+++ b/core/modules/config_translation/config_translation.module
@@ -140,7 +140,11 @@ function config_translation_config_translation_info(&$info) {
+    && $mapper_manager->hasTranslatable($info['config_prefix'] . '.' . $entity->id())) {

Sorry, I didn't look closely enough before. This should be $mapper->hasTranslatable() (which checks for the appropriate config names itself). Therefore the instanceof check should be sufficient.

tstoeckler’s picture

Oh wait. We don't have $mapper. Hmm... Well we should certainly add a method to ConfigMapperManager to get the mapper for an entity. I'll see if I can cook something up tomorrow.

Xano’s picture

Status: Needs work » Needs review
FileSize
916 bytes
tstoeckler’s picture

Status: Needs review » Needs work

Ohh, it's so wrong that we use the entity type as the plugin ID. We changed to making the plugin ID equal the base route name, but apparently forgot entity mappers.

This is now missing the instanceof check. Even better than that, however, we should simply be checking whether the mapper exists. E.g. if field UI module is disabled, for fields you wouldn't get a proper mapper, but simply FALSE. (I guess hook_operations_info() wouldn't be run on fields in that case, but still...)

Xano’s picture

Status: Needs work » Needs review
FileSize
1.33 KB
tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Looks good. Xano pointed out that createInstance() doesn't just return FALSE, but throws an exception. That's why we need to go the detour of checking getDefinition() first.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 12: drupal_2140925_12.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review

12: drupal_2140925_12.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 12: drupal_2140925_12.patch, failed testing.

tstoeckler’s picture

Status: Needs work » Needs review

12: drupal_2140925_12.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 12: drupal_2140925_12.patch, failed testing.

tstoeckler’s picture

+++ b/core/modules/config_translation/config_translation.module
@@ -140,14 +140,18 @@ function config_translation_config_translation_info(&$info) {
+    $mapper = $manager->createInstance($entity->entityType());
+    if (\Drupal::currentUser()->hasPermission('translate configuration') && $mapper->hasTranslatable()) {

So @webflo smartly points out that this cannot work, as the mapper does not know about the specific entity, but only about the entity type. We need to initialize the entity-specific information with setEntity() first. Tests++

Gábor Hojtsy’s picture

Ideally this code would do the same access checking that we have on the route or at least consider that possibility. Eg if the config is in undefined language, it is not translatable either. That may have UX implications, eg. confusion to users that some entities in a list have a translate buttons, others don't. So all considered with UX this patch may be better at the end.

Xano’s picture

We should move the access check to hook_entity_access() so it's reusable through entities' access controllers. Once #1839516: Introduce entity operation providers is in, this is a requirement for operation link access control anyway.

Gábor Hojtsy’s picture

Sounds like that would need some refactoring, since not all config are entities, so we need to delegate the access checking through the config mapper then which can rely on entity access in the entity mapper then :)

Xano’s picture

Well, this issue is about the entity operation only. Does this access check go wrong elsewhere for non-entities too?

Gábor Hojtsy’s picture

No. But to factor out the access checking in the route access class to an entity access hook, we'll need to then re-integrate it in the route access class (via the config mapper).

Xano’s picture

I assume you are talking about \Drupal\config_translation\Access\ConfigTranslationOverviewAccess and/or \Drupal\config_translation\Access\ConfigNameCheck? I am not sure what the problem is you are describing, but can't it be solved by using multiple access requirements on routes? One can be the entity access check, whereas the other points to these classes.

Gábor Hojtsy’s picture

Right, we need a combination of access checks, which is what I am saying. The route access depends on the entity access as well as dynamic path components (eg target language of the translation). So all I am saying it needs to delegate parts of the access check to avoid code duplications.

webflo’s picture

Status: Needs work » Needs review
FileSize
1.59 KB

The plugin id for field instance is not field_instance (thanks to config_translation_config_translation_info). Fiel instance translation depends on the parent entity type.

Gábor Hojtsy’s picture

Looks like this may not be very extensible then... Eg. view modes and form modes would also depend on the parent entity?

tstoeckler’s picture

So, I was about to RTBC this, when I saw the discussion above:
@Xano/@Gábor Hojtsy: So what's the proposed solution, that is not really clear to me from the above?

tstoeckler’s picture

I just reviewed #1839516: Introduce entity operation providers and now understand #20 - #26. I will post an updated patch that implements entity access if no-one beats me to it.

Xano’s picture

Status: Needs review » Needs work

The last submitted patch, 27: config_translation_entity_operation_alter-2140925-27.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review
FileSize
1.56 KB

Re-roll.

Status: Needs review » Needs work

The last submitted patch, 33: drupal_2140925_33.patch, failed testing.

Gábor Hojtsy’s picture

So now the block and field translation links that are looked for in the test are not found.

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.

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

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

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

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

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

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

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

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

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should 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.

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

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

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

DieterHolvoet’s picture

FileSize
1.47 KB

Re-roll

DieterHolvoet’s picture

FileSize
880 bytes

Re-rolled patch isn't working anymore, I updated the patch to check access using the url object.

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

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

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

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

Issue summary: View changes
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +Bug Smash Initiative, +Needs issue summary update, +Needs steps to reproduce

Is this still a problem? All discussion here happened for Drupal 8. Has this been fixed in the meantime?

The issue summary does not have specific steps to reproduce so it is not clear what entities have this problem. Adding tag for steps to reproduce.

Since we need more information to move forward with this issue, I am setting the status at Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks

quietone’s picture

Status: Postponed (maintainer needs more info) » Closed (outdated)

Since no additional information has been supplied I am closing this issue.

If you are experiencing this problem on a supported version of Drupal reopen the issue, by setting the status to 'Active', and provide complete steps to reproduce the issue (starting from "Install Drupal core").

Thanks!