From ed959e492a27213fc97b11a986c5778b85addbd2 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 4 Jul 2019 11:17:56 +0200 Subject: [PATCH 01/41] Show only authorized collections list during submission --- src/app/core/data/collection-data.service.ts | 17 +++++++++++++++++ ...submission-form-collection.component.spec.ts | 16 +++++++++++++++- .../submission-form-collection.component.ts | 11 +++++++++-- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/app/core/data/collection-data.service.ts b/src/app/core/data/collection-data.service.ts index 993954a360..762838e9ae 100644 --- a/src/app/core/data/collection-data.service.ts +++ b/src/app/core/data/collection-data.service.ts @@ -19,6 +19,7 @@ import { Observable } from 'rxjs/internal/Observable'; import { FindAllOptions } from './request.models'; import { RemoteData } from './remote-data'; import { PaginatedList } from './paginated-list'; +import { SearchParam } from '../cache/models/search-param.model'; @Injectable() export class CollectionDataService extends ComColDataService { @@ -40,6 +41,22 @@ export class CollectionDataService extends ComColDataService { super(); } + /** + * Get all collections whom user has authorization to submit to by community + * + * @return boolean + * true if the user has at least one collection to submit to + */ + getAuthorizedCollectionByCommunity(communityId): Observable>> { + const searchHref = 'findAuthorizedByCommunity'; + const options = new FindAllOptions(); + options.elementsPerPage = 1000; + options.searchParams = [new SearchParam('uuid', communityId)]; + + return this.searchBy(searchHref, options).pipe( + filter((collections: RemoteData>) => !collections.isResponsePending)); + } + /** * Find whether there is a collection whom user has authorization to submit to * diff --git a/src/app/submission/form/collection/submission-form-collection.component.spec.ts b/src/app/submission/form/collection/submission-form-collection.component.spec.ts index 679500a670..fc34094ce0 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.spec.ts +++ b/src/app/submission/form/collection/submission-form-collection.component.spec.ts @@ -8,6 +8,7 @@ import { filter } from 'rxjs/operators'; import { TranslateModule } from '@ngx-translate/core'; import { NgbModule } from '@ng-bootstrap/ng-bootstrap'; import { Store } from '@ngrx/store'; +import { cold } from 'jasmine-marbles'; import { SubmissionServiceStub } from '../../../shared/testing/submission-service-stub'; import { mockSubmissionId, mockSubmissionRestResponse } from '../../../shared/mocks/mock-submission'; @@ -24,7 +25,7 @@ import { PaginatedList } from '../../../core/data/paginated-list'; import { PageInfo } from '../../../core/shared/page-info.model'; import { Collection } from '../../../core/shared/collection.model'; import { createTestComponent } from '../../../shared/testing/utils'; -import { cold } from 'jasmine-marbles'; +import { CollectionDataService } from '../../../core/data/collection-data.service'; const subcommunities = [Object.assign(new Community(), { name: 'SubCommunity 1', @@ -125,6 +126,12 @@ const mockCommunity2 = Object.assign(new Community(), { const mockCommunityList = observableOf(new RemoteData(true, true, true, undefined, new PaginatedList(new PageInfo(), [mockCommunity, mockCommunity2]))); +const mockCommunityCollectionList = observableOf(new RemoteData(true, true, true, + undefined, new PaginatedList(new PageInfo(), [mockCommunity1Collection1, mockCommunity1Collection2]))); + +const mockCommunity2CollectionList = observableOf(new RemoteData(true, true, true, + undefined, new PaginatedList(new PageInfo(), [mockCommunity2Collection1, mockCommunity2Collection2]))); + const mockCollectionList = [ { communities: [ @@ -193,6 +200,11 @@ describe('SubmissionFormCollectionComponent Component', () => { const communityDataService: any = jasmine.createSpyObj('communityDataService', { findAll: jasmine.createSpy('findAll') }); + + const collectionDataService: any = jasmine.createSpyObj('collectionDataService', { + getAuthorizedCollectionByCommunity: jasmine.createSpy('getAuthorizedCollectionByCommunity') + }); + const store: any = jasmine.createSpyObj('store', { dispatch: jasmine.createSpy('dispatch'), select: jasmine.createSpy('select') @@ -214,6 +226,7 @@ describe('SubmissionFormCollectionComponent Component', () => { TestComponent ], providers: [ + { provide: CollectionDataService, useValue: collectionDataService }, { provide: SubmissionJsonPatchOperationsService, useClass: SubmissionJsonPatchOperationsServiceStub }, { provide: SubmissionService, useClass: SubmissionServiceStub }, { provide: CommunityDataService, useValue: communityDataService }, @@ -284,6 +297,7 @@ describe('SubmissionFormCollectionComponent Component', () => { it('should init collection list properly', () => { communityDataService.findAll.and.returnValue(mockCommunityList); + collectionDataService.getAuthorizedCollectionByCommunity.and.returnValues(mockCommunityCollectionList, mockCommunity2CollectionList); comp.ngOnChanges({ currentCollectionId: new SimpleChange(null, collectionId, true) diff --git a/src/app/submission/form/collection/submission-form-collection.component.ts b/src/app/submission/form/collection/submission-form-collection.component.ts index b576834091..eb7459eaf4 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.ts +++ b/src/app/submission/form/collection/submission-form-collection.component.ts @@ -35,6 +35,8 @@ import { PaginatedList } from '../../../core/data/paginated-list'; import { SubmissionService } from '../../submission.service'; import { SubmissionObject } from '../../../core/submission/models/submission-object.model'; import { SubmissionJsonPatchOperationsService } from '../../../core/submission/submission-json-patch-operations.service'; +import { CollectionDataService } from '../../../core/data/collection-data.service'; +import { FindAllOptions } from '../../../core/data/request.models'; /** * An interface to represent a collection entry @@ -145,12 +147,14 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { * * @param {ChangeDetectorRef} cdr * @param {CommunityDataService} communityDataService + * @param {CollectionDataService} collectionDataService * @param {JsonPatchOperationsBuilder} operationsBuilder * @param {SubmissionJsonPatchOperationsService} operationsService * @param {SubmissionService} submissionService */ constructor(protected cdr: ChangeDetectorRef, private communityDataService: CommunityDataService, + private collectionDataService: CollectionDataService, private operationsBuilder: JsonPatchOperationsBuilder, private operationsService: SubmissionJsonPatchOperationsService, private submissionService: SubmissionService) { @@ -189,16 +193,19 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { if (hasValue(changes.currentCollectionId) && hasValue(changes.currentCollectionId.currentValue)) { this.selectedCollectionId = this.currentCollectionId; + const findOptions: FindAllOptions = { + elementsPerPage: 100 + }; // @TODO replace with search/top browse endpoint // @TODO implement community/subcommunity hierarchy - const communities$ = this.communityDataService.findAll().pipe( + const communities$ = this.communityDataService.findAll(findOptions).pipe( find((communities: RemoteData>) => isNotEmpty(communities.payload)), mergeMap((communities: RemoteData>) => communities.payload.page)); const listCollection$ = communities$.pipe( flatMap((communityData: Community) => { - return communityData.collections.pipe( + return this.collectionDataService.getAuthorizedCollectionByCommunity(communityData.uuid).pipe( find((collections: RemoteData>) => !collections.isResponsePending && collections.hasSucceeded), mergeMap((collections: RemoteData>) => collections.payload.page), filter((collectionData: Collection) => isNotEmpty(collectionData)), From b665456b9d8bf30c8d559863316127b126225fd7 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 4 Jul 2019 13:03:18 +0200 Subject: [PATCH 02/41] Fixed issues with relation group field --- src/app/shared/chips/models/chips-item.model.ts | 8 +++++++- .../ds-dynamic-form-ui/models/ds-dynamic-concat.model.ts | 7 +++++++ .../relation-group/dynamic-relation-group.components.ts | 4 +++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/app/shared/chips/models/chips-item.model.ts b/src/app/shared/chips/models/chips-item.model.ts index 540f94166f..913232fa71 100644 --- a/src/app/shared/chips/models/chips-item.model.ts +++ b/src/app/shared/chips/models/chips-item.model.ts @@ -2,6 +2,7 @@ import { isObject, uniqueId } from 'lodash'; import { hasValue, isNotEmpty } from '../../empty.util'; import { FormFieldMetadataValueObject } from '../../form/builder/models/form-field-metadata-value.model'; import { ConfidenceType } from '../../../core/integration/models/confidence-type'; +import { PLACEHOLDER_PARENT_METADATA } from '../../form/builder/ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.model'; export interface ChipsItemIcon { metadata: string; @@ -62,7 +63,7 @@ export class ChipsItem { if (this._item.hasOwnProperty(icon.metadata) && (((typeof this._item[icon.metadata] === 'string') && hasValue(this._item[icon.metadata])) || (this._item[icon.metadata] as FormFieldMetadataValueObject).hasValue()) - && !(this._item[icon.metadata] as FormFieldMetadataValueObject).hasPlaceholder()) { + && !this.hasPlaceholder(this._item[icon.metadata])) { if ((icon.visibleWhenAuthorityEmpty || (this._item[icon.metadata] as FormFieldMetadataValueObject).confidence !== ConfidenceType.CF_UNSET) && isNotEmpty(icon.style)) { @@ -109,4 +110,9 @@ export class ChipsItem { this.display = value; } + + private hasPlaceholder(value: any) { + return (typeof value === 'string') ? (value === PLACEHOLDER_PARENT_METADATA) : + (value as FormFieldMetadataValueObject).hasPlaceholder() + } } diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-concat.model.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-concat.model.ts index fc618023f9..66bdf97dad 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-concat.model.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-concat.model.ts @@ -1,4 +1,7 @@ import { DynamicFormControlLayout, DynamicFormGroupModel, DynamicFormGroupModelConfig, serializable } from '@ng-dynamic-forms/core'; + +import { Subject } from 'rxjs'; + import { isNotEmpty } from '../../../../empty.util'; import { DsDynamicInputModel } from './ds-dynamic-input.model'; import { FormFieldMetadataValueObject } from '../../models/form-field-metadata-value.model'; @@ -16,12 +19,16 @@ export class DynamicConcatModel extends DynamicFormGroupModel { @serializable() separator: string; @serializable() hasLanguages = false; isCustomGroup = true; + valueUpdates: Subject; constructor(config: DynamicConcatModelConfig, layout?: DynamicFormControlLayout) { super(config, layout); this.separator = config.separator + ' '; + + this.valueUpdates = new Subject(); + this.valueUpdates.subscribe((value: string) => this.value = value); } get value() { diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.components.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.components.ts index fde8d4b7bf..1485993375 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.components.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.components.ts @@ -130,7 +130,9 @@ export class DsDynamicRelationGroupComponent extends DynamicFormControlComponent ? null : this.selectedChipItem.item[model.name]; if (isNotNull(value)) { - model.valueUpdates.next(this.formBuilderService.isInputModel(model) ? value.value : value); + const nextValue = (this.formBuilderService.isInputModel(model) && (typeof value !== 'string')) ? + value.value : value; + model.valueUpdates.next(nextValue); } }); }); From fe1e4931c32fd6c00e2b120f4be257f24ebec45b Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Mon, 20 May 2019 12:53:07 +0200 Subject: [PATCH 03/41] hide upload section on init when is not mandatory and there a no file uploaded in the submission --- .../core/submission/submission-response-parsing.service.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/app/core/submission/submission-response-parsing.service.ts b/src/app/core/submission/submission-response-parsing.service.ts index 21135be463..5206227d4e 100644 --- a/src/app/core/submission/submission-response-parsing.service.ts +++ b/src/app/core/submission/submission-response-parsing.service.ts @@ -131,7 +131,10 @@ export class SubmissionResponseParsingService extends BaseResponseParsingService // Iterate over all workspaceitem's sections Object.keys(item.sections) .forEach((sectionId) => { - if (typeof item.sections[sectionId] === 'object' && isNotEmpty(item.sections[sectionId])) { + if (typeof item.sections[sectionId] === 'object' && (isNotEmpty(item.sections[sectionId]) && + // When Upload section is disabled, add to submission only if there are files + (!item.sections[sectionId].hasOwnProperty('files') || isNotEmpty((item.sections[sectionId] as any).files)))) { + const normalizedSectionData = Object.create({}); // Iterate over all sections property Object.keys(item.sections[sectionId]) From 4a1530eea5e31d327b4350299831b9a4d38bbc67 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 15 May 2019 13:15:10 +0200 Subject: [PATCH 04/41] fixed issue with submission footer z-index --- src/styles/_custom_variables.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/styles/_custom_variables.scss b/src/styles/_custom_variables.scss index be03d719c5..b4de04ad9e 100644 --- a/src/styles/_custom_variables.scss +++ b/src/styles/_custom_variables.scss @@ -11,7 +11,7 @@ $drop-zone-area-inner-z-index: 1021; $login-logo-height:72px; $login-logo-width:72px; $submission-header-z-index: 1001; -$submission-footer-z-index: 1000; +$submission-footer-z-index: 999; $main-z-index: 0; $nav-z-index: 10; From 6856f95cb94ca7fdc911a79c9455317656b33389 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Fri, 5 Jul 2019 13:10:51 +0200 Subject: [PATCH 05/41] Fixed an issue when submission metadata auto save is triggered on a typeahead field --- .../dynamic-typeahead.component.html | 3 +- .../dynamic-typeahead.component.spec.ts | 47 +++++++++++++++++-- .../typeahead/dynamic-typeahead.component.ts | 30 ++++++++---- 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/typeahead/dynamic-typeahead.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/typeahead/dynamic-typeahead.component.html index 51e7667200..449481152d 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/typeahead/dynamic-typeahead.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/typeahead/dynamic-typeahead.component.html @@ -28,7 +28,8 @@ aria-hidden="true" [authorityValue]="currentValue" (whenClickOnConfidenceNotAccepted)="whenClickOnConfidenceNotAccepted($event)"> - { inputElement.value = 'test value'; inputElement.dispatchEvent(new Event('input')); - expect((typeaheadComp.model as any).value).toEqual(new FormFieldMetadataValueObject('test value')) + expect(typeaheadComp.inputValue).toEqual(new FormFieldMetadataValueObject('test value')) }); @@ -173,19 +173,56 @@ describe('DsDynamicTypeaheadComponent test suite', () => { }); - it('should emit blur Event onBlur', () => { + it('should emit blur Event onBlur when popup is closed', () => { spyOn(typeaheadComp.blur, 'emit'); + spyOn(typeaheadComp.instance, 'isPopupOpen').and.returnValue(false); typeaheadComp.onBlur(new Event('blur')); expect(typeaheadComp.blur.emit).toHaveBeenCalled(); }); - it('should emit change Event onBlur when AuthorityOptions.closed is false', () => { + it('should not emit blur Event onBlur when popup is opened', () => { + spyOn(typeaheadComp.blur, 'emit'); + spyOn(typeaheadComp.instance, 'isPopupOpen').and.returnValue(true); + const input = typeaheadFixture.debugElement.query(By.css('input')); + + input.nativeElement.blur(); + expect(typeaheadComp.blur.emit).not.toHaveBeenCalled(); + }); + + it('should emit change Event onBlur when AuthorityOptions.closed is false and inputValue is changed', () => { typeaheadComp.inputValue = 'test value'; typeaheadFixture.detectChanges(); spyOn(typeaheadComp.blur, 'emit'); spyOn(typeaheadComp.change, 'emit'); - typeaheadComp.onBlur(new Event('blur')); - // expect(typeaheadComp.change.emit).toHaveBeenCalled(); + spyOn(typeaheadComp.instance, 'isPopupOpen').and.returnValue(false); + typeaheadComp.onBlur(new Event('blur', )); + expect(typeaheadComp.change.emit).toHaveBeenCalled(); + expect(typeaheadComp.blur.emit).toHaveBeenCalled(); + }); + + it('should not emit change Event onBlur when AuthorityOptions.closed is false and inputValue is not changed', () => { + typeaheadComp.inputValue = 'test value'; + typeaheadComp.model = new DynamicTypeaheadModel(TYPEAHEAD_TEST_MODEL_CONFIG); + (typeaheadComp.model as any).value = 'test value'; + typeaheadFixture.detectChanges(); + spyOn(typeaheadComp.blur, 'emit'); + spyOn(typeaheadComp.change, 'emit'); + spyOn(typeaheadComp.instance, 'isPopupOpen').and.returnValue(false); + typeaheadComp.onBlur(new Event('blur', )); + expect(typeaheadComp.change.emit).not.toHaveBeenCalled(); + expect(typeaheadComp.blur.emit).toHaveBeenCalled(); + }); + + it('should not emit change Event onBlur when AuthorityOptions.closed is false and inputValue is null', () => { + typeaheadComp.inputValue = null; + typeaheadComp.model = new DynamicTypeaheadModel(TYPEAHEAD_TEST_MODEL_CONFIG); + (typeaheadComp.model as any).value = 'test value'; + typeaheadFixture.detectChanges(); + spyOn(typeaheadComp.blur, 'emit'); + spyOn(typeaheadComp.change, 'emit'); + spyOn(typeaheadComp.instance, 'isPopupOpen').and.returnValue(false); + typeaheadComp.onBlur(new Event('blur', )); + expect(typeaheadComp.change.emit).not.toHaveBeenCalled(); expect(typeaheadComp.blur.emit).toHaveBeenCalled(); }); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/typeahead/dynamic-typeahead.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/typeahead/dynamic-typeahead.component.ts index ace6812858..136d1db1c2 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/typeahead/dynamic-typeahead.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/typeahead/dynamic-typeahead.component.ts @@ -1,4 +1,4 @@ -import { ChangeDetectorRef, Component, EventEmitter, Input, OnInit, Output } from '@angular/core'; +import { ChangeDetectorRef, Component, EventEmitter, Input, OnInit, Output, ViewChild } from '@angular/core'; import { FormGroup } from '@angular/forms'; import { @@ -8,14 +8,13 @@ import { } from '@ng-dynamic-forms/core'; import { catchError, debounceTime, distinctUntilChanged, filter, map, merge, switchMap, tap } from 'rxjs/operators'; import { Observable, of as observableOf, Subject } from 'rxjs'; -import { NgbTypeaheadSelectItemEvent } from '@ng-bootstrap/ng-bootstrap'; +import { NgbTypeahead, NgbTypeaheadSelectItemEvent } from '@ng-bootstrap/ng-bootstrap'; import { AuthorityService } from '../../../../../../core/integration/authority.service'; import { DynamicTypeaheadModel } from './dynamic-typeahead.model'; import { IntegrationSearchOptions } from '../../../../../../core/integration/models/integration-options.model'; -import { isEmpty, isNotEmpty } from '../../../../../empty.util'; +import { isEmpty, isNotEmpty, isNotNull } from '../../../../../empty.util'; import { FormFieldMetadataValueObject } from '../../../models/form-field-metadata-value.model'; - import { ConfidenceType } from '../../../../../../core/integration/models/confidence-type'; @Component({ @@ -32,6 +31,8 @@ export class DsDynamicTypeaheadComponent extends DynamicFormControlComponent imp @Output() change: EventEmitter = new EventEmitter(); @Output() focus: EventEmitter = new EventEmitter(); + @ViewChild('instance') instance: NgbTypeahead; + searching = false; searchOptions: IntegrationSearchOptions; searchFailed = false; @@ -105,16 +106,26 @@ export class DsDynamicTypeaheadComponent extends DynamicFormControlComponent imp onInput(event) { if (!this.model.authorityOptions.closed && isNotEmpty(event.target.value)) { this.inputValue = new FormFieldMetadataValueObject(event.target.value); - this.model.valueUpdates.next(this.inputValue); } } onBlur(event: Event) { - if (!this.model.authorityOptions.closed && isNotEmpty(this.inputValue)) { - this.change.emit(this.inputValue); - this.inputValue = null; + if (!this.instance.isPopupOpen()) { + if (!this.model.authorityOptions.closed && isNotEmpty(this.inputValue)) { + if (isNotNull(this.inputValue) && this.model.value !== this.inputValue) { + this.model.valueUpdates.next(this.inputValue); + this.change.emit(this.inputValue); + } + this.inputValue = null; + } + this.blur.emit(event); + } else { + // prevent on blur propagation if typeahed suggestions are showed + event.preventDefault(); + event.stopImmediatePropagation(); + // set focus on input again, this is to avoid to lose changes when no suggestion is selected + (event.target as HTMLInputElement).focus(); } - this.blur.emit(event); } onChange(event: Event) { @@ -141,4 +152,5 @@ export class DsDynamicTypeaheadComponent extends DynamicFormControlComponent imp this.click$.next(this.formatter(this.currentValue)); } } + } From 016afd84e466638213a4a6f5386367db6ba61f24 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Mon, 8 Jul 2019 10:40:41 +0200 Subject: [PATCH 06/41] Fixed edit link for workflow --- .../claimed-task/claimed-task-actions.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/shared/mydspace-actions/claimed-task/claimed-task-actions.component.html b/src/app/shared/mydspace-actions/claimed-task/claimed-task-actions.component.html index 4b9b93e7e3..3a8cb0cded 100644 --- a/src/app/shared/mydspace-actions/claimed-task/claimed-task-actions.component.html +++ b/src/app/shared/mydspace-actions/claimed-task/claimed-task-actions.component.html @@ -2,7 +2,7 @@ {{'submission.workflow.tasks.claimed.edit' | translate}} From 7667cab772fd51ca5ebe8381cf144553bc7a9bd9 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 15 May 2019 17:58:37 +0200 Subject: [PATCH 07/41] Added hint message to form fields --- .../ds-dynamic-form-ui/models/ds-dynamic-input.model.ts | 7 ++----- .../models/ds-dynamic-qualdrop.model.ts | 8 ++++++-- src/app/shared/form/builder/parsers/field-parser.ts | 2 ++ .../shared/form/builder/parsers/onebox-field-parser.ts | 4 ++++ src/app/shared/form/form.component.scss | 5 +++++ 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-input.model.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-input.model.ts index 860c481820..4e4a944319 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-input.model.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-input.model.ts @@ -28,6 +28,7 @@ export class DsDynamicInputModel extends DynamicInputModel { constructor(config: DsDynamicInputModelConfig, layout?: DynamicFormControlLayout) { super(config, layout); + this.hint = config.hint; this.readOnly = config.readOnly; this.value = config.value; this.language = config.language; @@ -57,11 +58,7 @@ export class DsDynamicInputModel extends DynamicInputModel { } get hasLanguages(): boolean { - if (this.languageCodes && this.languageCodes.length > 1) { - return true; - } else { - return false; - } + return this.languageCodes && this.languageCodes.length > 1; } get language(): string { diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-qualdrop.model.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-qualdrop.model.ts index 6bd5a604a0..5d2cbc58b7 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-qualdrop.model.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-qualdrop.model.ts @@ -1,5 +1,5 @@ -import { DynamicFormControlLayout, DynamicFormGroupModel, DynamicInputModelConfig, serializable } from '@ng-dynamic-forms/core'; -import { DsDynamicInputModel, DsDynamicInputModelConfig } from './ds-dynamic-input.model'; +import { DynamicFormControlLayout, DynamicFormGroupModel, serializable } from '@ng-dynamic-forms/core'; +import { DsDynamicInputModel } from './ds-dynamic-input.model'; import { Subject } from 'rxjs'; import { DynamicFormGroupModelConfig } from '@ng-dynamic-forms/core/src/model/form-group/dynamic-form-group.model'; import { LanguageCode } from '../../models/form-field-language-value.model'; @@ -12,6 +12,7 @@ export interface DsDynamicQualdropModelConfig extends DynamicFormGroupModelConfi languageCodes?: LanguageCode[]; language?: string; readOnly: boolean; + hint?: string; } export class DynamicQualdropModel extends DynamicFormGroupModel { @@ -20,6 +21,7 @@ export class DynamicQualdropModel extends DynamicFormGroupModel { @serializable() languageUpdates: Subject; @serializable() hasLanguages = false; @serializable() readOnly: boolean; + @serializable() hint: string; isCustomGroup = true; constructor(config: DsDynamicQualdropModelConfig, layout?: DynamicFormControlLayout) { @@ -33,6 +35,8 @@ export class DynamicQualdropModel extends DynamicFormGroupModel { this.languageUpdates.subscribe((lang: string) => { this.language = lang; }); + + this.hint = config.hint; } get value() { diff --git a/src/app/shared/form/builder/parsers/field-parser.ts b/src/app/shared/form/builder/parsers/field-parser.ts index 28e3fb8fb5..dd37a45fba 100644 --- a/src/app/shared/form/builder/parsers/field-parser.ts +++ b/src/app/shared/form/builder/parsers/field-parser.ts @@ -190,6 +190,8 @@ export abstract class FieldParser { controlModel.placeholder = this.configData.label; + controlModel.hint = this.configData.hints; + if (this.configData.mandatory && setErrors) { this.markAsRequired(controlModel); } diff --git a/src/app/shared/form/builder/parsers/onebox-field-parser.ts b/src/app/shared/form/builder/parsers/onebox-field-parser.ts index d347f38eee..284656cc95 100644 --- a/src/app/shared/form/builder/parsers/onebox-field-parser.ts +++ b/src/app/shared/form/builder/parsers/onebox-field-parser.ts @@ -24,6 +24,7 @@ export class OneboxFieldParser extends FieldParser { const clsGroup = { element: { control: 'form-row', + hint: 'ds-form-qualdrop-hint' } }; @@ -54,8 +55,10 @@ export class OneboxFieldParser extends FieldParser { inputSelectGroup.id = newId.replace(/\./g, '_') + QUALDROP_GROUP_SUFFIX; inputSelectGroup.group = []; inputSelectGroup.legend = this.configData.label; + inputSelectGroup.hint = this.configData.hints; const selectModelConfig: DynamicSelectModelConfig = this.initModel(newId + QUALDROP_METADATA_SUFFIX, label); + selectModelConfig.hint = null; this.setOptions(selectModelConfig); if (isNotEmpty(fieldValue)) { selectModelConfig.value = fieldValue.metadata; @@ -63,6 +66,7 @@ export class OneboxFieldParser extends FieldParser { inputSelectGroup.group.push(new DynamicSelectModel(selectModelConfig, clsSelect)); const inputModelConfig: DsDynamicInputModelConfig = this.initModel(newId + QUALDROP_VALUE_SUFFIX, label, true); + inputModelConfig.hint = null; this.setValues(inputModelConfig, fieldValue); inputSelectGroup.readOnly = selectModelConfig.disabled && inputModelConfig.readOnly; diff --git a/src/app/shared/form/form.component.scss b/src/app/shared/form/form.component.scss index 1d5e034290..ed10941f09 100644 --- a/src/app/shared/form/form.component.scss +++ b/src/app/shared/form/form.component.scss @@ -44,3 +44,8 @@ .right-addon input { padding-right: $spacer * 2.25; } + +.ds-form-qualdrop-hint { + top: -$spacer; + position: relative; +} From cf73625830af5215b013b97e7366009e6329ffe4 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 11 Jul 2019 19:12:31 +0200 Subject: [PATCH 08/41] Fixed comments --- .../objects/submission-objects.reducer.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/app/submission/objects/submission-objects.reducer.ts b/src/app/submission/objects/submission-objects.reducer.ts index 1a65783945..8c111dde67 100644 --- a/src/app/submission/objects/submission-objects.reducer.ts +++ b/src/app/submission/objects/submission-objects.reducer.ts @@ -361,7 +361,7 @@ const addError = (state: SubmissionObjectState, action: InertSectionErrorsAction * @param state * the current state * @param action - * an RemoveSectionErrorsAction + * a RemoveSectionErrorsAction * @return SubmissionObjectState * the new state, with the section's errors updated. */ @@ -416,7 +416,7 @@ function initSubmission(state: SubmissionObjectState, action: InitSubmissionForm * @param state * the current state * @param action - * an ResetSubmissionFormAction + * a ResetSubmissionFormAction * @return SubmissionObjectState * the new state, with the section removed. */ @@ -439,7 +439,7 @@ function resetSubmission(state: SubmissionObjectState, action: ResetSubmissionFo * @param state * the current state * @param action - * an CompleteInitSubmissionFormAction + * a CompleteInitSubmissionFormAction * @return SubmissionObjectState * the new state, with the section removed. */ @@ -461,7 +461,7 @@ function completeInit(state: SubmissionObjectState, action: CompleteInitSubmissi * @param state * the current state * @param action - * an SaveSubmissionFormAction | SaveSubmissionSectionFormAction + * a SaveSubmissionFormAction | SaveSubmissionSectionFormAction * | SaveForLaterSubmissionFormAction | SaveAndDepositSubmissionAction * @return SubmissionObjectState * the new state, with the flag set to true. @@ -491,7 +491,7 @@ function saveSubmission(state: SubmissionObjectState, * @param state * the current state * @param action - * an SaveSubmissionFormSuccessAction | SaveForLaterSubmissionFormSuccessAction + * a SaveSubmissionFormSuccessAction | SaveForLaterSubmissionFormSuccessAction * | SaveSubmissionSectionFormSuccessAction | SaveSubmissionFormErrorAction * | SaveForLaterSubmissionFormErrorAction | SaveSubmissionSectionFormErrorAction * @return SubmissionObjectState @@ -521,7 +521,7 @@ function completeSave(state: SubmissionObjectState, * @param state * the current state * @param action - * an DepositSubmissionAction + * a DepositSubmissionAction * @return SubmissionObjectState * the new state, with the deposit flag changed. */ @@ -544,7 +544,7 @@ function startDeposit(state: SubmissionObjectState, action: DepositSubmissionAct * @param state * the current state * @param action - * an DepositSubmissionSuccessAction or DepositSubmissionErrorAction + * a DepositSubmissionSuccessAction or a DepositSubmissionErrorAction * @return SubmissionObjectState * the new state, with the deposit flag changed. */ @@ -586,7 +586,7 @@ function changeCollection(state: SubmissionObjectState, action: ChangeSubmission * @param state * the current state * @param action - * an SetActiveSectionAction + * a SetActiveSectionAction * @return SubmissionObjectState * the new state, with the active section. */ @@ -676,7 +676,7 @@ function updateSectionData(state: SubmissionObjectState, action: UpdateSectionDa * @param state * the current state * @param action - * an DisableSectionAction + * a DisableSectionAction * @param enabled * enabled or disabled section. * @return SubmissionObjectState @@ -705,7 +705,7 @@ function changeSectionState(state: SubmissionObjectState, action: EnableSectionA * @param state * the current state * @param action - * an SectionStatusChangeAction + * a SectionStatusChangeAction * @return SubmissionObjectState * the new state, with the section new validity status. */ @@ -769,7 +769,7 @@ function newFile(state: SubmissionObjectState, action: NewUploadedFileAction): S * @param state * the current state * @param action - * a EditFileDataAction action + * an EditFileDataAction action * @return SubmissionObjectState * the new state, with the edited file. */ From ec105b35a9c937daf1690e30438001cbac04bf3a Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 25 Jul 2019 10:03:49 +0200 Subject: [PATCH 09/41] Disabled search button on lookup field on edit mode --- .../lookup/dynamic-lookup.component.spec.ts | 40 +++++++++++++++++++ .../models/lookup/dynamic-lookup.component.ts | 2 +- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.spec.ts index df2252163d..4a1d636adb 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.spec.ts @@ -237,6 +237,12 @@ describe('Dynamic Lookup component', () => { it('should init component properly', () => { expect(lookupComp.firstInputValue).toBe(''); + const de = lookupFixture.debugElement.queryAll(By.css('button')); + const searchBtnEl = de[0].nativeElement; + const editBtnEl = de[1].nativeElement; + expect(searchBtnEl.disabled).toBe(true); + expect(editBtnEl.disabled).toBe(true); + expect(editBtnEl.textContent.trim()).toBe('form.edit'); }); it('should return search results', fakeAsync(() => { @@ -297,6 +303,7 @@ describe('Dynamic Lookup component', () => { expect(lookupComp.model.value).not.toBeDefined(); }); + }); describe('and init model value is not empty', () => { @@ -318,6 +325,19 @@ describe('Dynamic Lookup component', () => { it('should init component properly', () => { expect(lookupComp.firstInputValue).toBe('test'); }); + + it('should have search button disabled on edit mode', () => { + lookupComp.editMode = true; + lookupFixture.detectChanges(); + + const de = lookupFixture.debugElement.queryAll(By.css('button')); + const searchBtnEl = de[0].nativeElement; + const saveBtnEl = de[1].nativeElement; + expect(searchBtnEl.disabled).toBe(true); + expect(saveBtnEl.disabled).toBe(false); + expect(saveBtnEl.textContent.trim()).toBe('form.save'); + + }); }); }); @@ -340,7 +360,14 @@ describe('Dynamic Lookup component', () => { }); it('should render two input element', () => { const de = lookupFixture.debugElement.queryAll(By.css('input.form-control')); + const deBtn = lookupFixture.debugElement.queryAll(By.css('button')); + const searchBtnEl = deBtn[0].nativeElement; + const editBtnEl = deBtn[1].nativeElement; + expect(de.length).toBe(2); + expect(searchBtnEl.disabled).toBe(true); + expect(editBtnEl.disabled).toBe(true); + expect(editBtnEl.textContent.trim()).toBe('form.edit'); }); }); @@ -418,6 +445,19 @@ describe('Dynamic Lookup component', () => { expect(lookupComp.firstInputValue).toBe('Name'); expect(lookupComp.secondInputValue).toBe('Lastname'); }); + + it('should have search button disabled on edit mode', () => { + lookupComp.editMode = true; + lookupFixture.detectChanges(); + + const de = lookupFixture.debugElement.queryAll(By.css('button')); + const searchBtnEl = de[0].nativeElement; + const saveBtnEl = de[1].nativeElement; + expect(searchBtnEl.disabled).toBe(true); + expect(saveBtnEl.disabled).toBe(false); + expect(saveBtnEl.textContent.trim()).toBe('form.save'); + + }); }); }); }); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.ts index cba352484d..597f39b271 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.ts @@ -159,7 +159,7 @@ export class DsDynamicLookupComponent extends DynamicFormControlComponent implem } public isSearchDisabled() { - return isEmpty(this.firstInputValue); + return isEmpty(this.firstInputValue) || this.editMode; } public onBlurEvent(event: Event) { From 93b415a6bed431b220e2cd4f716beced838127d8 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 25 Jul 2019 10:05:14 +0200 Subject: [PATCH 10/41] Checked there is no pending save when a new save operation is dispatched --- src/app/submission/submission.service.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/app/submission/submission.service.ts b/src/app/submission/submission.service.ts index 82185a8eae..f35536d560 100644 --- a/src/app/submission/submission.service.ts +++ b/src/app/submission/submission.service.ts @@ -184,7 +184,11 @@ export class SubmissionService { * The submission id */ dispatchSave(submissionId) { - this.store.dispatch(new SaveSubmissionFormAction(submissionId)); + this.getSubmissionSaveProcessingStatus(submissionId).pipe( + find((isPending: boolean) => !isPending) + ).subscribe(() => { + this.store.dispatch(new SaveSubmissionFormAction(submissionId)); + }) } /** From a2180c19ac2dc0d425d934d46252f911f206041a Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 25 Jul 2019 11:58:21 +0200 Subject: [PATCH 11/41] during submission retrieve surrent collection name by current collection id --- .../submission-form-collection.component.html | 3 +-- ...bmission-form-collection.component.spec.ts | 10 +++++++--- .../submission-form-collection.component.ts | 19 ++++++------------- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/app/submission/form/collection/submission-form-collection.component.html b/src/app/submission/form/collection/submission-form-collection.component.html index 37ada35155..2cd1e928f7 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.html +++ b/src/app/submission/form/collection/submission-form-collection.component.html @@ -12,8 +12,7 @@ (click)="onClose()" [disabled]="(disabled$ | async)" ngbDropdownToggle> - - {{ selectedCollectionName$ | async }} + {{ selectedCollectionName$ | async }}
{ }); const collectionDataService: any = jasmine.createSpyObj('collectionDataService', { + findById: jasmine.createSpy('findById'), getAuthorizedCollectionByCommunity: jasmine.createSpy('getAuthorizedCollectionByCommunity') }); @@ -297,6 +301,7 @@ describe('SubmissionFormCollectionComponent Component', () => { it('should init collection list properly', () => { communityDataService.findAll.and.returnValue(mockCommunityList); + collectionDataService.findById.and.returnValue(mockCommunity1Collection1Rd); collectionDataService.getAuthorizedCollectionByCommunity.and.returnValues(mockCommunityCollectionList, mockCommunity2CollectionList); comp.ngOnChanges({ @@ -308,9 +313,8 @@ describe('SubmissionFormCollectionComponent Component', () => { b: mockCollectionList })); - expect(comp.selectedCollectionName$).toBeObservable(cold('(ab|)', { - a: '', - b: 'Community 1-Collection 1' + expect(comp.selectedCollectionName$).toBeObservable(cold('(a|)', { + a: 'Community 1-Collection 1' })); }); diff --git a/src/app/submission/form/collection/submission-form-collection.component.ts b/src/app/submission/form/collection/submission-form-collection.component.ts index eb7459eaf4..9cf01b173c 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.ts +++ b/src/app/submission/form/collection/submission-form-collection.component.ts @@ -193,6 +193,12 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { if (hasValue(changes.currentCollectionId) && hasValue(changes.currentCollectionId.currentValue)) { this.selectedCollectionId = this.currentCollectionId; + + this.selectedCollectionName$ = this.collectionDataService.findById(this.currentCollectionId).pipe( + find((collectionRD: RemoteData) => isNotEmpty(collectionRD.payload)), + map((collectionRD: RemoteData) => collectionRD.payload.name) + ); + const findOptions: FindAllOptions = { elementsPerPage: 100 }; @@ -219,19 +225,6 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { startWith([]) ); - this.selectedCollectionName$ = communities$.pipe( - flatMap((communityData: Community) => { - return communityData.collections.pipe( - find((collections: RemoteData>) => !collections.isResponsePending && collections.hasSucceeded), - mergeMap((collections: RemoteData>) => collections.payload.page), - filter((collectionData: Collection) => isNotEmpty(collectionData)), - filter((collectionData: Collection) => collectionData.id === this.selectedCollectionId), - map((collectionData: Collection) => collectionData.name) - ); - }), - startWith('') - ); - const searchTerm$ = this.searchField.valueChanges.pipe( debounceTime(200), distinctUntilChanged(), From 49c341b05aa1cff73aeba34a2fb403394e9e4341 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 25 Jul 2019 12:54:23 +0200 Subject: [PATCH 12/41] Fixed concat field hint message --- src/app/shared/form/builder/parsers/concat-field-parser.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/shared/form/builder/parsers/concat-field-parser.ts b/src/app/shared/form/builder/parsers/concat-field-parser.ts index 135dda5de3..6323905555 100644 --- a/src/app/shared/form/builder/parsers/concat-field-parser.ts +++ b/src/app/shared/form/builder/parsers/concat-field-parser.ts @@ -47,6 +47,7 @@ export class ConcatFieldParser extends FieldParser { const input1ModelConfig: DynamicInputModelConfig = this.initModel(id + CONCAT_FIRST_INPUT_SUFFIX, label, false, false); const input2ModelConfig: DynamicInputModelConfig = this.initModel(id + CONCAT_SECOND_INPUT_SUFFIX, label, true, false); + input2ModelConfig.hint = ' '; if (this.configData.mandatory) { input1ModelConfig.required = true; From 0ab2ca18809423b5a420399a916f513cb3c1e2e4 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 25 Jul 2019 12:59:16 +0200 Subject: [PATCH 13/41] added variable representing collection change processing status --- .../submission-form-collection.component.html | 5 +++-- .../collection/submission-form-collection.component.ts | 10 ++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/app/submission/form/collection/submission-form-collection.component.html b/src/app/submission/form/collection/submission-form-collection.component.html index 2cd1e928f7..6f4a8a864c 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.html +++ b/src/app/submission/form/collection/submission-form-collection.component.html @@ -10,9 +10,10 @@ class="btn btn-outline-primary" (blur)="onClose()" (click)="onClose()" - [disabled]="(disabled$ | async)" + [disabled]="(disabled$ | async) || (processingChange$ | async)" ngbDropdownToggle> - {{ selectedCollectionName$ | async }} + + {{ selectedCollectionName$ | async }}
(true); + /** + * A boolean representing if a collection change operation is processing + * @type {BehaviorSubject} + */ + public processingChange$ = new BehaviorSubject(false); + /** * The search form control * @type {FormControl} @@ -265,7 +271,7 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { */ onSelect(event) { this.searchField.reset(); - this.disabled$.next(true); + this.processingChange$.next(true); this.operationsBuilder.replace(this.pathCombiner.getPath(), event.collection.id, true); this.subs.push(this.operationsService.jsonPatchByResourceID( this.submissionService.getSubmissionObjectLinkName(), @@ -277,7 +283,7 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { this.selectedCollectionName$ = observableOf(event.collection.name); this.collectionChange.emit(submissionObject[0]); this.submissionService.changeSubmissionCollection(this.submissionId, event.collection.id); - this.disabled$.next(false); + this.processingChange$.next(false); this.cdr.detectChanges(); }) ); From 82426700fdaf3c4c9d09137c0a8b3635b8334db5 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 25 Jul 2019 13:00:26 +0200 Subject: [PATCH 14/41] Added getAuthorizedCollection method to CollectionDataService --- src/app/core/data/collection-data.service.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/app/core/data/collection-data.service.ts b/src/app/core/data/collection-data.service.ts index 762838e9ae..643eb55404 100644 --- a/src/app/core/data/collection-data.service.ts +++ b/src/app/core/data/collection-data.service.ts @@ -41,11 +41,26 @@ export class CollectionDataService extends ComColDataService { super(); } + /** + * Get all collections whom user has authorization to submit + * + * @return Observable>> + * collection list + */ + getAuthorizedCollection(): Observable>> { + const searchHref = 'findAuthorized'; + const options = new FindAllOptions(); + options.elementsPerPage = 1000; + + return this.searchBy(searchHref, options).pipe( + filter((collections: RemoteData>) => !collections.isResponsePending)); + } + /** * Get all collections whom user has authorization to submit to by community * - * @return boolean - * true if the user has at least one collection to submit to + * @return Observable>> + * collection list */ getAuthorizedCollectionByCommunity(communityId): Observable>> { const searchHref = 'findAuthorizedByCommunity'; From 2ff1b88bac89f40c6cb6eeee25944490a39e5929 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 25 Jul 2019 19:14:57 +0200 Subject: [PATCH 15/41] Fixed issue with collection change --- .../submission/sections/upload/section-upload.component.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/app/submission/sections/upload/section-upload.component.ts b/src/app/submission/sections/upload/section-upload.component.ts index 826385af45..9dbd1079f4 100644 --- a/src/app/submission/sections/upload/section-upload.component.ts +++ b/src/app/submission/sections/upload/section-upload.component.ts @@ -155,14 +155,14 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { filter((submissionObject: SubmissionObjectEntry) => isUndefined(this.collectionId) || this.collectionId !== submissionObject.collection), tap((submissionObject: SubmissionObjectEntry) => this.collectionId = submissionObject.collection), flatMap((submissionObject: SubmissionObjectEntry) => this.collectionDataService.findById(submissionObject.collection)), - find((rd: RemoteData) => isNotUndefined((rd.payload))), + filter((rd: RemoteData) => isNotUndefined((rd.payload))), tap((collectionRemoteData: RemoteData) => this.collectionName = collectionRemoteData.payload.name), flatMap((collectionRemoteData: RemoteData) => { return this.resourcePolicyService.findByHref( (collectionRemoteData.payload as any)._links.defaultAccessConditions ); }), - find((defaultAccessConditionsRemoteData: RemoteData) => + filter((defaultAccessConditionsRemoteData: RemoteData) => defaultAccessConditionsRemoteData.hasSucceeded), tap((defaultAccessConditionsRemoteData: RemoteData) => { if (isNotEmpty(defaultAccessConditionsRemoteData.payload)) { @@ -171,7 +171,6 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { } }), flatMap(() => config$), - take(1), flatMap((config: SubmissionUploadsModel) => { this.availableAccessConditionOptions = isNotEmpty(config.accessConditionOptions) ? config.accessConditionOptions : []; From 50024c8c559216053fc8323c91a4a2b8a816ae4e Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 25 Jul 2019 19:16:08 +0200 Subject: [PATCH 16/41] During submission retrieve collection list only at first load --- .../submission-form-collection.component.ts | 71 ++++++++++--------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/src/app/submission/form/collection/submission-form-collection.component.ts b/src/app/submission/form/collection/submission-form-collection.component.ts index 5a0e86103c..1fbccb6958 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.ts +++ b/src/app/submission/form/collection/submission-form-collection.component.ts @@ -209,43 +209,46 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { elementsPerPage: 100 }; - // @TODO replace with search/top browse endpoint - // @TODO implement community/subcommunity hierarchy - const communities$ = this.communityDataService.findAll(findOptions).pipe( - find((communities: RemoteData>) => isNotEmpty(communities.payload)), - mergeMap((communities: RemoteData>) => communities.payload.page)); + // Retrieve collection list only when is the first change + if (changes.currentCollectionId.isFirstChange()) { + // @TODO replace with search/top browse endpoint + // @TODO implement community/subcommunity hierarchy + const communities$ = this.communityDataService.findAll(findOptions).pipe( + find((communities: RemoteData>) => isNotEmpty(communities.payload)), + mergeMap((communities: RemoteData>) => communities.payload.page)); - const listCollection$ = communities$.pipe( - flatMap((communityData: Community) => { - return this.collectionDataService.getAuthorizedCollectionByCommunity(communityData.uuid).pipe( - find((collections: RemoteData>) => !collections.isResponsePending && collections.hasSucceeded), - mergeMap((collections: RemoteData>) => collections.payload.page), - filter((collectionData: Collection) => isNotEmpty(collectionData)), - map((collectionData: Collection) => ({ - communities: [{ id: communityData.id, name: communityData.name }], - collection: { id: collectionData.id, name: collectionData.name } - })) - ); - }), - reduce((acc: any, value: any) => [...acc, ...value], []), - startWith([]) - ); + const listCollection$ = communities$.pipe( + flatMap((communityData: Community) => { + return this.collectionDataService.getAuthorizedCollectionByCommunity(communityData.uuid).pipe( + find((collections: RemoteData>) => !collections.isResponsePending && collections.hasSucceeded), + mergeMap((collections: RemoteData>) => collections.payload.page), + filter((collectionData: Collection) => isNotEmpty(collectionData)), + map((collectionData: Collection) => ({ + communities: [{ id: communityData.id, name: communityData.name }], + collection: { id: collectionData.id, name: collectionData.name } + })) + ); + }), + reduce((acc: any, value: any) => [...acc, ...value], []), + startWith([]) + ); - const searchTerm$ = this.searchField.valueChanges.pipe( - debounceTime(200), - distinctUntilChanged(), - startWith('') - ); + const searchTerm$ = this.searchField.valueChanges.pipe( + debounceTime(200), + distinctUntilChanged(), + startWith('') + ); - this.searchListCollection$ = combineLatest(searchTerm$, listCollection$).pipe( - map(([searchTerm, listCollection]) => { - this.disabled$.next(isEmpty(listCollection)); - if (isEmpty(searchTerm)) { - return listCollection; - } else { - return listCollection.filter((v) => v.collection.name.toLowerCase().indexOf(searchTerm.toLowerCase()) > -1).slice(0, 5); - } - })); + this.searchListCollection$ = combineLatest(searchTerm$, listCollection$).pipe( + map(([searchTerm, listCollection]) => { + this.disabled$.next(isEmpty(listCollection)); + if (isEmpty(searchTerm)) { + return listCollection; + } else { + return listCollection.filter((v) => v.collection.name.toLowerCase().indexOf(searchTerm.toLowerCase()) > -1).slice(0, 5); + } + })); + } } } From 3fe67b8da1a01d0688d07f1b7b53ab41b7312c38 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 25 Jul 2019 19:18:11 +0200 Subject: [PATCH 17/41] changed elementsPerPage on CollectionDataService --- src/app/core/data/collection-data.service.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/core/data/collection-data.service.ts b/src/app/core/data/collection-data.service.ts index 643eb55404..626ddf233d 100644 --- a/src/app/core/data/collection-data.service.ts +++ b/src/app/core/data/collection-data.service.ts @@ -42,7 +42,7 @@ export class CollectionDataService extends ComColDataService { } /** - * Get all collections whom user has authorization to submit + * Get all collections the user is authorized to submit to * * @return Observable>> * collection list @@ -50,14 +50,14 @@ export class CollectionDataService extends ComColDataService { getAuthorizedCollection(): Observable>> { const searchHref = 'findAuthorized'; const options = new FindAllOptions(); - options.elementsPerPage = 1000; + options.elementsPerPage = 100; return this.searchBy(searchHref, options).pipe( filter((collections: RemoteData>) => !collections.isResponsePending)); } /** - * Get all collections whom user has authorization to submit to by community + * Get all collections the user is authorized to submit to, by community * * @return Observable>> * collection list @@ -65,7 +65,7 @@ export class CollectionDataService extends ComColDataService { getAuthorizedCollectionByCommunity(communityId): Observable>> { const searchHref = 'findAuthorizedByCommunity'; const options = new FindAllOptions(); - options.elementsPerPage = 1000; + options.elementsPerPage = 100; options.searchParams = [new SearchParam('uuid', communityId)]; return this.searchBy(searchHref, options).pipe( From 45ab326355d48d7b1cc1e204920585f309b3b198 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Fri, 26 Jul 2019 15:29:00 +0200 Subject: [PATCH 18/41] Added FindAllOptions param to getAuthorizedCollection and getAuthorizedCollectionByCommunity --- src/app/core/data/collection-data.service.ts | 11 +++++------ .../submission-form-collection.component.ts | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/app/core/data/collection-data.service.ts b/src/app/core/data/collection-data.service.ts index 626ddf233d..04e483604c 100644 --- a/src/app/core/data/collection-data.service.ts +++ b/src/app/core/data/collection-data.service.ts @@ -44,13 +44,12 @@ export class CollectionDataService extends ComColDataService { /** * Get all collections the user is authorized to submit to * + * @param options The [[FindAllOptions]] object * @return Observable>> * collection list */ - getAuthorizedCollection(): Observable>> { + getAuthorizedCollection(options: FindAllOptions = {}): Observable>> { const searchHref = 'findAuthorized'; - const options = new FindAllOptions(); - options.elementsPerPage = 100; return this.searchBy(searchHref, options).pipe( filter((collections: RemoteData>) => !collections.isResponsePending)); @@ -59,13 +58,13 @@ export class CollectionDataService extends ComColDataService { /** * Get all collections the user is authorized to submit to, by community * + * @param communityId The community id + * @param options The [[FindAllOptions]] object * @return Observable>> * collection list */ - getAuthorizedCollectionByCommunity(communityId): Observable>> { + getAuthorizedCollectionByCommunity(communityId: string, options: FindAllOptions = {}): Observable>> { const searchHref = 'findAuthorizedByCommunity'; - const options = new FindAllOptions(); - options.elementsPerPage = 100; options.searchParams = [new SearchParam('uuid', communityId)]; return this.searchBy(searchHref, options).pipe( diff --git a/src/app/submission/form/collection/submission-form-collection.component.ts b/src/app/submission/form/collection/submission-form-collection.component.ts index 1fbccb6958..79d2f2a7bc 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.ts +++ b/src/app/submission/form/collection/submission-form-collection.component.ts @@ -206,7 +206,7 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { ); const findOptions: FindAllOptions = { - elementsPerPage: 100 + elementsPerPage: 1000 }; // Retrieve collection list only when is the first change @@ -219,7 +219,7 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { const listCollection$ = communities$.pipe( flatMap((communityData: Community) => { - return this.collectionDataService.getAuthorizedCollectionByCommunity(communityData.uuid).pipe( + return this.collectionDataService.getAuthorizedCollectionByCommunity(communityData.uuid, findOptions).pipe( find((collections: RemoteData>) => !collections.isResponsePending && collections.hasSucceeded), mergeMap((collections: RemoteData>) => collections.payload.page), filter((collectionData: Collection) => isNotEmpty(collectionData)), From 861f8ddb6c770f0a31caf53a0e9aa11b64c7605e Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Fri, 26 Jul 2019 19:41:28 +0200 Subject: [PATCH 19/41] Show hint message only when there is an error message --- .../ds-dynamic-form-control-container.component.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html index cead04f797..5692c27d20 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html @@ -14,7 +14,8 @@ - +
{{ message | translate:model.validators }} From 509fd0d802d8ff7070e6ab3a4569c747c32c8bb0 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Fri, 26 Jul 2019 19:43:15 +0200 Subject: [PATCH 20/41] Fixed an issue while editing repeatable fields --- .../sections/form/section-form.component.ts | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/app/submission/sections/form/section-form.component.ts b/src/app/submission/sections/form/section-form.component.ts index ef817a7568..2269ccd5f1 100644 --- a/src/app/submission/sections/form/section-form.component.ts +++ b/src/app/submission/sections/form/section-form.component.ts @@ -64,6 +64,12 @@ export class SubmissionSectionformComponent extends SectionModelComponent { */ public isLoading = true; + /** + * A map representing all field on their way to be removed + * @type {Map} + */ + protected fieldsOnTheirWayToBeRemoved: Map = new Map(); + /** * The form config * @type {SubmissionFormsModel} @@ -295,6 +301,7 @@ export class SubmissionSectionformComponent extends SectionModelComponent { }), distinctUntilChanged()) .subscribe((sectionState: SubmissionSectionObject) => { + this.fieldsOnTheirWayToBeRemoved = new Map(); this.updateForm(sectionState.data as WorkspaceitemSectionFormObject, sectionState.errors); }) ) @@ -348,11 +355,24 @@ export class SubmissionSectionformComponent extends SectionModelComponent { * the [[DynamicFormControlEvent]] emitted */ onRemove(event: DynamicFormControlEvent): void { + const fieldId = this.formBuilderService.getId(event.model); + const fieldIndex = this.formOperationsService.getArrayIndexFromEvent(event); + + // Keep track that this field will be removed + if (this.fieldsOnTheirWayToBeRemoved.has(fieldId)) { + const indexes = this.fieldsOnTheirWayToBeRemoved.get(fieldId); + indexes.push(fieldIndex); + this.fieldsOnTheirWayToBeRemoved.set(fieldId, indexes); + } else { + this.fieldsOnTheirWayToBeRemoved.set(fieldId, [fieldIndex]); + } + this.formOperationsService.dispatchOperationsFromEvent( this.pathCombiner, event, this.previousValue, - this.hasStoredValue(this.formBuilderService.getId(event.model), this.formOperationsService.getArrayIndexFromEvent(event))); + this.hasStoredValue(fieldId, fieldIndex)); + } /** @@ -365,9 +385,23 @@ export class SubmissionSectionformComponent extends SectionModelComponent { */ hasStoredValue(fieldId, index): boolean { if (isNotEmpty(this.sectionData.data)) { - return this.sectionData.data.hasOwnProperty(fieldId) && isNotEmpty(this.sectionData.data[fieldId][index]); + return this.sectionData.data.hasOwnProperty(fieldId) && + isNotEmpty(this.sectionData.data[fieldId][index]) && + !this.isFieldToRemove(fieldId, index); } else { return false; } } + + /** + * Check if the specified field is on the way to be removed + * + * @param fieldId + * the section data retrieved from the serverù + * @param index + * the section data retrieved from the server + */ + isFieldToRemove(fieldId, index) { + return this.fieldsOnTheirWayToBeRemoved.has(fieldId) && this.fieldsOnTheirWayToBeRemoved.get(fieldId).includes(index); + } } From 0cc130a8b90a668806438c688bebe6642ddf6fff Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Tue, 20 Aug 2019 11:14:15 +0200 Subject: [PATCH 21/41] Fixed bitstream download in mydspace page --- .../item-detail-preview.component.html | 4 ++-- .../item-detail-preview.component.spec.ts | 14 +++++++++++ .../item-detail-preview.component.ts | 24 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.html b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.html index 04e128c49a..ab2c24c435 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.html +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.html @@ -15,11 +15,11 @@
- +
{{('mydspace.results.no-files' | translate)}} diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.spec.ts b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.spec.ts index d0af614a6c..182c5eca39 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.spec.ts +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.spec.ts @@ -12,6 +12,16 @@ import { MockTranslateLoader } from '../../../mocks/mock-translate-loader'; import { ItemDetailPreviewFieldComponent } from './item-detail-preview-field/item-detail-preview-field.component'; import { FileSizePipe } from '../../../utils/file-size-pipe'; import { VarDirective } from '../../../utils/var.directive'; +import { FileService } from '../../../../core/shared/file.service'; +import { HALEndpointService } from '../../../../core/shared/hal-endpoint.service'; +import { HALEndpointServiceStub } from '../../../testing/hal-endpoint-service-stub'; + +function getMockFileService(): FileService { + return jasmine.createSpyObj('FileService', { + downloadFile: jasmine.createSpy('downloadFile'), + getFileNameFromResponseContentDisposition: jasmine.createSpy('getFileNameFromResponseContentDisposition') + }); +} let component: ItemDetailPreviewComponent; let fixture: ComponentFixture; @@ -59,6 +69,10 @@ describe('ItemDetailPreviewComponent', () => { }), ], declarations: [ItemDetailPreviewComponent, ItemDetailPreviewFieldComponent, TruncatePipe, FileSizePipe, VarDirective], + providers: [ + { provide: FileService, useValue: getMockFileService() }, + { provide: HALEndpointService, useValue: new HALEndpointServiceStub('workspaceitems') } + ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(ItemDetailPreviewComponent, { set: { changeDetection: ChangeDetectionStrategy.Default } diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.ts b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.ts index d26bfc4589..fa15c71168 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.ts +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.ts @@ -1,12 +1,15 @@ import { Component, Input } from '@angular/core'; import { Observable } from 'rxjs'; +import { first } from 'rxjs/operators'; import { Item } from '../../../../core/shared/item.model'; import { MyDspaceItemStatusType } from '../../../object-collection/shared/mydspace-item-status/my-dspace-item-status-type'; import { fadeInOut } from '../../../animations/fade'; import { Bitstream } from '../../../../core/shared/bitstream.model'; import { MyDSpaceResult } from '../../../../+my-dspace-page/my-dspace-result.model'; +import { FileService } from '../../../../core/shared/file.service'; +import { HALEndpointService } from '../../../../core/shared/hal-endpoint.service'; /** * This component show metadata for the given item object in the detail view. @@ -54,6 +57,16 @@ export class ItemDetailPreviewComponent { */ public thumbnail$: Observable; + /** + * Initialize instance variables + * + * @param {FileService} fileService + * @param {HALEndpointService} halService + */ + constructor(private fileService: FileService, + private halService: HALEndpointService) { + } + /** * Initialize all instance variables */ @@ -62,4 +75,15 @@ export class ItemDetailPreviewComponent { this.bitstreams$ = this.item.getFiles(); } + /** + * Perform bitstream download + */ + public downloadBitstreamFile(uuid: string) { + this.halService.getEndpoint('bitstreams').pipe( + first()) + .subscribe((url) => { + const fileUrl = `${url}/${uuid}/content`; + this.fileService.downloadFile(fileUrl); + }); + } } From 3651b16b2c9b1d1879a9503477ab33e0cf231de7 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 21 Aug 2019 09:52:38 +0200 Subject: [PATCH 22/41] Fixed issue with form value init within relation fields --- .../dynamic-relation-group.components.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.components.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.components.ts index 1485993375..62b6b4effa 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.components.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.components.ts @@ -129,11 +129,11 @@ export class DsDynamicRelationGroupComponent extends DynamicFormControlComponent || this.selectedChipItem.item[model.name].value === PLACEHOLDER_PARENT_METADATA) ? null : this.selectedChipItem.item[model.name]; - if (isNotNull(value)) { - const nextValue = (this.formBuilderService.isInputModel(model) && (typeof value !== 'string')) ? - value.value : value; - model.valueUpdates.next(nextValue); - } + + const nextValue = (this.formBuilderService.isInputModel(model) && isNotNull(value) && (typeof value !== 'string')) ? + value.value : value; + model.valueUpdates.next(nextValue); + }); }); @@ -231,7 +231,7 @@ export class DsDynamicRelationGroupComponent extends DynamicFormControlComponent flatMap((valueModel) => { const returnList: Array> = []; valueModel.forEach((valueObj) => { - const returnObj = Object.keys(valueObj).map((fieldName) => { + const returnObj = Object.keys(valueObj).map((fieldName) => { let return$: Observable; if (isObject(valueObj[fieldName]) && valueObj[fieldName].hasAuthority() && isNotEmpty(valueObj[fieldName].authority)) { const fieldId = fieldName.replace(/\./g, '_'); @@ -255,7 +255,7 @@ export class DsDynamicRelationGroupComponent extends DynamicFormControlComponent } else { return$ = observableOf(valueObj[fieldName]); } - return return$.pipe(map((entry) => ({[fieldName]: entry}))); + return return$.pipe(map((entry) => ({ [fieldName]: entry }))); }); returnList.push(combineLatest(returnObj)); From 5bcb361c6e36f886698df53562ae502ecbe78aac Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 21 Aug 2019 09:54:16 +0200 Subject: [PATCH 23/41] Fixed issue with model value update within scrollable fields --- .../dynamic-scrollable-dropdown.component.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts index e51fc78ec3..5eda1372eb 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts @@ -2,7 +2,7 @@ import { ChangeDetectorRef, Component, EventEmitter, Input, OnInit, Output } fro import { FormGroup } from '@angular/forms'; import { Observable, of as observableOf } from 'rxjs'; -import { catchError, first, tap } from 'rxjs/operators'; +import { catchError, distinctUntilChanged, first, tap } from 'rxjs/operators'; import { NgbDropdown } from '@ng-bootstrap/ng-bootstrap'; import { DynamicFormControlComponent, @@ -71,7 +71,13 @@ export class DsDynamicScrollableDropdownComponent extends DynamicFormControlComp } this.pageInfo = object.pageInfo; this.cdr.detectChanges(); - }) + }); + + this.group.get(this.model.id).valueChanges.pipe(distinctUntilChanged()) + .subscribe((value) => { + this.setCurrentValue(value); + }); + } inputFormatter = (x: AuthorityValue): string => x.display || x.value; From c1b241dc299392842f6c5b4edae415867f5b6cd2 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 22 Aug 2019 11:29:31 +0200 Subject: [PATCH 24/41] Fixed issue with lookup field which updated model on input event --- .../lookup/dynamic-lookup.component.html | 10 ++++---- .../lookup/dynamic-lookup.component.spec.ts | 4 ++-- .../models/lookup/dynamic-lookup.component.ts | 23 +++++++++++-------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html index cb2d1fe217..3cfb5980c6 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html @@ -20,11 +20,10 @@ [disabled]="isInputDisabled()" [placeholder]="model.placeholder | translate" [readonly]="model.readOnly" - (change)="$event.preventDefault()" + (change)="onChange($event)" (blur)="onBlurEvent($event); $event.stopPropagation(); sdRef.close();" (focus)="onFocusEvent($event); $event.stopPropagation(); sdRef.close();" - (click)="$event.stopPropagation(); $event.stopPropagation(); sdRef.close();" - (input)="onInput($event)"> + (click)="$event.stopPropagation(); $event.stopPropagation(); sdRef.close();">
@@ -40,11 +39,10 @@ [disabled]="firstInputValue.length === 0 || isInputDisabled()" [placeholder]="model.secondPlaceholder | translate" [readonly]="model.readOnly" - (change)="$event.preventDefault()" + (change)="onChange($event)" (blur)="onBlurEvent($event); $event.stopPropagation(); sdRef.close();" (focus)="onFocusEvent($event); $event.stopPropagation(); sdRef.close();" - (click)="$event.stopPropagation(); sdRef.close();" - (input)="onInput($event)"> + (click)="$event.stopPropagation(); sdRef.close();">
diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.html b/src/app/shared/auth-nav-menu/auth-nav-menu.component.html index b560283ad5..4df07880d8 100644 --- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.html +++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.html @@ -3,7 +3,8 @@
{{ 'nav.login' | translate }}
- +
diff --git a/src/app/shared/log-in/log-in.component.spec.ts b/src/app/shared/log-in/log-in.component.spec.ts index 43004b79c7..8ad423629d 100644 --- a/src/app/shared/log-in/log-in.component.spec.ts +++ b/src/app/shared/log-in/log-in.component.spec.ts @@ -58,8 +58,7 @@ describe('LogInComponent', () => { {provide: AuthService, useClass: AuthServiceStub}, {provide: APP_BASE_HREF, useValue: '/'}, {provide: Router, useClass: RouterStub}, - {provide: RouteService, useValue: routeServiceStub }, - {provide: HostWindowService, useValue: new HostWindowServiceStub(900) } + {provide: RouteService, useValue: routeServiceStub } ], schemas: [ CUSTOM_ELEMENTS_SCHEMA @@ -151,6 +150,8 @@ describe('LogInComponent', () => { const authService: AuthService = TestBed.get(AuthService); spyOn(authService, 'setRedirectUrl'); + component.isStandalonePage = false; + fixture.detectChanges(); expect(authService.setRedirectUrl).not.toHaveBeenCalledWith(); @@ -193,8 +194,7 @@ describe('LogInComponent on small screen', () => { {provide: AuthService, useClass: AuthServiceStub}, {provide: APP_BASE_HREF, useValue: '/'}, {provide: Router, useClass: RouterStub}, - {provide: RouteService, useValue: routeServiceStub }, - {provide: HostWindowService, useValue: new HostWindowServiceStub(300) } + {provide: RouteService, useValue: routeServiceStub } ], schemas: [ CUSTOM_ELEMENTS_SCHEMA @@ -230,6 +230,7 @@ describe('LogInComponent on small screen', () => { it('should set the redirect url on init', () => { const authService: AuthService = TestBed.get(AuthService); spyOn(authService, 'setRedirectUrl'); + component.isStandalonePage = true; fixture.detectChanges(); // set FormControl values component.form.controls.email.setValue('user'); diff --git a/src/app/shared/log-in/log-in.component.ts b/src/app/shared/log-in/log-in.component.ts index 5097b2a0c7..3a4ff151bf 100644 --- a/src/app/shared/log-in/log-in.component.ts +++ b/src/app/shared/log-in/log-in.component.ts @@ -1,5 +1,5 @@ import {filter, map, pairwise, take, takeUntil, takeWhile, tap} from 'rxjs/operators'; -import { Component, OnDestroy, OnInit } from '@angular/core'; +import {Component, Input, OnDestroy, OnInit} from '@angular/core'; import { FormBuilder, FormGroup, Validators } from '@angular/forms'; import { select, Store } from '@ngrx/store'; @@ -86,13 +86,14 @@ export class LogInComponent implements OnDestroy, OnInit { private redirectUrl = LOGIN_ROUTE; + @Input() isStandalonePage: boolean; + /** * @constructor * @param {AuthService} authService * @param {FormBuilder} formBuilder * @param {RouteService} routeService * @param {Router} router - * @param {HostWindowService} windowService * @param {Store} store */ constructor( @@ -100,7 +101,6 @@ export class LogInComponent implements OnDestroy, OnInit { private formBuilder: FormBuilder, private routeService: RouteService, private router: Router, - private windowService: HostWindowService, private store: Store ) { } @@ -114,17 +114,14 @@ export class LogInComponent implements OnDestroy, OnInit { this.isAuthenticated = this.store.pipe(select(isAuthenticated)); // for mobile login, set the redirect url to the previous route - this.windowService.isXs().pipe(take(1)) - .subscribe((isMobile) => { - if (isMobile) { - this.routeService.getHistory().pipe( - take(1) - ).subscribe((history) => { - const previousIndex = history.length - 2; - this.setRedirectUrl(history[previousIndex]); - }); - } + if (this.isStandalonePage) { + this.routeService.getHistory().pipe( + take(1) + ).subscribe((history) => { + const previousIndex = history.length - 2; + this.setRedirectUrl(history[previousIndex]); }); + } // set formGroup this.form = this.formBuilder.group({ From 0935bd4afd6902cf0f364ff0e200dbd7f3b9a882 Mon Sep 17 00:00:00 2001 From: Michael W Spalti Date: Thu, 5 Sep 2019 17:49:56 -0700 Subject: [PATCH 31/41] Proposed authentication service method that sets redirect url and dispatches auth request. --- src/app/core/auth/auth.service.spec.ts | 43 +++++- src/app/core/auth/auth.service.ts | 40 ++++-- .../shared/log-in/log-in.component.spec.ts | 131 ------------------ src/app/shared/log-in/log-in.component.ts | 35 +---- src/app/shared/testing/auth-service-stub.ts | 3 + src/app/shared/testing/route-service-stub.ts | 2 +- 6 files changed, 79 insertions(+), 175 deletions(-) diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index ab2e6fd86b..6044e9ecb6 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -4,7 +4,7 @@ import { ActivatedRoute, Router } from '@angular/router'; import { Store, StoreModule } from '@ngrx/store'; import { REQUEST } from '@nguniversal/express-engine/tokens'; -import { of as observableOf } from 'rxjs'; +import {of, of as observableOf} from 'rxjs'; import { authReducer, AuthState } from './auth.reducer'; import { NativeWindowRef, NativeWindowService } from '../services/window.service'; @@ -23,11 +23,14 @@ import { AppState } from '../../app.reducer'; import { ClientCookieService } from '../services/client-cookie.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { getMockRemoteDataBuildService } from '../../shared/mocks/mock-remote-data-build.service'; +import {routeServiceStub} from '../../shared/testing/route-service-stub'; +import {RouteService} from '../services/route.service'; describe('AuthService test', () => { let mockStore: Store; let authService: AuthService; + let routeServiceMock: RouteService; let authRequest; let window; let routerStub; @@ -74,6 +77,7 @@ describe('AuthService test', () => { { provide: NativeWindowService, useValue: window }, { provide: REQUEST, useValue: {} }, { provide: Router, useValue: routerStub }, + { provide: RouteService, useValue: routeServiceStub }, { provide: ActivatedRoute, useValue: routeStub }, { provide: Store, useValue: mockStore }, { provide: RemoteDataBuildService, useValue: rdbService }, @@ -82,6 +86,8 @@ describe('AuthService test', () => { ], }); authService = TestBed.get(AuthService); + routeServiceMock = TestBed.get(RouteService); + spyOn(authService, 'setRedirectUrl'); }); it('should return the authentication status object when user credentials are correct', () => { @@ -124,6 +130,31 @@ describe('AuthService test', () => { expect(authService.logout.bind(null)).toThrow(); }); + it ('should dispatch authentication', () => { + authService.doAuthentication(true, '', ''); + expect(mockStore.dispatch).toHaveBeenCalled(); + }); + + it ('should set redirect url to previous page', () => { + spyOn(routeServiceMock, 'getHistory').and.callThrough(); + authService.doAuthentication(true, '', ''); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(authService.setRedirectUrl).toHaveBeenCalledWith('/collection/123'); + }); + + it ('should set redirect url to current page', () => { + spyOn(routeServiceMock, 'getHistory').and.callThrough(); + authService.doAuthentication(false, '', ''); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(authService.setRedirectUrl).toHaveBeenCalledWith('/home'); + }); + + it ('should not set redirect url to /login', () => { + spyOn(routeServiceMock, 'getHistory').and.returnValue(of(['/login', '/login'])); + authService.doAuthentication(true, '', ''); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(authService.setRedirectUrl).not.toHaveBeenCalled(); + }); }); describe('', () => { @@ -138,6 +169,7 @@ describe('AuthService test', () => { { provide: AuthRequestService, useValue: authRequest }, { provide: REQUEST, useValue: {} }, { provide: Router, useValue: routerStub }, + { provide: RouteService, useValue: routeServiceStub }, { provide: RemoteDataBuildService, useValue: rdbService }, CookieService, AuthService @@ -145,13 +177,13 @@ describe('AuthService test', () => { }).compileComponents(); })); - beforeEach(inject([CookieService, AuthRequestService, Store, Router], (cookieService: CookieService, authReqService: AuthRequestService, store: Store, router: Router) => { + beforeEach(inject([CookieService, AuthRequestService, Store, Router, RouteService], (cookieService: CookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService) => { store .subscribe((state) => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, router, cookieService, store, rdbService); + authService = new AuthService({}, window, undefined, authReqService, router, routeService, cookieService, store, rdbService); })); it('should return true when user is logged in', () => { @@ -189,6 +221,7 @@ describe('AuthService test', () => { { provide: AuthRequestService, useValue: authRequest }, { provide: REQUEST, useValue: {} }, { provide: Router, useValue: routerStub }, + { provide: RouteService, useValue: routeServiceStub }, { provide: RemoteDataBuildService, useValue: rdbService }, ClientCookieService, CookieService, @@ -197,7 +230,7 @@ describe('AuthService test', () => { }).compileComponents(); })); - beforeEach(inject([ClientCookieService, AuthRequestService, Store, Router], (cookieService: ClientCookieService, authReqService: AuthRequestService, store: Store, router: Router) => { + beforeEach(inject([ClientCookieService, AuthRequestService, Store, Router, RouteService], (cookieService: ClientCookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService) => { const expiredToken: AuthTokenInfo = new AuthTokenInfo('test_token'); expiredToken.expires = Date.now() - (1000 * 60 * 60); authenticatedState = { @@ -212,7 +245,7 @@ describe('AuthService test', () => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, router, cookieService, store, rdbService); + authService = new AuthService({}, window, undefined, authReqService, router, routeService, cookieService, store, rdbService); storage = (authService as any).storage; spyOn(storage, 'get'); spyOn(storage, 'remove'); diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 08c94b02f2..615fee5d56 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -22,6 +22,7 @@ import { ResetAuthenticationMessagesAction, SetRedirectUrlAction } from './auth. import { NativeWindowRef, NativeWindowService } from '../services/window.service'; import { Base64EncodeUrl } from '../../shared/utils/encode-decode.util'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import {RouteService} from '../services/route.service'; export const LOGIN_ROUTE = '/login'; export const LOGOUT_ROUTE = '/logout'; @@ -45,6 +46,7 @@ export class AuthService { protected authRequestService: AuthRequestService, @Optional() @Inject(RESPONSE) private response: any, protected router: Router, + protected routeService: RouteService, protected storage: CookieService, protected store: Store, protected rdbService: RemoteDataBuildService @@ -337,7 +339,7 @@ export class AuthService { /** * Redirect to the route navigated before the login */ - public redirectToPreviousUrl() { + public redirectToPreviousUrl(isStandalonePage: boolean) { this.getRedirectUrl().pipe( take(1)) .subscribe((redirectUrl) => { @@ -346,18 +348,39 @@ export class AuthService { this.clearRedirectUrl(); this.router.onSameUrlNavigation = 'reload'; const url = decodeURIComponent(redirectUrl); - this.router.navigateByUrl(url); - /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ - // this._window.nativeWindow.location.href = url; + this.navigateToRedirectUrl(url); } else { - this.router.navigate(['/']); - /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ - // this._window.nativeWindow.location.href = '/'; + // If redirectUrl is empty use history. + this.routeService.getHistory().pipe( + take(1) + ).subscribe((history) => { + let redirUrl; + if (isStandalonePage) { + // For standalone login pages, use the previous route. + redirUrl = history[history.length - 2] || ''; + } else { + redirUrl = history[history.length - 1] || ''; + } + this.navigateToRedirectUrl(redirUrl); + }); } - }) + }); } + private navigateToRedirectUrl(url: string) { + // in case the user navigates directly to /login (via bookmark, etc), or the route history is not found. + if (isEmpty(url) || url.startsWith(LOGIN_ROUTE)) { + this.router.navigate(['/']); + /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ + // this._window.nativeWindow.location.href = '/'; + } else { + /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ + // this._window.nativeWindow.location.href = url; + this.router.navigate([url]); + } + } + /** * Refresh route navigated */ @@ -400,4 +423,5 @@ export class AuthService { this.store.dispatch(new SetRedirectUrlAction('')); this.storage.remove(REDIRECT_COOKIE); } + } diff --git a/src/app/shared/log-in/log-in.component.spec.ts b/src/app/shared/log-in/log-in.component.spec.ts index 8ad423629d..6df992fff7 100644 --- a/src/app/shared/log-in/log-in.component.spec.ts +++ b/src/app/shared/log-in/log-in.component.spec.ts @@ -16,8 +16,6 @@ import { AppState } from '../../app.reducer'; import {AppRoutingModule} from '../../app-routing.module'; import {PageNotFoundComponent} from '../../pagenotfound/pagenotfound.component'; import {APP_BASE_HREF} from '@angular/common'; -import {HostWindowService} from '../host-window.service'; -import {HostWindowServiceStub} from '../testing/host-window-service-stub'; import {RouterStub} from '../testing/router-stub'; import {Router} from '@angular/router'; import {RouteService} from '../services/route.service'; @@ -110,135 +108,6 @@ describe('LogInComponent', () => { expect(page.navigateSpy.calls.any()).toBe(true, 'Store.dispatch not invoked'); }); - it('should set the redirect url', () => { - fixture.detectChanges(); - - // set FormControl values - component.form.controls.email.setValue('user'); - component.form.controls.password.setValue('password'); - - const authService: AuthService = TestBed.get(AuthService); - spyOn(authService, 'setRedirectUrl'); - - component.submit(); - - // the redirect url should be set upon submit - expect(authService.setRedirectUrl).toHaveBeenCalled(); - }); - - it('should not set the redirect url to /login', () => { - fixture.detectChanges(); - - const router: Router = TestBed.get(Router); - router.navigateByUrl('/login') - - const authService: AuthService = TestBed.get(AuthService); - - // set FormControl values - component.form.controls.email.setValue('user'); - component.form.controls.password.setValue('password'); - - spyOn(authService, 'setRedirectUrl'); - - component.submit(); - - expect(authService.setRedirectUrl).not.toHaveBeenCalled(); - }); - - it('should not set the redirect url on init', () => { - - const authService: AuthService = TestBed.get(AuthService); - spyOn(authService, 'setRedirectUrl'); - - component.isStandalonePage = false; - - fixture.detectChanges(); - expect(authService.setRedirectUrl).not.toHaveBeenCalledWith(); - - }); - -}); - -describe('LogInComponent on small screen', () => { - - let component: LogInComponent; - let fixture: ComponentFixture; - let page: Page; - let user: EPerson; - - const authState = { - authenticated: false, - loaded: false, - loading: false, - }; - - beforeEach(() => { - user = EPersonMock; - }); - - beforeEach(async(() => { - // refine the test module by declaring the test component - TestBed.configureTestingModule({ - imports: [ - FormsModule, - ReactiveFormsModule, - StoreModule.forRoot(authReducer), - AppRoutingModule, - TranslateModule.forRoot() - ], - declarations: [ - LogInComponent, - PageNotFoundComponent - ], - providers: [ - {provide: AuthService, useClass: AuthServiceStub}, - {provide: APP_BASE_HREF, useValue: '/'}, - {provide: Router, useClass: RouterStub}, - {provide: RouteService, useValue: routeServiceStub } - ], - schemas: [ - CUSTOM_ELEMENTS_SCHEMA - ] - }) - .compileComponents(); - - })); - - beforeEach(inject([Store], (store: Store) => { - store - .subscribe((state) => { - (state as any).core = Object.create({}); - (state as any).core.auth = authState; - }); - - // create component and test fixture - fixture = TestBed.createComponent(LogInComponent); - - // get test component from the fixture - component = fixture.componentInstance; - - // create page - page = new Page(component, fixture); - - // verify the fixture is stable (no pending tasks) - fixture.whenStable().then(() => { - page.addPageElements(); - }); - - })); - - it('should set the redirect url on init', () => { - const authService: AuthService = TestBed.get(AuthService); - spyOn(authService, 'setRedirectUrl'); - component.isStandalonePage = true; - fixture.detectChanges(); - // set FormControl values - component.form.controls.email.setValue('user'); - component.form.controls.password.setValue('password'); - expect(authService.setRedirectUrl).toHaveBeenCalledWith('collection/123'); - - }); - }); /** diff --git a/src/app/shared/log-in/log-in.component.ts b/src/app/shared/log-in/log-in.component.ts index 3a4ff151bf..aa798548f4 100644 --- a/src/app/shared/log-in/log-in.component.ts +++ b/src/app/shared/log-in/log-in.component.ts @@ -1,9 +1,9 @@ -import {filter, map, pairwise, take, takeUntil, takeWhile, tap} from 'rxjs/operators'; +import {filter, map, takeWhile } from 'rxjs/operators'; import {Component, Input, OnDestroy, OnInit} from '@angular/core'; import { FormBuilder, FormGroup, Validators } from '@angular/forms'; import { select, Store } from '@ngrx/store'; -import {Observable, Subject} from 'rxjs'; +import {Observable} from 'rxjs'; import { AuthenticateAction, ResetAuthenticationMessagesAction @@ -19,10 +19,8 @@ import { CoreState } from '../../core/core.reducers'; import {isNotEmpty} from '../empty.util'; import { fadeOut } from '../animations/fade'; -import {AuthService, LOGIN_ROUTE} from '../../core/auth/auth.service'; +import {AuthService} from '../../core/auth/auth.service'; import {Router} from '@angular/router'; -import {HostWindowService} from '../host-window.service'; -import {RouteService} from '../services/route.service'; /** * /users/sign-in @@ -84,8 +82,6 @@ export class LogInComponent implements OnDestroy, OnInit { */ private alive = true; - private redirectUrl = LOGIN_ROUTE; - @Input() isStandalonePage: boolean; /** @@ -99,7 +95,6 @@ export class LogInComponent implements OnDestroy, OnInit { constructor( private authService: AuthService, private formBuilder: FormBuilder, - private routeService: RouteService, private router: Router, private store: Store ) { @@ -113,16 +108,6 @@ export class LogInComponent implements OnDestroy, OnInit { // set isAuthenticated this.isAuthenticated = this.store.pipe(select(isAuthenticated)); - // for mobile login, set the redirect url to the previous route - if (this.isStandalonePage) { - this.routeService.getHistory().pipe( - take(1) - ).subscribe((history) => { - const previousIndex = history.length - 2; - this.setRedirectUrl(history[previousIndex]); - }); - } - // set formGroup this.form = this.formBuilder.group({ email: ['', Validators.required], @@ -156,7 +141,7 @@ export class LogInComponent implements OnDestroy, OnInit { takeWhile(() => this.alive), filter((authenticated) => authenticated)) .subscribe(() => { - this.authService.redirectToPreviousUrl(); + this.authService.redirectToPreviousUrl(this.isStandalonePage); } ); } @@ -203,21 +188,11 @@ export class LogInComponent implements OnDestroy, OnInit { email.trim(); password.trim(); - this.setRedirectUrl(this.router.url); // dispatch AuthenticationAction this.store.dispatch(new AuthenticateAction(email, password)); + // clear form this.form.reset(); - } - /** - * Sets the redirect url if not equal to LOGIN_ROUTE - * @param url - */ - private setRedirectUrl(url: string) { - if (url !== this.redirectUrl) { - this.authService.setRedirectUrl(url) - } - } } diff --git a/src/app/shared/testing/auth-service-stub.ts b/src/app/shared/testing/auth-service-stub.ts index a6d24d5c8b..c5f9d92a53 100644 --- a/src/app/shared/testing/auth-service-stub.ts +++ b/src/app/shared/testing/auth-service-stub.ts @@ -96,6 +96,9 @@ export class AuthServiceStub { return observableOf(this.redirectUrl); } + public doAuthentication(isStandalonePage, email, password) { + return; + } public storeToken(token: AuthTokenInfo) { return; } diff --git a/src/app/shared/testing/route-service-stub.ts b/src/app/shared/testing/route-service-stub.ts index 22e039a815..a493f10a13 100644 --- a/src/app/shared/testing/route-service-stub.ts +++ b/src/app/shared/testing/route-service-stub.ts @@ -29,7 +29,7 @@ export const routeServiceStub: any = { return observableOf({}) }, getHistory: () => { - return observableOf(['/home','collection/123','/login']) + return observableOf(['/home','/collection/123','/home']) } /* tslint:enable:no-empty */ }; From d9fb68dce97405c890c2bc15ba2164eff98b22bf Mon Sep 17 00:00:00 2001 From: Michael W Spalti Date: Fri, 6 Sep 2019 12:18:28 -0700 Subject: [PATCH 32/41] Modified redirectToPreviousUrl to use the standalone page parameter if no redirect url is found in the store. Removed unused import that was causing merge conflict. Once again try to fix merge conflict. Added routeService to server module providers. Changed order of providers. Minor change to ServerAuthService to make method signature consistent with AuthService. Try adding RouteService to browser-app module to see if that fixes travis build. One more try at getting the CI build to work. Removed change to browser module. --- src/app/core/auth/auth.service.spec.ts | 57 ++++++++++--------- src/app/core/auth/server-auth.service.ts | 2 +- .../shared/log-in/log-in.component.spec.ts | 16 +----- src/app/shared/log-in/log-in.component.ts | 1 - src/modules/app/server-app.module.ts | 2 +- 5 files changed, 34 insertions(+), 44 deletions(-) diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index 6044e9ecb6..380c50d64b 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -86,8 +86,6 @@ describe('AuthService test', () => { ], }); authService = TestBed.get(AuthService); - routeServiceMock = TestBed.get(RouteService); - spyOn(authService, 'setRedirectUrl'); }); it('should return the authentication status object when user credentials are correct', () => { @@ -130,31 +128,6 @@ describe('AuthService test', () => { expect(authService.logout.bind(null)).toThrow(); }); - it ('should dispatch authentication', () => { - authService.doAuthentication(true, '', ''); - expect(mockStore.dispatch).toHaveBeenCalled(); - }); - - it ('should set redirect url to previous page', () => { - spyOn(routeServiceMock, 'getHistory').and.callThrough(); - authService.doAuthentication(true, '', ''); - expect(routeServiceMock.getHistory).toHaveBeenCalled(); - expect(authService.setRedirectUrl).toHaveBeenCalledWith('/collection/123'); - }); - - it ('should set redirect url to current page', () => { - spyOn(routeServiceMock, 'getHistory').and.callThrough(); - authService.doAuthentication(false, '', ''); - expect(routeServiceMock.getHistory).toHaveBeenCalled(); - expect(authService.setRedirectUrl).toHaveBeenCalledWith('/home'); - }); - - it ('should not set redirect url to /login', () => { - spyOn(routeServiceMock, 'getHistory').and.returnValue(of(['/login', '/login'])); - authService.doAuthentication(true, '', ''); - expect(routeServiceMock.getHistory).toHaveBeenCalled(); - expect(authService.setRedirectUrl).not.toHaveBeenCalled(); - }); }); describe('', () => { @@ -247,9 +220,12 @@ describe('AuthService test', () => { }); authService = new AuthService({}, window, undefined, authReqService, router, routeService, cookieService, store, rdbService); storage = (authService as any).storage; + routeServiceMock = TestBed.get(RouteService); + routerStub = TestBed.get(Router); spyOn(storage, 'get'); spyOn(storage, 'remove'); spyOn(storage, 'set'); + })); it('should throw false when token is not valid', () => { @@ -271,5 +247,32 @@ describe('AuthService test', () => { expect(storage.remove).toHaveBeenCalled(); }); + it ('should set redirect url to previous page', () => { + spyOn(routeServiceMock, 'getHistory').and.callThrough(); + authService.redirectToPreviousUrl(true); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(routerStub.navigate).toHaveBeenCalledWith(['/collection/123']); + }); + + it ('should set redirect url to current page', () => { + spyOn(routeServiceMock, 'getHistory').and.callThrough(); + authService.redirectToPreviousUrl(false); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(routerStub.navigate).toHaveBeenCalledWith(['/home']); + }); + + it ('should redirect to / and not to /login', () => { + spyOn(routeServiceMock, 'getHistory').and.returnValue(of(['/login', '/login'])); + authService.redirectToPreviousUrl(true); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(routerStub.navigate).toHaveBeenCalledWith(['/']); + }); + + it ('should redirect to / when no redirect url is found', () => { + spyOn(routeServiceMock, 'getHistory').and.returnValue(of([''])); + authService.redirectToPreviousUrl(true); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(routerStub.navigate).toHaveBeenCalledWith(['/']); + }); }); }); diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index c344683e38..50e3bc53e1 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -54,7 +54,7 @@ export class ServerAuthService extends AuthService { /** * Redirect to the route navigated before the login */ - public redirectToPreviousUrl() { + public redirectToPreviousUrl(isStandalonePage: boolean) { this.getRedirectUrl().pipe( take(1)) .subscribe((redirectUrl) => { diff --git a/src/app/shared/log-in/log-in.component.spec.ts b/src/app/shared/log-in/log-in.component.spec.ts index 6df992fff7..c3bee04d76 100644 --- a/src/app/shared/log-in/log-in.component.spec.ts +++ b/src/app/shared/log-in/log-in.component.spec.ts @@ -13,13 +13,6 @@ import { TranslateModule } from '@ngx-translate/core'; import { AuthService } from '../../core/auth/auth.service'; import { AuthServiceStub } from '../testing/auth-service-stub'; import { AppState } from '../../app.reducer'; -import {AppRoutingModule} from '../../app-routing.module'; -import {PageNotFoundComponent} from '../../pagenotfound/pagenotfound.component'; -import {APP_BASE_HREF} from '@angular/common'; -import {RouterStub} from '../testing/router-stub'; -import {Router} from '@angular/router'; -import {RouteService} from '../services/route.service'; -import {routeServiceStub} from '../testing/route-service-stub'; describe('LogInComponent', () => { @@ -45,18 +38,13 @@ describe('LogInComponent', () => { FormsModule, ReactiveFormsModule, StoreModule.forRoot(authReducer), - AppRoutingModule, TranslateModule.forRoot() ], declarations: [ - LogInComponent, - PageNotFoundComponent + LogInComponent ], providers: [ - {provide: AuthService, useClass: AuthServiceStub}, - {provide: APP_BASE_HREF, useValue: '/'}, - {provide: Router, useClass: RouterStub}, - {provide: RouteService, useValue: routeServiceStub } + {provide: AuthService, useClass: AuthServiceStub} ], schemas: [ CUSTOM_ELEMENTS_SCHEMA diff --git a/src/app/shared/log-in/log-in.component.ts b/src/app/shared/log-in/log-in.component.ts index aa798548f4..c7f67ea28b 100644 --- a/src/app/shared/log-in/log-in.component.ts +++ b/src/app/shared/log-in/log-in.component.ts @@ -88,7 +88,6 @@ export class LogInComponent implements OnDestroy, OnInit { * @constructor * @param {AuthService} authService * @param {FormBuilder} formBuilder - * @param {RouteService} routeService * @param {Router} router * @param {Store} store */ diff --git a/src/modules/app/server-app.module.ts b/src/modules/app/server-app.module.ts index bd3379c8de..dfe936df25 100644 --- a/src/modules/app/server-app.module.ts +++ b/src/modules/app/server-app.module.ts @@ -64,7 +64,7 @@ export function createTranslateLoader() { { provide: SubmissionService, useClass: ServerSubmissionService - }, + } ] }) export class ServerAppModule { From c3102b9d437f8d4aa2e0798d53fc21f2a2c85911 Mon Sep 17 00:00:00 2001 From: Michael W Spalti Date: Mon, 9 Sep 2019 12:15:09 -0700 Subject: [PATCH 33/41] A little cleanup in unit tests. --- src/app/shared/log-in/log-in.component.spec.ts | 2 +- src/app/shared/testing/auth-service-stub.ts | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/app/shared/log-in/log-in.component.spec.ts b/src/app/shared/log-in/log-in.component.spec.ts index c3bee04d76..13f9e5369a 100644 --- a/src/app/shared/log-in/log-in.component.spec.ts +++ b/src/app/shared/log-in/log-in.component.spec.ts @@ -3,7 +3,7 @@ import { async, ComponentFixture, inject, TestBed } from '@angular/core/testing' import { FormGroup, FormsModule, ReactiveFormsModule } from '@angular/forms'; import { By } from '@angular/platform-browser'; -import { Store, StoreModule, select } from '@ngrx/store'; +import { Store, StoreModule } from '@ngrx/store'; import { LogInComponent } from './log-in.component'; import { authReducer } from '../../core/auth/auth.reducer'; diff --git a/src/app/shared/testing/auth-service-stub.ts b/src/app/shared/testing/auth-service-stub.ts index c5f9d92a53..a6d24d5c8b 100644 --- a/src/app/shared/testing/auth-service-stub.ts +++ b/src/app/shared/testing/auth-service-stub.ts @@ -96,9 +96,6 @@ export class AuthServiceStub { return observableOf(this.redirectUrl); } - public doAuthentication(isStandalonePage, email, password) { - return; - } public storeToken(token: AuthTokenInfo) { return; } From 6425f6e7d1542c08c3ab0d9d187e1158d29eb0e4 Mon Sep 17 00:00:00 2001 From: Michael W Spalti Date: Mon, 9 Sep 2019 13:33:51 -0700 Subject: [PATCH 34/41] Removed the router injection that slipped into auth service during rebase, test now passes. --- src/app/shared/log-in/log-in.component.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/shared/log-in/log-in.component.ts b/src/app/shared/log-in/log-in.component.ts index c7f67ea28b..a6c26381e4 100644 --- a/src/app/shared/log-in/log-in.component.ts +++ b/src/app/shared/log-in/log-in.component.ts @@ -94,7 +94,6 @@ export class LogInComponent implements OnDestroy, OnInit { constructor( private authService: AuthService, private formBuilder: FormBuilder, - private router: Router, private store: Store ) { } From 4cc7432e91fbd1bf53479b31eeed85a09b40e7af Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Fri, 13 Sep 2019 17:37:08 +0200 Subject: [PATCH 35/41] Print error message when error occurred on SSR --- src/server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.ts b/src/server.ts index 0526f196ba..de0f8082bb 100644 --- a/src/server.ts +++ b/src/server.ts @@ -67,7 +67,7 @@ export function startServer(bootstrap: Type<{}> | NgModuleFactory<{}>) { function onHandleError(parentZoneDelegate, currentZone, targetZone, error) { if (!res._headerSent) { - console.warn('Error in SSR, serving for direct CSR'); + console.warn('Error in SSR, serving for direct CSR. Error details : ', error); res.sendFile('index.csr.html', { root: './src' }); } } From 0b878af92a30ff31bc67cc956863f8ec22b9e3b6 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Fri, 13 Sep 2019 17:38:12 +0200 Subject: [PATCH 36/41] Fix issue with find operator on SSR --- src/app/core/data/request.service.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 775118dbc0..0980d48537 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -3,7 +3,7 @@ import { HttpHeaders } from '@angular/common/http'; import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; import { Observable, race as observableRace } from 'rxjs'; -import { filter, find, map, mergeMap, take } from 'rxjs/operators'; +import { filter, map, mergeMap, take } from 'rxjs/operators'; import { cloneDeep, remove } from 'lodash'; import { AppState } from '../../app.reducer'; @@ -262,12 +262,13 @@ export class RequestService { */ private clearRequestsOnTheirWayToTheStore(request: GetRequest) { this.getByHref(request.href).pipe( - find((re: RequestEntry) => hasValue(re))) - .subscribe((re: RequestEntry) => { - if (!re.responsePending) { - remove(this.requestsOnTheirWayToTheStore, (item) => item === request.href); - } - }); + filter((re: RequestEntry) => hasValue(re)), + take(1) + ).subscribe((re: RequestEntry) => { + if (!re.responsePending) { + remove(this.requestsOnTheirWayToTheStore, (item) => item === request.href); + } + }); } /** From 8fe617bd74dd1b6b7ccc357632c103c7401be954 Mon Sep 17 00:00:00 2001 From: Michael W Spalti Date: Mon, 16 Sep 2019 13:14:45 -0700 Subject: [PATCH 37/41] Changed method name. --- src/app/core/auth/auth.service.spec.ts | 8 ++++---- src/app/core/auth/auth.service.ts | 2 +- src/app/core/auth/server-auth.service.ts | 2 +- src/app/shared/log-in/log-in.component.ts | 14 +++++++------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index 380c50d64b..a5c9f0e3a8 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -249,28 +249,28 @@ describe('AuthService test', () => { it ('should set redirect url to previous page', () => { spyOn(routeServiceMock, 'getHistory').and.callThrough(); - authService.redirectToPreviousUrl(true); + authService.redirectAfterLoginSuccess(true); expect(routeServiceMock.getHistory).toHaveBeenCalled(); expect(routerStub.navigate).toHaveBeenCalledWith(['/collection/123']); }); it ('should set redirect url to current page', () => { spyOn(routeServiceMock, 'getHistory').and.callThrough(); - authService.redirectToPreviousUrl(false); + authService.redirectAfterLoginSuccess(false); expect(routeServiceMock.getHistory).toHaveBeenCalled(); expect(routerStub.navigate).toHaveBeenCalledWith(['/home']); }); it ('should redirect to / and not to /login', () => { spyOn(routeServiceMock, 'getHistory').and.returnValue(of(['/login', '/login'])); - authService.redirectToPreviousUrl(true); + authService.redirectAfterLoginSuccess(true); expect(routeServiceMock.getHistory).toHaveBeenCalled(); expect(routerStub.navigate).toHaveBeenCalledWith(['/']); }); it ('should redirect to / when no redirect url is found', () => { spyOn(routeServiceMock, 'getHistory').and.returnValue(of([''])); - authService.redirectToPreviousUrl(true); + authService.redirectAfterLoginSuccess(true); expect(routeServiceMock.getHistory).toHaveBeenCalled(); expect(routerStub.navigate).toHaveBeenCalledWith(['/']); }); diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 615fee5d56..b3b95ce6ea 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -339,7 +339,7 @@ export class AuthService { /** * Redirect to the route navigated before the login */ - public redirectToPreviousUrl(isStandalonePage: boolean) { + public redirectAfterLoginSuccess(isStandalonePage: boolean) { this.getRedirectUrl().pipe( take(1)) .subscribe((redirectUrl) => { diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 50e3bc53e1..7e6876e43f 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -54,7 +54,7 @@ export class ServerAuthService extends AuthService { /** * Redirect to the route navigated before the login */ - public redirectToPreviousUrl(isStandalonePage: boolean) { + public redirectAfterLoginSuccess(isStandalonePage: boolean) { this.getRedirectUrl().pipe( take(1)) .subscribe((redirectUrl) => { diff --git a/src/app/shared/log-in/log-in.component.ts b/src/app/shared/log-in/log-in.component.ts index a6c26381e4..b6b97230dd 100644 --- a/src/app/shared/log-in/log-in.component.ts +++ b/src/app/shared/log-in/log-in.component.ts @@ -1,9 +1,9 @@ -import {filter, map, takeWhile } from 'rxjs/operators'; -import {Component, Input, OnDestroy, OnInit} from '@angular/core'; +import { filter, map, takeWhile } from 'rxjs/operators'; +import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { FormBuilder, FormGroup, Validators } from '@angular/forms'; import { select, Store } from '@ngrx/store'; -import {Observable} from 'rxjs'; +import { Observable } from 'rxjs'; import { AuthenticateAction, ResetAuthenticationMessagesAction @@ -17,10 +17,10 @@ import { } from '../../core/auth/selectors'; import { CoreState } from '../../core/core.reducers'; -import {isNotEmpty} from '../empty.util'; +import { isNotEmpty } from '../empty.util'; import { fadeOut } from '../animations/fade'; -import {AuthService} from '../../core/auth/auth.service'; -import {Router} from '@angular/router'; +import { AuthService } from '../../core/auth/auth.service'; +import { Router } from '@angular/router'; /** * /users/sign-in @@ -139,7 +139,7 @@ export class LogInComponent implements OnDestroy, OnInit { takeWhile(() => this.alive), filter((authenticated) => authenticated)) .subscribe(() => { - this.authService.redirectToPreviousUrl(this.isStandalonePage); + this.authService.redirectAfterLoginSuccess(this.isStandalonePage); } ); } From e70b9c59947ae4e4a8c2fc5d9d07b5740505121c Mon Sep 17 00:00:00 2001 From: Michael W Spalti Date: Tue, 17 Sep 2019 12:44:02 -0700 Subject: [PATCH 38/41] Added new auth redirect code for ssr. --- src/app/core/auth/auth.service.ts | 2 +- src/app/core/auth/server-auth.service.ts | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index b3b95ce6ea..5287e537ee 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -368,7 +368,7 @@ export class AuthService { } - private navigateToRedirectUrl(url: string) { + protected navigateToRedirectUrl(url: string) { // in case the user navigates directly to /login (via bookmark, etc), or the route history is not found. if (isEmpty(url) || url.startsWith(LOGIN_ROUTE)) { this.router.navigate(['/']); diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 7e6876e43f..cf4d4a658e 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -1,12 +1,12 @@ -import { map, switchMap, take } from 'rxjs/operators'; +import { filter, map, switchMap, take } from 'rxjs/operators'; import { Injectable } from '@angular/core'; import { Observable } from 'rxjs'; import { HttpHeaders } from '@angular/common/http'; import { HttpOptions } from '../dspace-rest-v2/dspace-rest-v2.service'; import { AuthStatus } from './models/auth-status.model'; -import { isNotEmpty } from '../../shared/empty.util'; -import { AuthService } from './auth.service'; +import { isEmpty, isNotEmpty } from '../../shared/empty.util'; +import { AuthService, LOGIN_ROUTE } from './auth.service'; import { AuthTokenInfo } from './models/auth-token-info.model'; import { CheckAuthenticationTokenAction } from './auth.actions'; import { EPerson } from '../eperson/models/eperson.model'; @@ -67,10 +67,15 @@ export class ServerAuthService extends AuthService { const url = decodeURIComponent(redirectUrl); this.router.navigateByUrl(url); } else { - this.router.navigate(['/']); + // If redirectUrl is empty use history. For ssr the history array should contain the requested url. + this.routeService.getHistory().pipe( + filter((history) => history.length > 0), + take(1) + ).subscribe((history) => { + this.navigateToRedirectUrl(history[history.length - 1] || ''); + }); } }) - } } From 73dd0147bc32af8faf626131f03d293a788bfc3d Mon Sep 17 00:00:00 2001 From: Michael W Spalti Date: Wed, 18 Sep 2019 10:37:12 -0700 Subject: [PATCH 39/41] Minor update of the imports in unit test. --- src/app/core/auth/auth.service.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index a5c9f0e3a8..4ece5eeadd 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -4,7 +4,7 @@ import { ActivatedRoute, Router } from '@angular/router'; import { Store, StoreModule } from '@ngrx/store'; import { REQUEST } from '@nguniversal/express-engine/tokens'; -import {of, of as observableOf} from 'rxjs'; +import { of, of as observableOf } from 'rxjs'; import { authReducer, AuthState } from './auth.reducer'; import { NativeWindowRef, NativeWindowService } from '../services/window.service'; @@ -23,8 +23,8 @@ import { AppState } from '../../app.reducer'; import { ClientCookieService } from '../services/client-cookie.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { getMockRemoteDataBuildService } from '../../shared/mocks/mock-remote-data-build.service'; -import {routeServiceStub} from '../../shared/testing/route-service-stub'; -import {RouteService} from '../services/route.service'; +import { routeServiceStub } from '../../shared/testing/route-service-stub'; +import { RouteService } from '../services/route.service'; describe('AuthService test', () => { From ca048736b51ead7f90b3fdebe2ecc150c0f1fc81 Mon Sep 17 00:00:00 2001 From: Michael W Spalti Date: Wed, 18 Sep 2019 12:13:41 -0700 Subject: [PATCH 40/41] Removed unnecessary import in unit test. --- src/app/core/auth/auth.service.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index 4ece5eeadd..5084dc8596 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -4,7 +4,7 @@ import { ActivatedRoute, Router } from '@angular/router'; import { Store, StoreModule } from '@ngrx/store'; import { REQUEST } from '@nguniversal/express-engine/tokens'; -import { of, of as observableOf } from 'rxjs'; +import { of as observableOf } from 'rxjs'; import { authReducer, AuthState } from './auth.reducer'; import { NativeWindowRef, NativeWindowService } from '../services/window.service'; @@ -262,14 +262,14 @@ describe('AuthService test', () => { }); it ('should redirect to / and not to /login', () => { - spyOn(routeServiceMock, 'getHistory').and.returnValue(of(['/login', '/login'])); + spyOn(routeServiceMock, 'getHistory').and.returnValue(observableOf(['/login', '/login'])); authService.redirectAfterLoginSuccess(true); expect(routeServiceMock.getHistory).toHaveBeenCalled(); expect(routerStub.navigate).toHaveBeenCalledWith(['/']); }); it ('should redirect to / when no redirect url is found', () => { - spyOn(routeServiceMock, 'getHistory').and.returnValue(of([''])); + spyOn(routeServiceMock, 'getHistory').and.returnValue(observableOf([''])); authService.redirectAfterLoginSuccess(true); expect(routeServiceMock.getHistory).toHaveBeenCalled(); expect(routerStub.navigate).toHaveBeenCalledWith(['/']); From ef832d34dc3a86d9ada8aac44dd84cd5b81ac52e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paulo=20Gra=C3=A7a?= Date: Thu, 26 Sep 2019 10:04:21 +0100 Subject: [PATCH 41/41] Remove duplicated "SearchFilterService" --- src/app/+search-page/search-page.module.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/+search-page/search-page.module.ts b/src/app/+search-page/search-page.module.ts index 816633ec38..6ca449460b 100644 --- a/src/app/+search-page/search-page.module.ts +++ b/src/app/+search-page/search-page.module.ts @@ -81,7 +81,6 @@ const components = [ SearchFilterService, SearchFixedFilterService, ConfigurationSearchPageGuard, - SearchFilterService, SearchConfigurationService ], entryComponents: [