fix: Add output for individual failures or skipped parts

Signed-off-by: Julius Härtl <jus@bitgrid.net>

lib/Db/Assignment.php

@@ -41,4 +41,17 @@ class Assignment extends RelationalEntity implements JsonSerializable {
 		$this->addType('type', 'integer');
 		$this->addResolvable('participant');
 	}
+
+	public function getTypeString(): string {
+		switch ($this->getType()) {
+			case self::TYPE_USER:
+				return 'user';
+			case self::TYPE_GROUP:
+				return 'group';
+			case self::TYPE_CIRCLE:
+				return 'circle';
+		}
+
+		return 'unknown';
+	}
 }

lib/Service/Importer/BoardImportCommandService.php

@@ -25,7 +25,6 @@ namespace OCA\Deck\Service\Importer;
 
 use OCA\Deck\Exceptions\ConflictException;
 use OCA\Deck\NotFoundException;
-use OCA\Deck\Service\Importer\Systems\DeckJsonService;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -78,11 +77,12 @@ class BoardImportCommandService extends BoardImportService {
 
 	protected function validateConfig(): void {
 		// FIXME: Make config optional for deck plain importer (but use a call on the importer insterad)
-		if ($this->getImportSystem() instanceof DeckJsonService) {
-			return;
-		}
 		try {
 			$config = $this->getInput()->getOption('config');
+			if (!$config) {
+				return;
+			}
+
 			if (is_string($config)) {
 				if (!is_file($config)) {
 					throw new NotFoundException('It\'s not a valid config file.');
@@ -191,17 +191,37 @@ class BoardImportCommandService extends BoardImportService {
 	public function bootstrap(): void {
 		$this->setSystem($this->getInput()->getOption('system'));
 		parent::bootstrap();
+
+		$this->registerErrorCollector(function ($error, $exception) {
+			$message = $error;
+			if ($exception instanceof \Throwable) {
+				$message .= ': ' . $exception->getMessage();
+			}
+			$this->getOutput()->writeln('<error>' . $message . '</error>');
+			if ($exception instanceof \Throwable && $this->getOutput()->isVeryVerbose()) {
+				$this->getOutput()->writeln($exception->getTraceAsString());
+			}
+		});
+
+		$this->registerOutputCollector(function ($info) {
+			if ($this->getOutput()->isVerbose()) {
+				$this->getOutput()->writeln('<info>' . $info . '</info>', );
+			}
+		});
 	}
 
 	public function import(): void {
 		$this->getOutput()->writeln('Starting import...');
 		$this->bootstrap();
+		$this->validateSystem();
+		$this->validateConfig();
 		$boards = $this->getImportSystem()->getBoards();
 
 		foreach ($boards as $board) {
+			try {
 				$this->reset();
 				$this->setData($board);
-			$this->getOutput()->writeln('Importing board "' . $this->getBoard()->getTitle() . '".');
+				$this->getOutput()->writeln('Importing board "' . $board->title . '".');
 				$this->importBoard();
 				$this->getOutput()->writeln('Assign users to board...');
 				$this->importAcl();
@@ -218,6 +238,9 @@ class BoardImportCommandService extends BoardImportService {
 				$this->getOutput()->writeln('Importing participants...');
 				$this->importCardAssignments();
 				$this->getOutput()->writeln('<info>Finished board import of "' . $this->getBoard()->getTitle() . '"</info>');
+			} catch (\Exception $e) {
+				$this->output->writeln('<error>Import failed for board ' . $board->title . ': ' . $e->getMessage() . '</error>');
+			}
 		}
 	}
 }

lib/Service/Importer/BoardImportService.php

@@ -49,6 +49,7 @@ use OCP\Comments\NotFoundException as CommentNotFoundException;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IUserManager;
 use OCP\Server;
+use Psr\Log\LoggerInterface;
 
 class BoardImportService {
 	private string $system = '';
@@ -70,6 +71,11 @@ class BoardImportService {
 	private $data;
 	private Board $board;
 
+	/** @var callable[] */
+	private array $errorCollectors = [];
+	/** @var callable[] */
+	private array $outputCollectors = [];
+
 	public function __construct(
 		private IUserManager $userManager,
 		private BoardMapper $boardMapper,
@@ -80,7 +86,8 @@ class BoardImportService {
 		private AttachmentMapper $attachmentMapper,
 		private CardMapper $cardMapper,
 		private ICommentsManager $commentsManager,
-		private IEventDispatcher $eventDispatcher
+		private IEventDispatcher $eventDispatcher,
+		private LoggerInterface $logger
 	) {
 		$this->board = new Board();
 		$this->disableCommentsEvents();
@@ -88,6 +95,28 @@ class BoardImportService {
 		$this->config = new \stdClass();
 	}
 
+	public function registerErrorCollector(callable $errorCollector): void {
+		$this->errorCollectors[] = $errorCollector;
+	}
+
+	public function registerOutputCollector(callable $outputCollector): void {
+		$this->outputCollectors[] = $outputCollector;
+	}
+
+	private function addError(string $message, $exception): void {
+		$message .= ' (on board ' . $this->getBoard()->getTitle() . ')';
+		foreach ($this->errorCollectors as $errorCollector) {
+			$errorCollector($message, $exception);
+		}
+		$this->logger->error($message, ['exception' => $exception]);
+	}
+
+	private function addOutput(string $message): void {
+		foreach ($this->outputCollectors as $outputCollector) {
+			$outputCollector($message);
+		}
+	}
+
 	private function disableCommentsEvents(): void {
 		if (defined('PHPUNIT_RUN')) {
 			return;
@@ -117,6 +146,7 @@ class BoardImportService {
 				$this->importComments();
 				$this->importCardAssignments();
 			} catch (\Throwable $th) {
+				$this->logger->error('Failed to import board', ['exception' => $th]);
 				throw new BadRequestException($th->getMessage());
 			}
 		}
@@ -195,6 +225,10 @@ class BoardImportService {
 
 	public function importBoard(): void {
 		$board = $this->getImportSystem()->getBoard();
+		if (!$this->userManager->userExists($board->getOwner())) {
+			throw new \Exception('Target owner ' . $board->getOwner() . ' not found. Please provide a mapping through the import config.');
+		}
+
 		if ($board) {
 			$this->boardMapper->insert($board);
 			$this->board = $board;
@@ -211,8 +245,13 @@ class BoardImportService {
 	public function importAcl(): void {
 		$aclList = $this->getImportSystem()->getAclList();
 		foreach ($aclList as $code => $acl) {
+			try {
 				$this->aclMapper->insert($acl);
 				$this->getImportSystem()->updateAcl($code, $acl);
+			} catch (\Exception $e) {
+				$this->addError('Failed to import acl rule for ' . $acl->getParticipant(), $e);
+
+			}
 		}
 		$this->getBoard()->setAcl($aclList);
 	}
@@ -220,8 +259,12 @@ class BoardImportService {
 	public function importLabels(): void {
 		$labels = $this->getImportSystem()->getLabels();
 		foreach ($labels as $code => $label) {
+			try {
 				$this->labelMapper->insert($label);
 				$this->getImportSystem()->updateLabel($code, $label);
+			} catch (\Exception $e) {
+				$this->addError('Failed to import label ' . $label->getTitle(), $e);
+			}
 		}
 		$this->getBoard()->setLabels($labels);
 	}
@@ -229,8 +272,12 @@ class BoardImportService {
 	public function importStacks(): void {
 		$stacks = $this->getImportSystem()->getStacks();
 		foreach ($stacks as $code => $stack) {
+			try {
 				$this->stackMapper->insert($stack);
 				$this->getImportSystem()->updateStack($code, $stack);
+			} catch (\Exception $e) {
+				$this->addError('Failed to import list ' . $stack->getTitle(), $e);
+			}
 		}
 		$this->getBoard()->setStacks(array_values($stacks));
 	}
@@ -238,6 +285,7 @@ class BoardImportService {
 	public function importCards(): void {
 		$cards = $this->getImportSystem()->getCards();
 		foreach ($cards as $code => $card) {
+			try {
 				$createdAt = $card->getCreatedAt();
 				$lastModified = $card->getLastModified();
 				$this->cardMapper->insert($card);
@@ -254,6 +302,9 @@ class BoardImportService {
 					$this->cardMapper->update($card, false);
 				}
 				$this->getImportSystem()->updateCard($code, $card);
+			} catch (\Exception $e) {
+				$this->addError('Failed to import card ' . $card->getTitle(), $e);
+			}
 		}
 	}
 
@@ -274,11 +325,15 @@ class BoardImportService {
 		$data = $this->getImportSystem()->getCardLabelAssignment();
 		foreach ($data as $cardId => $assignemnt) {
 			foreach ($assignemnt as $assignmentId => $labelId) {
+				try {
 					$this->assignCardToLabel(
 						$cardId,
 						$labelId
 					);
 					$this->getImportSystem()->updateCardLabelsAssignment($cardId, $assignmentId, $labelId);
+				} catch (\Exception $e) {
+					$this->addError('Failed to assign label ' . $labelId . ' to ' . $cardId, $e);
+				}
 			}
 		}
 	}
@@ -320,9 +375,14 @@ class BoardImportService {
 	public function importCardAssignments(): void {
 		$allAssignments = $this->getImportSystem()->getCardAssignments();
 		foreach ($allAssignments as $cardId => $assignments) {
-			foreach ($assignments as $assignmentId => $assignment) {
-				$this->assignmentMapper->insert($assignment);
-				$this->getImportSystem()->updateCardAssignment($cardId, $assignmentId, $assignment);
+			foreach ($assignments as $assignment) {
+				try {
+					$assignment = $this->assignmentMapper->insert($assignment);
+					$this->getImportSystem()->updateCardAssignment($cardId, (string)$assignment->getId(), $assignment);
+					$this->addOutput('Assignment ' . $assignment->getParticipant() . ' added');
+				} catch (NotFoundException $e) {
+					$this->addError('No origin or mapping found for card "' . $cardId . '" and ' . $assignment->getTypeString() .' assignment "' . $assignment->getParticipant(), $e);
+				}
 			}
 		}
 	}

tests/stub.phpstub

@@ -193,6 +193,8 @@ namespace Symfony\Component\Console\Output {
 	class OutputInterface {
 		public const VERBOSITY_VERBOSE = 1;
 		public function writeln($text, int $flat = 0) {}
+		public function isVerbose(): bool {}
+		public function isVeryVerbose(): bool {}
 	}
 }
 

tests/unit/Service/Importer/BoardImportServiceTest.php

@@ -43,6 +43,7 @@ use OCP\IDBConnection;
 use OCP\IUser;
 use OCP\IUserManager;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 
 class BoardImportServiceTest extends \Test\TestCase {
 	/** @var IDBConnection|MockObject */
@@ -92,7 +93,8 @@ class BoardImportServiceTest extends \Test\TestCase {
 			$this->attachmentMapper,
 			$this->cardMapper,
 			$this->commentsManager,
-			$this->eventDispatcher
+			$this->eventDispatcher,
+			$this->createMock(LoggerInterface::class),
 		);
 
 		$this->boardImportService->setSystem('trelloJson');
@@ -145,6 +147,9 @@ class BoardImportServiceTest extends \Test\TestCase {
 	}
 
 	public function testImportSuccess() {
+		$this->userManager->method('userExists')
+			->willReturn(true);
+
 		$this->boardMapper
 			->expects($this->once())
 			->method('insert');