From 7073129bbf5bb71d7547b88611a3de383dcdaea0 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Fri, 18 Oct 2024 23:21:32 +0100 Subject: [PATCH 1/3] Refactor getForeignLimit Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 5 +++++ src/BrowseForeigners.php | 11 ++--------- src/Controllers/BrowseForeignersController.php | 5 +++-- tests/unit/BrowseForeignersTest.php | 15 +++++++-------- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 34442e983d..72138c5c73 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1391,6 +1391,11 @@ parameters: count: 1 path: src/Console.php + - + message: "#^Cannot cast mixed to int\\.$#" + count: 1 + path: src/Controllers/BrowseForeignersController.php + - message: """ #^Call to deprecated method getInstance\\(\\) of class PhpMyAdmin\\\\Config\\: diff --git a/src/BrowseForeigners.php b/src/BrowseForeigners.php index cd27e7708b..6224252125 100644 --- a/src/BrowseForeigners.php +++ b/src/BrowseForeigners.php @@ -283,19 +283,12 @@ class BrowseForeigners return ''; } - /** - * Function to get foreign limit - * - * @param string|null $foreignShowAll foreign navigation - */ - public function getForeignLimit(string|null $foreignShowAll): string|null + public function getForeignLimit(string|null $foreignShowAll, int $pos): string { if ($foreignShowAll === __('Show all')) { - return null; + return ''; } - isset($_POST['pos']) ? $pos = $_POST['pos'] : $pos = 0; - return 'LIMIT ' . $pos . ', ' . $this->settings->maxRows . ' '; } } diff --git a/src/Controllers/BrowseForeignersController.php b/src/Controllers/BrowseForeignersController.php index 98ba603bfb..184e7d237d 100644 --- a/src/Controllers/BrowseForeignersController.php +++ b/src/Controllers/BrowseForeignersController.php @@ -48,13 +48,14 @@ final class BrowseForeignersController implements InvocableController $header->disableMenuAndConsole(); $header->setBodyId('body_browse_foreigners'); - $foreignLimit = $this->browseForeigners->getForeignLimit($foreignShowAll); + $pos = (int) $request->getParsedBodyParam('pos'); + $foreignLimit = $this->browseForeigners->getForeignLimit($foreignShowAll, $pos); $foreignData = $this->relation->getForeignData( $this->relation->getForeigners($database, $table), $field, true, $foreignFilter, - $foreignLimit ?? '', + $foreignLimit, true, ); diff --git a/tests/unit/BrowseForeignersTest.php b/tests/unit/BrowseForeignersTest.php index 9d396b59b3..59876c5e7c 100644 --- a/tests/unit/BrowseForeignersTest.php +++ b/tests/unit/BrowseForeignersTest.php @@ -31,20 +31,19 @@ class BrowseForeignersTest extends AbstractTestCase */ public function testGetForeignLimit(): void { - self::assertNull( - $this->browseForeigners->getForeignLimit('Show all'), + self::assertSame( + '', + $this->browseForeigners->getForeignLimit('Show all', 0), ); self::assertSame( 'LIMIT 0, 25 ', - $this->browseForeigners->getForeignLimit(null), + $this->browseForeigners->getForeignLimit(null, 0), ); - $_POST['pos'] = 10; - self::assertSame( 'LIMIT 10, 25 ', - $this->browseForeigners->getForeignLimit(null), + $this->browseForeigners->getForeignLimit(null, 10), ); $config = new Config(); @@ -53,12 +52,12 @@ class BrowseForeignersTest extends AbstractTestCase self::assertSame( 'LIMIT 10, 50 ', - $browseForeigners->getForeignLimit(null), + $browseForeigners->getForeignLimit(null, 10), ); self::assertSame( 'LIMIT 10, 50 ', - $browseForeigners->getForeignLimit('xyz'), + $browseForeigners->getForeignLimit('xyz', 10), ); } From 4369046c58ecdc0dbe61f7437809cfa9df0ee554 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Fri, 18 Oct 2024 23:30:38 +0100 Subject: [PATCH 2/3] Refactor getHtmlForGotoPage Signed-off-by: Kamil Tekiela --- psalm-baseline.xml | 4 ---- src/BrowseForeigners.php | 6 +++--- src/Controllers/BrowseForeignersController.php | 1 + tests/unit/BrowseForeignersTest.php | 9 +++++---- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 62ecb5480e..370f73d2cf 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -93,10 +93,6 @@ - - - settings->maxRows]]> - diff --git a/src/BrowseForeigners.php b/src/BrowseForeigners.php index 6224252125..8616dfff0e 100644 --- a/src/BrowseForeigners.php +++ b/src/BrowseForeigners.php @@ -143,8 +143,9 @@ class BrowseForeigners ForeignData $foreignData, string|null $fieldKey, string $currentValue, + int $pos, ): string { - $gotoPage = $this->getHtmlForGotoPage($foreignData); + $gotoPage = $this->getHtmlForGotoPage($foreignData, $pos); $foreignShowAll = ''; if ( $foreignData->dispRow !== null && @@ -255,9 +256,8 @@ class BrowseForeigners /** * Function to get html for the goto page option */ - private function getHtmlForGotoPage(ForeignData $foreignData): string + private function getHtmlForGotoPage(ForeignData $foreignData, int $pos): string { - isset($_POST['pos']) ? $pos = $_POST['pos'] : $pos = 0; if ($foreignData->dispRow === null) { return ''; } diff --git a/src/Controllers/BrowseForeignersController.php b/src/Controllers/BrowseForeignersController.php index 184e7d237d..93a54ba5f1 100644 --- a/src/Controllers/BrowseForeignersController.php +++ b/src/Controllers/BrowseForeignersController.php @@ -66,6 +66,7 @@ final class BrowseForeignersController implements InvocableController $foreignData, $fieldKey, $data, + $pos, )); return $this->response->response(); diff --git a/tests/unit/BrowseForeignersTest.php b/tests/unit/BrowseForeignersTest.php index 59876c5e7c..2f0069cd79 100644 --- a/tests/unit/BrowseForeignersTest.php +++ b/tests/unit/BrowseForeignersTest.php @@ -73,11 +73,10 @@ class BrowseForeignersTest extends AbstractTestCase $this->browseForeigners, BrowseForeigners::class, 'getHtmlForGotoPage', - [$foreignData], + [$foreignData, 0], ), ); - $_POST['pos'] = 15; $foreignData = new ForeignData(false, 5, '', [], ''); self::assertSame( @@ -86,7 +85,7 @@ class BrowseForeignersTest extends AbstractTestCase $this->browseForeigners, BrowseForeigners::class, 'getHtmlForGotoPage', - [$foreignData], + [$foreignData, 15], ), ); @@ -95,7 +94,7 @@ class BrowseForeignersTest extends AbstractTestCase $this->browseForeigners, BrowseForeigners::class, 'getHtmlForGotoPage', - [$foreignData], + [$foreignData, 15], ); self::assertStringStartsWith('Page number:', $result); @@ -161,6 +160,7 @@ class BrowseForeignersTest extends AbstractTestCase $foreignData, $fieldkey, $currentValue, + 0, ); self::assertStringContainsString( @@ -205,6 +205,7 @@ class BrowseForeignersTest extends AbstractTestCase $foreignData, $fieldkey, $currentValue, + 0, ); self::assertStringContainsString( From c2cbd15c6da72114b4162c9cb9c10b40ee549fc1 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Fri, 18 Oct 2024 23:39:39 +0100 Subject: [PATCH 3/3] Refactor getHtmlForRelationalFieldSelection Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 10 ------- psalm-baseline.xml | 6 ----- src/BrowseForeigners.php | 27 +++++++++---------- .../BrowseForeignersController.php | 4 +++ tests/unit/BrowseForeignersTest.php | 8 ++++-- 5 files changed, 23 insertions(+), 32 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 72138c5c73..6e74927fc8 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -148,21 +148,11 @@ parameters: count: 1 path: src/Bookmarks/BookmarkRepository.php - - - message: "#^Cannot cast mixed to string\\.$#" - count: 1 - path: src/BrowseForeigners.php - - message: "#^Parameter \\#1 \\$description of method PhpMyAdmin\\\\BrowseForeigners\\:\\:getDescriptionAndTitle\\(\\) expects string, mixed given\\.$#" count: 2 path: src/BrowseForeigners.php - - - message: "#^Parameter \\#1 \\$string of function htmlspecialchars expects string, mixed given\\.$#" - count: 1 - path: src/BrowseForeigners.php - - message: "#^Parameter \\#1 \\$state of static method PhpMyAdmin\\\\Charsets\\\\Charset\\:\\:fromServer\\(\\) expects array\\{Charset\\?\\: string, Description\\?\\: string, Default collation\\?\\: string, Maxlen\\?\\: string\\}, array\\ given\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 370f73d2cf..ba7dbe384f 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -87,12 +87,6 @@ - - - - - - diff --git a/src/BrowseForeigners.php b/src/BrowseForeigners.php index 8616dfff0e..0b49aa8c1d 100644 --- a/src/BrowseForeigners.php +++ b/src/BrowseForeigners.php @@ -130,20 +130,22 @@ class BrowseForeigners /** * Function to get html for relational field selection * - * @param string $db current database - * @param string $table current table - * @param string $field field - * @param string|null $fieldKey field key - * @param string $currentValue current columns's value + * @param string $db current database + * @param string $table current table + * @param string $field field + * @param string $fieldKey field key + * @param string $currentValue current columns's value */ public function getHtmlForRelationalFieldSelection( string $db, string $table, string $field, ForeignData $foreignData, - string|null $fieldKey, + string $fieldKey, string $currentValue, int $pos, + string $foreignFilter, + string|null $rownumber, ): string { $gotoPage = $this->getHtmlForGotoPage($foreignData, $pos); $foreignShowAll = ''; @@ -161,16 +163,13 @@ class BrowseForeigners . Url::getHiddenInputs($db, $table) . "\n" . '' . "\n" . '' . "\n"; + . htmlspecialchars($fieldKey) . '">' . "\n"; - if (isset($_POST['rownumber'])) { - $output .= ''; + if ($rownumber !== null) { + $output .= ''; } - $filterValue = isset($_POST['foreign_filter']) - ? htmlspecialchars($_POST['foreign_filter']) - : ''; + $filterValue = htmlspecialchars($foreignFilter); $output .= '
' . '
' . "\n" . '
the new description and title + * @return array{string, string} the new description and title */ private function getDescriptionAndTitle(string $description): array { diff --git a/src/Controllers/BrowseForeignersController.php b/src/Controllers/BrowseForeignersController.php index 93a54ba5f1..b74462536d 100644 --- a/src/Controllers/BrowseForeignersController.php +++ b/src/Controllers/BrowseForeignersController.php @@ -38,6 +38,8 @@ final class BrowseForeignersController implements InvocableController $foreignShowAll = $request->getParsedBodyParam('foreign_showAll'); /** @var string $foreignFilter */ $foreignFilter = $request->getParsedBodyParam('foreign_filter', ''); + /** @var string|null $rownumber */ + $rownumber = $request->getParsedBodyParam('rownumber'); if (! isset($database, $table, $field)) { return $this->response->response(); @@ -67,6 +69,8 @@ final class BrowseForeignersController implements InvocableController $fieldKey, $data, $pos, + $foreignFilter, + $rownumber, )); return $this->response->response(); diff --git a/tests/unit/BrowseForeignersTest.php b/tests/unit/BrowseForeignersTest.php index 2f0069cd79..1c80481f1b 100644 --- a/tests/unit/BrowseForeignersTest.php +++ b/tests/unit/BrowseForeignersTest.php @@ -151,8 +151,8 @@ class BrowseForeignersTest extends AbstractTestCase $foreignData = new ForeignData(false, 0, '', null, ''); $fieldkey = 'bar'; $currentValue = ''; - $_POST['rownumber'] = 1; - $_POST['foreign_filter'] = '5'; + $rownumber = '1'; + $foreignFilter = '5'; $result = $this->browseForeigners->getHtmlForRelationalFieldSelection( $db, $table, @@ -161,6 +161,8 @@ class BrowseForeignersTest extends AbstractTestCase $fieldkey, $currentValue, 0, + $foreignFilter, + $rownumber, ); self::assertStringContainsString( @@ -206,6 +208,8 @@ class BrowseForeignersTest extends AbstractTestCase $fieldkey, $currentValue, 0, + $foreignFilter, + $rownumber, ); self::assertStringContainsString(