Fix #133, #134: Don't add not substituted arguments and empty values …

…to query string (#135)
yiisoft · Dec 23, 2023 · 7839246 · 7839246
1 parent 286474f
commit 7839246
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,8 @@
 
 ## 3.0.1 under development
 
-- no changes in this release.
+- Bug #133: Don't add not substituted arguments to a query string (@rustamwin)
+- Bug #134: Don't add query string if it's empty (@rustamwin)
 
 ## 3.0.0 February 17, 2023
 

diff --git a/src/UrlGenerator.php b/src/UrlGenerator.php
@@ -254,7 +254,7 @@ private function missingArguments(array $parts, array $substitutions): array
             }
         }
 
-        // All required arguments are availgit logable.
+        // All required arguments are available.
         return [];
     }
 
@@ -297,10 +297,11 @@ private function generatePath(array $arguments, array $queryParameters, array $p
 
         $path = str_replace('//', '/', $path);
 
-        return $path . (
-            $notSubstitutedArguments !== [] || $queryParameters !== [] ?
-                '?' . http_build_query(array_merge($notSubstitutedArguments, $queryParameters))
-                : ''
-        );
+        $queryString = '';
+        if (!empty($queryParameters)) {
+            $queryString = http_build_query($queryParameters);
+        }
+
+        return $path . (!empty($queryString) ? '?' . $queryString : '');
     }
 }
diff --git a/tests/UrlGeneratorTest.php b/tests/UrlGeneratorTest.php
@@ -196,7 +196,7 @@ public function testQueryParametersAddedAsQueryString(): void
         $this->assertEquals('/test/post?id=12&sort=asc', $url);
     }
 
-    public function testExtraArgumentsAddedAsQueryString(): void
+    public function testQueryParametersAddedAsQueryStringWithEmptyValues(): void
     {
         $routes = [
             Route::get('/test/{name}')
@@ -205,8 +205,8 @@ public function testExtraArgumentsAddedAsQueryString(): void
 
         $url = $this
             ->createUrlGenerator($routes)
-            ->generate('test', ['name' => 'post', 'id' => 12, 'sort' => 'asc']);
-        $this->assertEquals('/test/post?id=12&sort=asc', $url);
+            ->generate('test', ['name' => 'post'], ['id' => null]);
+        $this->assertEquals('/test/post', $url);
     }
 
     public function testQueryParametersOverrideExtraArguments(): void
@@ -222,7 +222,7 @@ public function testQueryParametersOverrideExtraArguments(): void
         $this->assertEquals('/test/post?id=12&sort=asc', $url);
     }
 
-    public function testQueryParametersMergedWithExtraArguments(): void
+    public function testNotSubstitutedArgumentsRemove(): void
     {
         $routes = [
             Route::get('/test/{name}')
@@ -232,7 +232,7 @@ public function testQueryParametersMergedWithExtraArguments(): void
         $url = $this
             ->createUrlGenerator($routes)
             ->generate('test', ['name' => 'post', 'id' => 11], ['sort' => 'asc']);
-        $this->assertEquals('/test/post?id=11&sort=asc', $url);
+        $this->assertEquals('/test/post?sort=asc', $url);
     }
 
     public function testDefaultNotUsedForOptionalArgument(): void