fix: address the review findings from the beta promotion #1026

l10n/en.js

@@ -1073,7 +1073,11 @@ OC.L10N.register(
         "Application published": "Application published",
         "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.": "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.",
         "Application registered by": "Application registered by",
-        "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions."
+        "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.",
+        "The file could not be sent. Check your connection and try again.": "The file could not be sent. Check your connection and try again.",
+        "The organisation register is not configured, so there are no organisations to choose from.": "The organisation register is not configured, so there are no organisations to choose from.",
+        "The organisations could not be loaded. Reload the page to try again.": "The organisations could not be loaded. Reload the page to try again.",
+        "Whether a service desk is connected could not be loaded. Reload the page to try again.": "Whether a service desk is connected could not be loaded. Reload the page to try again."
     },
     "nplurals=2; plural=(n != 1);"
 )

l10n/en.json

@@ -1072,6 +1072,10 @@
         "Application published": "Application published",
         "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.": "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.",
         "Application registered by": "Application registered by",
-        "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions."
+        "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.",
+        "The file could not be sent. Check your connection and try again.": "The file could not be sent. Check your connection and try again.",
+        "The organisation register is not configured, so there are no organisations to choose from.": "The organisation register is not configured, so there are no organisations to choose from.",
+        "The organisations could not be loaded. Reload the page to try again.": "The organisations could not be loaded. Reload the page to try again.",
+        "Whether a service desk is connected could not be loaded. Reload the page to try again.": "Whether a service desk is connected could not be loaded. Reload the page to try again."
     }
 }

l10n/nl.js

@@ -1143,7 +1143,11 @@ OC.L10N.register(
         "Application published": "Applicatie gepubliceerd",
         "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.": "De publicatiedatum van de applicatie waar deze versie bij hoort, daarvan overgenomen. Een versie is alleen openbaar zolang haar applicatie dat is.",
         "Application registered by": "Applicatie geregistreerd door",
-        "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Wie de applicatie registreerde waar deze versie bij hoort, daarvan overgenomen. De applicatie van een leverancier is openbaar, en haar versies ook."
+        "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Wie de applicatie registreerde waar deze versie bij hoort, daarvan overgenomen. De applicatie van een leverancier is openbaar, en haar versies ook.",
+        "The file could not be sent. Check your connection and try again.": "Het bestand kon niet worden verstuurd. Controleer uw verbinding en probeer het opnieuw.",
+        "The organisation register is not configured, so there are no organisations to choose from.": "Het organisatieregister is niet ingesteld, dus er zijn geen organisaties om uit te kiezen.",
+        "The organisations could not be loaded. Reload the page to try again.": "De organisaties konden niet worden geladen. Herlaad de pagina om het opnieuw te proberen.",
+        "Whether a service desk is connected could not be loaded. Reload the page to try again.": "Kon niet worden geladen of er een servicedesk is gekoppeld. Herlaad de pagina om het opnieuw te proberen."
     },
     "nplurals=2; plural=(n != 1);"
 )

l10n/nl.json

@@ -1142,6 +1142,10 @@
         "Application published": "Applicatie gepubliceerd",
         "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.": "De publicatiedatum van de applicatie waar deze versie bij hoort, daarvan overgenomen. Een versie is alleen openbaar zolang haar applicatie dat is.",
         "Application registered by": "Applicatie geregistreerd door",
-        "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Wie de applicatie registreerde waar deze versie bij hoort, daarvan overgenomen. De applicatie van een leverancier is openbaar, en haar versies ook."
+        "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Wie de applicatie registreerde waar deze versie bij hoort, daarvan overgenomen. De applicatie van een leverancier is openbaar, en haar versies ook.",
+        "The file could not be sent. Check your connection and try again.": "Het bestand kon niet worden verstuurd. Controleer uw verbinding en probeer het opnieuw.",
+        "The organisation register is not configured, so there are no organisations to choose from.": "Het organisatieregister is niet ingesteld, dus er zijn geen organisaties om uit te kiezen.",
+        "The organisations could not be loaded. Reload the page to try again.": "De organisaties konden niet worden geladen. Herlaad de pagina om het opnieuw te proberen.",
+        "Whether a service desk is connected could not be loaded. Reload the page to try again.": "Kon niet worden geladen of er een servicedesk is gekoppeld. Herlaad de pagina om het opnieuw te proberen."
     }
 }

lib/AppInfo/Application.php

@@ -22,6 +22,7 @@
 
 use OCA\OpenRegister\Contract\ObjectServiceInterface;
 use OCA\OpenRegister\Event\ObjectCreatedEvent;
+use OCA\OpenRegister\Event\ObjectDeletedEvent;
 use OCA\OpenRegister\Event\ObjectUpdatedEvent;
 use OCA\OpenRegister\Event\UserProfileUpdatedEvent;
 use OCA\OpenRegister\Service\OrganisationService as OpenRegisterOrganisationService;
@@ -815,6 +816,7 @@ private function registerEventListeners(IRegistrationContext $context): void {
 		// A module version is public only while its application is (publication-field-rules).
 		$context->registerEventListener(ObjectCreatedEvent::class, ModuleVersionPublicationListener::class);
 		$context->registerEventListener(ObjectUpdatedEvent::class, ModuleVersionPublicationListener::class);
+		$context->registerEventListener(ObjectDeletedEvent::class, ModuleVersionPublicationListener::class);
 
 		// Sync user profile updates into the contactpersoon mirror.
 		$context->registerEventListener(UserProfileUpdatedEvent::class, UserProfileUpdatedEventListener::class);

lib/Controller/CmdbImportController.php

@@ -101,7 +101,7 @@ public function import(): JSONResponse {
 				['error' => $e->getErrorCode(), 'details' => $e->getDetails(), 'reason' => $e->getMessage()]
 			);
 			return $this->fromException(e: $e);
-		} catch (\Exception $e) {
+		} catch (\Throwable $e) {
 			$this->logger->error('CmdbImportController: import failed', ['exception' => $e]);
 			return $this->error(code: 'IMPORT_FAILED', status: Http::STATUS_INTERNAL_SERVER_ERROR);
 		}

lib/EventListener/ModuleVersionPublicationListener.php

@@ -21,6 +21,7 @@
 namespace OCA\Stackiq\EventListener;
 
 use OCA\OpenRegister\Event\ObjectCreatedEvent;
+use OCA\OpenRegister\Event\ObjectDeletedEvent;
 use OCA\OpenRegister\Event\ObjectUpdatedEvent;
 use OCA\Stackiq\Service\ModuleVersionPublicationService;
 use OCP\EventDispatcher\Event;
@@ -29,7 +30,7 @@
 use Throwable;
 
 /**
- * Hands every created or updated object to the publication mirror.
+ * Hands every created, updated or deleted object to the publication mirror.
  *
  * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is
  *
@@ -59,21 +60,18 @@ public function __construct(
 	 * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is
 	 */
 	public function handle(Event $event): void {
-		$object = null;
-		if ($event instanceof ObjectUpdatedEvent) {
-			$object = $event->getNewObject();
-		}
-
-		if ($event instanceof ObjectCreatedEvent) {
-			$object = $event->getObject();
-		}
+		try {
+			if ($event instanceof ObjectUpdatedEvent) {
+				$this->publication->objectSaved(object: $event->getNewObject(), previous: $event->getOldObject());
+			}
 
-		if ($object === null) {
-			return;
-		}
+			if ($event instanceof ObjectCreatedEvent) {
+				$this->publication->objectSaved(object: $event->getObject());
+			}
 
-		try {
-			$this->publication->objectSaved(object: $object);
+			if ($event instanceof ObjectDeletedEvent) {
+				$this->publication->objectDeleted(object: $event->getObject());
+			}
 		} catch (Throwable $e) {
 			$this->logger->error('ModuleVersionPublicationListener: could not mirror the publication', ['error' => $e->getMessage()]);
 		}

lib/Repair/BackfillModuleVersionPublication.php

@@ -7,7 +7,9 @@
  * so for anonymous readers they read as unpublished until their module is
  * saved again. That is the safe direction; this step makes the published ones
  * public again without waiting for an edit. It is idempotent: a version that
- * already holds its module's values is not written.
+ * already holds its module's values is not written. It reads the modules page
+ * by page, and after a pass in which nothing failed it records that in the app
+ * config, so later upgrades skip it.
  *
  * @category  Repair
  * @package   OCA\Stackiq\Repair
@@ -27,9 +29,12 @@
 namespace OCA\Stackiq\Repair;
 
 use OCA\OpenRegister\Contract\ObjectEntityInterface;
+use OCA\OpenRegister\Contract\ObjectServiceInterface;
+use OCA\Stackiq\AppInfo\Application;
 use OCA\Stackiq\Service\ModuleVersionPublicationService;
 use OCA\Stackiq\Service\SettingsService;
 use OCP\App\IAppManager;
+use OCP\IAppConfig;
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
 use Throwable;
@@ -41,17 +46,33 @@
  */
 class BackfillModuleVersionPublication implements IRepairStep {
 
+	/**
+	 * App-config key set after a pass in which every module and version was handled.
+	 *
+	 * @var string
+	 */
+	public const DONE_CONFIG_KEY = 'module_version_publication_backfilled';
+
+	/**
+	 * How many modules one read returns.
+	 *
+	 * @var integer
+	 */
+	public const PAGE_SIZE = 200;
+
 	/**
 	 * Constructor.
 	 *
 	 * @param IAppManager                     $appManager      Tells whether OpenRegister is installed.
 	 * @param SettingsService                 $settingsService Resolves the module schema and the object service.
 	 * @param ModuleVersionPublicationService $publication     The mirror.
+	 * @param IAppConfig                      $appConfig       Holds the done marker.
 	 */
 	public function __construct(
 		private readonly IAppManager $appManager,
 		private readonly SettingsService $settingsService,
 		private readonly ModuleVersionPublicationService $publication,
+		private readonly IAppConfig $appConfig,
 	) {
 	}//end __construct()
 
@@ -74,6 +95,10 @@ public function getName(): string {
 	 * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is
 	 */
 	public function run(IOutput $output): void {
+		if ($this->appConfig->getValueBool(Application::APP_ID, self::DONE_CONFIG_KEY, false) === true) {
+			return;
+		}
+
 		if (in_array('openregister', $this->appManager->getInstalledApps(), true) === false) {
 			$output->info('OpenRegister not installed, so there are no versions to update.');
 			return;
@@ -88,19 +113,54 @@ public function run(IOutput $output): void {
 		}
 
 		try {
-			$modules = $objects->setRegister($register)->setSchema($schema)->findAll([], false, false);
+			$result = $this->backfillAll(objects: $objects, register: $register, schema: $schema);
 		} catch (Throwable $e) {
 			$output->warning('Could not read the applications: ' . $e->getMessage());
 			return;
 		}
 
-		$written = 0;
-		foreach ((array) $modules as $module) {
-			if (($module instanceof ObjectEntityInterface) === true) {
-				$written += $this->publication->moduleSaved(module: $module);
-			}
+		$output->info($result['written'] . ' versions now follow their application\'s publication.');
+		if ($result['failed'] > 0) {
+			$output->warning($result['failed'] . ' versions or searches failed; the next upgrade tries again.');
+			return;
 		}
 
-		$output->info($written . ' versions now follow their application\'s publication.');
+		$this->appConfig->setValueBool(Application::APP_ID, self::DONE_CONFIG_KEY, true);
 	}//end run()
+
+	/**
+	 * Backfill every module, page by page.
+	 *
+	 * @param ObjectServiceInterface $objects  The object service.
+	 * @param int|string             $register The module register.
+	 * @param int|string             $schema   The module schema.
+	 *
+	 * @return array{written: int, failed: int} The versions written, and the versions or searches that failed.
+	 *
+	 * @throws Throwable When a page of modules cannot be read.
+	 */
+	private function backfillAll(ObjectServiceInterface $objects, int|string $register, int|string $schema): array {
+		$total  = ['written' => 0, 'failed' => 0];
+		$offset = 0;
+		do {
+			$modules = (array) $objects->setRegister($register)->setSchema($schema)->findAll(
+				['limit' => self::PAGE_SIZE, 'offset' => $offset],
+				false,
+				false
+			);
+
+			foreach ($modules as $module) {
+				if (($module instanceof ObjectEntityInterface) === true) {
+					$result            = $this->publication->backfillModule(module: $module);
+					$total['written'] += $result['written'];
+					$total['failed']  += $result['failed'];
+				}
+			}
+
+			$offset  += self::PAGE_SIZE;
+			$pageSize = count($modules);
+		} while ($pageSize === self::PAGE_SIZE);
+
+		return $total;
+	}//end backfillAll()
 }//end class

lib/Service/ArchiMateImportService.php

@@ -498,8 +498,7 @@ public function importArchiMateFileFromPathOptimized(array $options = []): array
 			);
 
 			if ($this->operationId !== null) {
-				$this->progressTracker->addError(message: $e->getMessage());
-				$this->progressTracker->completeOperation();
+				$this->progressTracker->failOperation(message: $e->getMessage());
 			}
 
 			return [

lib/Service/CmdbExportImportService.php

@@ -217,7 +217,7 @@ public function supportsMissingRecords(string $mode): bool {
 	 *
 	 * @param string $operationId The operation id.
 	 *
-	 * @return bool False when no `cmdb_import` operation has this id.
+	 * @return bool False when no running `cmdb_import` operation has this id.
 	 *
 	 * @spec openspec/changes/cmdb-export-import/tasks.md#task-7
 	 */
@@ -227,7 +227,10 @@ public function requestCancel(string $operationId): bool {
 		}
 
 		$progress = $this->progressTracker->getProgress(operationId: $operationId);
-		if (is_array($progress) === false || ($progress['operation_type'] ?? null) !== self::OPERATION_TYPE) {
+		if (is_array($progress) === false
+			|| ($progress['operation_type'] ?? null) !== self::OPERATION_TYPE
+			|| ($progress['status'] ?? null) !== 'running'
+		) {
 			return false;
 		}
 
@@ -282,25 +285,32 @@ public function import(string $path, array $options): array {
 		$this->progressTracker->setPhase(phase: 'processing_elements', data: ['total_items' => count($rows)]);
 
 		$updateExisting = (($options['updateExisting'] ?? true) !== false);
-		foreach ($rows as $index => $row) {
-			if ($this->progressTracker->isCancelRequested(operationId: $operationId) === true) {
-				$report->markCancelled();
-				break;
-			}
+		try {
+			foreach ($rows as $index => $row) {
+				if ($this->progressTracker->isCancelRequested(operationId: $operationId) === true) {
+					$report->markCancelled();
+					break;
+				}
 
-			$this->processRow(
-				row: $row,
-				municipalityUuid: $municipality['uuid'],
-				updateExisting: $updateExisting,
-				startedAt: $startedAt,
-				date1904: $workbook['date1904'],
-				report: $report
-			);
-			$this->progressTracker->updateProgress(processedItems: ($index + 1));
-		}
+				$this->processRow(
+					row: $row,
+					municipalityUuid: $municipality['uuid'],
+					updateExisting: $updateExisting,
+					startedAt: $startedAt,
+					date1904: $workbook['date1904'],
+					report: $report
+				);
+				$this->progressTracker->updateProgress(processedItems: ($index + 1));
+			}
 
-		$result = $report->toArray();
-		$this->finishOperation(report: $result);
+			$result = $report->toArray();
+			$this->finishOperation(report: $result);
+		} catch (Throwable $e) {
+			// Rows catch their own errors; this is the run itself failing, so the
+			// operation stops as failed instead of staying running until it expires.
+			$this->progressTracker->failOperation(message: $e->getMessage());
+			throw $e;
+		}//end try
 
 		$this->logger->info(
 			'CmdbExportImportService: import finished',