[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

@@ -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<IdentifierData> {
@@ -50,7 +55,7 @@ export class IdentifierDataService extends BaseDataService<IdentifierData> {
    * Should we allow registration of new DOIs via the item status page?
    */
   public getIdentifierRegistrationConfiguration(): Observable<string[]> {
-    return this.configurationService.findByPropertyName('identifiers.item-status.register').pipe(
+    return this.configurationService.findByPropertyName('identifiers.item-status.registerDOI').pipe(
       getFirstCompletedRemoteData(),
       map((propertyRD: RemoteData<ConfigurationProperty>) => propertyRD.hasSucceeded ? propertyRD.payload.values : [])
     );

src/app/core/data/item-data.service.ts

@@ -256,7 +256,9 @@ export abstract class BaseItemDataService extends IdentifiableDataService<Item>
       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);

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'
       ]

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<boolean> = this.configurationService.findByPropertyName('identifiers.item-status.register').pipe(
+      let registerConfigEnabled$: Observable<boolean> = this.configurationService.findByPropertyName('identifiers.item-status.registerDOI').pipe(
         map((enabled: RemoteData<ConfigurationProperty>) => {
           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

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<IdentifierData>;
 
-  /**
-   * 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 : [])
     );
   }
 }

src/app/shared/object-list/identifier-data/identifier.model.ts

@@ -1,12 +1,30 @@
 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;
 }