WIP: Code review fixes

classes/components/forms/context/OrcidSettingsForm.php

@@ -1,5 +1,19 @@
 <?php
 
+/**
+ * @file classes/components/form/context/OrcidSettingsForm.php
+ *
+ * Copyright (c) 2014-2024 Simon Fraser University
+ * Copyright (c) 2000-2024 John Willinsky
+ * Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
+ *
+ * @class OrcidSettingsForm
+ *
+ * @ingroup classes_controllers_form
+ *
+ * @brief Add settings for ORCID integration with OJS.
+ */
+
 namespace APP\components\forms\context;
 
 use APP\core\Application;
@@ -46,7 +60,7 @@ public function __construct(string $action, array $locales, \PKP\context\Context
                 'description' => __('orcidProfile.manager.settings.description'),
             ]));
 
-        // ORCID API settings can be configured globally via config file or from this settings form
+        // ORCID API settings can be configured globally via the site settings form or from this settings form
         if (OrcidManager::isGloballyConfigured()) {
             $site = Application::get()->getRequest()->getSite();
 
@@ -129,10 +143,10 @@ public function __construct(string $action, array $locales, \PKP\context\Context
                 'label' => __('orcidProfile.manager.settings.logSectionTitle'),
                 'description' => __('orcidProfile.manager.settings.logLevel.help'),
                 'options' => [
-                    ['value' => 'ERROR', 'label' => __('orcidProfile.manager.settings.logLevel.error')],
-                    ['value' => 'ALL', 'label' => __('orcidProfile.manager.settings.logLevel.all')],
+                    ['value' => OrcidManager::LOG_LEVEL_ERROR, 'label' => __('orcidProfile.manager.settings.logLevel.error')],
+                    ['value' => OrcidManager::LOG_LEVEL_INFO, 'label' => __('orcidProfile.manager.settings.logLevel.all')],
                 ],
-                'value' => $context->getData(OrcidManager::LOG_LEVEL) ?? 'ERROR',
+                'value' => $context->getData(OrcidManager::LOG_LEVEL) ?? OrcidManager::LOG_LEVEL_ERROR,
             ]));
     }
 

classes/migration/upgrade/v3_5_0/OrcidMigration.php → classes/migration/upgrade/v3_5_0/I9771_OrcidMigration.php

@@ -1,13 +1,25 @@
 <?php
 
+/**
+ * @file classes/migration/upgrade/v3_5_0/I9771_OrcidMigration.php
+ *
+ * Copyright (c) 2024 Simon Fraser University
+ * Copyright (c) 2024 John Willinsky
+ * Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
+ *
+ * @class I9771_OrcidMigration
+ *
+ * @brief Move ORCID integration settings from plugin to core application
+ */
+
 namespace APP\migration\upgrade\v3_5_0;
 
 use APP\facades\Repo;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Str;
 use PKP\migration\Migration;
 
-class OrcidMigration extends Migration
+class I9771_OrcidMigration extends Migration
 {
     /**
      * @inheritDoc

classes/orcid/OrcidManager.php

@@ -1,5 +1,17 @@
 <?php
 
+/**
+ * @file classes/orcid/OrcidManager.php
+ *
+ * Copyright (c) 2014-2024 Simon Fraser University
+ * Copyright (c) 2000-2024 John Willinsky
+ * Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
+ *
+ * @class OrcidManager
+ *
+ * @brief Manages using ORCID settings at site and journal level
+ */
+
 namespace APP\orcid;
 
 use APP\author\Author;
@@ -26,24 +38,26 @@ class OrcidManager
     public const ORCID_API_SCOPE_MEMBER = '/activities/update';
 
     public const ENABLED = 'orcidEnabled';
+    // TODO: Remove if not used
     public const PROFILE_API_PATH = 'orcidProfileAPIPath';
     public const CLIENT_ID = 'orcidClientId';
     public const CLIENT_SECRET = 'orcidClientSecret';
     public const SEND_MAIL_TO_AUTHORS_ON_PUBLICATION = 'orcidSendMailToAuthorsOnPublication';
     public const LOG_LEVEL = 'orcidLogLevel';
-    public const IS_SANDBOX = 'orcidIsSandBox';
     public const COUNTRY = 'orcidCountry';
     public const CITY = 'orcidCity';
     public const API_TYPE = 'orcidApiType';
     public const API_PUBLIC_PRODUCTION = 'publicProduction';
     public const API_PUBLIC_SANDBOX = 'publicSandbox';
     public const API_MEMBER_PRODUCTION = 'memberProduction';
     public const API_MEMBER_SANDBOX = 'memberSandbox';
+    public const LOG_LEVEL_ERROR = 'ERROR';
+    public const LOG_LEVEL_INFO = 'INFO';
 
     /**
-     * Check if there exist a valid orcid configuration section in the global config.inc.php of OJS.
+     * Check if ORCID is configured at the site-level of OJS.
      *
-     * @return boolean True, if the config file has api_url, client_id and client_secret set in an [orcid] section
+     * @return boolean True, if the ORCID settings at the site level are set for the API type, client ID, and client secret
      */
     public static function isGloballyConfigured(): bool
     {
@@ -65,6 +79,9 @@ public static function getIcon(): string
         return file_exists($path) ? file_get_contents($path) : '';
     }
 
+    /**
+     * Checks if ORCID functionality is enabled. Works at the context-level.
+     */
     public static function isEnabled(?Context $context = null): bool
     {
         if ($context === null) {
@@ -74,6 +91,11 @@ public static function isEnabled(?Context $context = null): bool
         return (bool) $context?->getData(self::ENABLED);
     }
 
+    /**
+     * Gets the main ORCID URL, either production or sandbox.
+     *
+     * Will first check if globally configured and prioritize site-level settings over context-level setting.
+     */
     public static function getOrcidUrl(?Context $context = null): string
     {
         if (self::isGloballyConfigured()) {
@@ -87,15 +109,20 @@ public static function getOrcidUrl(?Context $context = null): string
         return in_array($apiType, [self::API_PUBLIC_PRODUCTION, self::API_MEMBER_PRODUCTION]) ? self::ORCID_URL : self::ORCID_URL_SANDBOX;
     }
 
+    /**
+     * Gets the ORCID API URL, one of sandbox/production, member/public API URLs.
+     *
+     * Will first check if globally configured and prioritize site-level settings over context-level setting.
+     */
     public static function getApiPath(?Context $context = null): string
     {
         if (self::isGloballyConfigured()) {
-            $apiType = Application::get()->getRequest()->getSite()->getData(OrcidManager::API_TYPE);
+            $apiType = Application::get()->getRequest()->getSite()->getData(self::API_TYPE);
         } else {
             if ($context === null) {
                 $context = Application::get()->getRequest()->getContext();
             }
-            $apiType = $context->getData(OrcidManager::API_TYPE);
+            $apiType = $context->getData(self::API_TYPE);
         }
 
         return match ($apiType) {
@@ -106,22 +133,27 @@ public static function getApiPath(?Context $context = null): string
         };
     }
 
+    /**
+     * Returns whether the ORCID integration is set to use the sandbox domain.
+     *
+     * Will first check if globally configured and prioritize site-level settings over context-level setting.
+     */
     public static function isSandbox(?Context $context = null): bool
     {
         if (self::isGloballyConfigured()) {
-            $apiType = Application::get()->getRequest()->getSite()->getData(OrcidManager::API_TYPE);
+            $apiType = Application::get()->getRequest()->getSite()->getData(self::API_TYPE);
         } else {
             if ($context === null) {
                 $context = Application::get()->getRequest()->getContext();
             }
-            $apiType = $context->getData(OrcidManager::API_TYPE);
+            $apiType = $context->getData(self::API_TYPE);
         }
 
         return in_array($apiType, [self::API_PUBLIC_SANDBOX, self::API_MEMBER_SANDBOX]);
     }
 
     /**
-     * TODO: update as needed for new API
+     * Constructs an ORCID OAuth URL with correct scope/API based on configured settings
      *
      * @param string $handlerMethod Previously: containting a valid method of the OrcidProfileHandler
      * @param array $redirectParams Additional request parameters for the redirect URL
@@ -138,7 +170,6 @@ public static function buildOAuthUrl(string $handlerMethod, array $redirectParam
 
         $scope = self::isMemberApiEnabled() ? self::ORCID_API_SCOPE_MEMBER : self::ORCID_API_SCOPE_PUBLIC;
 
-        // TODO: This is the previous URL. Will be an API URL in new iteration
         // We need to construct a page url, but the request is using the component router.
         // Use the Dispatcher to construct the url and set the page router.
         $redirectUrl = $request->getDispatcher()->url(
@@ -160,6 +191,11 @@ public static function buildOAuthUrl(string $handlerMethod, array $redirectParam
         );
     }
 
+    /**
+     * Gets the configured city for use with review contributions.
+     *
+     * Will first check if globally configured and prioritize site-level settings over context-level setting.
+     */
     public static function getCity(?Context $context = null): string
     {
         if ($context === null) {
@@ -169,14 +205,25 @@ public static function getCity(?Context $context = null): string
         return $context->getData(self::CITY) ?? '';
     }
 
+    /**
+     * Gets the configured country for use with review contributions.
+     *
+     * Will first check if globally configured and prioritize site-level settings over context-level setting.
+     */
     public static function getCountry(?Context $context = null): string
     {
         if ($context === null) {
             $context = Application::get()->getRequest()->getContext();
         }
-        return $context->getData(OrcidManager::COUNTRY) ?? '';
+        return $context->getData(self::COUNTRY) ?? '';
     }
 
+
+    /**
+     * Returns true of member API (as opposed to public API) is in use.
+     *
+     * Will first check if globally configured and prioritize site-level settings over context-level setting.
+     */
     public static function isMemberApiEnabled(?Context $context = null): bool
     {
         if (self::isGloballyConfigured()) {
@@ -185,7 +232,7 @@ public static function isMemberApiEnabled(?Context $context = null): bool
             if ($context === null) {
                 $context = Application::get()->getRequest()->getContext();
             }
-            $apiType = $context->getData(OrcidManager::API_TYPE);
+            $apiType = $context->getData(self::API_TYPE);
         }
 
         if (in_array($apiType, [self::API_MEMBER_PRODUCTION, self::API_MEMBER_SANDBOX])) {
@@ -195,29 +242,44 @@ public static function isMemberApiEnabled(?Context $context = null): bool
         }
     }
 
+    /**
+     * Gets the currently configured log level. Can only bet set at the context-level, not the site-level.
+     */
     public static function getLogLevel(?Context $context = null): string
     {
         if ($context === null) {
             $context = Application::get()->getRequest()->getContext();
         }
 
-        return $context->getData(OrcidManager::LOG_LEVEL) ?? 'ERROR';
+        return $context->getData(self::LOG_LEVEL) ?? self::LOG_LEVEL_ERROR;
     }
 
+
+    /**
+     * Checks whether option to email authors for verification/authorization has been configured (context-level only).
+     */
     public static function shouldSendMailToAuthors(?Context $context = null): bool
     {
         if ($context === null) {
             $context = Application::get()->getRequest()->getContext();
         }
 
-        return $context->getData(OrcidManager::SEND_MAIL_TO_AUTHORS_ON_PUBLICATION) ?? false;
+        return $context->getData(self::SEND_MAIL_TO_AUTHORS_ON_PUBLICATION) ?? false;
     }
 
+    /**
+     * Helper method that gets OAuth endpoint for configured ORCID URL (production or sandbox)
+     */
     public static function getOauthPath(): string
     {
         return self::getOrcidUrl() . 'oauth/';
     }
 
+    /**
+     * Gets the configured client ID. Used to connect to the ORCID API.
+     *
+     * Will first check if globally configured and prioritize site-level settings over context-level setting.
+     */
     public static function getClientId(?Context $context = null): string
     {
         if (self::isGloballyConfigured()) {
@@ -231,6 +293,11 @@ public static function getClientId(?Context $context = null): string
         }
     }
 
+    /**
+     * Gets the configured client secret. Used to connect to the ORCID API.
+     *
+     * Will first check if globally configured and prioritize site-level settings over context-level setting.
+     */
     public static function getClientSecret(?Context $context = null): string
     {
         if (self::isGloballyConfigured()) {
@@ -264,21 +331,31 @@ public static function removeOrcidAccessToken(Author $author, bool $updateAuthor
         }
     }
 
+    /**
+     * Write out log message at the INFO level.
+     */
     public static function logInfo(string $message): void
     {
-        if (self::getLogLevel() !== 'INFO') {
+        if (self::getLogLevel() !== self::LOG_LEVEL_INFO) {
             return;
         }
-        self::writeLog($message, 'INFO');
+        self::writeLog($message, self::LOG_LEVEL_INFO);
     }
+
+    /**
+     * Write out log message at the ERROR level.
+     */
     public static function logError(string $message): void
     {
-        if (self::getLogLevel() !== 'ERROR') {
+        if (self::getLogLevel() !== self::LOG_LEVEL_ERROR) {
             return;
         }
-        self::writeLog($message, 'ERROR');
+        self::writeLog($message, self::LOG_LEVEL_ERROR);
     }
 
+    /**
+     * Helper method to write log message out to the configured log file.
+     */
     private static function writeLog(string $message, string $level): void
     {
         $fineStamp = date('Y-m-d H:i:s') . substr(microtime(), 1, 4);