[DSpace-CRIS] Porting of authority framework functionalities (Frontend)#4956
[DSpace-CRIS] Porting of authority framework functionalities (Frontend)#4956FrancescoMolinaro wants to merge 32 commits intoDSpace:mainfrom
Conversation
|
Hi @FrancescoMolinaro, |
artlowel
left a comment
There was a problem hiding this comment.
Thanks @FrancescoMolinaro
I haven't tested it yet, just did a code review for now. Most of it looks good, but I have a few inline comments
config/config.example.yml
Outdated
| followAuthorityMetadataValuesLimit: 5; | ||
|
|
||
| # When the search results are retrieved, for each item type the metadata with a valid authority value are inspected. | ||
| # Referenced items will be fetched with a find all by id strategy to avoid individual rest requests to efficiently display the search results. | ||
| followAuthorityMetadata: | ||
| - type: Publication | ||
| metadata: dc.contributor.author |
| /** | ||
| * A boolean representing if this Item is currently withdrawn or not | ||
| */ |
There was a problem hiding this comment.
This comment is for something else
config/config.example.yml
Outdated
| # Icons to be displayed next to an authority controlled value, to give indication of the source. | ||
| sourceIcons: | ||
| # Example of configuration for authority logo based on sources. | ||
| # The condigured icon will be displayed next to the authority value in submission and on item page or search results. |
| getExternalSourceEntryById(externalSourceId: string, entryId: string): Observable<RemoteData<ExternalSourceEntry>> { | ||
| const href$ = this.getEntryIDHref(externalSourceId, entryId).pipe( | ||
| isNotEmptyOperator(), | ||
| distinctUntilChanged(), | ||
| ); | ||
|
|
||
| return this.findByHref(href$) as any; | ||
| } |
There was a problem hiding this comment.
This method doesn't seem to be used anywhere
| public findById(id: string, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig<Item>[]): Observable<RemoteData<Item>> { | ||
|
|
||
| if (uuidValidate(id)) { | ||
| const href$ = this.getIDHrefObs(encodeURIComponent(id), ...linksToFollow); | ||
| return this.findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); | ||
| } else { | ||
| return this.findByCustomUrl(id, useCachedVersionIfAvailable, reRequestOnStale, linksToFollow); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I think this check should be moved to the resolver. findById does one, well defined thing in all dataservices. The resolver should parse the route and determine whether to call findById or findByCustom URL, or even cleaner: move custom URLs to a different subroute, so you can have a separate resolver as well.
| /** | ||
| * Returns true if this Metadatum's authority key starts with 'virtual::' | ||
| */ | ||
| get isVirtual(): boolean { | ||
| return hasValue(this.authority) && this.authority.startsWith(VIRTUAL_METADATA_PREFIX); | ||
| } | ||
|
|
||
| /** | ||
| * If this is a virtual Metadatum, it returns everything in the authority key after 'virtual::'. | ||
| * Returns undefined otherwise. | ||
| */ | ||
| get virtualValue(): string { | ||
| if (this.isVirtual) { | ||
| return this.authority.substring(this.authority.indexOf(VIRTUAL_METADATA_PREFIX) + VIRTUAL_METADATA_PREFIX.length); | ||
| } else { | ||
| return undefined; | ||
| } | ||
| } |
There was a problem hiding this comment.
These methods were moved out of the models, to a service in #2875. In general we want to avoid having methods in models, as they cause issues with caching serialization etc
| tap((isHierarchical: boolean) => { | ||
| if (this.model.value) { | ||
| this.setCurrentValue(this.model.value, isHierarchical); | ||
| } | ||
| }), |
There was a problem hiding this comment.
It's not a good idea to do "work" in a tap(). Because that code will only be executed if something is subscribed to the pipe. It's better to have a dedicated subscription to do the value updates, that way you can be sure it happens every time the source observable updates.
| this.orcidUrl$ = this.configurationService | ||
| .findByPropertyName('orcid.domain-url') | ||
| .pipe( | ||
| getFirstSucceededRemoteDataPayload(), |
There was a problem hiding this comment.
I suspect that, if orcid.domain-url isn't set on the server, there will never be a succeeded remote data, so this will never emit. Better deal with the failed case as well
|
Hi @artlowel , thank you very much for the review and for the feedback. I have addressed all the points raised, with the exception of the custom URL aspect. The custom URL functionality originates from #4814. Since this pull request builds upon that work, I believe it would be more appropriate to address any related issue directly within that original PR, if needed. During the implementation, we chose to place the ID format control within the Following the referenced PR, when the relevant metadata is present, the custom URL replaces the standard UUID/ID parameter in the page URL and effectively becomes a unique identifier for the item. For this reason, we considered it preferable to keep the validation logic inside the I would really appreaciate your thoughts on this reasoning and of course if it is not enough to justify the choice made I will make sure to adapt the code accordingly in the referenced pull request. |
|
Hi @FrancescoMolinaro, |
References
Description
Initial port of the DSpace-CRIS authority framework into core DSpace, enabling real-time entity lookup, confidence-based linking, and automatic graph maintenance through server-side consumers.
On the UI this PR introduces a new way to link authorithy controlled metadata to the related item, generating a link in search results or item page, this connection is handled in submission. (for configuration check the rest PR)
Through a rest configuration, is possible also to activate an "auto-fill" functionality, that will populate the submission fields based on the selected value (value must be authority controlled).
Exaple of config in cris-authority-metadatagenerator.xml:
auto-fill.webm
The information of the related item are previewed via a new pop-up component enabled on hover, visible on both search results and item page:
In addition the PR introduce also multilanguage support for authority controlled vocabularies, to achieve this we need to have a vocabulary version for each language enabled by the property: webui.supported.locales
For instance if we have
webui.supported.locales = en, de
we can create a translated vocabulary (srsc_de in this example) and then configure the authority as follows (authority.cfg):
vocabulary.plugin.srsc.hierarchy.store = true
vocabulary.plugin.srsc.hierarchy.suggest = true
vocabulary.plugin.srsc.delimiter = "::"
vocabulary.plugin.authority.store = true
authority.controlled.dc.subject = true
This will enable to display the translated value in the UI, if the language matches the translated one:
EN:
DE:
This last feature will be consolidated later in the merger with the cust layout functionality from CRIS (at the moment this is not presente in the exact same way on CRIS as there we rely on the layout feature that is not yet in scope).
The implementation will remain anyway as it will be useful also as fallback for not configured layouts.
Instructions for Reviewers
Once the item relation is enstablished (submitted) the metadata will be linking to the related item, with a preview of metadata visible on link hover.
The new configuration for the pop-hover preview is available in default-app.config.ts, under the name:
metadataLinkViewPopoverData
It provides the opportunity to customize the preview metadata based on entity type.
In submission is possible also to configure specific icons for each source of authority, see config:
sourceIcons (under authority config)
In general for the newly added configuration you can refer to the config.example.yml file.
Once submitted, the authorithy controlled value should then result in a clickable link the search results (if the value is displayed) and in a clickable link in the item page (see images above).
Is possible to configure also auto-fill and generated metadata based on the authority (see descriptio or rest PR).
For the multilanguage support on vocabularies a translated vocabulary and submission form are needed, in addition to the properties based on the vocabulary name and metadata (see description).
List of changes in this PR:
Enhanced search results and item page with linkable related items (authority controlled)
Added new popover component for metadata preview on hover.
Implemented custom pipe for icon/info handling in onebox component.
Improved editing of authority via admin view, improved handling of search filters (filter by authorithy not value).
Implemented auto-fill functionality in submission.
Added support for multilanguage visualization of controlled vocabulary values in item page.
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.