Merge pull request pkp#3946 from jonasraoni/feature/main/8333-add-rem…

…aining-fks

Feature/main/8333 Add remaining FKs

api/v1/issues/IssueController.php

@@ -199,7 +199,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
         Hook::call('API::issues::params', [&$collector, $illuminateRequest]);
 
         // You must be a manager or site admin to access unpublished Issues
-        $isAdmin = $currentUser->hasRole([Role::ROLE_ID_MANAGER], $context->getId()) || $currentUser->hasRole([Role::ROLE_ID_SITE_ADMIN], \PKP\core\PKPApplication::CONTEXT_SITE);
+        $isAdmin = $currentUser->hasRole([Role::ROLE_ID_MANAGER], $context->getId()) || $currentUser->hasRole([Role::ROLE_ID_SITE_ADMIN], \PKP\core\PKPApplication::SITE_CONTEXT_ID);
         if (isset($collector->isPublished) && !$collector->isPublished && !$isAdmin) {
             return response()->json([
                 'error' => __('api.submissions.403.unpublishedIssues'),

classes/issue/Collector.php

@@ -13,6 +13,7 @@
 
 namespace APP\issue;
 
+use APP\core\Application;
 use APP\facades\Repo;
 use Exception;
 use Illuminate\Database\Query\Builder;
@@ -21,7 +22,6 @@
 use Illuminate\Support\LazyCollection;
 use InvalidArgumentException;
 use PKP\core\interfaces\CollectorInterface;
-use PKP\core\PKPApplication;
 use PKP\plugins\Hook;
 
 class Collector implements CollectorInterface
@@ -42,7 +42,7 @@ class Collector implements CollectorInterface
 
     public ?int $offset = null;
 
-    /** @var array|null Context ID or PKPApplication::CONTEXT_ID_ALL to get from all contexts */
+    /** @var array|null Context ID */
     public ?array $contextIds = null;
 
     /** @var array|null List of issue IDs to include */
@@ -327,15 +327,14 @@ public function getQueryBuilder(): Builder
             )
         );
 
-        // Context
-        // Never permit a query without a context_id unless the PKPApplication::CONTEXT_ID_ALL wildcard
-        // has been set explicitly.
+        // Context: Never permit a query without a context_id unless the Application::SITE_CONTEXT_ID_ALL wildcard has been set explicitly.
         if (!isset($this->contextIds)) {
-            throw new Exception('Submissions can not be retrieved without a context id. Pass the Application::CONTEXT_ID_ALL wildcard to get submissions from any context.');
-        } elseif (!in_array(PKPApplication::CONTEXT_ID_ALL, $this->contextIds)) {
-            $q->whereIn('i.journal_id', $this->contextIds);
+            throw new Exception('Issues cannot be retrieved without a context id. Pass the Application::SITE_CONTEXT_ID_ALL wildcard to get issues from any context.');
         }
 
+        if (!in_array(Application::SITE_CONTEXT_ID_ALL, $this->contextIds)) {
+            $q->whereIn('i.journal_id', $this->contextIds);
+        }
         // Issue IDs
         $q->when($this->issueIds !== null, fn (Builder $q) => $q->whereIn('i.issue_id', $this->issueIds));
         // Published

classes/issue/DAO.php

@@ -213,7 +213,7 @@ public function resequenceCustomIssueOrders(int $contextId)
                     ],
                     [
                         'issue_id' => $item->issue_id,
-                        'journal_id' => (int) $contextId,
+                        'journal_id' => $contextId,
                         'seq' => $newSeq
                     ],
                 );
@@ -238,8 +238,8 @@ public function customIssueOrderingExists(int $contextId): bool
     public function getCustomIssueOrder(int $contextId, int $issueId): ?int
     {
         $results = DB::table('custom_issue_orders')
-            ->where('journal_id', '=', (int) $contextId)
-            ->where('issue_id', '=', (int) $issueId);
+            ->where('journal_id', '=', $contextId)
+            ->where('issue_id', '=', $issueId);
 
         $row = $results->first();
         return $row ? (int) $row->seq : null;
@@ -358,7 +358,7 @@ public function deleteAllPubIds(int $contextId, string $pubIdType): int
      *
      * From legacy IssueDAO
      *
-     * @param int $contextId optional
+     * @param int $contextId
      * @param string $pubIdType
      * @param string $pubIdSettingName optional
      * (e.g. crossref::registeredDoi)
@@ -374,7 +374,7 @@ public function getExportable($contextId, $pubIdType = null, $pubIdSettingName =
             ->when($pubIdType != null, fn (Builder $q) => $q->leftJoin('issue_settings AS ist', 'i.issue_id', '=', 'ist.issue_id'))
             ->when($pubIdSettingName, fn (Builder $q) => $q->leftJoin('issue_settings AS iss', fn (JoinClause $j) => $j->on('i.issue_id', '=', 'iss.issue_id')->where('iss.setting_name', '=', $pubIdSettingName)))
             ->where('i.published', '=', 1)
-            ->where('i.journal_id', '=', $contextId)
+            ->where('i.journal_id', '=', (int) $contextId)
             ->when($pubIdType != null, fn (Builder $q) => $q->where('ist.setting_name', '=', "pub-id::{$pubIdType}")->whereNotNull('ist.setting_value'))
             ->when(
                 $pubIdSettingName,

classes/journal/JournalDAO.php

@@ -147,15 +147,13 @@ public function anyPubIdExists(
      * Sets current_issue_id for context to null.
      * This is necessary because current_issue_id should explicitly be set to null rather than unset.
      *
-     * @param int $contextId
-     *
      * @return int
      */
-    public function removeCurrentIssue($contextId)
+    public function removeCurrentIssue(int $contextId)
     {
         return $this->update(
             "UPDATE {$this->tableName} SET current_issue_id = null WHERE {$this->primaryKeyColumn} = ?",
-            [(int) $contextId]
+            [$contextId]
         );
     }
 }

classes/migration/upgrade/v3_4_0/PreflightCheckMigration.php

@@ -234,7 +234,7 @@ protected function buildOrphanedEntityProcessor(): void
             $affectedRows = 0;
             $rows = DB::table('publications AS p')
                 ->join('publication_settings AS ps', 'ps.publication_id', '=', 'p.publication_id')
-                ->leftJoin('issues AS i', DB::raw('CAST(i.issue_id AS CHAR(20))'), '=', 'ps.setting_value')
+                ->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']);

classes/migration/upgrade/v3_5_0/I8333_AddMissingForeignKeys.php

@@ -0,0 +1,33 @@
+<?php
+
+/**
+ * @file classes/migration/upgrade/v3_5_0/I8333_AddMissingForeignKeys.php
+ *
+ * Copyright (c) 2023 Simon Fraser University
+ * Copyright (c) 2023 John Willinsky
+ * Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
+ *
+ * @class I8333_AddMissingForeignKeys
+ *
+ * @brief Upgrade/downgrade operations for introducing foreign key definitions to existing database relationships.
+ */
+
+namespace APP\migration\upgrade\v3_5_0;
+
+class I8333_AddMissingForeignKeys extends \PKP\migration\upgrade\v3_5_0\I8333_AddMissingForeignKeys
+{
+    protected function getContextTable(): string
+    {
+        return 'journals';
+    }
+
+    protected function getContextKeyField(): string
+    {
+        return 'journal_id';
+    }
+
+    protected function getContextSettingsTable(): string
+    {
+        return 'journal_settings';
+    }
+}

classes/observers/events/UsageEvent.php

@@ -34,11 +34,11 @@ class UsageEvent extends \PKP\observers\events\UsageEvent
     public function __construct(
         int $assocType,
         Context $context,
-        Submission $submission = null,
-        Representation $galley = null,
-        SubmissionFile $submissionFile = null,
-        Issue $issue = null,
-        IssueGalley $issueGalley = null
+        ?Submission $submission = null,
+        ?Representation $galley = null,
+        ?SubmissionFile $submissionFile = null,
+        ?Issue $issue = null,
+        ?IssueGalley $issueGalley = null
     ) {
         $this->issue = $issue;
         $this->issueGalley = $issueGalley;

classes/payment/ojs/OJSCompletedPaymentDAO.php

@@ -19,6 +19,7 @@
 
 namespace APP\payment\ojs;
 
+use APP\core\Application;
 use Illuminate\Support\Facades\DB;
 use PKP\core\Core;
 use PKP\db\DAOResultFactory;
@@ -30,20 +31,19 @@ class OJSCompletedPaymentDAO extends \PKP\db\DAO
     /**
      * Retrieve a CompletedPayment by its ID.
      *
-     * @param int $completedPaymentId
      * @param int $contextId optional
      *
      * @return CompletedPayment
      */
-    public function getById($completedPaymentId, $contextId = null)
+    public function getById(int $completedPaymentId, int $contextId = Application::SITE_CONTEXT_ID_ALL)
     {
-        $params = [(int) $completedPaymentId];
-        if ($contextId) {
-            $params[] = (int) $contextId;
+        $params = [$completedPaymentId];
+        if ($contextId !== Application::SITE_CONTEXT_ID_ALL) {
+            $params[] = $contextId;
         }
 
         $result = $this->retrieve(
-            'SELECT * FROM completed_payments WHERE completed_payment_id = ?' . ($contextId ? ' AND context_id = ?' : ''),
+            'SELECT * FROM completed_payments WHERE completed_payment_id = ?' . ($contextId !== Application::SITE_CONTEXT_ID_ALL ? ' AND context_id = ?' : ''),
             $params
         );
         $row = $result->current();
@@ -192,16 +192,15 @@ public function hasPaidPublication($userId, $articleId)
     /**
      * Retrieve an array of payments for a particular context ID.
      *
-     * @param int $contextId
      * @param ?DBResultRange $rangeInfo
      *
      * @return array Matching payments
      */
-    public function getByContextId($contextId, $rangeInfo = null)
+    public function getByContextId(int $contextId, $rangeInfo = null)
     {
         $result = $this->retrieveRange(
             'SELECT * FROM completed_payments WHERE context_id = ? ORDER BY timestamp DESC',
-            [(int) $contextId],
+            [$contextId],
             $rangeInfo
         );
 

classes/publication/Publication.php

@@ -27,11 +27,10 @@ class Publication extends PKPPublication
     /**
      * Get the URL to a localized cover image
      *
-     * @param int $contextId
      *
      * @return string
      */
-    public function getLocalizedCoverImageUrl($contextId)
+    public function getLocalizedCoverImageUrl(int $contextId)
     {
         $coverImage = $this->getLocalizedData('coverImage');
 

classes/section/Section.php

@@ -36,7 +36,7 @@ public function getAbbrev(?string $locale): string|array|null
         return $this->getData('abbrev', $locale);
     }
 
-    public function setAbbrev(string|array $abbrev, string $locale = null): void
+    public function setAbbrev(string|array $abbrev, ?string $locale = null): void
     {
         $this->setData('abbrev', $abbrev, $locale);
     }
@@ -51,7 +51,7 @@ public function getPolicy(?string $locale): string|array|null
         return $this->getData('policy', $locale);
     }
 
-    public function setPolicy(string|array $policy, string $locale = null): void
+    public function setPolicy(string|array $policy, ?string $locale = null): void
     {
         $this->setData('policy', $policy, $locale);
     }
@@ -186,7 +186,7 @@ public function getIdentifyType(?string $locale): string|array|null
     /**
      * Set string identifying type of items in this section.
      */
-    public function setIdentifyType(string|array $identifyType, string $locale = null): void
+    public function setIdentifyType(string|array $identifyType, ?string $locale = null): void
     {
         $this->setData('identifyType', $identifyType, $locale);
     }

classes/services/StatsIssueService.php

@@ -131,10 +131,8 @@ public function getDefaultArgs(): array
         return [
             'dateStart' => StatisticsHelper::STATISTICS_EARLIEST_DATE,
             'dateEnd' => date('Y-m-d', strtotime('yesterday')),
-
-            // Require a context to be specified to prevent unwanted data leakage
-            // if someone forgets to specify the context.
-            'contextIds' => [\PKP\core\PKPApplication::CONTEXT_ID_NONE],
+            // Require a context to be specified to prevent unwanted data leakage if someone forgets to specify the context.
+            'contextIds' => [],
         ];
     }
 

classes/services/queryBuilders/StatsIssueQueryBuilder.php

@@ -19,6 +19,7 @@
 
 use APP\core\Application;
 use APP\statistics\StatisticsHelper;
+use Exception;
 use Illuminate\Database\Query\Builder;
 use Illuminate\Support\Facades\DB;
 use PKP\plugins\Hook;
@@ -103,7 +104,11 @@ protected function _getObject(): Builder
     {
         $q = DB::table('metrics_issue');
 
-        if (!empty($this->contextIds)) {
+        if (empty($this->contextIds)) {
+            throw new Exception('Statistics cannot be retrieved without a context id. Pass the Application::SITE_CONTEXT_ID_ALL wildcard to get statistics from all contexts.');
+        }
+
+        if (!in_array(Application::SITE_CONTEXT_ID_ALL, $this->contextIds)) {
             $q->whereIn(StatisticsHelper::STATISTICS_DIMENSION_CONTEXT_ID, $this->contextIds);
         }
 

classes/submission/DAO.php

@@ -46,7 +46,7 @@ public function deleteById(int $id): int
      *
      * @return DAOResultFactory<Submission>
      */
-    public function getExportable($contextId, $pubIdType = null, $title = null, $author = null, $issueId = null, $pubIdSettingName = null, $pubIdSettingValue = null, $rangeInfo = null)
+    public function getExportable(int $contextId, $pubIdType = null, $title = null, $author = null, $issueId = null, $pubIdSettingName = null, $pubIdSettingValue = null, $rangeInfo = null)
     {
         $q = DB::table('submissions', 's')
             ->leftJoin('publications AS p', 's.current_publication_id', '=', 'p.publication_id')

dbscripts/xml/upgrade.xml

@@ -123,6 +123,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="PKP\migration\upgrade\v3_5_0\I9895_AddAppKeyToConfigFile"/>
 		<migration class="PKP\migration\upgrade\v3_5_0\InstallEmailTemplates"/>
 		<migration class="PKP\migration\upgrade\v3_5_0\I9197_MigrateAccessKeys"/>
@@ -137,7 +138,6 @@
 		<migration class="PKP\migration\upgrade\v3_5_0\I9566_SessionUpgrade"/>
 		<migration class="PKP\migration\upgrade\v3_5_0\I9809_ReviewCancelDate"/>
 		<migration class="PKP\migration\upgrade\v3_5_0\I9860_EditorialMastheadNavMenuItem"/>
-		<migration class="PKP\migration\upgrade\v3_5_0\I8826_AddMissingForeignKeys" />
 		<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"/>

pages/issue/IssueHandler.php

@@ -326,7 +326,7 @@ public function userCanViewGalley($request)
      * 	                                                            will be displayed.
      * @param bool                      $withSubscriptionDetails    Should include the subscription related information into the template
      */
-    public static function _setupIssueTemplate(Request $request, Issue $issue, Journal $journal = null, $showToc = false, $withSubscriptionDetails = true)
+    public static function _setupIssueTemplate(Request $request, Issue $issue, ?Journal $journal = null, $showToc = false, $withSubscriptionDetails = true)
     {
         $journal ??= $request->getJournal();
         $templateMgr = TemplateManager::getManager($request);

pages/oai/OAIHandler.php

@@ -16,6 +16,7 @@
 
 namespace APP\pages\oai;
 
+use APP\core\Application;
 use APP\handler\Handler;
 use APP\oai\ojs\JournalOAI;
 use Firebase\JWT\Key;
@@ -42,7 +43,7 @@ public function index($args, $request)
         PluginRegistry::loadCategory('oaiMetadataFormats', true);
 
         $oai = new JournalOAI(new OAIConfig($request->url(null, 'oai'), Config::getVar('oai', 'repository_id')));
-        if (!$request->getJournal() && $request->getRouter()->getRequestedContextPath($request) != 'index') {
+        if (!$request->getJournal() && $request->getRouter()->getRequestedContextPath($request) != Application::SITE_CONTEXT_PATH) {
             $dispatcher = $request->getDispatcher();
             return $dispatcher->handle404();
         }

tools/resolveAgencyDuplicates.php

@@ -8,8 +8,8 @@
 
 class resolveAgencyDuplicates extends \PKP\cliTool\CommandLineTool
 {
-    private string|null $command = null;
-    private string|null $agency_name = null;
+    private ?string $command = null;
+    private ?string $agency_name = null;
     private bool $forceFlag = false;
     /**
      * List of potential agencies to choose from along with related fields for resolution.