Replace _l() in PathController::adminOverview()

I think the way to go here is to add a variant to UnroutedUrlAssembler which includes path processing,
but this should be off by default. Does someone has an oppinion about that?

adding blocker tag, since this blocks #2343669: Remove _l() and _url(), so that the d8 blockers is accurate. Remove the blocker tag when this issue is fixed, and unpostpone 2343669 if this was the last blocker of that.

One simple solution would be to add opt in path processing functionality to the unrouted URL assembler.

So this is one way to fix it, this works at least, but I don't really like it, to be honest.

Okay, let's feed the url assembler a litte bit more.

\Drupal\Tests\Core\Utility\UnroutedUrlAssemblerTest. now passes.

1. +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
   @@ -27,17 +28,27 @@ class UnroutedUrlAssembler implements UnroutedUrlAssemblerInterface {
   +   * @var \Drupal\Core\PathProcessor\PathProcessorManager
   ...
   +   * @param \Drupal\Core\PathProcessor\PathProcessorManager $path_processor
   +   *   The path processor manager.
   ...
   +  public function __construct(RequestStack $request_stack, ConfigFactoryInterface $config, PathProcessorManager $path_processor) {
   

   Any reason why we don't have an interface for this? I realise it implements two existing interfaces so it would be an amalgam of those.

2. +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
   @@ -99,9 +110,16 @@ protected function buildLocalUrl($uri, array $options = []) {
   +
   
   @@ -126,6 +144,7 @@ protected function buildLocalUrl($uri, array $options = []) {
   +
   

   Unneeded change

+++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
@@ -99,9 +110,16 @@ protected function buildLocalUrl($uri, array $options = []) {
+    // Allow path processing, if needed.
+    if (!empty($options['path_processing'])) {
+      $this->pathProcessor->processOutbound($uri, $options);

Any reason to make it in constructor?

Issue summary/title says replace _l in PathController::adminOverview() but I don't see that change in the patch?

Just be clear, I never promised that this patch would be ready ... all I wanted to do for now, is to outline what my idea is.

Any reason to make it in constructor?

... its a special usecase, so you should always manually opt in, if possible.

Sensible solution IMO. And the patch is looking great.

1. +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
   @@ -27,17 +28,27 @@ class UnroutedUrlAssembler implements UnroutedUrlAssemblerInterface {
   +   * The output path processor.
   +   *
   +   * @var \Drupal\Core\PathProcessor\OutboundPathProcessorInterface
   +   */
   +  protected $pathProcessor;
   
   @@ -99,6 +110,12 @@ protected function buildLocalUrl($uri, array $options = []) {
   +    // Allow path processing, if needed.
   +    if (!empty($options['path_processing'])) {
   +      $uri = $this->pathProcessor->processOutbound($uri, $options);
   +    }
   

   I think either of these should explicitly mention PathController::adminOverview() as an example of a valid use case.

2. +++ b/core/tests/Drupal/Tests/Core/Utility/UnroutedUrlAssemblerTest.php
   @@ -90,6 +98,56 @@ public function providerTestAssembleWithExternalUrl() {
   +  public function testAssembleWithOutboundPathProcessorNotProcessing() {
   ...
   +  public function testAssembleWithOutboundPathProcessorWithProcessing() {
   

   I had to read these 3 times :P Could you shorten them a bit? :)

3. +++ b/core/tests/Drupal/Tests/Core/Utility/UnroutedUrlAssemblerTest.php
   @@ -90,6 +98,56 @@ public function providerTestAssembleWithExternalUrl() {
   +   * @param string $subdir
   

   string|FALSE

Thank you for your review!

I had to read these 3 times :P Could you shorten them a bit? :)

That is a total valid point.

This should be green now as well.

+++ b/core/includes/install.core.inc
@@ -1060,21 +1051,6 @@ function install_verify_database_settings() {
- * Verify that the database is ready (no existing Drupal installation).
- */
-function install_verify_database_ready() {
-  $system_schema = system_schema();
-  end($system_schema);
-  $table = key($system_schema);
-
-  if ($database = Database::getConnectionInfo()) {
-    if (Database::getConnection()->schema()->tableExists($table)) {
-      throw new AlreadyInstalledException(\Drupal::service('string_translation'));
-    }
-  }
-}
-

Is this from another issue?

#17: I think so. NW for that. The interdiff looks good, but the patch contains unrelated stuff. I think Daniel might've forgotten to do a hard reset :)

Yeah I forgot to merge the recent patch in.

I think this is ready. A simple addition, test coverage, supporting a rare use case (hence no CR needed IMHO) that helps us to resolve the url()/l() fallout (one of the last 5 after #2364161: Replace nearly all existing _l calls).

IS updated, beta evaluation added.

Nitpicks that can be resolved on commit:

1. +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
   @@ -27,17 +28,27 @@ class UnroutedUrlAssembler implements UnroutedUrlAssemblerInterface {
   +   * The output path processor.
   ...
   +   *   The output path processor.
   

   s/output/outbound/

2. +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
   @@ -99,6 +110,14 @@ protected function buildLocalUrl($uri, array $options = []) {
   +    // Allow path processing, if needed.
   

   s/path processing/(outbound) path processing/

3. +++ b/core/tests/Drupal/Tests/Core/Utility/UnroutedUrlAssemblerTest.php
   @@ -40,6 +40,13 @@ class UnroutedUrlAssemblerTest extends UnitTestCase {
   +   * The mocked path processor.
   

   outbound path processor

Needs a reroll...

And

+++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
@@ -99,6 +110,14 @@ protected function buildLocalUrl($uri, array $options = []) {
+    // Allow path processing, if needed.
+    // A valid usecase is the path alias overview form:

This comment needs reflowing...

Rerolled and addressed #20

Yeaaaaah!

Rerolled - #18 is probably fine too ..

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed a942dd5 and pushed to 8.0.x. Thanks!