Merge pull request #6186 from kenjis/fix-routes-group-slash

fix: `$routes->group('/', ...)` creates the route `foo///bar`
codeigniter4 · Jun 26, 2022 · 053d669 · 053d669
2 parents 74f12ec + 66fbd47
commit 053d669
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 2 deletions.
diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php
@@ -623,9 +623,9 @@ public function group(string $name, ...$params)
         $oldOptions = $this->currentOptions;
 
         // To register a route, we'll set a flag so that our router
-        // so it will see the group name.
+        // will see the group name.
         // If the group name is empty, we go on using the previously built group name.
-        $this->group = $name ? ltrim($oldGroup . '/' . $name, '/') : $oldGroup;
+        $this->group = $name ? trim($oldGroup . '/' . $name, '/') : $oldGroup;
 
         $callback = array_pop($params);
 

diff --git a/tests/system/Router/RouteCollectionTest.php b/tests/system/Router/RouteCollectionTest.php
@@ -375,6 +375,54 @@ static function ($routes) {
         $this->assertSame($expected, $routes->getRoutes());
     }
 
+    /**
+     * @dataProvider groupProvider
+     */
+    public function testNestedGroupingWorksWithRootPrefix(
+        string $group,
+        string $subgroup,
+        array $expected
+    ) {
+        $routes = $this->getCollector();
+
+        $routes->group($group, static function ($routes) use ($subgroup) {
+            $routes->group(
+                $subgroup,
+                static function ($routes) {
+                    $routes->add('users/list', '\Users::list');
+
+                    $routes->group('delegate', static function ($routes) {
+                        $routes->add('foo', '\Users::foo');
+                    });
+                }
+            );
+        });
+
+        $this->assertSame($expected, $routes->getRoutes());
+    }
+
+    public function groupProvider()
+    {
+        yield from [
+            ['admin', '/', [
+                'admin/users/list'   => '\Users::list',
+                'admin/delegate/foo' => '\Users::foo',
+            ]],
+            ['/', '', [
+                'users/list'   => '\Users::list',
+                'delegate/foo' => '\Users::foo',
+            ]],
+            ['', '', [
+                'users/list'   => '\Users::list',
+                'delegate/foo' => '\Users::foo',
+            ]],
+            ['', '/', [
+                'users/list'   => '\Users::list',
+                'delegate/foo' => '\Users::foo',
+            ]],
+        ];
+    }
+
     public function testHostnameOption()
     {
         $_SERVER['HTTP_HOST'] = 'example.com';