Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reintroduce App::$page as App::$urlBuildingIndexPage #2096

Merged
merged 21 commits into from
Sep 9, 2023
+33 −26
Merged

Reintroduce App::$page as App::$urlBuildingIndexPage #2096

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
67f0594
Improve App::url tests
abbadon1334 Aug 28, 2023
b2810b8
Skip $useRequestUrl=true due to REQUEST_URI
abbadon1334 Aug 28, 2023
3997714
Change REQUEST_URI to Psr7 methods
abbadon1334 Aug 28, 2023
4348ff1
Allow $useRequestUrl=true
abbadon1334 Aug 28, 2023
e4c3428
clean up
abbadon1334 Aug 28, 2023
7895ec9
Improve error message
abbadon1334 Aug 28, 2023
f1fefac
CS Fix
abbadon1334 Aug 28, 2023
308cc87
Merge branch 'develop' into add_tests_app_url
abbadon1334 Sep 3, 2023
23a5a6b
Add App::$urlBuildingPage to customize url building page
abbadon1334 Sep 4, 2023
f3f099f
Comment out App::url failing tests
abbadon1334 Sep 4, 2023
d5b4224
Merge branch 'develop' into add_tests_app_url
mvorisek Sep 9, 2023
47d6032
DEBUG REVERT ALL
mvorisek Sep 9, 2023
d4a0810
introduce App::$urlBuildingIndexPage prop
mvorisek Sep 9, 2023
89e3efa
drop protected App::getRequestUrl() method
mvorisek Sep 9, 2023
b490c1f
replace $_SERVER['REQUEST_URI'] with PSR7 request
mvorisek Sep 9, 2023
4d0e4c7
fix cs
mvorisek Sep 9, 2023
8b3bd58
original AppTest changes
mvorisek Sep 9, 2023
fccada4
no manual assert context dump
mvorisek Sep 9, 2023
b14f5fe
allow to create test App with defaults
mvorisek Sep 9, 2023
c3de25c
always use test App in tests
mvorisek Sep 9, 2023
ce40c14
Revert AppTest changes
mvorisek Sep 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,9 @@ public function stickyForget(string $name): void
public function url($page = [], $useRequestUrl = false, $extraRequestUrlArgs = []): string
{
if ($useRequestUrl) {
$page = $_SERVER['REQUEST_URI'];
$query = $this->getRequest()->getUri()->getQuery();
$page = $this->getRequest()->getUri()->getPath();
$page .= $query === '' ? '' : '?' . $query;
}

$pagePath = '';
Expand All @@ -719,11 +721,12 @@ public function url($page = [], $useRequestUrl = false, $extraRequestUrlArgs = [
}

$args = $extraRequestUrlArgs;
$queryParams = $this->getRequest()->getQueryParams();

// add sticky arguments
foreach ($this->stickyGetArguments as $k => $v) {
if ($v && isset($_GET[$k])) {
$args[$k] = $_GET[$k];
if ($v && isset($queryParams[$k])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be done in #2101

$args[$k] = $queryParams[$k];
} else {
unset($args[$k]);
}
Expand Down
144 changes: 144 additions & 0 deletions tests/AppTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
namespace Atk4\Ui\Tests;

use Atk4\Core\Phpunit\TestCase;
use Atk4\Ui\App;
use Atk4\Ui\Exception\LateOutputError;
use Atk4\Ui\HtmlTemplate;
use Nyholm\Psr7\Factory\Psr17Factory;

class AppTest extends TestCase
{
Expand Down Expand Up @@ -68,4 +70,146 @@ public function testUnexpectedOutputLateError(): void
ob_end_clean();
}
}

public function provideUrlBuildingCases(): iterable
{
return [
// simple cases
[[], false, [], '/', '/index.php', '/default.php8', '/'],
[[], false, [], '/test/', '/test/index.php', '/test/default.php8', '/test/'],
[[], true, [], '/', '/index.php', '/default.php8', '/'],
[[], true, [], '/test/', '/test/index.php', '/test/default.php8', '/test/'],

// simple cases with query args in request
[[], false, [], '/?test=atk4', '/index.php?test=atk4', '/default.php8?test=atk4', '/?test=atk4'],
[[], false, [], '/test/?test=atk4', '/test/index.php?test=atk4', '/test/default.php8?test=atk4', '/test/?test=atk4'],
[[], true, [], '/?test=atk4', '/index.php?test=atk4', '/default.php8?test=atk4', '/?test=atk4'],
[[], true, [], '/test/?test=atk4', '/test/index.php?test=atk4', '/test/default.php8?test=atk4', '/test/?test=atk4'],

// simple cases with extra query args in request
[[], false, ['extra_args' => 'atk4'], '/?test=atk4', '/index.php?extra_args=atk4&test=atk4', '/default.php8?extra_args=atk4&test=atk4', '/?extra_args=atk4&test=atk4'],
[[], false, ['extra_args' => 'atk4'], '/test/?test=atk4', '/test/index.php?extra_args=atk4&test=atk4', '/test/default.php8?extra_args=atk4&test=atk4', '/test/?extra_args=atk4&test=atk4'],
[[], true, ['extra_args' => 'atk4'], '/?test=atk4', '/index.php?extra_args=atk4&test=atk4', '/default.php8?extra_args=atk4&test=atk4', '/?extra_args=atk4&test=atk4'],
[[], true, ['extra_args' => 'atk4'], '/test/?test=atk4', '/test/index.php?extra_args=atk4&test=atk4', '/test/default.php8?extra_args=atk4&test=atk4', '/test/?extra_args=atk4&test=atk4'],

// simple cases with page as string
['test', false, [], '/', 'test.php', 'test.php8', 'test'],
['test/test/a', false, [], '/', 'test/test/a.php', 'test/test/a.php8', 'test/test/a'],
['test/index', false, [], '/', 'test/index.php', 'test/index.php8', 'test/index'],
['test/index', false, [], '/test/', 'test/index.php', 'test/index.php8', 'test/index'],
['test', true, [], '/', '/index.php', '/default.php8', '/'],
['test', true, [], '/request-url', '/request-url.php', '/request-url.php8', '/request-url'],
['test', true, [], '/request-url/', '/request-url/index.php', '/request-url/default.php8', '/request-url/'],
['test/index', true, [], '/test/', '/test/index.php', '/test/default.php8', '/test/'],
['test', false, [], '/', 'test.php', 'test.php8', 'test'],

// simple cases with page as array with 0 => string
[['test'], false, [], '/', 'test.php', 'test.php8', 'test'],
[['test/test/a'], false, [], '/', 'test/test/a.php', 'test/test/a.php8', 'test/test/a'],
[['test/index'], false, [], '/test/', 'test/index.php', 'test/index.php8', 'test/index'],
[['test'], true, [], '/', '/index.php', '/default.php8', '/'],
[['test'], true, [], '/request-url', '/request-url.php', '/request-url.php8', '/request-url'],
[['test'], true, [], '/request-url/', '/request-url/index.php', '/request-url/default.php8', '/request-url/'],
[['test/index'], true, [], '/test/', '/test/index.php', '/test/default.php8', '/test/'],
[['test'], false, [], '/', 'test.php', 'test.php8', 'test'],

// query args in page cases
[['test', 'extra_args' => 'atk4'], false, [], '/', 'test.php?extra_args=atk4', 'test.php8?extra_args=atk4', 'test?extra_args=atk4'],
[['test/test/a', 'extra_args' => 'atk4'], false, [], '/', 'test/test/a.php?extra_args=atk4', 'test/test/a.php8?extra_args=atk4', 'test/test/a?extra_args=atk4'],
[['test/index', 'extra_args' => 'atk4'], false, [], '/test/', 'test/index.php?extra_args=atk4', 'test/index.php8?extra_args=atk4', 'test/index?extra_args=atk4'],
[['test', 'extra_args' => 'atk4'], true, [], '/', '/index.php', '/default.php8', '/'],
[['test', 'extra_args' => 'atk4'], true, [], '/request-url', '/request-url.php', '/request-url.php8', '/request-url'],
[['test', 'extra_args' => 'atk4'], true, [], '/request-url/', '/request-url/index.php', '/request-url/default.php8', '/request-url/'],
[['test/index', 'extra_args' => 'atk4'], true, [], '/test/', '/test/index.php', '/test/default.php8', '/test/'],
[['test', 'extra_args' => 'atk4'], false, [], '/', 'test.php?extra_args=atk4', 'test.php8?extra_args=atk4', 'test?extra_args=atk4'],

// extra query args cases
[['test'], false, ['extra_args' => 'atk4'], '/', 'test.php?extra_args=atk4', 'test.php8?extra_args=atk4', 'test?extra_args=atk4'],
[['test/test/a'], false, ['extra_args' => 'atk4'], '/', 'test/test/a.php?extra_args=atk4', 'test/test/a.php8?extra_args=atk4', 'test/test/a?extra_args=atk4'],
[['test/index'], false, ['extra_args' => 'atk4'], '/test/', 'test/index.php?extra_args=atk4', 'test/index.php8?extra_args=atk4', 'test/index?extra_args=atk4'],
[['test'], true, ['extra_args' => 'atk4'], '/', '/index.php?extra_args=atk4', '/default.php8?extra_args=atk4', '/?extra_args=atk4'],
[['test'], true, ['extra_args' => 'atk4'], '/request-url', '/request-url.php?extra_args=atk4', '/request-url.php8?extra_args=atk4', '/request-url?extra_args=atk4'],
[['test'], true, ['extra_args' => 'atk4'], '/request-url/', '/request-url/index.php?extra_args=atk4', '/request-url/default.php8?extra_args=atk4', '/request-url/?extra_args=atk4'],
[['test/index'], true, ['extra_args' => 'atk4'], '/test/', '/test/index.php?extra_args=atk4', '/test/default.php8?extra_args=atk4', '/test/?extra_args=atk4'],
[['test'], false, ['extra_args' => 'atk4'], '/', 'test.php?extra_args=atk4', 'test.php8?extra_args=atk4', 'test?extra_args=atk4'],

// query args in page cases and query args in request cases and extra query args cases
[['test', 'page_args' => 'atk4'], false, ['extra_args' => 'atk4'], '/?extra_args=atk4&query_args=atk4&page_args=atk4', 'test.php?extra_args=atk4&query_args=atk4&page_args=atk4', 'test.php8?extra_args=atk4&query_args=atk4&page_args=atk4', 'test?extra_args=atk4&query_args=atk4&page_args=atk4'],
[['test/test/a', 'page_args' => 'atk4'], false, ['extra_args' => 'atk4'], '/?extra_args=atk4&query_args=atk4&page_args=atk4', 'test/test/a.php?extra_args=atk4&query_args=atk4&page_args=atk4', 'test/test/a.php8?extra_args=atk4&query_args=atk4&page_args=atk4', 'test/test/a?extra_args=atk4&query_args=atk4&page_args=atk4'],
[['test/index', 'page_args' => 'atk4'], false, ['extra_args' => 'atk4'], '/test/?extra_args=atk4&query_args=atk4&page_args=atk4', 'test/index.php?extra_args=atk4&query_args=atk4&page_args=atk4', 'test/index.php8?extra_args=atk4&query_args=atk4&page_args=atk4', 'test/index?extra_args=atk4&query_args=atk4&page_args=atk4'],
[['test', 'page_args' => 'atk4'], true, ['extra_args' => 'atk4'], '/', '/index.php?extra_args=atk4', '/default.php8?extra_args=atk4', '/?extra_args=atk4'],
[['test', 'page_args' => 'atk4'], true, ['extra_args' => 'atk4'], '/request-url', '/request-url.php?extra_args=atk4', '/request-url.php8?extra_args=atk4', '/request-url?extra_args=atk4'],
[['test', 'page_args' => 'atk4'], true, ['extra_args' => 'atk4'], '/request-url/', '/request-url/index.php?extra_args=atk4', '/request-url/default.php8?extra_args=atk4', '/request-url/?extra_args=atk4'],
[['test/index', 'page_args' => 'atk4'], true, ['extra_args' => 'atk4'], '/test/', '/test/index.php?extra_args=atk4', '/test/default.php8?extra_args=atk4', '/test/?extra_args=atk4'],
[['test', 'page_args' => 'atk4', 'check_unset_page' => false], false, ['extra_args' => 'atk4'], '/?extra_args=atk4&query_args=atk4&page_args=atk4', 'test.php?extra_args=atk4&query_args=atk4&page_args=atk4', 'test.php8?extra_args=atk4&query_args=atk4&page_args=atk4', 'test?extra_args=atk4&query_args=atk4&page_args=atk4'],
];
}

/**
* @dataProvider provideUrlBuildingCases
*
* @param string|array<0|string, string|int|false> $page URL as string or array with page name as first element and other GET arguments
* @param bool $useRequestUrl Simply return $_SERVER['REQUEST_URI'] if needed
* @param array<string, string> $extraRequestUrlArgs Additional URL arguments, deleting sticky can delete them
*/
public function testUrlBuilding($page, bool $useRequestUrl, array $extraRequestUrlArgs, string $requestUrl, string $exceptedStd, string $exceptedCustom, string $exceptedRouting): void
{
if ($useRequestUrl) {
$this->markTestSkipped('future improvements for App::url() with $useRequestUrl=true');
}

$factory = new Psr17Factory();
$request = $factory->createServerRequest('GET', 'http://127.0.0.1' . $requestUrl);
/*
$_SERVER = [
'REQUEST_METHOD' => 'GET',
'HTTP_HOST' => $request->getUri()->getHost(),
'REQUEST_URI' => $requestUrl,
'QUERY_STRING' => $request->getUri()->getQuery(),
];

parse_str($request->getUri()->getQuery(), $_GET);

foreach ($extraRequestUrlArgs as $key => $value) {
$_GET[$key] = $value;
}
*/
$stickyGetArguments = [
'__atk_json' => false,
'__atk_tab' => false,
];

foreach ($request->getQueryParams() as $key => $value) {
$stickyGetArguments[$key] = $value;
}

$app = new App([
mvorisek marked this conversation as resolved.
Show resolved Hide resolved
'request' => $request,
'stickyGetArguments' => $stickyGetArguments,
'catchExceptions' => false,
'alwaysRun' => false,
]);
self::assertSame($exceptedStd, $app->url($page, $useRequestUrl, $extraRequestUrlArgs), 'App::url test error case: standard');
self::assertSame($exceptedStd, $app->jsUrl($page, $useRequestUrl, $extraRequestUrlArgs), 'App::jsUrl test error case: standard');

$app = new App([
'stickyGetArguments' => $stickyGetArguments,
'urlBuildingPage' => 'default',
'urlBuildingExt' => '.php8',
'catchExceptions' => false,
'alwaysRun' => false,
]);
self::assertSame($exceptedCustom, $app->url($page, $useRequestUrl, $extraRequestUrlArgs), 'App::url test error case: custom page/ext');
self::assertSame($exceptedCustom, $app->jsUrl($page, $useRequestUrl, $extraRequestUrlArgs), 'App::jsUrl test error case: custom page/ext');

$app = new App([
'stickyGetArguments' => $stickyGetArguments,
'urlBuildingPage' => '',
'urlBuildingExt' => '',
'catchExceptions' => false,
'alwaysRun' => false,
]);
self::assertSame($exceptedRouting, $app->url($page, $useRequestUrl, $extraRequestUrlArgs), 'App::url test error case: routing');
self::assertSame($exceptedRouting, $app->jsUrl($page, $useRequestUrl, $extraRequestUrlArgs), 'App::jsUrl test error case: routing');
}
}