Skip to content

Commit

Permalink
Fix #35 (Dots in page URL break calendar navigation)
Browse files Browse the repository at this point in the history
The "page URLs" of CMSimple_XH are very special; they are usually not
processed by the usual PHP features, and as such are not subject to the
PHP URL parameter name replacements (spaces and periods are replaced by
underscores).  Thus, we must not process them with `http_build_query()`,
but rather refer to them via `$su`.

To avoid doing this in `CalendarController::modifyUrl()`, thereby
coupling the unrelated URL handling more to the controller, we extract
a simple `Url` class which hopefully does the job properly.

A nice side-effect of this new URL handling is that we automatically
produce more CMSimple_XH-like URLs for the starting page, whose "page
URL" is empty.  This is why we had to update a couple of tests.
  • Loading branch information
cmb69 committed Feb 9, 2023
1 parent 432e823 commit ec895a9
Show file tree
Hide file tree
Showing 21 changed files with 200 additions and 65 deletions.
19 changes: 5 additions & 14 deletions classes/CalendarController.php
Expand Up @@ -41,8 +41,8 @@ abstract class CalendarController
/** @var string */
protected $mode;

/** @var string */
private $scriptName;
/** @var Url */
protected $url;

/** @var string */
protected $pluginFolder;
Expand Down Expand Up @@ -70,7 +70,7 @@ abstract class CalendarController
* @param array<string,string> $lang
*/
public function __construct(
string $scriptName,
Url $url,
string $pluginFolder,
?CsrfProtector $csrfProtector,
array $config,
Expand All @@ -81,7 +81,7 @@ public function __construct(
bool $isAdmin,
string $type
) {
$this->scriptName = $scriptName;
$this->url = $url;
$this->pluginFolder = $pluginFolder;
$this->csrfProtector = $csrfProtector;
$this->config = $config;
Expand Down Expand Up @@ -184,7 +184,7 @@ protected function renderModeLinkView(): HtmlString
{
return new HtmlString($this->view->render('mode-link', [
'mode' => $mode = $this->mode === 'calendar' ? 'list' : 'calendar',
'url' => $this->modifyUrl(array('ocal_action' => $mode)),
'url' => $this->url->replace(['ocal_action' => $mode]),
]));
}

Expand All @@ -201,13 +201,4 @@ protected function renderToolbarView(): HtmlString
'states' => range(0, $this->config['state_max']),
]));
}

/** @param array<string,string> $newParams */
protected function modifyUrl(array $newParams): string
{
parse_str($_SERVER['QUERY_STRING'], $params);
$params = array_merge($params, $newParams);
$query = str_replace('=&', '&', http_build_query($params));
return "{$this->scriptName}?$query";
}
}
8 changes: 4 additions & 4 deletions classes/DailyCalendarController.php
Expand Up @@ -38,7 +38,7 @@ class DailyCalendarController extends CalendarController
* @param array<string,string> $lang
*/
public function __construct(
string $scriptName,
Url $url,
string $pluginFolder,
?CsrfProtector $csrfProtector,
array $config,
Expand All @@ -49,7 +49,7 @@ public function __construct(
bool $isAdmin
) {
parent::__construct(
$scriptName,
$url,
$pluginFolder,
$csrfProtector,
$config,
Expand Down Expand Up @@ -208,11 +208,11 @@ private function getPaginationItems(): array
);
$paginationItems = $pagination->getItems();
foreach ($paginationItems as $item) {
$item->url = $this->modifyUrl(array(
$item->url = $this->url->replace([
'ocal_year' => $item->year,
'ocal_month' => $item->monthOrWeek,
'ocal_action' => $this->mode
));
]);
}
return $paginationItems;
}
Expand Down
8 changes: 4 additions & 4 deletions classes/Dic.php
Expand Up @@ -39,10 +39,10 @@ public static function makeDefaultAdminController(): DefaultAdminController

public static function makeDailyCalendarController(): DailyCalendarController
{
global $sn, $pth, $plugin_cf, $plugin_tx, $_XH_csrfProtection;
global $sn, $su, $pth, $plugin_cf, $plugin_tx, $_XH_csrfProtection;

return new DailyCalendarController(
$sn,
new Url($sn, $su, ($su !== "" ? array_slice($_GET, 1) : $_GET)),
"{$pth['folder']['plugins']}ocal/",
$_XH_csrfProtection,
$plugin_cf['ocal'],
Expand All @@ -56,10 +56,10 @@ public static function makeDailyCalendarController(): DailyCalendarController

public static function makeHourlyCalendarController(): HourlyCalendarController
{
global $sn, $pth, $plugin_cf, $plugin_tx, $_XH_csrfProtection;
global $sn, $su, $pth, $plugin_cf, $plugin_tx, $_XH_csrfProtection;

return new HourlyCalendarController(
$sn,
new Url($sn, $su, ($su !== "" ? array_slice($_GET, 1) : $_GET)),
"{$pth['folder']['plugins']}ocal/",
$_XH_csrfProtection,
$plugin_cf['ocal'],
Expand Down
8 changes: 4 additions & 4 deletions classes/HourlyCalendarController.php
Expand Up @@ -38,7 +38,7 @@ class HourlyCalendarController extends CalendarController
* @param array<string,string> $lang
*/
public function __construct(
string $scriptName,
Url $url,
string $pluginFolder,
?CsrfProtector $csrfProtector,
array $config,
Expand All @@ -49,7 +49,7 @@ public function __construct(
bool $isAdmin
) {
parent::__construct(
$scriptName,
$url,
$pluginFolder,
$csrfProtector,
$config,
Expand Down Expand Up @@ -204,11 +204,11 @@ private function getPaginationItems(int $weekCount): array
);
$items = $pagination->getItems($weekCount);
foreach ($items as $item) {
$item->url = $this->modifyUrl(array(
$item->url = $this->url->replace([
'ocal_year' => $item->year,
'ocal_week' => $item->monthOrWeek,
'ocal_action' => $this->mode
));
]);
}
return $items;
}
Expand Down
56 changes: 56 additions & 0 deletions classes/Url.php
@@ -0,0 +1,56 @@
<?php

/**
* Copyright 2023 Christoph M. Becker
*
* This file is part of Ocal_XH.
*
* Ocal_XH is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Ocal_XH is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Ocal_XH. If not, see <http://www.gnu.org/licenses/>.
*/

namespace Ocal;

class Url
{
/** @var string */
private $base;

/** @var string */
private $page;

/** @var array<string,string> */
private $params;

/** @param array<string,string> $params */
public function __construct(string $base, string $page, array $params)
{
$this->base = $base;
$this->page = $page;
$this->params = $params;
}

/** @param array<string,string> $params */
public function replace(array $params): self
{
return new Url($this->base, $this->page, array_replace($this->params, $params));
}

public function __toString(): string
{
$rest = http_build_query($this->params, "", "&", PHP_QUERY_RFC3986);
$rest = preg_replace('/=(?=&|$)/', '', $rest);
$query = $this->page . ($rest !== "" ? "&{$rest}": "");
return $this->base . ($query !== "" ? "?{$query}" : "");
}
}
2 changes: 1 addition & 1 deletion tests/DailyCalendarControllerTest.php
Expand Up @@ -55,7 +55,7 @@ public function setUp(): void
$this->listService = $this->createStub(ListService::class);
$this->db = $this->createStub(Db::class);
$this->sut = new DailyCalendarController(
"/",
new Url("/", "", []),
"./",
$this->csrfProtector,
$config,
Expand Down
2 changes: 1 addition & 1 deletion tests/DailyCalendarControllerVisitorTest.php
Expand Up @@ -39,7 +39,7 @@ public function testContructorDoesNotCrash(): void
$listService = $this->createStub(ListService::class);
$db = $this->createStub(Db::class);
new DailyCalendarController(
"/",
new Url("/", "", []),
"./",
null,
$config,
Expand Down
2 changes: 1 addition & 1 deletion tests/HourlyCalendarControllerTest.php
Expand Up @@ -55,7 +55,7 @@ public function setUp(): void
$this->listService = $this->createStub(ListService::class);
$this->db = $this->createStub(Db::class);
$this->sut = new HourlyCalendarController(
"/",
new Url("/", "", []),
"./",
$this->csrfProtector,
$config,
Expand Down
86 changes: 86 additions & 0 deletions tests/UrlTest.php
@@ -0,0 +1,86 @@
<?php

/**
* Copyright 2023 Christoph M. Becker
*
* This file is part of Ocal_XH.
*
* Ocal_XH is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Ocal_XH is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Ocal_XH. If not, see <http://www.gnu.org/licenses/>.
*/

namespace Ocal;

use PHPUnit\Framework\TestCase;

class UrlTest extends TestCase
{
/** @dataProvider dataForUrlHasDesiredStringRepresentation */
public function testUrlHasDesiredStringRepresentation(Url $url, string $expected): void
{
$actual = (string) $url;
$this->assertEquals($expected, $actual);
}

public function dataForUrlHasDesiredStringRepresentation(): array
{
return [
'no_page' => [new Url("/", "", []), "/"],
'with_page' => [new Url("/", "Page", []), "/?Page"],
'with_params' => [new Url("/", "Page", ["foo" => "bar", "qux" => "bar"]), "/?Page&foo=bar&qux=bar"],
'params_but_no_page' => [new Url("/", "", ["foo" => "bar"]), "/?&foo=bar"],
'gh-35' => [new Url("/", "Ocal-1.0", []), "/?Ocal-1.0"],
];
}

/**
* @param array<string,string> $params
* @dataProvider dataForUrlCanBeModified
*/
public function testUrlCanBeModified(Url $url, array $params, string $expected): void
{
$actual = (string) $url->replace($params);
$this->assertEquals($expected, $actual);
}

public function dataForUrlCanBeModified(): array
{
return [
[
new Url("/", "Page", []),
[],
"/?Page",
],
[
new Url("/", "", []),
["foo" => "bar"],
"/?&foo=bar",
],
[
new Url("/", "Page", ["foo" => "bar"]),
[],
"/?Page&foo=bar",
],
[
new Url("/", "Page", ["foo" => "bar"]),
["baz" => "qux"],
"/?Page&foo=bar&baz=qux",
],
[
new Url("/", "Page", ["foo" => "bar"]),
["foo" => "baz"],
"/?Page&foo=baz",
],
];
}
}
Expand Up @@ -2,7 +2,7 @@
<div class="ocal_calendars" data-name="test-daily">
<!-- mode-link -->
<p class="ocal_mode">
<a class="ocal_button" href="/?ocal_action=list">List view</a>
<a class="ocal_button" href="/?&amp;ocal_action=list">List view</a>
</p>
<input type="hidden" name="xh_csrf_token" value="dcfff515ebf5bd421d5a0777afc6358b"> <!-- toolbar -->
<div class="ocal_toolbar">
Expand Down Expand Up @@ -90,8 +90,8 @@
</table>
<!-- pagination -->
<p class="ocal_pagination">
<a class="ocal_button" href="/?ocal_year=2023&amp;ocal_month=7&amp;ocal_action=calendar">Today</a>
<a class="ocal_button" href="/?ocal_year=2023&amp;ocal_month=8&amp;ocal_action=calendar">Next Month</a>
<a class="ocal_button" href="/?ocal_year=2024&amp;ocal_month=7&amp;ocal_action=calendar">Next Year</a>
<a class="ocal_button" href="/?&amp;ocal_year=2023&amp;ocal_month=7&amp;ocal_action=calendar">Today</a>
<a class="ocal_button" href="/?&amp;ocal_year=2023&amp;ocal_month=8&amp;ocal_action=calendar">Next Month</a>
<a class="ocal_button" href="/?&amp;ocal_year=2024&amp;ocal_month=7&amp;ocal_action=calendar">Next Year</a>
</p>
</div>
Expand Up @@ -2,7 +2,7 @@
<div class="ocal_calendars" data-name="test-daily">
<!-- mode-link -->
<p class="ocal_mode">
<a class="ocal_button" href="/?ocal_action=list">List view</a>
<a class="ocal_button" href="/?&amp;ocal_action=list">List view</a>
</p>
<input type="hidden" name="xh_csrf_token" value="dcfff515ebf5bd421d5a0777afc6358b"> <!-- toolbar -->
<div class="ocal_toolbar">
Expand Down Expand Up @@ -90,8 +90,8 @@
</table>
<!-- pagination -->
<p class="ocal_pagination">
<a class="ocal_button" href="/?ocal_year=2023&amp;ocal_month=7&amp;ocal_action=calendar">Today</a>
<a class="ocal_button" href="/?ocal_year=2023&amp;ocal_month=8&amp;ocal_action=calendar">Next Month</a>
<a class="ocal_button" href="/?ocal_year=2024&amp;ocal_month=7&amp;ocal_action=calendar">Next Year</a>
<a class="ocal_button" href="/?&amp;ocal_year=2023&amp;ocal_month=7&amp;ocal_action=calendar">Today</a>
<a class="ocal_button" href="/?&amp;ocal_year=2023&amp;ocal_month=8&amp;ocal_action=calendar">Next Month</a>
<a class="ocal_button" href="/?&amp;ocal_year=2024&amp;ocal_month=7&amp;ocal_action=calendar">Next Year</a>
</p>
</div>
Expand Up @@ -2,7 +2,7 @@
<div data-name="test-daily">
<!-- mode-link -->
<p class="ocal_mode">
<a class="ocal_button" href="/?ocal_action=calendar">Calendar view</a>
<a class="ocal_button" href="/?&amp;ocal_action=calendar">Calendar view</a>
</p>
<!-- statusbar -->
<div class="ocal_loaderbar"><img src="./images/ajax-loader-bar.gif" alt="loading"></div>
Expand All @@ -14,8 +14,8 @@
</dl>
<!-- pagination -->
<p class="ocal_pagination">
<a class="ocal_button" href="/?ocal_year=2023&amp;ocal_month=7&amp;ocal_action=list">Today</a>
<a class="ocal_button" href="/?ocal_year=2023&amp;ocal_month=8&amp;ocal_action=list">Next Month</a>
<a class="ocal_button" href="/?ocal_year=2024&amp;ocal_month=7&amp;ocal_action=list">Next Year</a>
<a class="ocal_button" href="/?&amp;ocal_year=2023&amp;ocal_month=7&amp;ocal_action=list">Today</a>
<a class="ocal_button" href="/?&amp;ocal_year=2023&amp;ocal_month=8&amp;ocal_action=list">Next Month</a>
<a class="ocal_button" href="/?&amp;ocal_year=2024&amp;ocal_month=7&amp;ocal_action=list">Next Year</a>
</p>
</div>
Expand Up @@ -2,7 +2,7 @@
<div data-name="test-daily">
<!-- mode-link -->
<p class="ocal_mode">
<a class="ocal_button" href="/?ocal_action=calendar">Calendar view</a>
<a class="ocal_button" href="/?&amp;ocal_action=calendar">Calendar view</a>
</p>
<!-- statusbar -->
<div class="ocal_loaderbar"><img src="./images/ajax-loader-bar.gif" alt="loading"></div>
Expand All @@ -19,8 +19,8 @@
</dl>
<!-- pagination -->
<p class="ocal_pagination">
<a class="ocal_button" href="/?ocal_year=2023&amp;ocal_month=7&amp;ocal_action=list">Today</a>
<a class="ocal_button" href="/?ocal_year=2023&amp;ocal_month=8&amp;ocal_action=list">Next Month</a>
<a class="ocal_button" href="/?ocal_year=2024&amp;ocal_month=7&amp;ocal_action=list">Next Year</a>
<a class="ocal_button" href="/?&amp;ocal_year=2023&amp;ocal_month=7&amp;ocal_action=list">Today</a>
<a class="ocal_button" href="/?&amp;ocal_year=2023&amp;ocal_month=8&amp;ocal_action=list">Next Month</a>
<a class="ocal_button" href="/?&amp;ocal_year=2024&amp;ocal_month=7&amp;ocal_action=list">Next Year</a>
</p>
</div>

0 comments on commit ec895a9

Please sign in to comment.