From 8d6ef4769954dae3f084cc0281701a6818de5b29 Mon Sep 17 00:00:00 2001 From: Jon Robson Date: Thu, 5 Jan 2023 12:02:40 -0800 Subject: [PATCH] Refactor SkinVector22::getTemplateData and add test coverage Bug: T318434 Change-Id: If06e6618adaae614ea7fe9b803b1f4fd80170d59 --- includes/SkinVector22.php | 136 ++++++++++++------------ tests/phpunit/unit/SkinVector22Test.php | 100 +++++++++++++++++ 2 files changed, 168 insertions(+), 68 deletions(-) create mode 100644 tests/phpunit/unit/SkinVector22Test.php diff --git a/includes/SkinVector22.php b/includes/SkinVector22.php index 7c0a53463..5a76566ca 100644 --- a/includes/SkinVector22.php +++ b/includes/SkinVector22.php @@ -131,6 +131,30 @@ class SkinVector22 extends SkinVector { ); } + /** + * Pulls the page tools menu out of $sidebar into $pageToolsMenu + * + * @param array &$sidebar + * @param array &$pageToolsMenu + */ + private static function extractPageToolsFromSidebar( &$sidebar, &$pageToolsMenu ) { + $restPortlets = $sidebar[ 'array-portlets-rest' ] ?? []; + $toolboxMenuIndex = array_search( + VectorComponentPageTools::TOOLBOX_ID, + array_column( + $restPortlets, + 'id' + ) + ); + + if ( $toolboxMenuIndex !== false ) { + // Splice removes the toolbox menu from the $restPortlets array + // and current returns the first value of array_splice, i.e. the $toolbox menu data. + $pageToolsMenu = array_splice( $restPortlets, $toolboxMenuIndex ); + $sidebar['array-portlets-rest'] = $restPortlets; + } + } + /** * @return array */ @@ -159,82 +183,58 @@ class SkinVector22 extends SkinVector { ); } - $toc = new VectorComponentTableOfContents( - $parentData['data-toc'], - $this->getContext(), - $this->getConfig() - ); - $config = $this->getConfig(); - $searchBox = new VectorComponentSearchBox( - $parentData['data-search-box'], - true, - // is primary mode of search - true, - 'searchform', - true, - $config, - Constants::SEARCH_BOX_INPUT_LOCATION_MOVED, - $this->getContext() - ); $isPageToolsEnabled = $featureManager->isFeatureEnabled( Constants::FEATURE_PAGE_TOOLS ); $sidebar = $parentData[ 'data-portlets-sidebar' ]; - $pageToolsMenus = []; - $restPortlets = $parentData[ 'data-portlets-sidebar' ][ 'array-portlets-rest' ]; + $pageToolsMenu = []; if ( $isPageToolsEnabled ) { - $toolboxMenuIndex = array_search( - VectorComponentPageTools::TOOLBOX_ID, - array_column( - $restPortlets, - 'id' - ) - ); - - if ( $toolboxMenuIndex !== false ) { - // Splice removes the toolbox menu from the $restPortlets array - // and current returns the first value of array_splice, i.e. the $toolbox menu data. - $pageToolsMenus = array_splice( $restPortlets, $toolboxMenuIndex ); - $parentData[ 'data-portlets-sidebar' ]['array-portlets-rest'] = $restPortlets; - $sidebar = $parentData[ 'data-portlets-sidebar' ]; - } + self::extractPageToolsFromSidebar( $sidebar, $pageToolsMenu ); } - $mainMenu = new VectorComponentMainMenu( - $sidebar, - $this->shouldLanguageAlertBeInSidebar(), - $parentData['data-portlets']['data-languages'] ?? [], - $this->getContext(), - $this->getUser(), - VectorServices::getFeatureManager(), - $this, - ); - $mainMenuDropdown = new VectorComponentDropdown( - $mainMenu::ID . '-dropdown', - $this->msg( $mainMenu::ID . '-label' )->text(), - $mainMenu::ID . '-dropdown' . ' mw-ui-icon-flush-left mw-ui-icon-flush-right', - 'menu' - ); - - $pageTools = new VectorComponentPageTools( - array_merge( [ $parentData['data-portlets']['data-actions'] ?? [] ], $pageToolsMenus ), - $this->getContext(), - $this->getUser(), - $featureManager - ); - $pageToolsDropdown = new VectorComponentDropdown( - $pageTools::ID . '-dropdown', - $this->msg( 'toolbox' )->text(), - $pageTools::ID . '-dropdown', - ); - $components = [ - 'data-toc' => $toc, - 'data-search-box' => $searchBox, - 'data-main-menu' => $mainMenu, - 'data-main-menu-dropdown' => $mainMenuDropdown, - 'data-page-tools' => $isPageToolsEnabled ? $pageTools : null, - 'data-page-tools-dropdown' => $isPageToolsEnabled ? $pageToolsDropdown : null, + 'data-toc' => new VectorComponentTableOfContents( + $parentData['data-toc'], + $this->getContext(), + $this->getConfig() + ), + 'data-search-box' => new VectorComponentSearchBox( + $parentData['data-search-box'], + true, + // is primary mode of search + true, + 'searchform', + true, + $config, + Constants::SEARCH_BOX_INPUT_LOCATION_MOVED, + $this->getContext() + ), + 'data-main-menu' => new VectorComponentMainMenu( + $sidebar, + $this->shouldLanguageAlertBeInSidebar(), + $parentData['data-portlets']['data-languages'] ?? [], + $this->getContext(), + $this->getUser(), + VectorServices::getFeatureManager(), + $this, + ), + 'data-main-menu-dropdown' => new VectorComponentDropdown( + VectorComponentMainMenu::ID . '-dropdown', + $this->msg( VectorComponentMainMenu::ID . '-label' )->text(), + VectorComponentMainMenu::ID . '-dropdown' . ' mw-ui-icon-flush-left mw-ui-icon-flush-right', + 'menu' + ), + 'data-page-tools' => $isPageToolsEnabled ? new VectorComponentPageTools( + array_merge( [ $parentData['data-portlets']['data-actions'] ?? [] ], $pageToolsMenu ), + $this->getContext(), + $this->getUser(), + $featureManager + ) : null, + 'data-page-tools-dropdown' => $isPageToolsEnabled ? new VectorComponentDropdown( + VectorComponentPageTools::ID . '-dropdown', + $this->msg( 'toolbox' )->text(), + VectorComponentPageTools::ID . '-dropdown', + ) : null, ]; foreach ( $components as $key => $component ) { // Array of components or null values. diff --git a/tests/phpunit/unit/SkinVector22Test.php b/tests/phpunit/unit/SkinVector22Test.php new file mode 100644 index 000000000..981b7aba5 --- /dev/null +++ b/tests/phpunit/unit/SkinVector22Test.php @@ -0,0 +1,100 @@ + 'p-navigation', + ]; + private const SUPPORT = [ + 'id' => 'p-support', + ]; + private const TOOLBOX = [ + 'id' => 'p-tb', + ]; + private const WIKIBASE = [ + 'id' => 'p-wikibase-otherprojects', + ]; + + public function provideExtractPageToolsFromSidebar() { + return [ + [ + [], + [], [], + 'No change if sidebar is missing keys' + ], + [ + [ + 'data-portlets-first' => self::MAIN, + 'array-portlets-rest' => [ + self::SUPPORT + ], + ], + [ + 'data-portlets-first' => self::MAIN, + 'array-portlets-rest' => [ + self::SUPPORT + ], + ], + [], + 'No change if no toolbox found' + ], + [ + [ + 'data-portlets-first' => self::TOOLBOX, + 'array-portlets-rest' => [ self::SUPPORT ], + ], + [ + 'data-portlets-first' => self::TOOLBOX, + 'array-portlets-rest' => [ self::SUPPORT ], + ], + [], + 'A toolbox in first part of sidebar is ignored.' + ], + + [ + [ + 'data-portlets-first' => self::MAIN, + 'array-portlets-rest' => [ self::SUPPORT, self::TOOLBOX, self::WIKIBASE ], + ], + // new expected sidebar + [ + 'data-portlets-first' => self::MAIN, + 'array-portlets-rest' => [ + self::SUPPORT + ], + ], + // new expected page tools menu + [ + self::TOOLBOX, self::WIKIBASE + ], + 'Toolbox and any items after it are pulled out.' + ], + ]; + } + + /** + * @covers ::extractPageToolsFromSidebar + * @dataProvider provideExtractPageToolsFromSidebar + */ + public function testExtractPageToolsFromSidebar( $sidebar, $expectedSidebar, $expectedPageTools, $msg ) { + $pageTools = []; + $extractPageToolsFromSidebar = new ReflectionMethod( + SkinVector22::class, + 'extractPageToolsFromSidebar' + ); + $extractPageToolsFromSidebar->setAccessible( true ); + $extractPageToolsFromSidebar->invokeArgs( null, [ &$sidebar, &$pageTools ] ); + $this->assertEquals( $expectedSidebar, $sidebar ); + $this->assertEquals( $expectedPageTools, $pageTools, $msg ); + } +}