From 2c384d9cb5b59432772747ec85a86bd86eecbb11 Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Mon, 21 Feb 2022 11:42:06 +0000
Subject: [PATCH] allow no parameters for url_for(), fixes #642

---
 app/controllers/search/module.php             | 32 ++++++++++-------
 app/controllers/studip_controller.php         | 24 ++++++-------
 tests/unit/_bootstrap.php                     | 13 +++++++
 .../unit/lib/classes/StudipControllerTest.php | 35 +++++++++++--------
 4 files changed, 66 insertions(+), 38 deletions(-)

diff --git a/app/controllers/search/module.php b/app/controllers/search/module.php
index b9f3d22f0b1..edede226fde 100644
--- a/app/controllers/search/module.php
+++ b/app/controllers/search/module.php
@@ -65,10 +65,6 @@ class Search_ModuleController extends MVVController
         return MVV::isVisibleSearch();
     }
 
-    public function after_filter($action, $args) {
-        parent::after_filter($action, $args);
-    }
-
     public function index_action()
     {
         $template = $this->get_template_factory()
@@ -134,8 +130,11 @@ class Search_ModuleController extends MVVController
 
         $sidebar = Sidebar::get();
 
-        $widget = new SelectWidget(_('Semesterauswahl'),
-            $this->url_for('',['sterm' => $this->sterm]), 'sem_select');
+        $widget = new SelectWidget(
+            _('Semesterauswahl'),
+            $this->indexURL(['sterm' => $this->sterm]),
+            'sem_select'
+        );
         $options = [];
         $semester = Semester::findAllVisible();
         unset($semester[0]);
@@ -160,8 +159,11 @@ class Search_ModuleController extends MVVController
                 || count($drill_down['studiengaenge']['objects'])
                 || count($drill_down['faecher']['objects'])) {
 
-            $widget = new SelectWidget(_('Studiengänge'),
-                $this->url_for('',['sterm' => $this->sterm, 'type' => 'Studiengang']), 'id');
+            $widget = new SelectWidget(
+                _('Studiengänge'),
+                $this->indexURL(['sterm' => $this->sterm, 'type' => 'Studiengang']),
+                'id'
+            );
             $options = [0 => 'Alle'];
             if(!empty($drill_down['studiengaenge']['objects'])){
                 foreach ($drill_down['studiengaenge']['objects'] as $studiengang) {
@@ -173,8 +175,11 @@ class Search_ModuleController extends MVVController
             $sidebar->addWidget($widget, 'studiengaenge_filter');
 
 
-            $widget = new SelectWidget(_('Fächer'),
-                $this->url_for('',['sterm' => $this->sterm, 'type' => 'Fach']), 'id');
+            $widget = new SelectWidget(
+                _('Fächer'),
+                $this->indexURL(['sterm' => $this->sterm, 'type' => 'Fach']),
+                'id'
+            );
             $options = [0 => 'Alle'];
             if(!empty($drill_down['faecher']['objects'])){
                 foreach ($drill_down['faecher']['objects'] as $fach) {
@@ -186,8 +191,11 @@ class Search_ModuleController extends MVVController
             $sidebar->addWidget($widget, 'faecher_filter');
 
 
-            $widget = new SelectWidget(_('Verantwortliche Einrichtungen'),
-                $this->url_for('',['sterm' => $this->sterm, 'type' => 'Fachbereich']), 'id');
+            $widget = new SelectWidget(
+                _('Verantwortliche Einrichtungen'),
+                $this->indexURL(['sterm' => $this->sterm, 'type' => 'Fachbereich']),
+                'id'
+            );
             $widget->class = 'institute-list';
             $options = [0 => 'Alle'];
             $widget->addElement(new SelectElement(0, _('Alle')), 'select-all');
diff --git a/app/controllers/studip_controller.php b/app/controllers/studip_controller.php
index 7c0991e4806..fd52034cd8f 100644
--- a/app/controllers/studip_controller.php
+++ b/app/controllers/studip_controller.php
@@ -252,6 +252,14 @@ abstract class StudipController extends Trails_Controller
     {
         $args = func_get_args();
 
+        // Try to create route if none given
+        if ($to === '') {
+            $to = $this->parent_controller
+                ? $this->parent_controller->current_action
+                : $this->current_action;
+            return $this->action_url($to);
+        }
+
         // Create url for a specific action
         // TODO: This seems odd. You kinda specify an absolute path
         //       to receive a relative url. Meh...
@@ -287,14 +295,6 @@ abstract class StudipController extends Trails_Controller
         //preserve fragment
         [$to, $fragment] = explode('#', $to);
 
-        // Try to create route if none given
-        if (!$to) {
-            $to  = '/';
-            $to .= $this->parent_controller
-                 ? $this->parent_controller->current_action
-                 : $this->current_action;
-        }
-
         $url = parent::url_for($to);
 
         if ($fragment) {
@@ -365,15 +365,15 @@ abstract class StudipController extends Trails_Controller
      */
     private function adjustToArguments(...$args): string
     {
-        if ($this->isURL($args[0]) && count($args) > 1) {
+        if (count($args) > 1 && $this->isURL($args[0])) {
             throw new InvalidArgumentException('Method may not be used with a URL and multiple parameters');
         }
 
-        if (count($args) > 1 || !$this->isURL($args[0])) {
-            return $this->url_for(...$args);
+        if (count($args) === 1 && $this->isURL($args[0])) {
+            return $args[0];
         }
 
-        return $args[0];
+        return $this->url_for(...$args);
     }
 
     /**
diff --git a/tests/unit/_bootstrap.php b/tests/unit/_bootstrap.php
index 7ea48011c90..068ba58cf0d 100644
--- a/tests/unit/_bootstrap.php
+++ b/tests/unit/_bootstrap.php
@@ -60,6 +60,19 @@ StudipAutoloader::addAutoloadPath('lib/plugins/engine');
 StudipAutoloader::addAutoloadPath('lib/plugins/core');
 StudipAutoloader::addAutoloadPath('lib/plugins/db');
 
+StudipAutoloader::addClassLookup('StudipController', 'app/controllers/studip_controller.php');
+$trails_classes = [
+    'Trails_Dispatcher', 'Trails_Response', 'Trails_Controller',
+    'Trails_Inflector', 'Trails_Flash',
+    'Trails_Exception', 'Trails_DoubleRenderError', 'Trails_MissingFile',
+    'Trails_RoutingError', 'Trails_UnknownAction', 'Trails_UnknownController',
+    'Trails_SessionRequiredException',
+];
+StudipAutoloader::addClassLookup(
+    $trails_classes,
+    'vendor/trails/trails.php'
+);
+
 // load config-variables
 StudipFileloader::load(
     'config_defaults.inc.php config_local.inc.php',
diff --git a/tests/unit/lib/classes/StudipControllerTest.php b/tests/unit/lib/classes/StudipControllerTest.php
index db2d6591aac..fae7628d269 100644
--- a/tests/unit/lib/classes/StudipControllerTest.php
+++ b/tests/unit/lib/classes/StudipControllerTest.php
@@ -12,14 +12,6 @@ final class StudipControllerTest extends Codeception\Test\Unit
 {
     private $old_uri;
 
-    public static function setUpBeforeClass(): void
-    {
-        parent::setUpBeforeClass();
-
-        require_once 'vendor/trails/trails-abridged.php';
-        require_once 'app/controllers/studip_controller.php';
-    }
-
     public function setUp(): void
     {
         parent::setUp();
@@ -47,9 +39,7 @@ final class StudipControllerTest extends Codeception\Test\Unit
     private function getController(): StudipController
     {
         $dispatcher = $this->getDispatcher();
-        return new class($dispatcher) extends StudipController {
-
-        };
+        return new StudipControllerTestController($dispatcher);
     }
 
     /**
@@ -76,7 +66,7 @@ final class StudipControllerTest extends Codeception\Test\Unit
     }
 
     /**
-     * @dataProvider UrlForProvider
+     * @dataProvider RedirectProvider
      * @covers StudipController::redirect
      */
     public function testRedirect(string $expected, ...$args): void
@@ -106,7 +96,7 @@ final class StudipControllerTest extends Codeception\Test\Unit
     }
 
     /**
-     * @dataProvider UrlForProvider
+     * @dataProvider RedirectProvider
      * @covers StudipController::relocate
      */
     public function testRelocate(string $expected, ...$args): void
@@ -136,7 +126,7 @@ final class StudipControllerTest extends Codeception\Test\Unit
     }
 
     /**
-     * @dataProvider UrlForProvider
+     * @dataProvider RedirectProvider
      * @covers StudipController::relocate
      * @backupGlobals enabled
      */
@@ -187,6 +177,7 @@ final class StudipControllerTest extends Codeception\Test\Unit
     public function UrlForProvider(): array
     {
         return [
+            '0-action'                 => ['dispatch.php/studip_controller_test/foo'],
             '1-action'                 => ['dispatch.php/foo', 'foo'],
             '1-action-and-parameter'   => ['dispatch.php/foo?bar=42', 'foo', ['bar' => 42]],
             '1-action-and-parameters'  => ['dispatch.php/foo?bar=42&baz=23', 'foo', ['bar' => 42, 'baz' => 23]],
@@ -197,6 +188,13 @@ final class StudipControllerTest extends Codeception\Test\Unit
         ];
     }
 
+    public function RedirectProvider(): array
+    {
+        $result = $this->UrlForProvider();
+        unset($result['0-action']);
+        return $result;
+    }
+
     public function absoluteUrlForProvider(): array
     {
         return [
@@ -214,3 +212,12 @@ final class StudipControllerTest extends Codeception\Test\Unit
     }
 }
 
+class StudipControllerTestController extends StudipController
+{
+    public function __construct($dispatcher)
+    {
+        parent::__construct($dispatcher);
+
+        $this->current_action = 'foo';
+    }
+}
-- 
GitLab