Clicking on paths /admin/compact/on or /admin/compact/off gives a fatal error:

Symfony\Component\Routing\Exception\RouteNotFoundException: Route "front" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 127 of drupal-8/core/lib/Drupal/Core/Routing/RouteProvider.php).

For example: click on "Hide descriptions" or "Show descriptions" in admin/config.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pwolanin’s picture

Status: Active » Needs review
Issue tags: +Needs tests
FileSize
592 bytes

This fixes the bug, but we should add some minimal test coverage too.

pfrenssen’s picture

Here's a test.

pfrenssen’s picture

Issue tags: -Needs tests

Removing tag.

tstoeckler’s picture

tstoeckler’s picture

+++ b/core/modules/system/lib/Drupal/system/Tests/System/AdminTest.php
@@ -42,6 +42,9 @@ function setUp() {
+    // Configure 'admin' as front page, so we can test redirects.
+    \Drupal::config('system.site')->set('page.front', 'admin')->save();

This code looks strange, can you comment on why this was needed?

pfrenssen’s picture

If I do not put that line there the front page is not defined and the redirect is not executed.

By default the frontpage is 'node', but the node module is not enabled in this test, so I opted for 'admin'.

pfrenssen’s picture

Status: Needs review » Needs work

Woops, I copy/pasted these lines and forgot to change "on" into "off"

     $this->drupalGet('admin/compact/off');
+    $this->assertResponse(200, 'A valid page is returned after turning on compact mode.');
+    $this->assertUrl('<front>', array(), 'The user is redirected to the front page after turning on compact mode.');

Shall we reword the comment for the front page configuration as well? Maybe

Configure the front page so we can test redirects to it. Usually this is set to 'node' but the node module is not enabled in this test.

tstoeckler’s picture

Yes, a more elaborate comment would be great.

I still don't know really get why that is necessary.

As far as I know, the default front page is 'user' in D8 and user.module should always be enabled. But let's not hold this up on any more on that detail. As long as it's green...

andypost’s picture

+++ b/core/modules/system/lib/Drupal/system/Tests/System/AdminTest.php
@@ -42,6 +42,9 @@ function setUp() {
+    \Drupal::config('system.site')->set('page.front', 'admin')->save();

should be $this->container->get(...

pfrenssen’s picture

Assigned: Unassigned » pfrenssen

Will address remarks.

pfrenssen’s picture

Assigned: Unassigned » pfrenssen
FileSize
2.49 KB
2.48 KB
1.9 KB

@tstoeckler, you're right. The front page is by default indeed set to 'user', but if a user is logged in this redirects to 'user/{uid}'. This causes $this->assertUrl('<front>') to fail, since it looks at the redirected url and this does not match the original url.

The test now checks for the redirected url.

pfrenssen’s picture

Assigned: pfrenssen » Unassigned
Status: Needs work » Needs review
dawehner’s picture

Assigned: pfrenssen » Unassigned

Haven't we decided to go with 'front' as routename?

tim.plunkett’s picture

If so, we need to change it back.

'<front>':
  pattern: '/'
  requirements:
    _access: 'TRUE'
tstoeckler’s picture

Thanks @pfrenssen for figuring that out. It's much clearer now, especially with the comment!

pfrenssen’s picture

It's been over two weeks, let's see if this needs a reroll.

pfrenssen’s picture

#11: 2076551-11-tests_only.patch queued for re-testing.

pfrenssen’s picture

#11: 2076551-11.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2076551-11.patch, failed testing.

pfrenssen’s picture

Status: Needs work » Needs review
FileSize
2.48 KB
1.9 KB

Rerolled against latest HEAD.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to go.

pwolanin’s picture

#20: 2076551-20.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2076551-20.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

#20: 2076551-20.patch queued for re-testing.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

Added manual testing instructions.