Merge remote-tracking branch 'origin/main' into CST-4633-search-refactoring

2025-10-07 01:54:15 +00:00 · 2022-01-14 15:21:19 +01:00
parent b7c46ffe29 6a7691000b
commit 746d0201e8
16 changed files with 109 additions and 47 deletions
--- a/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts
+++ b/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts
@@ -63,7 +63,7 @@ export class BrowseByDatePageComponent extends BrowseByMetadataPageComponent {
        this.browseId = params.id || this.defaultBrowseId;
        this.startsWith = +params.startsWith || params.startsWith;
        const searchOptions = browseParamsToOptions(params, currentPage, currentSort, this.browseId);
-        this.updatePageWithItems(searchOptions, this.value);
+        this.updatePageWithItems(searchOptions, this.value, undefined);
        this.updateParent(params.scope);
        this.updateStartsWithOptions(this.browseId, metadataKeys, params.scope);
      }));
--- a/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts
+++ b/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts
@@ -99,6 +99,11 @@ export class BrowseByMetadataPageComponent implements OnInit {
   */
  value = '';

+  /**
+   * The authority key (may be undefined) associated with {@link #value}.
+   */
+   authority: string;
+
  /**
   * The current startsWith option (fetched and updated from query-params)
   */
@@ -123,11 +128,12 @@ export class BrowseByMetadataPageComponent implements OnInit {
        })
      ).subscribe(([params, currentPage, currentSort]: [Params, PaginationComponentOptions, SortOptions]) => {
          this.browseId = params.id || this.defaultBrowseId;
+          this.authority = params.authority;
          this.value = +params.value || params.value || '';
          this.startsWith = +params.startsWith || params.startsWith;
          const searchOptions = browseParamsToOptions(params, currentPage, currentSort, this.browseId);
          if (isNotEmpty(this.value)) {
-            this.updatePageWithItems(searchOptions, this.value);
+            this.updatePageWithItems(searchOptions, this.value, this.authority);
          } else {
            this.updatePage(searchOptions);
          }
@@ -166,8 +172,8 @@ export class BrowseByMetadataPageComponent implements OnInit {
   *                          scope: string }
   * @param value          The value of the browse-entry to display items for
   */
-  updatePageWithItems(searchOptions: BrowseEntrySearchOptions, value: string) {
-    this.items$ = this.browseService.getBrowseItemsFor(value, searchOptions);
+  updatePageWithItems(searchOptions: BrowseEntrySearchOptions, value: string, authority: string) {
+    this.items$ = this.browseService.getBrowseItemsFor(value, authority, searchOptions);
  }

  /**
--- a/src/app/browse-by/browse-by-title-page/browse-by-title-page.component.ts
+++ b/src/app/browse-by/browse-by-title-page/browse-by-title-page.component.ts
@@ -46,7 +46,7 @@ export class BrowseByTitlePageComponent extends BrowseByMetadataPageComponent {
        })
      ).subscribe(([params, currentPage, currentSort]: [Params, PaginationComponentOptions, SortOptions]) => {
        this.browseId = params.id || this.defaultBrowseId;
-        this.updatePageWithItems(browseParamsToOptions(params, currentPage, currentSort, this.browseId), undefined);
+        this.updatePageWithItems(browseParamsToOptions(params, currentPage, currentSort, this.browseId), undefined, undefined);
        this.updateParent(params.scope);
      }));
    this.updateStartsWithTextOptions();
--- a/src/app/core/browse/browse.service.spec.ts
+++ b/src/app/core/browse/browse.service.spec.ts
@@ -129,6 +129,7 @@ describe('BrowseService', () => {
  describe('getBrowseEntriesFor and findList', () => {
    // should contain special characters such that url encoding can be tested as well
    const mockAuthorName = 'Donald Smith & Sons';
+    const mockAuthorityKey = 'some authority key ?=;';

    beforeEach(() => {
      requestService = getMockRequestService(getRequestEntry$(true));
@@ -155,7 +156,7 @@ describe('BrowseService', () => {
      it('should call hrefOnlyDataService.findAllByHref with the expected href', () => {
        const expected = browseDefinitions[1]._links.items.href + '?filterValue=' + encodeURIComponent(mockAuthorName);

-        scheduler.schedule(() => service.getBrowseItemsFor(mockAuthorName, new BrowseEntrySearchOptions(browseDefinitions[1].id)).subscribe());
+        scheduler.schedule(() => service.getBrowseItemsFor(mockAuthorName, undefined, new BrowseEntrySearchOptions(browseDefinitions[1].id)).subscribe());
        scheduler.flush();

        expect(getFirstUsedArgumentOfSpyMethod(hrefOnlyDataService.findAllByHref)).toBeObservable(cold('(a|)', {
@@ -164,6 +165,20 @@ describe('BrowseService', () => {
      });

    });
+    describe('when getBrowseItemsFor is called with a valid filter value and authority key', () => {
+      it('should call hrefOnlyDataService.findAllByHref with the expected href', () => {
+        const expected = browseDefinitions[1]._links.items.href +
+          '?filterValue=' + encodeURIComponent(mockAuthorName) +
+          '&filterAuthority=' + encodeURIComponent(mockAuthorityKey);
+
+        scheduler.schedule(() => service.getBrowseItemsFor(mockAuthorName, mockAuthorityKey, new BrowseEntrySearchOptions(browseDefinitions[1].id)).subscribe());
+        scheduler.flush();
+
+        expect(getFirstUsedArgumentOfSpyMethod(hrefOnlyDataService.findAllByHref)).toBeObservable(cold('(a|)', {
+          a: expected
+        }));
+      });
+    });
  });

  describe('getBrowseURLFor', () => {
--- a/src/app/core/browse/browse.service.ts
+++ b/src/app/core/browse/browse.service.ts
@@ -105,7 +105,7 @@ export class BrowseService {
   * @param options                   Options to narrow down your search
   * @returns {Observable<RemoteData<PaginatedList<Item>>>}
   */
-  getBrowseItemsFor(filterValue: string, options: BrowseEntrySearchOptions): Observable<RemoteData<PaginatedList<Item>>> {
+  getBrowseItemsFor(filterValue: string, filterAuthority: string, options: BrowseEntrySearchOptions): Observable<RemoteData<PaginatedList<Item>>> {
    const href$ = this.getBrowseDefinitions().pipe(
      getBrowseDefinitionLinks(options.metadataDefinition),
      hasValueOperator(),
@@ -132,6 +132,9 @@ export class BrowseService {
        if (isNotEmpty(filterValue)) {
          args.push(`filterValue=${encodeURIComponent(filterValue)}`);
        }
+        if (isNotEmpty(filterAuthority)) {
+          args.push(`filterAuthority=${encodeURIComponent(filterAuthority)}`);
+        }
        if (isNotEmpty(args)) {
          href = new URLCombiner(href, `?${args.join('&')}`).toString();
        }
--- a/src/app/process-page/form/process-form.component.spec.ts
+++ b/src/app/process-page/form/process-form.component.spec.ts
@@ -87,4 +87,15 @@ describe('ProcessFormComponent', () => {
    component.submitForm({ controls: {} } as any);
    expect(scriptService.invoke).toHaveBeenCalled();
  });
+
+  describe('when undefined parameters are provided', () => {
+    beforeEach(() => {
+      component.parameters = undefined;
+    });
+
+    it('should invoke the script with an empty array of parameters', () => {
+      component.submitForm({ controls: {} } as any);
+      expect(scriptService.invoke).toHaveBeenCalledWith(script.id, [], jasmine.anything());
+    });
+  });
 });
--- a/src/app/process-page/form/process-form.component.ts
+++ b/src/app/process-page/form/process-form.component.ts
@@ -12,6 +12,7 @@ import { Router } from '@angular/router';
 import { getFirstCompletedRemoteData } from '../../core/shared/operators';
 import { RemoteData } from '../../core/data/remote-data';
 import { getProcessListRoute } from '../process-page-routing.paths';
+import { isEmpty } from '../../shared/empty.util';

 /**
 * Component to create a new script
@@ -35,7 +36,7 @@ export class ProcessFormComponent implements OnInit {
  /**
   * The parameter values to use to start the process
   */
-  @Input() public parameters: ProcessParameter[];
+  @Input() public parameters: ProcessParameter[] = [];

  /**
   * Optional files that are used as parameter values
@@ -69,6 +70,9 @@ export class ProcessFormComponent implements OnInit {
   * @param form
   */
  submitForm(form: NgForm) {
+    if (isEmpty(this.parameters)) {
+      this.parameters = [];
+    }
    if (!this.validateForm(form) || this.isRequiredMissing()) {
      return;
    }
--- a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html
+++ b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html
@@ -1,5 +1,5 @@
 <div class="d-flex flex-row">
-    <a *ngIf="linkType != linkTypes.None" [target]="(linkType == linkTypes.ExternalLink) ? '_blank' : '_self'" rel="noopener noreferrer" [routerLink]="" [queryParams]="{value: object.value, startsWith: undefined}" [queryParamsHandling]="'merge'" class="lead">
+    <a *ngIf="linkType != linkTypes.None" [target]="(linkType == linkTypes.ExternalLink) ? '_blank' : '_self'" rel="noopener noreferrer" [routerLink]="" [queryParams]="getQueryParams()" [queryParamsHandling]="'merge'" class="lead">
        {{object.value}}
    </a>
    <span *ngIf="linkType == linkTypes.None" class="lead">
--- a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts
+++ b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts
@@ -15,4 +15,16 @@ import { listableObjectComponent } from '../../object-collection/shared/listable
 * This component is automatically used to create a list view for BrowseEntry objects when used in ObjectCollectionComponent
 */
@listableObjectComponent(BrowseEntry, ViewMode.ListElement)
-export class BrowseEntryListElementComponent extends AbstractListableElementComponent<BrowseEntry> {}
+export class BrowseEntryListElementComponent extends AbstractListableElementComponent<BrowseEntry> {
+
+  /**
+   * Get the query params to access the item page of this browse entry.
+   */
+  public getQueryParams(): {[param: string]: any} {
+    return {
+      value: this.object.value,
+      authority: !!this.object.authority ? this.object.authority : undefined,
+      startsWith: undefined,
+    };
+  }
+}
--- a/src/app/shared/search/models/facet-value.model.ts
+++ b/src/app/shared/search/models/facet-value.model.ts
@@ -24,6 +24,12 @@ export class FacetValue implements HALResource {
  @autoserialize
  count: number;

+  /**
+   * The Authority Value for this facet
+   */
+  @autoserialize
+  authorityKey?: string;
+
  /**
   * The {@link HALLink}s for this FacetValue
   */
--- a/src/app/shared/search/search.utils.spec.ts
+++ b/src/app/shared/search/search.utils.spec.ts
@@ -12,6 +12,7 @@ describe('Search Utils', () => {
    let facetValueWithSearchHref: FacetValue;
    let facetValueWithoutSearchHref: FacetValue;
    let searchFilterConfig: SearchFilterConfig;
+    let facetValueWithSearchHrefAuthority: FacetValue;

    beforeEach(() => {
      facetValueWithSearchHref = Object.assign(new FacetValue(), {
@@ -22,6 +23,11 @@ describe('Search Utils', () => {
          }
        }
      });
+      facetValueWithSearchHrefAuthority = Object.assign(new FacetValue(), {
+        value: 'Value with search href',
+        authorityKey: 'uuid',
+        }
+      );
      facetValueWithoutSearchHref = Object.assign(new FacetValue(), {
        value: 'Value without search href'
      });
@@ -34,6 +40,10 @@ describe('Search Utils', () => {
      expect(getFacetValueForType(facetValueWithSearchHref, searchFilterConfig)).toEqual('Value with search href,operator');
    });

+    it('should retrieve the correct value from the Facet', () => {
+      expect(getFacetValueForType(facetValueWithSearchHrefAuthority, searchFilterConfig)).toEqual('uuid,authority');
+    });
+
    it('should return the facet value with an equals operator by default', () => {
      expect(getFacetValueForType(facetValueWithoutSearchHref, searchFilterConfig)).toEqual('Value without search href,equals');
    });
--- a/src/app/shared/search/search.utils.ts
+++ b/src/app/shared/search/search.utils.ts
@@ -16,6 +16,10 @@ export function getFacetValueForType(facetValue: FacetValue, searchFilterConfig:
      return values[1];
    }
  }
+  if (facetValue.authorityKey) {
+    return addOperatorToFilterValue(facetValue.authorityKey, 'authority');
+  }
+
  return addOperatorToFilterValue(facetValue.value, 'equals');
 }

--- a/src/app/submission/form/footer/submission-form-footer.component.html
+++ b/src/app/submission/form/footer/submission-form-footer.component.html
@@ -41,7 +41,7 @@
      <button *ngIf="(showDepositAndDiscard | async)"
              type="button"
              class="btn btn-success"
-              [disabled]="(submissionIsInvalid | async) || (processingSaveStatus | async) || (processingDepositStatus | async)"
+              [disabled]="(processingSaveStatus | async) || (processingDepositStatus | async)"
              (click)="deposit($event)">
        <span><i class="fas fa-plus"></i> {{'submission.general.deposit' | translate}}</span>
      </button>
--- a/src/app/submission/form/footer/submission-form-footer.component.spec.ts
+++ b/src/app/submission/form/footer/submission-form-footer.component.spec.ts
@@ -201,13 +201,13 @@ describe('SubmissionFormFooterComponent Component', () => {
      });
    });

-    it('should have deposit button disabled when submission is not valid', () => {
+    it('should not have deposit button disabled when submission is not valid', () => {
      comp.showDepositAndDiscard = observableOf(true);
      compAsAny.submissionIsInvalid = observableOf(true);
      fixture.detectChanges();
      const depositBtn: any = fixture.debugElement.query(By.css('.btn-success'));

-      expect(depositBtn.nativeElement.disabled).toBeTruthy();
+      expect(depositBtn.nativeElement.disabled).toBeFalsy();
    });

    it('should not have deposit button disabled when submission is valid', () => {
--- a/src/app/submission/objects/submission-objects.effects.spec.ts
+++ b/src/app/submission/objects/submission-objects.effects.spec.ts
@@ -54,6 +54,8 @@ import { Item } from '../../core/shared/item.model';
 import { WorkspaceitemDataService } from '../../core/submission/workspaceitem-data.service';
 import { WorkflowItemDataService } from '../../core/submission/workflowitem-data.service';
 import { HALEndpointService } from '../../core/shared/hal-endpoint.service';
+import { SubmissionObjectDataService } from '../../core/submission/submission-object-data.service';
+import { mockSubmissionObjectDataService } from '../../shared/testing/submission-oject-data-service.mock';

 describe('SubmissionObjectEffects test suite', () => {
  let submissionObjectEffects: SubmissionObjectEffects;
@@ -63,6 +65,7 @@ describe('SubmissionObjectEffects test suite', () => {
  let notificationsServiceStub;
  let submissionServiceStub;
  let submissionJsonPatchOperationsServiceStub;
+  let submissionObjectDataServiceStub;
  const collectionId: string = mockSubmissionCollectionId;
  const submissionId: string = mockSubmissionId;
  const submissionDefinitionResponse: any = mockSubmissionDefinitionResponse;
@@ -75,6 +78,9 @@ describe('SubmissionObjectEffects test suite', () => {
    notificationsServiceStub = new NotificationsServiceStub();
    submissionServiceStub =  new SubmissionServiceStub();
    submissionJsonPatchOperationsServiceStub = new SubmissionJsonPatchOperationsServiceStub();
+    submissionObjectDataServiceStub = mockSubmissionObjectDataService;
+
+    submissionServiceStub.hasUnsavedModification.and.returnValue(observableOf(true));

    TestBed.configureTestingModule({
      imports: [
@@ -99,6 +105,7 @@ describe('SubmissionObjectEffects test suite', () => {
        { provide: WorkflowItemDataService, useValue: {} },
        { provide: WorkflowItemDataService, useValue: {} },
        { provide: HALEndpointService, useValue: {} },
+        { provide: SubmissionObjectDataService, useValue: submissionObjectDataServiceStub },
      ],
    });

@@ -879,7 +886,7 @@ describe('SubmissionObjectEffects test suite', () => {
      expect(submissionObjectEffects.saveAndDeposit$).toBeObservable(expected);
    });

-    it('should not allow to deposit when there are errors', () => {
+    it('should return a SAVE_SUBMISSION_FORM_SUCCESS action when there are errors', () => {
      store.nextState({
        submission: {
          objects: submissionState
@@ -902,31 +909,8 @@ describe('SubmissionObjectEffects test suite', () => {

      submissionJsonPatchOperationsServiceStub.jsonPatchByResourceType.and.returnValue(observableOf(response));

-      const errorsList = parseSectionErrors(mockSectionsErrors);
      const expected = cold('--b-', {
-        b: [
-          new UpdateSectionDataAction(
-            submissionId,
-            'traditionalpageone',
-            mockSectionsData.traditionalpageone as any,
-            errorsList.traditionalpageone || [],
-            errorsList.traditionalpageone || []
-          ),
-          new UpdateSectionDataAction(
-            submissionId,
-            'license',
-            mockSectionsData.license as any,
-            errorsList.license || [],
-            errorsList.license || []
-          ),
-          new UpdateSectionDataAction(
-            submissionId,
-            'upload',
-            mockSectionsData.upload as any,
-            errorsList.upload || [],
-            errorsList.upload || []
-          )
-        ]
+        b: new SaveSubmissionFormSuccessAction(submissionId, response as any[])
      });

      expect(submissionObjectEffects.saveAndDeposit$).toBeObservable(expected);
--- a/src/app/submission/objects/submission-objects.effects.ts
+++ b/src/app/submission/objects/submission-objects.effects.ts
@@ -196,19 +196,26 @@ export class SubmissionObjectEffects {
   */
  @Effect() saveAndDeposit$ = this.actions$.pipe(
    ofType(SubmissionObjectActionTypes.SAVE_AND_DEPOSIT_SUBMISSION),
-    withLatestFrom(this.store$),
-    switchMap(([action, currentState]: [SaveAndDepositSubmissionAction, any]) => {
-      return this.operationsService.jsonPatchByResourceType(
-        this.submissionService.getSubmissionObjectLinkName(),
-        action.payload.submissionId,
-        'sections').pipe(
+    withLatestFrom(this.submissionService.hasUnsavedModification()),
+    switchMap(([action, hasUnsavedModification]: [SaveAndDepositSubmissionAction, boolean]) => {
+      let response$: Observable<SubmissionObject[]>;
+      if (hasUnsavedModification) {
+        response$ = this.operationsService.jsonPatchByResourceType(
+          this.submissionService.getSubmissionObjectLinkName(),
+          action.payload.submissionId,
+          'sections') as Observable<SubmissionObject[]>;
+      } else {
+        response$ = this.submissionObjectService.findById(action.payload.submissionId).pipe(
+          getFirstSucceededRemoteDataPayload(),
+          map((submissionObject: SubmissionObject) => [submissionObject])
+        );
+      }
+      return response$.pipe(
        map((response: SubmissionObject[]) => {
          if (this.canDeposit(response)) {
            return new DepositSubmissionAction(action.payload.submissionId);
          } else {
-            this.notificationsService.warning(null, this.translate.get('submission.sections.general.sections_not_valid'));
-            return this.parseSaveResponse((currentState.submission as SubmissionState).objects[action.payload.submissionId],
-              response, action.payload.submissionId, currentState.forms);
+            return new SaveSubmissionFormSuccessAction(action.payload.submissionId, response);
          }
        }),
        catchError(() => observableOf(new SaveSubmissionFormErrorAction(action.payload.submissionId))));