pkp/pkp-lib#10157 fixed queries and added migration

Hafsa-Naeem · Aug 25, 2024 · 70fc656 · 70fc656
1 parent 7cfb8f3
commit 70fc656
Show file tree

Hide file tree

Showing 9 changed files with 108 additions and 72 deletions.
diff --git a/classes/migration/install/OJSMigration.php b/classes/migration/install/OJSMigration.php
@@ -38,7 +38,7 @@ public function up(): void
             $table->foreign('review_form_id', 'sections_review_form_id')->references('review_form_id')->on('review_forms')->onDelete('set null');
             $table->index(['review_form_id'], 'sections_review_form_id');
 
-            $table->float('seq', 8, 2)->default(0);
+            $table->float('seq')->default(0);
             $table->smallInteger('editor_restricted')->default(0);
             $table->smallInteger('meta_indexed')->default(0);
             $table->smallInteger('meta_reviewed')->default(1);
@@ -114,12 +114,12 @@ public function up(): void
             $table->unique(['issue_id', 'locale', 'setting_name'], 'issue_settings_unique');
         });
         // Add partial index (DBMS-specific)
-        switch (DB::getDriverName()) {
-            case 'mysql': DB::unprepared('CREATE INDEX issue_settings_name_value ON issue_settings (setting_name(50), setting_value(150))');
-                break;
-            case 'pgsql': DB::unprepared("CREATE INDEX issue_settings_name_value ON issue_settings (setting_name, setting_value) WHERE setting_name IN ('medra::registeredDoi', 'datacite::registeredDoi')");
-                break;
-        }
+        match (DB::getDriverName()) {
+            'mysql', 'mariadb' =>
+                DB::unprepared('CREATE INDEX issue_settings_name_value ON issue_settings (setting_name(50), setting_value(150))'),
+            'pgsql' =>
+                DB::unprepared("CREATE INDEX issue_settings_name_value ON issue_settings (setting_name, setting_value) WHERE setting_name IN ('medra::registeredDoi', 'datacite::registeredDoi')")
+        };
 
         Schema::create('issue_files', function (Blueprint $table) {
             $table->comment('Relationships between issues and issue files, such as cover images.');
@@ -154,7 +154,7 @@ public function up(): void
             $table->index(['file_id'], 'issue_galleys_file_id');
 
             $table->string('label', 255)->nullable();
-            $table->float('seq', 8, 2)->default(0);
+            $table->float('seq')->default(0);
             $table->string('url_path', 64)->nullable();
 
             $table->index(['url_path'], 'issue_galleys_url_path');
@@ -189,7 +189,7 @@ public function up(): void
             $table->foreign('journal_id', 'custom_issue_orders_journal_id')->references('journal_id')->on('journals')->onDelete('cascade');
             $table->index(['journal_id'], 'custom_issue_orders_journal_id');
 
-            $table->float('seq', 8, 2)->default(0);
+            $table->float('seq')->default(0);
 
             $table->unique(['issue_id'], 'custom_issue_orders_unique');
         });
@@ -206,7 +206,7 @@ public function up(): void
             $table->foreign('section_id', 'custom_section_orders_section_id')->references('section_id')->on('sections')->onDelete('cascade');
             $table->index(['section_id'], 'custom_section_orders_section_id');
 
-            $table->float('seq', 8, 2)->default(0);
+            $table->float('seq')->default(0);
 
             $table->unique(['issue_id', 'section_id'], 'custom_section_orders_unique');
         });
@@ -228,7 +228,7 @@ public function up(): void
             $table->foreign('section_id', 'publications_section_id')->references('section_id')->on('sections')->onDelete('set null');
             $table->index(['section_id'], 'publications_section_id');
 
-            $table->float('seq', 8, 2)->default(0);
+            $table->float('seq')->default(0);
 
             $table->bigInteger('submission_id');
             $table->foreign('submission_id', 'publications_submission_id')->references('submission_id')->on('submissions')->onDelete('cascade');
@@ -243,7 +243,7 @@ public function up(): void
             $table->index(['doi_id'], 'publications_doi_id');
 
             $table->bigInteger('issue_id')->nullable();
-            $table->foreign('issue_id')->references('issue_id')->on('issues')->onDelete('set null');
+            $table->foreign('issue_id')->references('issue_id')->on('issues')->nullOnDelete();
             $table->index(['issue_id'], 'publications_issue_id_index');
 
             $table->index(['url_path'], 'publications_url_path');
@@ -278,7 +278,7 @@ public function up(): void
             $table->foreign('submission_file_id')->references('submission_file_id')->on('submission_files');
             $table->index(['submission_file_id'], 'publication_galleys_submission_file_id');
 
-            $table->float('seq', 8, 2)->default(0);
+            $table->float('seq')->default(0);
             $table->string('remote_url', 2047)->nullable();
             $table->smallInteger('is_approved')->default(0);
             $table->string('url_path', 64)->nullable();
@@ -306,12 +306,12 @@ public function up(): void
             $table->unique(['galley_id', 'locale', 'setting_name'], 'publication_galley_settings_unique');
         });
         // Add partial index (DBMS-specific)
-        switch (DB::getDriverName()) {
-            case 'mysql': DB::unprepared('CREATE INDEX publication_galley_settings_name_value ON publication_galley_settings (setting_name(50), setting_value(150))');
-                break;
-            case 'pgsql': DB::unprepared('CREATE INDEX publication_galley_settings_name_value ON publication_galley_settings (setting_name, setting_value)');
-                break;
-        }
+        match (DB::getDriverName()) {
+            'mysql', 'mariadb' =>
+                DB::unprepared('CREATE INDEX publication_galley_settings_name_value ON publication_galley_settings (setting_name(50), setting_value(150))'),
+            'pgsql' =>
+                DB::unprepared('CREATE INDEX publication_galley_settings_name_value ON publication_galley_settings (setting_name, setting_value)')
+        };
 
         // Subscription types.
         Schema::create('subscription_types', function (Blueprint $table) {
@@ -322,14 +322,14 @@ public function up(): void
             $table->foreign('journal_id', 'subscription_types_journal_id')->references('journal_id')->on('journals')->onDelete('cascade');
             $table->index(['journal_id'], 'subscription_types_journal_id');
 
-            $table->float('cost', 8, 2);
+            $table->decimal('cost', 8, 2)->unsigned();
             $table->string('currency_code_alpha', 3);
             $table->smallInteger('duration')->nullable();
             $table->smallInteger('format');
             $table->smallInteger('institutional')->default(0);
             $table->smallInteger('membership')->default(0);
             $table->smallInteger('disable_public_display');
-            $table->float('seq', 8, 2);
+            $table->float('seq');
         });
 
         // Locale-specific subscription type data
@@ -419,7 +419,7 @@ public function up(): void
             $table->index(['user_id'], 'completed_payments_user_id');
 
             $table->bigInteger('assoc_id')->nullable();
-            $table->float('amount', 8, 2);
+            $table->decimal('amount', 8, 2)->unsigned();
             $table->string('currency_code_alpha', 3)->nullable();
             $table->string('payment_method_plugin_name', 80)->nullable();
         });

diff --git a/classes/migration/upgrade/v3_4_0/I7901_Duplicate_OAI_IDs.php b/classes/migration/upgrade/v3_4_0/I7901_Duplicate_OAI_IDs.php
@@ -21,30 +21,30 @@ class I7901_Duplicate_OAI_IDs extends \PKP\migration\Migration
      */
     public function up(): void
     {
-        switch (DB::getDriverName()) {
-            case 'mysql':
+        match (DB::getDriverName()) {
+            'mysql', 'mariadb' =>
                 DB::unprepared(
-                    'DELETE dot
+                    "DELETE dot
                     FROM data_object_tombstones dot
                     JOIN submissions s ON (dot.data_object_id = s.submission_id)
                     JOIN journals j ON (j.journal_id = s.context_id)
                     JOIN publications p ON (s.current_publication_id = p.publication_id)
-                    JOIN issues i ON (i.issue_id = p.issue_id)
-                    WHERE i.published = 1 AND j.enabled = 1 AND p.status = 3'
-                );
-                break;
-            case 'pgsql':
+                    JOIN publication_settings psissue ON (psissue.publication_id = p.publication_id AND psissue.setting_name='issueId' AND psissue.locale='')
+                    JOIN issues i ON (CAST(i.issue_id AS CHAR(20)) = psissue.setting_value)
+                    WHERE i.published = 1 AND j.enabled = 1 AND p.status = 3"
+                ),
+            'pgsql' =>
                 DB::unprepared(
-                    'DELETE FROM data_object_tombstones dot
-                    USING submissions s, journals j, publications p, issues i
+                    "DELETE FROM data_object_tombstones dot
+                    USING submissions s, journals j, publications p, publication_settings psissue, issues i
                     WHERE dot.data_object_id = s.submission_id
                     AND j.journal_id = s.context_id
                     AND s.current_publication_id = p.publication_id
-                    AND i.issue_id = p.issue_id
-                    AND i.published = 1 AND j.enabled = 1 AND p.status = 3'
-                );
-                break;
-        }
+                    AND psissue.publication_id = p.publication_id
+                    AND psissue.setting_name='issueId' AND psissue.locale='' AND (CAST(i.issue_id AS CHAR(20)) = psissue.setting_value)
+                    AND i.published = 1 AND j.enabled = 1 AND p.status = 3"
+                )
+        };
     }
 
     /**

diff --git a/classes/migration/upgrade/v3_4_0/PreflightCheckMigration.php b/classes/migration/upgrade/v3_4_0/PreflightCheckMigration.php
@@ -90,23 +90,9 @@ protected function buildOrphanedEntityProcessor(): void
             }
             $affectedRows += $this->deleteOptionalReference('publications', 'section_id', 'sections', 'section_id');
             // Remaining cleanups are inherited
-            $rows = DB::table('publications AS p')
-                ->leftJoin('issues AS i', 'p.issue_id', '=', 'i.issue_id')
-                ->whereNull('i.issue_id')
-                ->whereNotNull('p.issue_id')  // Ensure we're only looking at non-null issue_ids
-                ->select('p.submission_id', 'p.publication_id', 'p.issue_id')
-                ->get();
-
-            foreach ($rows as $row) {
-                $this->_installer->log("The publication ID ({$row->publication_id}) for the submission ID {$row->submission_id} is assigned to an invalid issue ID \"{$row->issue_id}\", its issue_id will be set to NULL.");
-                $affectedRows += DB::table('publications')
-                    ->where('publication_id', '=', $row->publication_id)
-                    ->update(['issue_id' => null]);  // Reset invalid issue_id to null
-            }
             return $affectedRows;
         });
 
-
         $this->addTableProcessor('publication_galleys', function (): int {
             $affectedRows = 0;
             // Depends directly on ~3 entities: doi_id->dois.doi_id(not found in previous version) publication_id->publications.publication_id submission_file_id->submission_files.submission_file_id
@@ -243,6 +229,25 @@ protected function buildOrphanedEntityProcessor(): void
             return $affectedRows;
         });
 
+        // Support for the issueId setting
+        $this->addTableProcessor('publication_settings', function (): int {
+            $affectedRows = 0;
+            $rows = DB::table('publications AS p')
+                ->join('publication_settings AS ps', 'ps.publication_id', '=', 'p.publication_id')
+                ->leftJoin('issues AS i', 'ps.setting_value', '=', DB::raw('CAST(i.issue_id AS CHAR(20))'))
+                ->where('ps.setting_name', 'issueId')
+                ->whereNull('i.issue_id')
+                ->get(['p.submission_id', 'p.publication_id', 'ps.setting_value']);
+            foreach ($rows as $row) {
+                $this->_installer->log("The publication ID ({$row->publication_id}) for the submission ID {$row->submission_id} is assigned to an invalid issue ID \"{$row->setting_value}\", its value will be updated to NULL");
+                $affectedRows += DB::table('publication_settings')
+                    ->where('publication_id', '=', $row->publication_id)
+                    ->where('setting_name', 'issueId')
+                    ->where('setting_value', $row->setting_value)
+                    ->delete();
+            }
+            return $affectedRows;
+        });
     }
 
     protected function getEntityRelationships(): array
@@ -296,7 +301,7 @@ protected function getEntityRelationships(): array
     protected function dropForeignKeys(): void
     {
         parent::dropForeignKeys();
-        if (DB::getDoctrineSchemaManager()->introspectTable('publication_galleys')->hasForeignKey('publication_galleys_submission_file_id_foreign')) {
+        if ($this->hasForeignKey('publication_galleys', 'publication_galleys_submission_file_id_foreign')) {
             Schema::table('publication_galleys', fn (Blueprint $table) => $table->dropForeign('publication_galleys_submission_file_id_foreign'));
         }
     }

diff --git a/classes/migration/upgrade/v3_5_0/I10157_CreateAndMigrateIssueIdField.php b/classes/migration/upgrade/v3_5_0/I10157_CreateAndMigrateIssueIdField.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace APP\migration\upgrade\v3_5_0;
+
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Schema;
+use PKP\migration\Migration;
+
+class I10157_CreateAndMigrateIssueIdField extends Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        // adding new column to the publications table
+        Schema::table('publications', function (Blueprint $table) {
+            $table->bigInteger('issue_id')->nullable()->after('submission_id');
+        });
+
+        // migrating data from publication_settings to the new issue_id column
+        DB::statement('UPDATE publications p JOIN publication_settings ps ON p.publication_id = ps.publication_id AND ps.setting_name = "issueId" SET p.issue_id = ps.setting_value');
+
+        // clear the old data of issueId in publication_settings
+        DB::table('publication_settings')->where('setting_name', 'issueId')->delete();
+    }
+
+    /**
+     * Reverse the migrations.
+     */
+    public function down(): void
+    {
+        // remove the column in case rolling back
+        Schema::table('publications', function (Blueprint $table) {
+            $table->dropColumn('issue_id');
+        });
+    }
+}
diff --git a/classes/oai/ojs/OAIDAO.php b/classes/oai/ojs/OAIDAO.php
@@ -272,7 +272,6 @@ public function _getRecordsRecordSetQuery($setIds, $from, $until, $set, $submiss
             ])
             ->join('publications AS p', 'a.current_publication_id', '=', 'p.publication_id')
             ->join('issues AS i', 'i.issue_id', '=', 'p.issue_id')
-            ->join('issues AS i', DB::raw('CAST(i.issue_id AS CHAR(20))'), '=', 'psissue.setting_value')
             ->join('sections AS s', 's.section_id', '=', 'p.section_id')
             ->join('journals AS j', 'j.journal_id', '=', 'a.context_id')
             ->where('i.published', '=', 1)

diff --git a/classes/search/ArticleSearchDAO.php b/classes/search/ArticleSearchDAO.php
@@ -92,8 +92,7 @@ public function getPhraseResults($journal, $phrase, $publishedFrom = null, $publ
             FROM
                 submissions s
                 JOIN publications p ON (p.publication_id = s.current_publication_id)
-                JOIN issues i ON (i.issue_id = p.issue_id AND i.journal_id = s.context_id)
-                JOIN issues i ON (CAST(i.issue_id AS CHAR(20)) = ps.setting_value AND i.journal_id = s.context_id)
+                JOIN issues i ON (i.issue_id = p.issue_id)
                 JOIN submission_search_objects o ON (s.submission_id = o.submission_id)
                 JOIN journals j ON j.journal_id = s.context_id
                 LEFT JOIN journal_settings js ON j.journal_id = js.journal_id AND js.setting_name = \'publishingMode\'

diff --git a/classes/services/queryBuilders/StatsGeoQueryBuilder.php b/classes/services/queryBuilders/StatsGeoQueryBuilder.php
@@ -45,13 +45,12 @@ public function filterByIssues(array $issueIds): self
     protected function _getAppSpecificQuery(Builder &$q): void
     {
         if (!empty($this->issueIds)) {
-            $issueSubmissionIds = DB::table('publications as p')
-                ->select('p.submission_id')
-                ->distinct()
-                ->where('p.status', Submission::STATUS_PUBLISHED)
-                ->whereIn('p.issue_id', $this->issueIds);
-            $q->joinSub($issueSubmissionIds, 'is', function ($join) {
-                $join->on('metrics_submission_geo_monthly.' . PKPStatisticsHelper::STATISTICS_DIMENSION_SUBMISSION_ID, '=', 'is.submission_id');
+            $q->whereExists(function ($query) {
+                $query->select(DB::raw(1))
+                    ->from('publications as p')
+                    ->whereRaw('p.submission_id = metrics_submission_geo_monthly.' . PKPStatisticsHelper::STATISTICS_DIMENSION_SUBMISSION_ID)
+                    ->where('p.status', Submission::STATUS_PUBLISHED)
+                    ->whereIn('p.issue_id', $this->issueIds);
             });
         }
     }

diff --git a/classes/submission/Collector.php b/classes/submission/Collector.php
@@ -54,20 +54,12 @@ public function getQueryBuilder(): Builder
 
         // By issue IDs
         if (is_array($this->issueIds)) {
-            $q->whereIn('s.submission_id', function ($query) {
-                $query->select('issue_p.submission_id')
-                    ->from('publications AS issue_p')
-                    ->whereIn('issue_p.issue_id', $this->issueIds);
-            });
+            $q->whereIn('po.issue_id', $this->issueIds);
         }
 
         // By section IDs
         if (is_array($this->sectionIds)) {
-            $q->whereIn('s.submission_id', function ($query) {
-                $query->select('section_p.submission_id')
-                    ->from('publications AS section_p')
-                    ->whereIn('section_p.section_id', $this->sectionIds);
-            });
+            $q->whereIn('po.section_id', $this->sectionIds);
         }
 
         return $q;

diff --git a/dbscripts/xml/upgrade.xml b/dbscripts/xml/upgrade.xml
@@ -68,6 +68,7 @@
 		<migration class="PKP\migration\upgrade\v3_4_0\I2890_AddSetNullForOnDeleteToReviewRoundIdForeign"/>
 		<migration class="APP\migration\upgrade\v3_4_0\I7901_Duplicate_OAI_IDs"/>
 		<migration class="PKP\migration\upgrade\v3_4_0\I7366_UpdateUserAPIKeySettings"/>
+		<migration class="PKP\migration\upgrade\v3_4_0\I7463_LocaleColumn"/>
 		<migration class="PKP\migration\upgrade\v3_4_0\I8093_UpdateUserGroupRelationTablesFK"/>
 		<migration class="APP\migration\upgrade\v3_4_0\I7725_DecisionConstantsUpdate"/>
 		<migration class="PKP\migration\upgrade\v3_4_0\I4789_AddReviewerRequestResentColumns"/>
@@ -128,6 +129,7 @@
 	<upgrade minversion="3.1.0.0" maxversion="3.4.9.9">
 		<migration class="PKP\migration\upgrade\v3_5_0\PreflightCheckMigration" fallback="3.4.9.9" />
 		<migration class="APP\migration\upgrade\v3_5_0\I8333_AddMissingForeignKeys" />
+		<migration class="APP\migration\upgrade\v3_5_0\I9892_FloatToDecimalColumnTypeUpdate" />
 		<migration class="PKP\migration\upgrade\v3_5_0\I9895_AddAppKeyToConfigFile"/>
 		<migration class="PKP\migration\upgrade\v3_5_0\I9678_RemoveScheduledTasksTable"/>
 		<migration class="PKP\migration\upgrade\v3_5_0\InstallEmailTemplates"/>
@@ -146,6 +148,7 @@
 		<migration class="APP\migration\upgrade\v3_5_0\I9937_EditorialTeamToEditorialHistory"/>
 		<migration class="PKP\migration\upgrade\v3_5_0\I10041_UserGroupsAndUserUserGroupsMastheadValues"/>
         <migration class="PKP\migration\upgrade\v3_5_0\I9771_OrcidMigration"/>
+		<migration class="PKP\migration\upgrade\v3_5_0\I10157_CreateAndMigrateIssueIdField.php"/>
 	</upgrade>
 
 	<!-- update plugin configuration - should be done as the final upgrade task -->