Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkp/pkp-lib#1550 Controlled vocabulary support #10833

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

pkp/pkp-lib#1550 Controlled vocabulary support #10833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions api/v1/vocabs/PKPVocabController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,25 @@ public function getMany(Request $illuminateRequest): JsonResponse
->withLocales([$locale])
->when(
$term,
fn ($query) => $query->withSetting($vocab, $term, ControlledVocabEntryMatch::PARTIAL)
fn ($query) => $query->withSetting('name', $term, ControlledVocabEntryMatch::PARTIAL)
)
->get();
} else {
$entries = [];
Hook::call('API::vocabs::getMany', [$vocab, &$entries, $illuminateRequest, response(), $request]);
}

$data = [];
foreach ($entries as $entry) {
$data[] = $entry->getLocalizedData('name', $locale);
}

$data = array_values(array_unique($data));
$data = collect($entries)
->map(fn (ControlledVocabEntry $entry): array => $entry->getEntryData($locale))
->unique(fn (array $entryData): string =>
($entryData[ControlledVocabEntry::CONTROLLED_VOCAB_ENTRY_IDENTIFIER] ?? '') .
($entryData[ControlledVocabEntry::CONTROLLED_VOCAB_ENTRY_SOURCE] ?? '') .
$entryData['name']
)
->values()
->toArray();

Hook::call('API::vocabs::external', [$vocab, $term, $locale, &$data, &$entries, $illuminateRequest, response(), $request]);

return response()->json($data, Response::HTTP_OK);
}
Expand Down
4 changes: 3 additions & 1 deletion classes/components/forms/FieldAutosuggestPreset.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

namespace PKP\components\forms;

use PKP\controlledVocab\ControlledVocabEntry;

class FieldAutosuggestPreset extends FieldBaseAutosuggest
{
/** @copydoc Field::$component */
Expand Down Expand Up @@ -75,7 +77,7 @@ protected function mapSelected($value)
}
return [
'value' => $value,
'label' => $value,
'label' => $value['name'] ?? $value,
];
}
}
4 changes: 3 additions & 1 deletion classes/components/forms/FieldControlledVocab.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

namespace PKP\components\forms;

use PKP\controlledVocab\ControlledVocabEntry;

class FieldControlledVocab extends FieldBaseAutosuggest
{
/** @copydoc Field::$component */
Expand Down Expand Up @@ -58,7 +60,7 @@ public function mapSelected($value)
{
return [
'value' => $value,
'label' => $value,
'label' => $value['name'] ?? $value,
];
}
}
24 changes: 20 additions & 4 deletions classes/components/forms/publication/PKPMetadataForm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

namespace PKP\components\forms\publication;

use APP\core\Application;
use APP\facades\Repo;
use APP\publication\Publication;
use PKP\controlledVocab\ControlledVocab;
use PKP\components\forms\FieldControlledVocab;
Expand Down Expand Up @@ -54,7 +56,7 @@ public function __construct(string $action, array $locales, Publication $publica
'isMultilingual' => true,
'apiUrl' => str_replace('__vocab__', ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_KEYWORD, $suggestionUrlBase),
'locales' => $this->locales,
'value' => (array) $publication->getData('keywords'),
'value' => $this->getVocabEntryData(ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_KEYWORD),
]));
}

Expand All @@ -65,7 +67,7 @@ public function __construct(string $action, array $locales, Publication $publica
'isMultilingual' => true,
'apiUrl' => str_replace('__vocab__', ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_SUBJECT, $suggestionUrlBase),
'locales' => $this->locales,
'value' => (array) $publication->getData('subjects'),
'value' => $this->getVocabEntryData(ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_SUBJECT),
]));
}

Expand All @@ -76,7 +78,7 @@ public function __construct(string $action, array $locales, Publication $publica
'isMultilingual' => true,
'apiUrl' => str_replace('__vocab__', ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_DISCIPLINE, $suggestionUrlBase),
'locales' => $this->locales,
'value' => (array) $publication->getData('disciplines'),
'value' => $this->getVocabEntryData(ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_DISCIPLINE),
]));
}

Expand All @@ -87,7 +89,7 @@ public function __construct(string $action, array $locales, Publication $publica
'isMultilingual' => true,
'apiUrl' => str_replace('__vocab__', ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_AGENCY, $suggestionUrlBase),
'locales' => $this->locales,
'value' => (array) $publication->getData('supportingAgencies'),
'value' => $this->getVocabEntryData(ControlledVocab::CONTROLLED_VOCAB_SUBMISSION_AGENCY),
]));
}

Expand Down Expand Up @@ -155,4 +157,18 @@ protected function enabled(string $setting): bool
}
return (bool) $this->context->getData($setting);
}

/**
* Get vocab entry data
*/
protected function getVocabEntryData(string $symbolic): array
{
return Repo::controlledVocab()->getBySymbolic(
$symbolic,
Application::ASSOC_TYPE_PUBLICATION,
$this->publication->getId(),
[],
Repo::controlledVocab()::AS_ENTRY_DATA
);
}
}
26 changes: 25 additions & 1 deletion classes/controlledVocab/ControlledVocabEntry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
class ControlledVocabEntry extends Model
{
use ModelWithSettings;

public const CONTROLLED_VOCAB_ENTRY_IDENTIFIER = 'identifier';
public const CONTROLLED_VOCAB_ENTRY_SOURCE = 'source';
Comment on lines +31 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason we are defining these core settings columns as const here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, where would be a better place?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking that just use these as settings columns in the getSettings method and not define these as const . Just wondering is there any chance to specifically use the fields by any plugin or have any special use later ? Better to use const when each of that information need to be specifically use for special purpose multiple time .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plugins should use these when creating suggestions for the fields. Downstream services may use 'identifier' (e.g. when it's url) but probably don't have use for 'service'. Also at some point in the future there could be more data added than just 'name', 'identifier', and 'source'. Currently core doesn't use identifier and source separately but that may change in the future

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case we can leave it as it is for now .


/**
* @copydoc \Illuminate\Database\Eloquent\Model::$table
Expand Down Expand Up @@ -67,6 +70,8 @@ protected function casts(): array
'controlled_vocab_entry_id' => 'integer',
'controlled_vocab_id' => 'integer',
'seq' => 'float',
self::CONTROLLED_VOCAB_ENTRY_IDENTIFIER => 'string',
self::CONTROLLED_VOCAB_ENTRY_SOURCE => 'string',
];
}

Expand All @@ -91,7 +96,7 @@ public function getMultilingualProps(): array
*/
public function getSettings(): array
{
return array_merge($this->settings, ['name']);
return array_merge($this->settings, ['name', self::CONTROLLED_VOCAB_ENTRY_IDENTIFIER, self::CONTROLLED_VOCAB_ENTRY_SOURCE]);
}

/**
Expand Down Expand Up @@ -190,4 +195,23 @@ public function scopeWithSetting(
)
);
}

/**
* Get entry related data
*/
public function getEntryData(string $locale = null): ?array
{
$multilingualProps = array_flip($this->getMultilingualProps());
$attributes = Arr::mapWithKeys($this->getSettings(), function (string $prop) use ($locale, $multilingualProps): array {
$propData = $this->getAttribute($prop);
$data = isset($multilingualProps[$prop]) && $locale ? $propData[$locale] ?? null : $propData;
return $data ? [$prop => $data] : [];
});

if (!isset($attributes['name'])) {
return null;
}

return $attributes;
}
}
33 changes: 26 additions & 7 deletions classes/controlledVocab/Repository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

class Repository
{
const AS_ENTRY_DATA = true;

/**
* Fetch a Controlled Vocab by symbolic info, building it if needed.
*/
Expand All @@ -46,7 +48,8 @@ public function getBySymbolic(
string $symbolic,
int $assocType,
?int $assocId,
?array $locales = []
?array $locales = [],
bool $asEntryData = !Repository::AS_ENTRY_DATA
): array
{
$result = [];
Expand All @@ -58,9 +61,9 @@ public function getBySymbolic(
)
->when(!empty($locales), fn ($query) => $query->withLocales($locales))
->get()
->each(function ($entry) use (&$result) {
->each(function ($entry) use (&$result, $asEntryData) {
foreach ($entry->name as $locale => $value) {
$result[$locale][] = $value;
$result[$locale][] = $asEntryData ? $entry->getEntryData($locale) : $value;
Copy link
Member

@touhidurabir touhidurabir Jan 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, what are the chances to standardise the output to one single format , so with $asEntryData set to true, we get

[
    "en" => [
      [
        "identifier" => "http://www.yso.fi/onto/koko/p36127",
        "source" => "finto",
        "term" => "timber construction",
      ],
      [
        "identifier" => "http://www.yso.fi/onto/koko/p62290",
        "source" => "finto",
        "term" => "development aids",
      ],
      [
        "term" => "test",
      ],
    ],
  ]

and $asEntryData set to false, we get

[
    "en" => [
      "timber construction",
      "development aids",
      "test",
    ],
  ]

if we try to standardise it as

[
    "en" => [
      ['name' => "timber construction"],
      ['name' => "development aids"],
      ['name' => "test"],
    ],
  ]

what about that ? or perhaps that will be too much changes as I guess need to port that changes to a lot of places .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about that ? or perhaps that will be too much changes as I guess need to port that changes to a lot of places .
I was thinking this too

}
});

Expand All @@ -80,6 +83,11 @@ public function insertBySymbolic(
{
$controlledVocab = $this->build($symbolic, $assocType, $assocId);
$controlledVocab->load('controlledVocabEntries');
$controlledVocabEntry = new ControlledVocabEntry;
$controlledVocabEntrySettings = $controlledVocabEntry->getSettings();
$multilingualProps = array_flip($controlledVocabEntry->getMultilingualProps());
$idKey = ControlledVocabEntry::CONTROLLED_VOCAB_ENTRY_IDENTIFIER;
$srcKey = ControlledVocabEntry::CONTROLLED_VOCAB_ENTRY_SOURCE;

if ($deleteFirst) {
ControlledVocabEntry::query()
Expand All @@ -93,17 +101,28 @@ public function insertBySymbolic(
collect($vocabs)
->each(
fn (array|string $entries, string $locale) => collect(array_values(Arr::wrap($entries)))
->reject(fn (string|array $vocab) => is_array($vocab) && isset($vocab[$idKey]) && !isset($vocab[$srcKey])) // Remove vocabs that have id but not source
->unique(fn (string|array $vocab): string => ($vocab[$idKey] ?? '') . ($vocab[$srcKey] ?? '') . ($vocab['name'] ?? $vocab))
->each(
fn (string $vocab, int $index) =>
fn (array|string $vocab, int $index) =>
ControlledVocabEntry::create([
'controlledVocabId' => $controlledVocab->id,
'seq' => $index + 1,
'name' => [
$locale => $vocab
],
...is_array($vocab)
? collect($vocab)
->only($controlledVocabEntrySettings)
->whereNotNull()
->map(fn ($prop, string $propName) => isset($multilingualProps[$propName])
? [$locale => $prop]
: $prop
)
->toArray()
: ['name' => [$locale => $vocab]],
])
)
);

$this->resequence($controlledVocab->id);
}

/**
Expand Down
76 changes: 72 additions & 4 deletions schemas/publication.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,24 @@
"nullable"
],
"items": {
"type": "string"
"type": "object",
"properties": {
"name": {
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above as if term and name represent same thing and we are already using name, can you use name here and everywhere ?

},
"source": {
"type": "string",
"validation": [
"nullable"
]
},
"identifier": {
"type": "string",
"validation": [
"nullable"
]
}
}
}
},
"doiObject": {
Expand Down Expand Up @@ -180,7 +197,24 @@
"nullable"
],
"items": {
"type": "string"
"type": "object",
"properties": {
"name": {
"type": "string"
},
"source": {
"type": "string",
"validation": [
"nullable"
]
},
"identifier": {
"type": "string",
"validation": [
"nullable"
]
}
}
}
},
"lastModified": {
Expand Down Expand Up @@ -256,7 +290,24 @@
"nullable"
],
"items": {
"type": "string"
"type": "object",
"properties": {
"name": {
"type": "string"
},
"source": {
"type": "string",
"validation": [
"nullable"
]
},
"identifier": {
"type": "string",
"validation": [
"nullable"
]
}
}
}
},
"submissionId": {
Expand All @@ -281,7 +332,24 @@
"nullable"
],
"items": {
"type": "string"
"type": "object",
"properties": {
"name": {
"type": "string"
},
"source": {
"type": "string",
"validation": [
"nullable"
]
},
"identifier": {
"type": "string",
"validation": [
"nullable"
]
}
}
}
},
"status": {
Expand Down