From 838cde48417c0a2cefe855e617cc9d8a5feaf787 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Thu, 19 Jan 2023 15:01:04 +1300 Subject: [PATCH] [TLC-249] Addressing review feedback Adding comments and tidying some comments, imports Expect text for status not integer Send a 'type' parameter with a DOI registration Rename item-status.register to registerDOI As per todonohue's feedback on 2022-01-18 --- src/app/core/data/identifier-data.service.ts | 7 ++++++- src/app/core/data/item-data.service.ts | 4 +++- .../item-status/item-status.component.spec.ts | 2 +- .../item-status/item-status.component.ts | 9 +++------ .../identifier-data.component.ts | 19 +++++------------- .../identifier-data/identifier.model.ts | 20 ++++++++++++++++++- 6 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/app/core/data/identifier-data.service.ts b/src/app/core/data/identifier-data.service.ts index d494b5171d..be4c5c8b59 100644 --- a/src/app/core/data/identifier-data.service.ts +++ b/src/app/core/data/identifier-data.service.ts @@ -20,6 +20,11 @@ import { map } from 'rxjs/operators'; import {ConfigurationProperty} from '../shared/configuration-property.model'; import {ConfigurationDataService} from './configuration-data.service'; +/** + * The service handling all REST requests to get item identifiers like handles and DOIs + * from the /identifiers endpoint, as well as the backend configuration that controls whether a 'Register DOI' + * button appears for admins in the item status page + */ @Injectable() @dataService(IDENTIFIERS) export class IdentifierDataService extends BaseDataService { @@ -50,7 +55,7 @@ export class IdentifierDataService extends BaseDataService { * Should we allow registration of new DOIs via the item status page? */ public getIdentifierRegistrationConfiguration(): Observable { - return this.configurationService.findByPropertyName('identifiers.item-status.register').pipe( + return this.configurationService.findByPropertyName('identifiers.item-status.registerDOI').pipe( getFirstCompletedRemoteData(), map((propertyRD: RemoteData) => propertyRD.hasSucceeded ? propertyRD.payload.values : []) ); diff --git a/src/app/core/data/item-data.service.ts b/src/app/core/data/item-data.service.ts index 5306dd468b..79719e502c 100644 --- a/src/app/core/data/item-data.service.ts +++ b/src/app/core/data/item-data.service.ts @@ -256,7 +256,9 @@ export abstract class BaseItemDataService extends IdentifiableDataService let headers = new HttpHeaders(); headers = headers.append('Content-Type', 'application/json'); options.headers = headers; - const request = new PostRequest(requestId, href, JSON.stringify({}), options); + // Pass identifier type as a simple parameter, no need for full JSON data + let hrefWithParams: string = this.buildHrefWithParams(href, [new RequestParam("type", "doi")]); + const request = new PostRequest(requestId, hrefWithParams, JSON.stringify({}), options); this.requestService.send(request); }); return this.rdbService.buildFromRequestUUID(requestId); diff --git a/src/app/item-page/edit-item-page/item-status/item-status.component.spec.ts b/src/app/item-page/edit-item-page/item-status/item-status.component.spec.ts index ce8293ce79..1f447deb9e 100644 --- a/src/app/item-page/edit-item-page/item-status/item-status.component.spec.ts +++ b/src/app/item-page/edit-item-page/item-status/item-status.component.spec.ts @@ -41,7 +41,7 @@ describe('ItemStatusComponent', () => { mockConfigurationDataService = jasmine.createSpyObj('configurationDataService', { findByPropertyName: createSuccessfulRemoteDataObject$(Object.assign(new ConfigurationProperty(), { - name: 'identifiers.item-status.register', + name: 'identifiers.item-status.registerDOI', values: [ 'true' ] diff --git a/src/app/item-page/edit-item-page/item-status/item-status.component.ts b/src/app/item-page/edit-item-page/item-status/item-status.component.ts index df636d78dc..20f167fb80 100644 --- a/src/app/item-page/edit-item-page/item-status/item-status.component.ts +++ b/src/app/item-page/edit-item-page/item-status/item-status.component.ts @@ -3,7 +3,7 @@ import { fadeIn, fadeInOut } from '../../../shared/animations/fade'; import { Item } from '../../../core/shared/item.model'; import { ActivatedRoute } from '@angular/router'; import { ItemOperation } from '../item-operation/itemOperation.model'; -import {distinctUntilChanged, first, map, mergeMap, startWith, switchMap, toArray} from 'rxjs/operators'; +import {distinctUntilChanged, first, map, mergeMap, toArray} from 'rxjs/operators'; import {BehaviorSubject, Observable, from as observableFrom, Subscription, combineLatest, of} from 'rxjs'; import { RemoteData } from '../../../core/data/remote-data'; import { getItemEditRoute, getItemPageRoute } from '../../item-page-routing-paths'; @@ -12,11 +12,8 @@ import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; import { hasValue } from '../../../shared/empty.util'; import { getAllSucceededRemoteDataPayload, - getFirstCompletedRemoteData, getFirstSucceededRemoteData, - getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators'; import {IdentifierDataService} from '../../../core/data/identifier-data.service'; -import {IdentifierData} from '../../../shared/object-list/identifier-data/identifier-data.model'; import {Identifier} from '../../../shared/object-list/identifier-data/identifier.model'; import {ConfigurationProperty} from '../../../core/shared/configuration-property.model'; import {ConfigurationDataService} from '../../../core/data/configuration-data.service'; @@ -111,7 +108,7 @@ export class ItemStatusComponent implements OnInit { ); // Observable for configuration determining whether the Register DOI feature is enabled - let registerConfigEnabled$: Observable = this.configurationService.findByPropertyName('identifiers.item-status.register').pipe( + let registerConfigEnabled$: Observable = this.configurationService.findByPropertyName('identifiers.item-status.registerDOI').pipe( map((enabled: RemoteData) => { let show = false; if (enabled.hasSucceeded) { @@ -161,7 +158,7 @@ export class ItemStatusComponent implements OnInit { if (hasValue(identifier) && identifier.identifierType === 'doi') { // The item has some kind of DOI no_doi = false; - if (identifier.identifierStatus === '10' || identifier.identifierStatus === '11' + if (identifier.identifierStatus === 'PENDING' || identifier.identifierStatus === 'MINTED' || identifier.identifierStatus == null) { // The item's DOI is pending, minted or null. // It isn't registered, reserved, queued for registration or reservation or update, deleted diff --git a/src/app/shared/object-list/identifier-data/identifier-data.component.ts b/src/app/shared/object-list/identifier-data/identifier-data.component.ts index 85989f34c6..5b495bfa62 100644 --- a/src/app/shared/object-list/identifier-data/identifier-data.component.ts +++ b/src/app/shared/object-list/identifier-data/identifier-data.component.ts @@ -1,10 +1,8 @@ import { Component, Input } from '@angular/core'; -import { catchError, map } from 'rxjs/operators'; -import { Observable, of as observableOf } from 'rxjs'; +import { map } from 'rxjs/operators'; +import { Observable } from 'rxjs'; import { hasValue } from '../../empty.util'; -import { environment } from 'src/environments/environment'; import { Item } from 'src/app/core/shared/item.model'; -import { AccessStatusDataService } from 'src/app/core/data/access-status-data.service'; import {IdentifierData} from './identifier-data.model'; import {IdentifierDataService} from '../../../core/data/identifier-data.service'; @@ -13,18 +11,13 @@ import {IdentifierDataService} from '../../../core/data/identifier-data.service' templateUrl: './identifier-data.component.html' }) /** - * Component rendering the access status of an item as a badge + * Component rendering an identifier, eg. DOI or handle */ export class IdentifierDataComponent { @Input() item: Item; identifiers$: Observable; - /** - * Whether to show the access status badge or not - */ - showAccessStatus: boolean; - /** * Initialize instance variables * @@ -34,11 +27,11 @@ export class IdentifierDataComponent { ngOnInit(): void { if (this.item == null) { - // Do not show the badge if the feature is inactive or if the item is null. + // Do not show the identifier if the feature is inactive or if the item is null. return; } if (this.item.identifiers == null) { - // In case the access status has not been loaded, do it individually. + // In case the identifier has not been loaded, do it individually. this.item.identifiers = this.identifierDataService.getIdentifierDataFor(this.item); } this.identifiers$ = this.item.identifiers.pipe( @@ -49,8 +42,6 @@ export class IdentifierDataComponent { return null; } }), - // EMpty array if none - //map((identifiers: IdentifierData) => hasValue(identifiers.identifiers) ? identifiers.identifiers : []) ); } } diff --git a/src/app/shared/object-list/identifier-data/identifier.model.ts b/src/app/shared/object-list/identifier-data/identifier.model.ts index 87b162afe1..f528824b36 100644 --- a/src/app/shared/object-list/identifier-data/identifier.model.ts +++ b/src/app/shared/object-list/identifier-data/identifier.model.ts @@ -1,12 +1,30 @@ -import {autoserialize} from 'cerialize'; +import { autoserialize } from 'cerialize'; +/** + * Identifier model. Identifiers using this model are returned in lists from the /item/{id}/identifiers endpoint + * + * @author Kim Shepherd + */ export class Identifier { + /** + * The value of the identifier, eg. http://hdl.handle.net/123456789/123 or https://doi.org/test/doi/1234 + */ @autoserialize value: string; + /** + * The type of identiifer, eg. "doi", or "handle", or "other" + */ @autoserialize identifierType: string; + /** + * The status of the identifier. Some schemes, like DOI, will have a different status based on whether it is + * queued for remote registration, reservation, or update, or has been registered, simply minted locally, etc. + */ @autoserialize identifierStatus: string; + /** + * The type of resource, in this case Identifier + */ @autoserialize type: string; }