pkp#10821 fix checkDuplicate, changePubId, pubIdExists, publisher-id …

…storing
bozana · Jan 20, 2025 · 3fdd61a · 3fdd61a
1 parent adacbdb
commit 3fdd61a
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 38 deletions.
diff --git a/classes/galley/DAO.php b/classes/galley/DAO.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * @file classes/galley/DAO.php
  *
@@ -204,10 +205,10 @@ public function pubIdExists(string $pubIdType, string $pubId, int $excludePubObj
      */
     public function changePubId($pubObjectId, $pubIdType, $pubId)
     {
-        DB::table('publication_galley_settings')
-            ->where('setting_name', 'pub-id::' . $pubIdType)
-            ->where('galley_id', (int) $pubObjectId)
-            ->update(['setting_value' => (string) $pubId]);
+        DB::table('publication_galley_settings')->updateOrInsert(
+            ['galley_id' => (int) $pubObjectId, 'locale' => '', 'setting_name' => 'pub-id::' . $pubIdType],
+            ['setting_value' => (string) $pubId]
+        );
     }
 
     /**

diff --git a/classes/plugins/PKPPubIdPlugin.php b/classes/plugins/PKPPubIdPlugin.php
@@ -316,7 +316,7 @@ public function verifyData($fieldName, $fieldValue, $pubObject, int $contextId,
             }
             $newPubId = $this->constructPubId($pubIdPrefix, $fieldValue, $contextId);
 
-            if (!$this->checkDuplicate($newPubId, get_class($pubObject), $pubObject->getId(), $contextId)) {
+            if (!$this->checkDuplicate($newPubId, $pubObject, $contextId)) {
                 $errorMsg = $this->getNotUniqueErrorMsg();
                 return false;
             }
@@ -470,30 +470,28 @@ public function setStoredPubId(&$pubObject, $pubId)
      * Check for duplicate public identifiers.
      *
      * Checks to see if a pubId has already been assigned to any object
-     * in the context.
+     * in the context (excluding the given object).
      *
      * @param string $pubId
-     * @param string $pubObjectType Class name of the pub object being checked
-     * @param int $excludeId This object id will not be checked for duplicates
+     * @param object $pubObject Pub object being checked
      *
      * @return bool
      */
-    public function checkDuplicate($pubId, $pubObjectType, $excludeId, int $contextId)
+    public function checkDuplicate($pubId, $pubObject, int $contextId)
     {
         foreach ($this->getPubObjectTypes() as $type => $fqcn) {
-            if ($type === 'Publication') {
-                $typeDao = Repo::publication()->dao;
-            } elseif ($type === 'Representation') {
-                $typeDao = Application::getRepresentationDAO();
-            } elseif ($type === 'SubmissionFile') {
-                $typeDao = Repo::submissionFile()->dao;
+            if (!$pubObject instanceof $fqcn) {
+                continue;
             }
-            $excludeTypeId = $type === $pubObjectType ? $excludeId : null;
-            if (isset($typeDao) && $typeDao->pubIdExists($this->getPubIdType(), $pubId, $excludeTypeId, $contextId)) {
+            $typeDao = match ($type) {
+                'Publication' => Repo::publication()->dao,
+                'Representation' => Application::getRepresentationDAO(),
+                'SubmissionFile' => Repo::submissionFile()->dao,
+            };
+            if (isset($typeDao) && $typeDao->pubIdExists($this->getPubIdType(), $pubId, $pubObject->getId(), $contextId)) {
                 return false;
             }
         }
-
         return true;
     }
 

diff --git a/classes/publication/DAO.php b/classes/publication/DAO.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * @file classes/publication/DAO.php
  *
@@ -13,8 +14,8 @@
 
 namespace PKP\publication;
 
-use APP\facades\Repo;
 use APP\core\Application;
+use APP\facades\Repo;
 use APP\publication\Publication;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Enumerable;
@@ -278,12 +279,14 @@ public function pubIdExists(string $pubIdType, string $pubId, int $excludePubObj
     public function changePubId($pubObjectId, $pubIdType, $pubId)
     {
         DB::table($this->settingsTable)
-            ->update([
-                'publication_id' => (int) $pubObjectId,
-                'locale' => '',
-                'setting_name' => 'pub-id::' . $pubIdType,
-                'setting_value' => (string) $pubId
-            ]);
+            ->updateOrInsert(
+                [
+                    'publication_id' => (int) $pubObjectId,
+                    'locale' => '',
+                    'setting_name' => 'pub-id::' . (string) $pubIdType,
+                ],
+                ['setting_value' => (string) $pubId]
+            );
     }
 
     /**

diff --git a/classes/submission/Representation.php b/classes/submission/Representation.php
@@ -172,7 +172,7 @@ public function getContextId()
     /**
      * @copydoc \PKP\core\DataObject::getDAO()
      */
-    public function getDAO(): \PKP\db\DAO
+    public function getDAO(): \PKP\galley\DAO
     {
         return Application::getRepresentationDAO();
     }

diff --git a/classes/submissionFile/DAO.php b/classes/submissionFile/DAO.php
@@ -353,16 +353,15 @@ public function pubIdExists(
         int $excludePubObjectId,
         int $contextId
     ): bool {
-        $result = DB::table($this->settingsTable . ' as sfs')
+        return DB::table($this->settingsTable . ' as sfs')
             ->join('submission_files AS sf', 'sfs.submission_file_id', '=', 'sf.submission_file_id')
             ->join('submissions AS s', 'sf.submission_id', '=', 's.submission_id')
             ->where([
-                'sfs.setting_name' => 'pub-id::' . (string) $pubIdType,
-                'sfs.setting_value' => (string) $pubId,
-                'sfs.submission_file_id' => (int) $excludePubObjectId,
-                's.context_id' => (int) $contextId
-            ])->getCountForPagination();
-        return $result > 0;
+                ['sfs.setting_name', '=', "pub-id::{$pubIdType}"],
+                ['sfs.setting_value', '=', (string) $pubId],
+                ['sfs.submission_file_id', '<>', (int) $excludePubObjectId],
+                ['s.context_id', '=', (int) $contextId]
+            ])->count() > 0;
     }
 
     /**
@@ -374,10 +373,10 @@ public function changePubId($pubObjectId, $pubIdType, $pubId)
             ->updateOrInsert(
                 [
                     'submission_file_id' => (int) $pubObjectId,
+                    'locale' => '',
                     'setting_name' => 'pub-id::' . (string) $pubIdType,
-                    'setting_value' => (string) $pubId
                 ],
-                ['locale' => '']
+                ['setting_value' => (string) $pubId]
             );
     }
 

diff --git a/controllers/tab/pubIds/form/PKPPublicIdentifiersForm.php b/controllers/tab/pubIds/form/PKPPublicIdentifiersForm.php
@@ -223,21 +223,21 @@ public function execute(...$functionArgs)
         parent::execute(...$functionArgs);
 
         $pubObject = $this->getPubObject();
-        $pubObject->setStoredPubId('publisher-id', $this->getData('publisherId'));
+        if ($this->getData('publisherId')) {
+            $pubObject->setStoredPubId('publisher-id', $this->getData('publisherId'));
+        }
 
         $pubIdPluginHelper = new PKPPubIdPluginHelper();
         $pubIdPluginHelper->execute($this->getContextId(), $this, $pubObject);
 
         if ($pubObject instanceof Representation) {
             $representationDao = Application::getRepresentationDAO();
             $representationDao->updateObject($pubObject);
-
             return;
         }
 
         if ($pubObject instanceof SubmissionFile) {
             Repo::submissionFile()->edit($pubObject, []);
-
             return;
         }
     }