From b6fc78057d4ae0de7118e0bc73e03629f8c56a51 Mon Sep 17 00:00:00 2001 From: Claus Due Date: Mon, 28 Oct 2024 13:46:10 +0100 Subject: [PATCH] [BUGFIX] Detach colPos list items provider from BackendLayoutView --- .../HookSubscribers/ColumnPositionItems.php | 58 ++++ .../Overrides/BackendLayoutView.php | 83 +----- Configuration/Services.yaml | 2 + Configuration/TCA/Overrides/tt_content.php | 2 +- .../ColumnPositionItemsTest.php | 103 +++++++ .../Overrides/BackendLayoutViewTest.php | 267 +----------------- 6 files changed, 168 insertions(+), 347 deletions(-) create mode 100644 Classes/Integration/HookSubscribers/ColumnPositionItems.php create mode 100644 Tests/Unit/Integration/HookSubscribers/ColumnPositionItemsTest.php diff --git a/Classes/Integration/HookSubscribers/ColumnPositionItems.php b/Classes/Integration/HookSubscribers/ColumnPositionItems.php new file mode 100644 index 000000000..f0c877ef7 --- /dev/null +++ b/Classes/Integration/HookSubscribers/ColumnPositionItems.php @@ -0,0 +1,58 @@ +recordService = $recordService; + $this->providerResolver = $providerResolver; + } + + /** + * Gets colPos items to be shown in the forms engine. + * This method is called as "itemsProcFunc" with the accordant context + * for tt_content.colPos. + */ + public function colPosListItemProcFunc(array &$parameters): void + { + $parentRecordUid = ColumnNumberUtility::calculateParentUid($parameters['row']['colPos']); + $parentRecord = $this->recordService->getSingle('tt_content', '*', $parentRecordUid); + $provider = $this->providerResolver->resolvePrimaryConfigurationProvider('tt_content', null, $parentRecord); + if ($parentRecord && $provider instanceof GridProviderInterface) { + $grid = $provider->getGrid($parentRecord); + /** @var SelectOption $dividerItem */ + $dividerItem = GeneralUtility::makeInstance( + SelectOption::class, + 'LLL:EXT:flux/Resources/Private/Language/locallang.xlf:flux.backendLayout.columnsInParent', + '--div--' + ); + $parameters['items'][] = $dividerItem->toArray(); + foreach ($grid->getRows() as $row) { + foreach ($row->getColumns() as $column) { + /** @var SelectOption $item */ + $item = GeneralUtility::makeInstance( + SelectOption::class, + $column->getLabel(), + ColumnNumberUtility::calculateColumnNumberForParentAndColumn( + $parentRecordUid, + $column->getColumnPosition() + ) + ); + $parameters['items'][] = $item->toArray(); + } + } + } + } +} diff --git a/Classes/Integration/Overrides/BackendLayoutView.php b/Classes/Integration/Overrides/BackendLayoutView.php index c91988be5..850b5868b 100644 --- a/Classes/Integration/Overrides/BackendLayoutView.php +++ b/Classes/Integration/Overrides/BackendLayoutView.php @@ -9,10 +9,8 @@ * LICENSE.md file that was distributed with this source code. */ -use FluidTYPO3\Flux\Integration\FormEngine\SelectOption; use FluidTYPO3\Flux\Provider\Interfaces\GridProviderInterface; use FluidTYPO3\Flux\Provider\ProviderResolver; -use FluidTYPO3\Flux\Utility\ColumnNumberUtility; use FluidTYPO3\Flux\Utility\DoctrineQueryProxy; use TYPO3\CMS\Core\Database\ConnectionPool; use TYPO3\CMS\Core\Utility\GeneralUtility; @@ -21,7 +19,6 @@ class BackendLayoutView extends \TYPO3\CMS\Backend\View\BackendLayoutView { protected ?GridProviderInterface $provider = null; protected array $record = []; - protected bool $addingItemsForContent = false; public function setProvider(GridProviderInterface $provider): void { @@ -33,43 +30,11 @@ public function setRecord(array $record): void $this->record = $record; } - /** - * Gets colPos items to be shown in the forms engine. - * This method is called as "itemsProcFunc" with the accordant context - * for tt_content.colPos. - */ - public function colPosListItemProcFunc(array $parameters): void - { - $this->record = $parameters['row']; - $this->addingItemsForContent = true; - parent::colPosListItemProcFunc($parameters); - $this->addingItemsForContent = false; - } - /** * @param int $pageId */ public function getSelectedBackendLayout($pageId): ?array { - if ($this->addingItemsForContent) { - $identifier = $this->getSelectedCombinedIdentifier($pageId); - if ($identifier === false) { - return null; - } - - // Early return parent method's output if selected identifier is not from Flux - if (substr((string) $identifier, 0, 6) !== 'flux__') { - return parent::getSelectedBackendLayout($pageId); - } - $pageRecord = $this->loadRecordFromTable('pages', (int)$pageId); - if (!$pageRecord) { - return null; - } - $pageLevelProvider = $this->resolvePrimaryProviderForRecord('pages', $pageRecord); - if ($pageLevelProvider instanceof GridProviderInterface) { - return $pageLevelProvider->getGrid($pageRecord)->buildExtendedBackendLayoutArray(0); - } - } // Delegate resolving of backend layout structure to the Provider, which will return a Grid, which can create // a full backend layout data array. if ($this->provider instanceof GridProviderInterface) { @@ -95,51 +60,6 @@ protected function resolveParentRecordUid(array $record): int return (int) $uid; } - /** - * Override which will merge allowed colPos values from two places: - * - * 1) The currently selected backend layout (which may be a Flux-based - * or any other type). - * 2) If a Provider can be resolved for the parent record and it has - * a grid, items from that grid are included. - * - * The result is a "colPos" items collection which includes page columns - * and columns directly inside the current parent. - * - * @param int $pageId - * @param array $items - */ - protected function addColPosListLayoutItems($pageId, $items): array - { - $layout = $this->getSelectedBackendLayout($pageId); - if (isset($layout, $layout['__items'])) { - $items = $layout['__items']; - } - if ($this->addingItemsForContent) { - $parentRecordUid = ColumnNumberUtility::calculateParentUid((integer) ($this->record['colPos'] ?? 0)); - if ($parentRecordUid > 0) { - $parentRecord = $this->loadRecordFromTable('tt_content', $parentRecordUid); - if (!$parentRecord) { - return $items; - } - $provider = $this->resolvePrimaryProviderForRecord('tt_content', $parentRecord); - if ($provider) { - $label = $this->getLanguageService()->sL( - 'LLL:EXT:flux/Resources/Private/Language/locallang.xlf:flux.backendLayout.columnsInParent' - ); - $items = array_merge( - $items, - [ - (new SelectOption($label, '--div--'))->toArray() - ], - $provider->getGrid($parentRecord)->buildExtendedBackendLayoutArray($parentRecordUid)['__items'] - ); - } - } - } - return $items; - } - /** * @codeCoverageIgnore */ @@ -157,6 +77,9 @@ protected function loadRecordFromTable(string $table, int $uid): ?array return $results[0] ?? null; } + /** + * @codeCoverageIgnore + */ protected function resolvePrimaryProviderForRecord(string $table, array $record): ?GridProviderInterface { /** @var ProviderResolver $providerResolver */ diff --git a/Configuration/Services.yaml b/Configuration/Services.yaml index 3e29d46b7..efe075744 100644 --- a/Configuration/Services.yaml +++ b/Configuration/Services.yaml @@ -24,6 +24,8 @@ services: arguments: $locator: !tagged_locator { tag: 'flux.datatransformer', index_by: 'identifier' } + FluidTYPO3\Flux\Integration\HookSubscribers\ColumnPositionItems: + public: true FluidTYPO3\Flux\Backend\BackendLayoutDataProvider: public: true FluidTYPO3\Flux\Backend\PageLayoutDataProvider: diff --git a/Configuration/TCA/Overrides/tt_content.php b/Configuration/TCA/Overrides/tt_content.php index 57e26ff91..90842f68f 100644 --- a/Configuration/TCA/Overrides/tt_content.php +++ b/Configuration/TCA/Overrides/tt_content.php @@ -4,7 +4,7 @@ \FluidTYPO3\Flux\Integration\MultipleItemsProcFunc::register( 'tt_content', 'colPos', - \FluidTYPO3\Flux\Integration\Overrides\BackendLayoutView::class . '->colPosListItemProcFunc' + \FluidTYPO3\Flux\Integration\HookSubscribers\ColumnPositionItems::class . '->colPosListItemProcFunc' ); $GLOBALS['TCA']['tt_content']['columns']['pi_flexform']['label'] = 'LLL:EXT:flux/Resources/Private/Language/locallang.xlf:tt_content.pi_flexform'; diff --git a/Tests/Unit/Integration/HookSubscribers/ColumnPositionItemsTest.php b/Tests/Unit/Integration/HookSubscribers/ColumnPositionItemsTest.php new file mode 100644 index 000000000..b5e6fe1a7 --- /dev/null +++ b/Tests/Unit/Integration/HookSubscribers/ColumnPositionItemsTest.php @@ -0,0 +1,103 @@ +recordService = $this->getMockBuilder(WorkspacesAwareRecordService::class) + ->onlyMethods(['getSingle']) + ->disableOriginalConstructor() + ->getMock(); + $this->providerResolver = $this->getMockBuilder(ProviderResolver::class) + ->onlyMethods(['resolvePrimaryConfigurationProvider']) + ->disableOriginalConstructor() + ->getMock(); + } + + public function testDoesNotProcessWithoutParentRecord(): void + { + $this->recordService->method('getSingle')->willReturn(null); + $this->providerResolver->method('resolvePrimaryConfigurationProvider')->willReturn( + $this->getMockBuilder(ProviderInterface::class)->getMockForAbstractClass() + ); + $subject = new ColumnPositionItems($this->recordService, $this->providerResolver); + + $parameters = ['row' => ['colPos' => 101]]; + $subject->colPosListItemProcFunc($parameters); + self::assertSame($parameters, $parameters); + } + + public function testDoesNotProcessWithoutProvider(): void + { + $this->recordService->method('getSingle')->willReturn(['uid' => 123]); + $this->providerResolver->method('resolvePrimaryConfigurationProvider')->willReturn(null); + $subject = new ColumnPositionItems($this->recordService, $this->providerResolver); + + $parameters = ['row' => ['colPos' => 101]]; + $subject->colPosListItemProcFunc($parameters); + self::assertSame($parameters, $parameters); + } + + public function testDoesNotProcessWithoutProviderAndParentRecord(): void + { + $this->recordService->method('getSingle')->willReturn(null); + $this->providerResolver->method('resolvePrimaryConfigurationProvider')->willReturn(null); + $subject = new ColumnPositionItems($this->recordService, $this->providerResolver); + + $parameters = ['row' => ['colPos' => 101]]; + $subject->colPosListItemProcFunc($parameters); + self::assertSame($parameters, $parameters); + } + + public function testAddsExpectedItems(): void + { + $grid = Grid::create(); + $grid->createContainer(Row::class, 'row')->createContainer(Column::class, 'col')->setColumnPosition(3); + + $provider = $this->getMockBuilder(ProviderInterface::class)->getMockForAbstractClass(); + $provider->method('getGrid')->willReturn($grid); + + $this->recordService->method('getSingle')->willReturn(['uid' => 123]); + $this->providerResolver->method('resolvePrimaryConfigurationProvider')->willReturn($provider); + $subject = new ColumnPositionItems($this->recordService, $this->providerResolver); + + $parameters = ['row' => ['colPos' => 103]]; + $expected = $parameters; + $expected['items'] = [ + (new SelectOption( + 'LLL:EXT:flux/Resources/Private/Language/locallang.xlf:flux.backendLayout.columnsInParent', + '--div--' + ))->toArray(), + (new SelectOption( + 'LLL:EXT:flux/Resources/Private/Language/locallang.xlf:flux..columns.col', + 103 + ))->toArray(), + ]; + + $subject->colPosListItemProcFunc($parameters); + self::assertSame($expected, $parameters); + } +} diff --git a/Tests/Unit/Integration/Overrides/BackendLayoutViewTest.php b/Tests/Unit/Integration/Overrides/BackendLayoutViewTest.php index 052e66524..2dcec90ca 100644 --- a/Tests/Unit/Integration/Overrides/BackendLayoutViewTest.php +++ b/Tests/Unit/Integration/Overrides/BackendLayoutViewTest.php @@ -1,5 +1,5 @@ setRecord($record); $this->assertSame($record, $this->getInaccessiblePropertyValue($instance, 'record')); } - - public function testColPosListItemProcFuncWithoutSelectedIdentifier(): void - { - $subject = $this->getMockBuilder(BackendLayoutView::class) - ->onlyMethods(['getSelectedCombinedIdentifier', 'determinePageId']) - ->disableOriginalConstructor() - ->getMock(); - $subject->expects(self::once())->method('determinePageId')->willReturn(1); - $subject->expects(self::once())->method('getSelectedCombinedIdentifier')->willReturn(false); - - $parameters = ['row' => ['uid' => 123, 'colPos' => 1], 'table' => 'tt_content', 'items' => []]; - - $subject->colPosListItemProcFunc($parameters); - } - - public function testColPosListItemProcFuncWithForeignSelectedIdentifier(): void - { - $subject = $this->getMockBuilder(BackendLayoutView::class) - ->onlyMethods( - [ - 'getBackendLayoutForPage', - 'getSelectedCombinedIdentifier', - 'determinePageId', - 'loadRecordFromTable', - ] - ) - ->disableOriginalConstructor() - ->getMock(); - $subject->method('getSelectedCombinedIdentifier')->willReturn('foreign__foobar'); - $subject->method('determinePageId')->willReturn(0); - $subject->expects(self::never())->method('loadRecordFromTable'); - $subject->method('getBackendLayoutForPage')->willReturn( - $this->getMockBuilder(BackendLayout::class)->disableOriginalConstructor()->getMock() - ); - - $parameters = ['row' => ['uid' => 123, 'colPos' => 1], 'table' => 'tt_content', 'items' => []]; - - $subject->colPosListItemProcFunc($parameters); - } - - public function testColPosListItemProcFuncWithoutPageRecord(): void - { - $subject = $this->getMockBuilder(BackendLayoutView::class) - ->onlyMethods( - [ - 'getBackendLayoutForPage', - 'getSelectedCombinedIdentifier', - 'determinePageId', - 'loadRecordFromTable', - 'resolvePrimaryProviderForRecord', - ] - ) - ->disableOriginalConstructor() - ->getMock(); - $subject->method('getSelectedCombinedIdentifier')->willReturn('flux__foobar'); - $subject->method('determinePageId')->willReturn(0); - $subject->method('loadRecordFromTable')->willReturn(null); - $subject->method('getBackendLayoutForPage')->willReturn( - $this->getMockBuilder(BackendLayout::class)->disableOriginalConstructor()->getMock() - ); - $subject->expects(self::never())->method('resolvePrimaryProviderForRecord'); - - $parameters = ['row' => ['uid' => 123, 'colPos' => 1], 'table' => 'tt_content', 'items' => []]; - - $subject->colPosListItemProcFunc($parameters); - } - - public function testColPosListItemProcFuncWithPageLevelProvider(): void - { - $grid = $this->getMockBuilder(Grid::class) - ->onlyMethods(['buildExtendedBackendLayoutArray']) - ->disableOriginalConstructor() - ->getMock(); - $grid->expects(self::once())->method('buildExtendedBackendLayoutArray')->with(0); - - $provider = $this->getMockBuilder(GridProviderInterface::class)->getMockForAbstractClass(); - $provider->method('getGrid')->willReturn($grid); - - $this->providerResolver->method('resolvePrimaryConfigurationProvider')->willReturn($provider); - - $subject = $this->getMockBuilder(BackendLayoutView::class) - ->onlyMethods( - [ - 'getBackendLayoutForPage', - 'getSelectedCombinedIdentifier', - 'determinePageId', - 'loadRecordFromTable', - ] - ) - ->disableOriginalConstructor() - ->getMock(); - $subject->method('getSelectedCombinedIdentifier')->willReturn('flux__foobar'); - $subject->method('determinePageId')->willReturn(0); - $subject->method('loadRecordFromTable')->willReturn(['uid' => 123]); - $subject->method('getBackendLayoutForPage')->willReturn( - $this->getMockBuilder(BackendLayout::class)->disableOriginalConstructor()->getMock() - ); - - $parameters = ['row' => ['uid' => 123, 'colPos' => 1], 'table' => 'tt_content', 'items' => []]; - - $subject->colPosListItemProcFunc($parameters); - } - - /** - * @param int|array $uid - * @dataProvider getColPosListItemProcFuncWithDelegateProviderTestValues - */ - public function testColPosListItemProcFuncWithDelegateProvider($uid): void - { - $grid = $this->getMockBuilder(Grid::class) - ->onlyMethods(['buildExtendedBackendLayoutArray']) - ->disableOriginalConstructor() - ->getMock(); - $grid->expects(self::once())->method('buildExtendedBackendLayoutArray')->with(1); - - $provider = $this->getMockBuilder(GridProviderInterface::class)->getMockForAbstractClass(); - $provider->method('getGrid')->willReturn($grid); - - $subject = $this->getMockBuilder(BackendLayoutView::class) - ->onlyMethods( - [ - 'getBackendLayoutForPage', - 'getSelectedCombinedIdentifier', - 'determinePageId', - 'loadRecordFromTable', - 'resolvePrimaryProviderForRecord', - ] - ) - ->disableOriginalConstructor() - ->getMock(); - $subject->method('getSelectedCombinedIdentifier')->willReturn('flux__foobar'); - $subject->method('determinePageId')->willReturn(0); - $subject->method('loadRecordFromTable')->willReturn(['uid' => 123]); - $subject->method('getBackendLayoutForPage')->willReturn( - $this->getMockBuilder(BackendLayout::class)->disableOriginalConstructor()->getMock() - ); - $subject->method('resolvePrimaryProviderForRecord')->willReturn(null); - $subject->setProvider($provider); - - $parameters = [ - 'row' => ['uid' => 123, 'colPos' => 1, 'l18n_parent' => $uid], - 'table' => 'tt_content', - 'items' => [] - ]; - - $subject->colPosListItemProcFunc($parameters); - } - - public function getColPosListItemProcFuncWithDelegateProviderTestValues(): array - { - return [ - 'uid as int' => [1], - 'uid as array with int' => [[1]], - ]; - } - - public function testColPosListItemProcFuncWithoutProviders(): void - { - $this->providerResolver->method('resolvePrimaryConfigurationProvider')->willReturn(null); - - $subject = $this->getMockBuilder(BackendLayoutView::class) - ->onlyMethods( - [ - 'getBackendLayoutForPage', - 'getSelectedCombinedIdentifier', - 'determinePageId', - 'loadRecordFromTable', - 'resolvePrimaryProviderForRecord', - ] - ) - ->disableOriginalConstructor() - ->getMock(); - $subject->expects(self::atLeastOnce())->method('getSelectedCombinedIdentifier')->willReturn('flux__foobar'); - $subject->method('determinePageId')->willReturn(0); - $subject->method('loadRecordFromTable')->willReturn(['uid' => 123]); - $subject->method('getBackendLayoutForPage')->willReturn(null); - - $parameters = [ - 'row' => ['uid' => 123, 'colPos' => 1, 'l18n_parent' => 1], - 'table' => 'tt_content', - 'items' => [] - ]; - - $subject->colPosListItemProcFunc($parameters); - } - - public function testAddColPosListLayoutItemsWithPageRecord(): void - { - $grid = $this->getMockBuilder(Grid::class) - ->onlyMethods(['buildExtendedBackendLayoutArray']) - ->disableOriginalConstructor() - ->getMock(); - $grid->expects(self::once()) - ->method('buildExtendedBackendLayoutArray') - ->with(123) - ->willReturn(['__items' => []]); - - $provider = $this->getMockBuilder(GridProviderInterface::class)->getMockForAbstractClass(); - $provider->method('getGrid')->willReturn($grid); - - $this->providerResolver->method('resolvePrimaryConfigurationProvider')->willReturn($provider); - - $languageService = $this->getMockBuilder(LanguageService::class) - ->onlyMethods(['sL']) - ->disableOriginalConstructor() - ->getMock(); - $languageService->method('sL')->willReturn('Label'); - - $subject = $this->getMockBuilder(BackendLayoutView::class) - ->onlyMethods( - [ - 'determinePageId', - 'getSelectedBackendLayout', - 'loadRecordFromTable', - 'getLanguageService', - 'resolvePrimaryProviderForRecord', - ] - ) - ->disableOriginalConstructor() - ->getMock(); - $subject->method('determinePageId')->willReturn(123); - $subject->method('loadRecordFromTable')->willReturn(['uid' => 123]); - $subject->method('getSelectedBackendLayout')->willReturn(['__items' => []]); - $subject->method('getLanguageService')->willReturn($languageService); - $subject->method('resolvePrimaryProviderForRecord')->willReturn($provider); - - $subject->setRecord(['uid' => 123, 'colPos' => 12300]); - - $this->setInaccessiblePropertyValue($subject, 'addingItemsForContent', true); - - $output = $this->callInaccessibleMethod($subject, 'addColPosListLayoutItems', 123, []); - self::assertSame( - [(new SelectOption('Label', '--div--'))->toArray()], - $output - ); - } - - public function testAddColPosListLayoutItemsWithoutPageRecord(): void - { - $subject = $this->getMockBuilder(BackendLayoutView::class) - ->onlyMethods( - [ - 'determinePageId', - 'getSelectedBackendLayout', - 'loadRecordFromTable', - 'getLanguageService', - ] - ) - ->disableOriginalConstructor() - ->getMock(); - $subject->method('determinePageId')->willReturn(123); - $subject->method('loadRecordFromTable')->willReturn(null); - $subject->method('getSelectedBackendLayout')->willReturn(['__items' => []]); - - $subject->setRecord(['uid' => 123, 'colPos' => 12300]); - - $this->setInaccessiblePropertyValue($subject, 'addingItemsForContent', true); - - $output = $this->callInaccessibleMethod($subject, 'addColPosListLayoutItems', 123, []); - self::assertSame([], $output); - } }