Steps to reproduce:

  1. Install Drupal with the standard profile
  2. Enable the Content Translation module
  3. Enable comment translation for the Article bundle
  4. Create a new article
  5. Post a comment
  6. Access the comment translation overview through the "translate" link

Expected result: the translation overview page is shown.
Actual result: the following exception is thrown:

Symfony\Component\Routing\Exception\RouteNotFoundException: Route "comment_permalink" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 127 of /var/www/test.dd/core/lib/Drupal/Core/Routing/RouteProvider.php). 

The exception is thrown while rendering comment local tasks/actions.

Related

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plach’s picture

Issue summary: View changes

Updated issue summary.

plach’s picture

Issue summary: View changes

Updated issue summary.

Berdir’s picture

Try comment.permalink instead of comment_permalink. Same for the others in that file.

plach’s picture

Already tried that and I get a new exception:

Symfony\Component\Routing\Exception\MissingMandatoryParametersException: Some mandatory parameters are missing ("comment") to generate a URL for route "/comment/{comment}". in Symfony\Component\Routing\Generator\UrlGenerator->doGenerate() (line 155 of /var/www/test.dd/core/vendor/symfony/routing/Symfony/Component/Routing/Generator/UrlGenerator.php). 

Not sure how I am supposed to proceed :(

Test coverage for this can be provided by simply reverting the change introduced in #2004626-87: Make non-configurable field translation settings available in the content language settings.

Gábor Hojtsy’s picture

Status: Active » Needs review
FileSize
3.16 KB

All local tasks in comment.local_tasks.yml need to use the comment DOT notation, not underscores. I noted this in an unrelated issue and the fix got removed in that patch, since it still worked kind of with comment_menu() having those tabs also. This would be the cleanup for comment stuff AFAIS.

Status: Needs review » Needs work

The last submitted patch, comment-routing-tabs.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
3.08 KB

Rerolled for current head, less changes needed in local_tasks.yml, since the task names now use dots. The route names don't, so the tabs basically refer to nonexistent routes.

plach’s picture

Status: Needs review » Needs work

Later I will move here the tests added in #2004626: Make non-configurable field translation settings available in the content language settings. Those should provide us the test coverage we are currently missing.

plach’s picture

Still failing with:

Symfony\Component\Routing\Exception\MissingMandatoryParametersException: Some mandatory parameters are missing ("comment") to generate a URL for route "/comment/{comment}". in Symfony\Component\Routing\Generator\UrlGenerator->doGenerate() (line 155 of /var/www/test.dd/core/vendor/symfony/routing/Symfony/Component/Routing/Generator/UrlGenerator.php). 

It seems there's an inconsistency between the expected variable name (comment) and the actual variable name (entity).

plach’s picture

Status: Needs work » Needs review
plach’s picture

Issue summary: View changes

Updated issue summary.

dawehner’s picture

This is my idea to fix it.

Status: Needs review » Needs work

The last submitted patch, content_translation-2106349-9.patch, failed testing.

plach’s picture

This is needed to fix some unintended failures in #7. It should be incorporated in the next patch.

webchick’s picture

Priority: Major » Critical
dawehner’s picture

Status: Needs work » Needs review
FileSize
2.93 KB
13.02 KB

This fixes nearly all of the tests.

Could it be that the entity_test_mul should have a label entity key in order to be translatable?

Status: Needs review » Needs work

The last submitted patch, content_translation-2106349-13.patch, failed testing.

plach’s picture

Priority: Critical » Major
Status: Needs work » Needs review
FileSize
13.68 KB
677 bytes

Of course :)

plach’s picture

Priority: Major » Critical

d.o.--

Berdir’s picture

+++ b/core/modules/content_translation/lib/Drupal/content_translation/Tests/ContentTranslationUITest.php
@@ -99,6 +100,22 @@ protected function assertBasicTranslation() {
+      if ($entity->hasTranslation($langcode)) {
+        debug($entity->getTranslation($langcode)->label());
+        $this->assertText($entity->getTranslation($langcode)->label(), format_string('Label correctly shown for %language translation', array('%language' => $langcode)));

debug() is still in there...

Status: Needs review » Needs work

The last submitted patch, content_translation-2106349-15.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, content_translation-2106349-15.patch, failed testing.

plach’s picture

Status: Needs work » Needs review
FileSize
13.67 KB
869 bytes

Rerolled and removed the debug leftover.

YesCT’s picture

#2102125-19: Big Local Task Conversion related to the dot underscore naming for local tasks.

YesCT’s picture

Issue summary: View changes

Updated issue summary.

dawehner’s picture

Issue tags: +WSCCI

The interdiff in #2106349-15: Comment translation overview has broken local tasks looks just perfect. I would love to RTBC that.

YesCT’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -WSCCI

I read the patch.
titles and labels are matching, uses correct naming convention, removes local tasks from hook_menu that are in comment.local_tasks.yml or already in comment.routing.yml
weight of delete is different. trying manually: both before and after, delete is first in the list of links: " delete edit reply approve"
also, I dont see /view in the patched version. I'll try it manually. Both before and after the patch comment/1/view gives page not found, so ok.

this looks ok to me. rtbc.

YesCT’s picture

Issue tags: +WSCCI

replacing tag lost in cross post.

YesCT’s picture

Issue summary: View changes

Updated issue summary.

catch’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/core/modules/content_translation/lib/Drupal/content_translation/Access/ContentTranslationOverviewAccess.php
    @@ -45,7 +45,8 @@ public function appliesTo() {
    -    if ($entity = $request->attributes->get('entity')) {
    +    $entity_type = $request->attributes->get('_entity_type');
    +    if ($entity = $request->attributes->get($entity_type)) {
           // Get entity base info.
           $entity_type = $entity->entityType();
           $bundle = $entity->bundle();
    

    Why do we get $entity_type from the request, then later get it again from $entity?

dawehner’s picture

Status: Needs work » Needs review
FileSize
1.8 KB
13.92 KB

Why do we get $entity_type from the request, then later get it again from $entity?

There is no reason at all.

Removed the double usage of $entity_type.

plach’s picture

Status: Needs review » Reviewed & tested by the community

Changes look sensible to me: RTBC again if the testbot agrees.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.