#10292 resolved all but one TODO

pkp · Oct 18, 2024 · e1964a3 · e1964a3
1 parent cd906b9
commit e1964a3
Show file tree

Hide file tree

Showing 8 changed files with 15 additions and 60 deletions.
diff --git a/classes/controlledVocab/ControlledVocab.php b/classes/controlledVocab/ControlledVocab.php
@@ -104,32 +104,6 @@ public static function hasDefinedVocabSymbolic(string $vocab): bool
         return in_array($vocab, static::getDefinedVocabSymbolic());
     }
 
-    // TODO: Investigate if this is necessary anymore
-    /**
-     * Get the locale field names for this controlled vocab
-     */
-    public function getLocaleFieldNames(): array
-    {
-        if (!$this->symbolic) {
-            return [];
-        }
-
-        return static::hasDefinedVocabSymbolic($this->symbolic)
-            ? [$this->symbolic]
-            : [];
-    }
-
-    // TODO: Investigate if this is necessary anymore
-    /**
-     * Compatibility function for including note IDs in grids.
-     *
-     * @deprecated 3.5.0 Use $model->id instead. Can be removed once the DataObject pattern is removed.
-     */
-    public function getId(): int
-    {
-        return $this->id;
-    }
-
     /**
      * Get all controlled vocab entries for this controlled vocab
      */
@@ -216,10 +190,7 @@ public function enumerate(?string $settingName = null): array
                 'e.controlled_vocab_entry_id',
                 DB::raw(
                     'COALESCE (l.setting_value, p.setting_value, n.setting_value) as setting_value'
-                ),
-                DB::raw(
-                    'COALESCE (l.setting_type, p.setting_type, n.setting_type) as setting_type'
-                ),
+                )
             ])
             ->where('e.controlled_vocab_id', $this->id)
             ->orderBy('e.seq')

diff --git a/classes/controlledVocab/ControlledVocabEntry.php b/classes/controlledVocab/ControlledVocabEntry.php
@@ -52,7 +52,7 @@ class ControlledVocabEntry extends Model
     public $timestamps = false;
 
     /**
-     * @inheritDoc
+     * @copydoc \PKP\core\traits\ModelWithSettings::getSettingsTable
      */
     public function getSettingsTable(): string
     {
@@ -72,15 +72,15 @@ protected function casts(): array
     }
 
     /**
-     * @inheritDoc
+     * @copydoc \PKP\core\traits\ModelWithSettings::getSchemaName
      */
     public static function getSchemaName(): ?string
     {
         return null;
     }
 
     /**
-     * @inheritDoc
+     * @copydoc \PKP\core\traits\ModelWithSettings::getMultilingualProps
      */
     public function getMultilingualProps(): array
     {
@@ -92,7 +92,7 @@ public function getMultilingualProps(): array
     }
 
     /**
-     * @inheritDoc
+     * @copydoc \PKP\core\traits\ModelWithSettings::getSettings
      */
     public function getSettings(): array
     {

diff --git a/classes/metadata/MetadataProperty.php b/classes/metadata/MetadataProperty.php
@@ -429,8 +429,7 @@ public function isValid($value, $locale = null)
                                         fn ($query) => $query->withSymbolic($symbolic)->withAssoc($assocType, $assocId)
                                     )
                                     ->withLocale($locale)
-                                    // TODO: Investigate if this need to be 'name' or $symbolic for settingName
-                                    ->withSetting('name', $value)
+                                    ->withSetting($symbolic, $value)
                                     ->first();
 
                                 if (!is_null($entry)) {

diff --git a/classes/user/interest/Repository.php b/classes/user/interest/Repository.php
@@ -123,7 +123,6 @@ public function setInterestsForUser(User $user, string|array|null $interests = n
                     ])->id
                 );
 
-            // TODO: Investigate the impact of applied patch from https://github.com/pkp/pkp-lib/issues/10423
             collect($currentInterests->pluck('id'))
                 ->merge($newInterestIds)
                 ->each(fn ($interestId) => UserInterest::create([

diff --git a/classes/user/interest/UserInterest.php b/classes/user/interest/UserInterest.php
@@ -91,17 +91,6 @@ public function controlledVocabEntries(): HasMany
         return $this->hasMany(ControlledVocabEntry::class, 'controlled_vocab_entry_id', 'controlled_vocab_entry_id');
     }
 
-    // TODO: Investigate if this is necessary anymore
-    /**
-     * Compatibility function for including note IDs in grids.
-     *
-     * @deprecated 3.5.0 Use $model->id instead. Can be removed once the DataObject pattern is removed.
-     */
-    public function getId(): int
-    {
-        return $this->id;
-    }
-
     /**
      * Scope a query to only include interests with a specific user id
      */

diff --git a/tests/classes/form/validation/FormValidatorControlledVocabTest.php b/tests/classes/form/validation/FormValidatorControlledVocabTest.php
@@ -45,7 +45,6 @@ public function testIsValid()
             $assocId
         );
 
-        // TODO : Investigate if possible to insert dummy symbolic in `controlled_vocab_entry_settings` table
         $controlledVocabEntryId1 = ControlledVocabEntry::create([
             'controlledVocabId' => $testControlledVocab->id,
             ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_KEYWORD => [
@@ -80,7 +79,7 @@ public function testIsValid()
         $form->setData('testData', 3);
         self::assertFalse($validator->isValid());
 
-        // Delete the test entries
+        // Delete the test vocab along with entries
         ControlledVocab::find($testControlledVocab->id)->delete();
     }
 }
diff --git a/tests/classes/metadata/MetadataPropertyTest.php b/tests/classes/metadata/MetadataPropertyTest.php
@@ -20,13 +20,13 @@
 
 use APP\facades\Repo;
 use InvalidArgumentException;
+use PKP\controlledVocab\ControlledVocab;
 use PKP\controlledVocab\ControlledVocabEntry;
 use PKP\metadata\MetadataDescription;
 use PKP\metadata\MetadataProperty;
 use PKP\tests\PKPTestCase;
 use stdClass;
 use PHPUnit\Framework\Attributes\CoversClass;
-use PKP\user\interest\UserInterest;
 
 #[CoversClass(MetadataProperty::class)]
 class MetadataPropertyTest extends PKPTestCase
@@ -148,32 +148,31 @@ public function testValidateControlledVocabulary()
     {
         // Build a test vocabulary. (Assoc type and id are 0 to simulate a site-wide vocabulary).
         $vocab = Repo::controlledVocab()->build(
-            UserInterest::CONTROLLED_VOCAB_INTEREST, 0, 0
+            ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_KEYWORD, 0, 0
         );
 
-        // TODO : Investigate if possible to insert dummy symbolic in `controlled_vocab_entry_settings` table
         $controlledVocabEntry = ControlledVocabEntry::create([
             'controlledVocabId' => $vocab->id,
-            UserInterest::CONTROLLED_VOCAB_INTEREST => [
+            ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_KEYWORD => [
                 'en' => 'testEntry',
             ],
         ]);
 
         $metadataProperty = new MetadataProperty(
             'testElement',
             [], 
-            [MetadataProperty::METADATA_PROPERTY_TYPE_VOCABULARY => UserInterest::CONTROLLED_VOCAB_INTEREST]
+            [MetadataProperty::METADATA_PROPERTY_TYPE_VOCABULARY => ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_KEYWORD]
         );
 
         // This validator checks numeric values
         self::assertEquals(
-            [MetadataProperty::METADATA_PROPERTY_TYPE_VOCABULARY => UserInterest::CONTROLLED_VOCAB_INTEREST],
+            [MetadataProperty::METADATA_PROPERTY_TYPE_VOCABULARY => ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_KEYWORD],
             $metadataProperty->isValid($controlledVocabEntry->id)
         );
         self::assertFalse($metadataProperty->isValid($controlledVocabEntry->id + 1));
 
-        // Delete the test vocabulary entry
-        $controlledVocabEntry->delete();
+        // Delete the test vocab along with entry
+        $vocab->delete();
 
     }
 

diff --git a/tests/classes/validation/ValidatorControlledVocabTest.php b/tests/classes/validation/ValidatorControlledVocabTest.php
@@ -40,7 +40,6 @@ public function testValidatorControlledVocab()
             $assocId
         );
 
-        // TODO : Investigate if possible to insert dummy symbolic in `controlled_vocab_entry_settings` table
         $controlledVocabEntryId1 = ControlledVocabEntry::create([
             'controlledVocabId' => $testControlledVocab->id,
             ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_KEYWORD => [
@@ -64,7 +63,7 @@ public function testValidatorControlledVocab()
         self::assertTrue($validator->isValid($controlledVocabEntryId2));
         self::assertFalse($validator->isValid(3));
 
-        // Delete the test entried
+        // Delete the test vocab along with entries
         ControlledVocab::find($testControlledVocab->id)->delete();
     }
 }