From bc81b3ca9cb0ea6b5ca6bd18605054bf0ee75871 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Thu, 26 Mar 2026 13:56:18 +0000 Subject: [PATCH] Fixed review suggestions --- app/plugins/EnvSource/config/plugin.json | 2 +- app/resources/locales/en_US/information.po | 5 ++- app/src/Command/UpgradeCommand.php | 42 +++++++++------------- 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/app/plugins/EnvSource/config/plugin.json b/app/plugins/EnvSource/config/plugin.json index e9243c991..ee0be22e5 100644 --- a/app/plugins/EnvSource/config/plugin.json +++ b/app/plugins/EnvSource/config/plugin.json @@ -28,7 +28,7 @@ "id": {}, "external_identity_source_id": {}, "redirect_on_duplicate": { "type": "url" }, - "mva_delimiter": { "type": "string", "size": "5" }, + "mva_delimiter": { "type": "string", "size": "1" }, "sync_on_login": { "type": "boolean"}, "default_affiliation_type_id": { "type": "integer", "foreignkey": { "table": "types", "column": "id" } }, "address_type_id": { "type": "integer", "foreignkey": { "table": "types", "column": "id" } }, diff --git a/app/resources/locales/en_US/information.po b/app/resources/locales/en_US/information.po index ff4d639f9..0351b7595 100644 --- a/app/resources/locales/en_US/information.po +++ b/app/resources/locales/en_US/information.po @@ -183,7 +183,10 @@ msgstr "Executing post-database tasks for version {0}" msgid "ug.tasks.pre" msgstr "Executing pre-database tasks for version {0}" -msgid "ug.tasks.schema.reload" +msgid "ug.tasks.cacheEnvSourcespMode" +msgstr "Caching EnvSource SP modes" + +msgid "ug.schema.reload" msgstr "Force a global schema reload so that post-upgrade tasks are aware of the new schema" msgid "ug.version.current" diff --git a/app/src/Command/UpgradeCommand.php b/app/src/Command/UpgradeCommand.php index 558056ae1..7d17fcf8f 100644 --- a/app/src/Command/UpgradeCommand.php +++ b/app/src/Command/UpgradeCommand.php @@ -48,19 +48,20 @@ class UpgradeCommand extends BaseCommand protected $io = null; /** - * Cache of EnvSource SP Modes partitioned by CO ID. + * Cache of EnvSource SP Modes. * * This array temporarily stores the `sp_mode` values (e.g., 'SH', 'SS', 'O') * from the database prior to schema migration. It allows post-migration tasks * to accurately map the deprecated `sp_mode` column data into the new * `mva_delimiter` column structure. * - * Format: `[ coId => [ envSourceId => 'sp_mode_value' ] ]` + * Format: `[ envSourceId => 'sp_mode_value' ]` * - * @var array> + * @var array */ private array $EnvSourceSpMode = []; + // A list of known versions, must be semantic versioning compliant. The value // is a "blocker" if it is a version that prevents an upgrade from happening. // For example, if a user attempts to upgrade from 1.0.0 to 1.2.0, and 1.1.0 @@ -115,8 +116,8 @@ class UpgradeCommand extends BaseCommand 'checkGroupNames' => ['global' => true], 'createDefaultGroups' => ['perCO' => true, 'perCOU' => true], 'installMostlyStaticPages' => ['perCO' => true], - 'cacheEnvSourcespMode' => ['perCO' => true], - 'assignEnvSourceMvaDelimiter' => ['perCO' => true] + 'cacheEnvSourcespMode' => ['global' => true], + 'assignEnvSourceMvaDelimiter' => ['global' => true] ]; /** @@ -258,7 +259,7 @@ public function execute(Arguments $args, ConsoleIo $io) $this->executeCommand(DatabaseCommand::class); // Force a global schema reload so that post-upgrade tasks are aware of the new schema - $this->io->out(__d('information', 'ug.tasks.schema.reload')); + $this->io->out(__d('information', 'ug.schema.reload')); // Clear all loaded table instances so they are re-instantiated with the new schema TableRegistry::getTableLocator()->clear(); @@ -414,53 +415,44 @@ protected function assignUuids() { /** - * Cache the SP Mode for EnvSource instances associated with a given CO. + * Cache the SP Mode for all EnvSource instances. * This is used prior to database migrations where the sp_mode column is * removed or altered, allowing post-migration tasks to map the old value * to the new schema structure (e.g., mva_delimiter). * * @since COmanage Registry v5.2.0 - * @param int $coId CO ID to cache the configurations for * @return void */ - protected function cacheEnvSourcespMode(int $coId): void + protected function cacheEnvSourcespMode(): void { - $MspsTable = $this->getTableLocator()->get('EnvSource.EnvSources'); + $EnvSourcesTable = $this->getTableLocator()->get('EnvSource.EnvSources'); - $modes = $MspsTable->find('list', [ + $this->EnvSourceSpMode = $EnvSourcesTable->find('list', [ 'keyField' => 'id', 'valueField' => 'sp_mode' ]) - ->innerJoinWith('ExternalIdentitySources') - ->where(['ExternalIdentitySources.co_id' => $coId]) ->applyOptions(['archived' => true]) ->toArray(); - - $this->EnvSourceSpMode[$coId] = $modes; } /** - * Cache the SP Mode for EnvSource instances associated with a given CO. - * This is used prior to database migrations where the sp_mode column is - * removed or altered, allowing post-migration tasks to map the old value - * to the new schema structure (e.g., mva_delimiter). + * Map cached EnvSource SP Modes to the new mva_delimiter column. * - * @param int $coId CO ID to cache the configurations for * @return void * @throws \Exception * @since COmanage Registry v5.2.0 */ - protected function assignEnvSourceMvaDelimiter(int $coId): void + protected function assignEnvSourceMvaDelimiter(): void { - if (empty($this->EnvSourceSpMode[$coId])) { - // Nothing to migrate for this CO + if (empty($this->EnvSourceSpMode)) { + // Nothing to migrate return; } $EnvSourcesTable = $this->getTableLocator()->get('EnvSource.EnvSources'); - // Iterate over the cached id => sp_mode array for this CO - foreach ($this->EnvSourceSpMode[$coId] as $envSourceId => $spMode) { + // Iterate over the cached id => sp_mode array + foreach ($this->EnvSourceSpMode as $envSourceId => $spMode) { $delimiter = ';'; // Map the old SP Mode to the new multi-value delimiter