Skip to content

Commit

Permalink
Fix broken assignment of features and categories
Browse files Browse the repository at this point in the history
We migrated the part of the import to use DataHandler.
We didn't invest too much time as budgets are low.
Still the bugs are covered with tests and fixed.

Relates: #10782
  • Loading branch information
DanielSiepmann committed Nov 2, 2023
1 parent 0784945 commit fefc1c3
Show file tree
Hide file tree
Showing 12 changed files with 274 additions and 128 deletions.
43 changes: 13 additions & 30 deletions Classes/Domain/Model/Event.php
Original file line number Diff line number Diff line change
Expand Up @@ -426,43 +426,15 @@ public function addCategory(Category $category): void
*/
public function getCategories(): array
{
$categories = $this->categories->toArray();

usort($categories, function (Category $catA, Category $catB) {
return $catA->getSorting() <=> $catB->getSorting();
});

return $categories;
}

/**
* @param ObjectStorage<Category> $categories
*/
public function setCategories(ObjectStorage $categories): void
{
$this->categories = $categories;
return $this->getSortedCategory($this->categories);
}

/**
* @return array<Category>
*/
public function getFeatures(): array
{
$features = $this->features->toArray();

usort($features, function (Category $catA, Category $catB) {
return $catA->getSorting() <=> $catB->getSorting();
});

return $features;
}

/**
* @param ObjectStorage<Category> $features
*/
public function setFeatures(ObjectStorage $features): void
{
$this->features = $features;
return $this->getSortedCategory($this->features);
}

public function setLanguageUid(int $languageUid): void
Expand All @@ -489,4 +461,15 @@ public function setSourceUrl(string $url): void
{
$this->sourceUrl = $url;
}

private function getSortedCategory(ObjectStorage $categories): array
{
$categories = $categories->toArray();

usort($categories, function (Category $catA, Category $catB) {
return $catA->getSorting() <=> $catB->getSorting();
});

return $categories;
}
}
42 changes: 30 additions & 12 deletions Classes/Service/DestinationDataImportService.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
use Wrm\Events\Service\DestinationDataImportService\CategoriesAssignment;
use Wrm\Events\Service\DestinationDataImportService\CategoriesAssignment\Import as CategoryImport;
use Wrm\Events\Service\DestinationDataImportService\DataFetcher;
use Wrm\Events\Service\DestinationDataImportService\DataHandler;
use Wrm\Events\Service\DestinationDataImportService\DataHandler\Assignment;
use Wrm\Events\Service\DestinationDataImportService\DatesFactory;
use Wrm\Events\Service\DestinationDataImportService\Events\CategoriesAssignEvent;
use Wrm\Events\Service\DestinationDataImportService\Events\EventImportEvent;
Expand Down Expand Up @@ -111,6 +113,11 @@ class DestinationDataImportService
*/
private $cacheManager;

/**
* @var DataHandler
*/
private $dataHandler;

/**
* @var EventDispatcher
*/
Expand All @@ -131,6 +138,7 @@ class DestinationDataImportService
* @param LocationAssignment $locationAssignment
* @param Slugger $slugger
* @param CacheManager $cacheManager
* @param DataHandler $dataHandler
* @param EventDispatcher $eventDispatcher
*/
public function __construct(
Expand All @@ -147,6 +155,7 @@ public function __construct(
LocationAssignment $locationAssignment,
Slugger $slugger,
CacheManager $cacheManager,
DataHandler $dataHandler,
EventDispatcher $eventDispatcher
) {
$this->eventRepository = $eventRepository;
Expand All @@ -162,6 +171,7 @@ public function __construct(
$this->locationAssignment = $locationAssignment;
$this->slugger = $slugger;
$this->cacheManager = $cacheManager;
$this->dataHandler = $dataHandler;
$this->eventDispatcher = $eventDispatcher;
}

Expand Down Expand Up @@ -236,16 +246,6 @@ public function processData(array $data): int
$this->locationAssignment->getLocation($event)
);

// Set Categories
if ($event['categories'] ?? false) {
$this->setCategories($event['categories']);
}

// Set Features
if ($event['features']) {
$this->setFeatures($event['features']);
}

// Set Organizer
if ($event['addresses'] ?? false) {
$this->setOrganizer($event['addresses']);
Expand Down Expand Up @@ -291,6 +291,16 @@ public function processData(array $data): int
$this->logger->info('Persist database');
$this->eventRepository->update($this->tmpCurrentEvent);
$this->persistenceManager->persistAll();

// Apply changes via DataHandler (The new way)
$this->logger->info('Apply changes via DataHandler');
if ($event['categories'] ?? false) {
$this->setCategories($event['categories']);
}
if ($event['features']) {
$this->setFeatures($event['features']);
}

$this->logger->info('Update slugs');
$this->slugger->update('tx_events_domain_model_event');
$this->slugger->update('tx_events_domain_model_date');
Expand All @@ -317,7 +327,11 @@ private function setCategories(array $categories): void
);
$this->eventDispatcher->dispatch($event);

$this->tmpCurrentEvent->setCategories($event->getCategories());
$this->dataHandler->storeAssignments(new Assignment(
$this->tmpCurrentEvent->getUid(),
'categories',
$event->getCategories()->toArray()
));
}

private function setFeatures(array $features): void
Expand All @@ -329,7 +343,11 @@ private function setFeatures(array $features): void
true
));

$this->tmpCurrentEvent->setFeatures($features);
$this->dataHandler->storeAssignments(new Assignment(
$this->tmpCurrentEvent->getUid(),
'features',
$features->toArray()
));
}

private function setDates(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Wrm\Events\Service\DestinationDataImportService;

use TYPO3\CMS\Extbase\Persistence\Generic\PersistenceManager;
use TYPO3\CMS\Extbase\Persistence\ObjectStorage;
use Wrm\Events\Domain\Model\Category;
use Wrm\Events\Domain\Repository\CategoryRepository;
Expand All @@ -20,10 +21,17 @@ class CategoriesAssignment
*/
private $repository;

/**
* @var PersistenceManager
*/
private $persistenceManager;

public function __construct(
CategoryRepository $repository
CategoryRepository $repository,
PersistenceManager $persistenceManager
) {
$this->repository = $repository;
$this->persistenceManager = $persistenceManager;
}

/**
Expand Down Expand Up @@ -58,6 +66,8 @@ public function getCategories(
$categories->attach($category);
}

$this->persistenceManager->persistAll();

return $categories;
}
}
68 changes: 68 additions & 0 deletions Classes/Service/DestinationDataImportService/DataHandler.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

declare(strict_types=1);

/*
* Copyright (C) 2023 Daniel Siepmann <coding@daniel-siepmann.de>
*
* This program 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 2
* of the License, or (at your option) any later version.
*
* This program 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 this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
* 02110-1301, USA.
*/

namespace Wrm\Events\Service\DestinationDataImportService;

use TYPO3\CMS\Core\DataHandling\DataHandler as Typo3DataHandler;
use TYPO3\CMS\Core\Log\Logger;
use TYPO3\CMS\Core\Log\LogManager;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use Wrm\Events\Service\DestinationDataImportService\DataHandler\Assignment;

final class DataHandler
{
/**
* @var Logger
*/
private $logger;

public function __construct(
LogManager $logManager
) {
$this->logger = $logManager->getLogger(__CLASS__);
}

public function storeAssignments(
Assignment $assignment
): void {
$data = [
'tx_events_domain_model_event' => [
$assignment->getUid() => [
$assignment->getColumnName() => implode(',', $assignment->getUids()),
],
],
];

$this->logger->debug('Import assignment.', $data);
$dataHandler = GeneralUtility::makeInstance(Typo3DataHandler::class);
$dataHandler->start($data, []);
$dataHandler->process_datamap();

if ($dataHandler->errorLog !== []) {
$this->logger->error('Error during import of assignments.', [
'assignment' => $assignment,
'errors' => $dataHandler->errorLog,
]);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php

declare(strict_types=1);

/*
* Copyright (C) 2023 Daniel Siepmann <coding@daniel-siepmann.de>
*
* This program 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 2
* of the License, or (at your option) any later version.
*
* This program 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 this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
* 02110-1301, USA.
*/

namespace Wrm\Events\Service\DestinationDataImportService\DataHandler;

use InvalidArgumentException;
use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject;

final class Assignment
{
/**
* @var int
*/
private $uid;

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

/**
* @var int[]
*/
private $uids;

/**
* @param AbstractDomainObject[] $assignments
*/
public function __construct(
int $uid,
string $columnName,
array $assignments,
) {
$this->uid = $uid;
$this->columnName = $columnName;
$this->uids = array_map(static function (AbstractDomainObject $model): int {
$uid = $model->getUid();
if (is_int($uid) === false) {
throw new InvalidArgumentException('Only object with uid supported.', 1698936965);
}
return $uid;
}, $assignments);
}

public function getUid(): int
{
return $this->uid;
}

public function getColumnName(): string
{
return $this->columnName;
}

/**
* @return int[]
*/
public function getUids(): array
{
return $this->uids;
}
}
15 changes: 15 additions & 0 deletions Documentation/Changelog/3.5.2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@ Fixes
An unexpected value (3) was submitted on selection.
We now migrated the property to look and act the same as hidden input.

* Fix broken assignment of features and categories.
The import contained a bug that lead to summing up relations between events and
categories and features.
The same relations would be added over and over again with every new import.
This leads to performance issues when editing or storing events through TYPO3
backend.
The import is adjusted to no longer use Extbase but DataHandler for this task.
This circumvents the issues introduced by Extbase.
This is the first step to migration of import to be fully using DataHandler +
doctrine DBAL in the future.
One drawback is the slowdown of the import.
But we don't expect people to run it that often.
Also a working import with proper logging (history, permissions, etc.) seems better
than a broken but fast import.

Tasks
-----

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
"tx_events_domain_model_event",,,,,,
,"uid","pid","features",,,
,1,2,1,,,
,2,2,1,,,
,1,2,6,,,
,2,2,4,,,
,3,2,0,,,
"sys_category",,,,,,
,"uid","pid","title","parent","hidden",
,1,2,"Top Category",0,0,
,2,2,"Event Category Parent",1,0,
,4,2,"Event Feature Parent",1,0,
,5,3,"vorhandenes Feature",4,0,
,5,3,"vorhandenes Feature",4,1,
,6,3,"Barrierefrei",4,1,
,7,3,"Zielgruppe Jugendliche",4,1,
,8,3,"Karten an der Abendkasse",4,1,
Expand Down
Loading

0 comments on commit fefc1c3

Please sign in to comment.