From 64f726bf30cd0ff9a4efa4c67221091e53c30573 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 21 Dec 2020 15:40:33 +0100 Subject: [PATCH 01/20] 75316: Deselect relationship on failure + show only enabled externalSources --- .../core/data/relationship-type.service.ts | 2 +- ...namic-lookup-relation-modal.component.html | 2 +- ...dynamic-lookup-relation-modal.component.ts | 34 ++++++++++++----- .../relationship.effects.ts | 37 +++++++++++++++++-- .../models/relationship-options.model.ts | 1 + .../selectable-list.service.ts | 11 ++++++ src/assets/i18n/en.json5 | 6 +++ 7 files changed, 77 insertions(+), 16 deletions(-) diff --git a/src/app/core/data/relationship-type.service.ts b/src/app/core/data/relationship-type.service.ts index 5df11011a3..5ce0720eb2 100644 --- a/src/app/core/data/relationship-type.service.ts +++ b/src/app/core/data/relationship-type.service.ts @@ -60,7 +60,7 @@ export class RelationshipTypeService extends DataService { } else if (type.rightwardType === label) { return this.checkType(type, secondType, firstType); } else { - return []; + return [null]; } }), ); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html index 909dcb1934..3442ddb1ba 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html @@ -22,7 +22,7 @@ - >>; + externalSourcesRD$: Observable; /** * The total amount of internal items for the current options @@ -121,6 +126,7 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy private externalSourceService: ExternalSourceService, private lookupRelationService: LookupRelationService, private searchConfigService: SearchConfigurationService, + private rdbService: RemoteDataBuildService, private zone: NgZone, private store: Store, private router: Router, @@ -141,7 +147,18 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy this.context = Context.EntitySearchModal; } - this.externalSourcesRD$ = this.externalSourceService.findAll(); + if (isNotEmpty(this.relationshipOptions.externalSources)) { + this.externalSourcesRD$ = this.rdbService.aggregate( + this.relationshipOptions.externalSources.map((source) => this.externalSourceService.findById(source)) + ).pipe( + getAllSucceededRemoteDataPayload() + ); + } else { + this.externalSourcesRD$ = this.externalSourceService.findAll().pipe( + getAllSucceededRemoteDataPayload(), + map((list: PaginatedList) => list.page) + ); + } this.setTotals(); } @@ -224,16 +241,13 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy ); const externalSourcesAndOptions$ = observableCombineLatest( - this.externalSourcesRD$.pipe( - getAllSucceededRemoteData(), - getRemoteDataPayload() - ), + this.externalSourcesRD$, this.searchConfigService.paginatedSearchOptions ); this.totalExternal$ = externalSourcesAndOptions$.pipe( switchMap(([sources, options]) => - observableCombineLatest(...sources.page.map((source: ExternalSource) => this.lookupRelationService.getTotalExternalResults(source, options)))) + observableCombineLatest(...sources.map((source: ExternalSource) => this.lookupRelationService.getTotalExternalResults(source, options)))) ); } diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts index 86557b806e..65e7cfb574 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts @@ -15,7 +15,7 @@ import { UpdateRelationshipNameVariantAction } from './relationship.actions'; import { Item } from '../../../../../core/shared/item.model'; -import { hasNoValue, hasValue } from '../../../../empty.util'; +import { hasNoValue, hasValue, hasValueOperator } from '../../../../empty.util'; import { Relationship } from '../../../../../core/shared/item-relationships/relationship.model'; import { RelationshipType } from '../../../../../core/shared/item-relationships/relationship-type.model'; import { RelationshipTypeService } from '../../../../../core/data/relationship-type.service'; @@ -30,6 +30,10 @@ import { ServerSyncBufferActionTypes } from '../../../../../core/cache/server-sy import { JsonPatchOperationsActionTypes } from '../../../../../core/json-patch/json-patch-operations.actions'; import { followLink } from '../../../../utils/follow-link-config.model'; import { RemoteData } from '../../../../../core/data/remote-data'; +import { NotificationsService } from '../../../../notifications/notifications.service'; +import { SelectableListService } from '../../../../object-list/selectable-list/selectable-list.service'; +import { TranslateService } from '@ngx-translate/core'; +import { error } from 'util'; const DEBOUNCE_TIME = 500; @@ -152,7 +156,10 @@ export class RelationshipEffects { private submissionObjectService: SubmissionObjectDataService, private store: Store, private objectCache: ObjectCacheService, - private requestService: RequestService + private requestService: RequestService, + private notificationsService: NotificationsService, + private translateService: TranslateService, + private selectableListService: SelectableListService, ) { } @@ -166,6 +173,9 @@ export class RelationshipEffects { return this.relationshipTypeService.getRelationshipTypeByLabelAndTypes(relationshipType, type1, type2) .pipe( mergeMap((type: RelationshipType) => { + if (type === null) { + return [null]; + } else { const isSwitched = type.rightwardType === relationshipType; if (isSwitched) { return this.relationshipService.addRelationship(type.id, item2, item1, nameVariant, undefined); @@ -173,9 +183,28 @@ export class RelationshipEffects { return this.relationshipService.addRelationship(type.id, item1, item2, undefined, nameVariant); } } - ), + }), take(1), - switchMap(() => this.refreshWorkspaceItemInCache(submissionId)), + switchMap((rd: RemoteData) => { + if (hasNoValue(rd) || rd.hasFailed) { + // An error occurred, deselect the object from the selectable list and display an error notification + const listId = `list-${submissionId}-${relationshipType}`; + this.selectableListService.findSelectedByCondition(listId, (object: any) => hasValue(object.indexableObject) && object.indexableObject.uuid === item2.uuid).pipe( + take(1), + hasValueOperator() + ).subscribe((selected) => { + this.selectableListService.deselectSingle(listId, selected); + }); + let errorContent; + if (hasNoValue(rd)) { + errorContent = this.translateService.instant('relationships.add.error.relationship-type.content', { type: relationshipType }); + } else { + errorContent = this.translateService.instant('relationships.add.error.server.content'); + } + this.notificationsService.error(this.translateService.instant('relationships.add.error.title'), errorContent); + } + return this.refreshWorkspaceItemInCache(submissionId); + }), ).subscribe((submissionObject: SubmissionObject) => this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false))); } diff --git a/src/app/shared/form/builder/models/relationship-options.model.ts b/src/app/shared/form/builder/models/relationship-options.model.ts index 4568aa69ec..eaa62961a2 100644 --- a/src/app/shared/form/builder/models/relationship-options.model.ts +++ b/src/app/shared/form/builder/models/relationship-options.model.ts @@ -8,6 +8,7 @@ export class RelationshipOptions { filter: string; searchConfiguration: string; nameVariants: string; + externalSources: string[]; get metadataField() { return RELATION_METADATA_PREFIX + this.relationshipType; diff --git a/src/app/shared/object-list/selectable-list/selectable-list.service.ts b/src/app/shared/object-list/selectable-list/selectable-list.service.ts index 5a86c0b65d..91ecdf1a1a 100644 --- a/src/app/shared/object-list/selectable-list/selectable-list.service.ts +++ b/src/app/shared/object-list/selectable-list/selectable-list.service.ts @@ -91,4 +91,15 @@ export class SelectableListService { distinctUntilChanged() ); } + + /** + * Find a selected object by a custom condition + * @param id The ID of the selectable list to search in + * @param condition The condition that the required object has to match + */ + findSelectedByCondition(id: string, condition: (object: ListableObject) => boolean): Observable { + return this.getSelectableList(id).pipe( + map((state: SelectableListState) => (hasValue(state) && isNotEmpty(state.selection)) ? state.selection.find((selected) => condition(selected)) : undefined) + ); + } } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 603acb3f48..a99ac57448 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -2654,6 +2654,12 @@ + "relationships.add.error.relationship-type.content": "No suitable match could be found for relationship type {{ type }} between the two items", + + "relationships.add.error.server.content": "The server returned an error", + + "relationships.add.error.title": "Unable to add relationship", + "relationships.isAuthorOf": "Authors", "relationships.isAuthorOf.Person": "Authors (persons)", From a08b515335b5a1777b135d69a5c566f77fa45dfd Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 21 Dec 2020 15:41:11 +0100 Subject: [PATCH 02/20] 75316: Only show external sources when configured --- .../dynamic-lookup-relation-modal.component.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts index c92702a57d..672ab852ee 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts @@ -153,11 +153,6 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy ).pipe( getAllSucceededRemoteDataPayload() ); - } else { - this.externalSourcesRD$ = this.externalSourceService.findAll().pipe( - getAllSucceededRemoteDataPayload(), - map((list: PaginatedList) => list.page) - ); } this.setTotals(); From d13eabe3863907e6f95a191b987e39973475fb17 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 21 Dec 2020 17:32:24 +0100 Subject: [PATCH 03/20] 75316: Test fixes --- .../core/data/relationship-type.service.spec.ts | 13 +++++++------ ...amic-lookup-relation-modal.component.spec.ts | 12 ++++++++++-- .../relationship.effects.spec.ts | 17 +++++++++++++++++ 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/app/core/data/relationship-type.service.spec.ts b/src/app/core/data/relationship-type.service.spec.ts index 14669e68fa..910b7662dc 100644 --- a/src/app/core/data/relationship-type.service.spec.ts +++ b/src/app/core/data/relationship-type.service.spec.ts @@ -9,10 +9,10 @@ import { import { ObjectCacheService } from '../cache/object-cache.service'; import { ItemType } from '../shared/item-relationships/item-type.model'; import { RelationshipType } from '../shared/item-relationships/relationship-type.model'; -import { PageInfo } from '../shared/page-info.model'; -import { buildPaginatedList } from './paginated-list.model'; import { RelationshipTypeService } from './relationship-type.service'; import { RequestService } from './request.service'; +import { createPaginatedList } from '../../shared/testing/utils.test'; +import { hasValueOperator } from '../../shared/empty.util'; describe('RelationshipTypeService', () => { let service: RelationshipTypeService; @@ -62,7 +62,7 @@ describe('RelationshipTypeService', () => { rightType: createSuccessfulRemoteDataObject$(orgUnitType) }); - buildList = createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), [relationshipType1, relationshipType2])); + buildList = createSuccessfulRemoteDataObject(createPaginatedList([relationshipType1, relationshipType2])); rdbService = getMockRemoteDataBuildService(undefined, observableOf(buildList)); objectCache = Object.assign({ /* tslint:disable:no-empty */ @@ -100,9 +100,10 @@ describe('RelationshipTypeService', () => { describe('getRelationshipTypeByLabelAndTypes', () => { it('should return the type filtered by label and type strings', (done) => { - const expected = service.getRelationshipTypeByLabelAndTypes(relationshipType1.leftwardType, publicationTypeString, personTypeString); - expected.subscribe((e) => { - expect(e).toBe(relationshipType1); + service.getRelationshipTypeByLabelAndTypes(relationshipType1.leftwardType, publicationTypeString, personTypeString).pipe( + hasValueOperator() + ).subscribe((e) => { + expect(e.id).toEqual(relationshipType1.id); done(); }); }); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts index 2b6f3019f4..88763b011c 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts @@ -20,6 +20,7 @@ import { createSuccessfulRemoteDataObject$ } from '../../../../remote-data.utils import { createPaginatedList } from '../../../../testing/utils.test'; import { ExternalSourceService } from '../../../../../core/data/external-source.service'; import { LookupRelationService } from '../../../../../core/data/lookup-relation.service'; +import { RemoteDataBuildService } from '../../../../../core/cache/builders/remote-data-build.service'; describe('DsDynamicLookupRelationModalComponent', () => { let component: DsDynamicLookupRelationModalComponent; @@ -38,6 +39,7 @@ describe('DsDynamicLookupRelationModalComponent', () => { let pSearchOptions; let externalSourceService; let lookupRelationService; + let rdbService; let submissionId; const externalSources = [ @@ -68,18 +70,23 @@ describe('DsDynamicLookupRelationModalComponent', () => { filter: 'filter', relationshipType: 'isAuthorOfPublication', nameVariants: true, - searchConfiguration: 'personConfig' + searchConfiguration: 'personConfig', + externalSources: ['orcidV2', 'sherpaPublisher'] }); nameVariant = 'Doe, J.'; metadataField = 'dc.contributor.author'; pSearchOptions = new PaginatedSearchOptions({}); externalSourceService = jasmine.createSpyObj('externalSourceService', { - findAll: createSuccessfulRemoteDataObject$(createPaginatedList(externalSources)) + findAll: createSuccessfulRemoteDataObject$(createPaginatedList(externalSources)), + findById: createSuccessfulRemoteDataObject$(externalSources[0]) }); lookupRelationService = jasmine.createSpyObj('lookupRelationService', { getTotalLocalResults: observableOf(totalLocal), getTotalExternalResults: observableOf(totalExternal) }); + rdbService = jasmine.createSpyObj('rdbService', { + aggregate: createSuccessfulRemoteDataObject$(externalSources) + }); submissionId = '1234'; } @@ -103,6 +110,7 @@ describe('DsDynamicLookupRelationModalComponent', () => { provide: RelationshipService, useValue: { getNameVariant: () => observableOf(nameVariant) } }, { provide: RelationshipTypeService, useValue: {} }, + { provide: RemoteDataBuildService, useValue: rdbService }, { provide: Store, useValue: { // tslint:disable-next-line:no-empty diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts index 215049e408..432ac44859 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts @@ -20,6 +20,9 @@ import { SubmissionObjectDataService } from '../../../../../core/submission/subm import { WorkspaceItem } from '../../../../../core/submission/models/workspaceitem.model'; import { ObjectCacheService } from '../../../../../core/cache/object-cache.service'; import { RequestService } from '../../../../../core/data/request.service'; +import { NotificationsService } from '../../../../notifications/notifications.service'; +import { TranslateService } from '@ngx-translate/core'; +import { SelectableListService } from '../../../../object-list/selectable-list/selectable-list.service'; describe('RelationshipEffects', () => { let relationEffects: RelationshipEffects; @@ -45,6 +48,9 @@ describe('RelationshipEffects', () => { let relationship; let mockRelationshipService; let mockRelationshipTypeService; + let notificationsService; + let translateService; + let selectableListService; let testScheduler: TestScheduler; function init() { @@ -95,6 +101,14 @@ describe('RelationshipEffects', () => { getRelationshipTypeByLabelAndTypes: () => observableOf(relationshipType) }; + notificationsService = jasmine.createSpyObj('notificationsService', ['error']); + translateService = jasmine.createSpyObj('translateService', { + instant: 'translated-message' + }); + selectableListService = jasmine.createSpyObj('selectableListService', { + findSelectedByCondition: observableOf({}), + deselectSingle: {} + }); } beforeEach(waitForAsync(() => { @@ -114,6 +128,9 @@ describe('RelationshipEffects', () => { { provide: Store, useValue: jasmine.createSpyObj('store', ['dispatch']) }, { provide: ObjectCacheService, useValue: {} }, { provide: RequestService, useValue: {} }, + { provide: NotificationsService, useValue: notificationsService }, + { provide: TranslateService, useValue: translateService }, + { provide: SelectableListService, useValue: selectableListService }, ], }); })); From e4d518fca7acd958865f1f844f2379b9f59fe78b Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 13 Jan 2021 14:24:06 +0100 Subject: [PATCH 04/20] fix issue where a relationshiptype wouldn't be found if the subscriber unsubscribed before all types were checked --- .../core/data/relationship-type.service.ts | 86 ++++++++++++++----- 1 file changed, 65 insertions(+), 21 deletions(-) diff --git a/src/app/core/data/relationship-type.service.ts b/src/app/core/data/relationship-type.service.ts index 5ce0720eb2..818a46cc2a 100644 --- a/src/app/core/data/relationship-type.service.ts +++ b/src/app/core/data/relationship-type.service.ts @@ -2,9 +2,9 @@ import { HttpClient } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { Store } from '@ngrx/store'; import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; -import { filter, find, map, mergeMap, switchMap } from 'rxjs/operators'; +import { map, mergeMap, switchMap, toArray } from 'rxjs/operators'; import { AppState } from '../../app.reducer'; -import { isNotUndefined } from '../../shared/empty.util'; +import { hasValue } from '../../shared/empty.util'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { followLink } from '../../shared/utils/follow-link-config.model'; import { dataService } from '../cache/builders/build-decorators'; @@ -15,7 +15,7 @@ import { HALEndpointService } from '../shared/hal-endpoint.service'; import { ItemType } from '../shared/item-relationships/item-type.model'; import { RelationshipType } from '../shared/item-relationships/relationship-type.model'; import { RELATIONSHIP_TYPE } from '../shared/item-relationships/relationship-type.resource-type'; -import { getFirstSucceededRemoteData } from '../shared/operators'; +import { getFirstSucceededRemoteData, getFirstCompletedRemoteData } from '../shared/operators'; import { DataService } from './data.service'; import { DefaultChangeAnalyzer } from './default-change-analyzer.service'; import { ItemDataService } from './item-data.service'; @@ -23,6 +23,16 @@ import { PaginatedList } from './paginated-list.model'; import { RemoteData } from './remote-data'; import { RequestService } from './request.service'; + +/** + * Check if one side of a RelationshipType is the ItemType with the given label + * + * @param typeRd the RemoteData for an ItemType + * @param label the label to check. e.g. Author + */ +const checkSide = (typeRd: RemoteData, label: string): boolean => + typeRd.hasSucceeded && typeRd.payload.label === label; + /** * The service handling all relationship type requests */ @@ -45,36 +55,70 @@ export class RelationshipTypeService extends DataService { } /** - * Get the RelationshipType for a relationship type by label - * @param label + * Find a RelationshipType object by its label, and the types of the items on either side. + * + * TODO this should be implemented as a rest endpoint, we shouldn't have logic on the client that + * requires using a huge page-size in order to process "everything". + * + * @param relationshipTypeLabel The name of the relationType we're looking for + * e.g. isAuthorOfPublication + * @param firstItemType The type of one of the sides of the relationship e.g. Publication + * @param secondItemType The type of the other side of the relationship e.g. Author */ - getRelationshipTypeByLabelAndTypes(label: string, firstType: string, secondType: string): Observable { + getRelationshipTypeByLabelAndTypes(relationshipTypeLabel: string, firstItemType: string, secondItemType: string): Observable { + // Retrieve all relationship types from the server in a single page return this.findAll({ currentPage: 1, elementsPerPage: 9999 }, true, true, followLink('leftType'), followLink('rightType')) .pipe( getFirstSucceededRemoteData(), - /* Flatten the page so we can treat it like an observable */ + // Emit each type in the page array separately switchMap((typeListRD: RemoteData>) => typeListRD.payload.page), - mergeMap((type: RelationshipType) => { - if (type.leftwardType === label) { - return this.checkType(type, firstType, secondType); - } else if (type.rightwardType === label) { - return this.checkType(type, secondType, firstType); + // Check each type individually, to see if it matches the provided types + mergeMap((relationshipType: RelationshipType) => { + if (relationshipType.leftwardType === relationshipTypeLabel) { + return this.checkType(relationshipType, firstItemType, secondItemType); + } else if (relationshipType.rightwardType === relationshipTypeLabel) { + return this.checkType(relationshipType, secondItemType, firstItemType); } else { return [null]; } }), + // Wait for all types to be checked and emit once, with the results combined back into an + // array + toArray(), + // Look for a match in the array and emit it if found, or null if one isn't found + map((types: RelationshipType[]) => { + const match = types.find((type: RelationshipType) => hasValue(type)); + if (hasValue(match)) { + return match + } else { + return null; + } + }) ); } - // Check if relationship type matches the given types - // returns a void observable if there's not match - // returns an observable that emits the relationship type when there is a match - private checkType(type: RelationshipType, firstType: string, secondType: string): Observable { - const entityTypes = observableCombineLatest([type.leftType.pipe(getFirstSucceededRemoteData()), type.rightType.pipe(getFirstSucceededRemoteData())]); - return entityTypes.pipe( - find(([leftTypeRD, rightTypeRD]: [RemoteData, RemoteData]) => leftTypeRD.payload.label === firstType && rightTypeRD.payload.label === secondType), - filter((types) => isNotUndefined(types)), - map(() => type) + /** + * Check if the given RelationshipType has the given itemTypes on its left and right sides. + * Returns an observable of the given RelationshipType if it matches, null if it doesn't + * + * @param type The RelationshipType to check + * @param leftItemType The item type that should be on the left side + * @param rightItemType The item type that should be on the right side + * @private + */ + private checkType(type: RelationshipType, leftItemType: string, rightItemType: string): Observable { + return observableCombineLatest([ + type.leftType.pipe(getFirstCompletedRemoteData()), + type.rightType.pipe(getFirstCompletedRemoteData()) + ]).pipe( + map(([leftTypeRD, rightTypeRD]: [RemoteData, RemoteData]) => { + if (checkSide(leftTypeRD, leftItemType) && checkSide(rightTypeRD, rightItemType) + ) { + return type; + } else { + return null; + } + }) ); } } From 3ffef2bffad6bb5a5d104405366ec3344fb578c0 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 14 Jan 2021 13:27:49 +0100 Subject: [PATCH 05/20] 75316: TSLint fix --- src/app/core/data/relationship-type.service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/core/data/relationship-type.service.ts b/src/app/core/data/relationship-type.service.ts index 818a46cc2a..b55a0ade8a 100644 --- a/src/app/core/data/relationship-type.service.ts +++ b/src/app/core/data/relationship-type.service.ts @@ -23,7 +23,6 @@ import { PaginatedList } from './paginated-list.model'; import { RemoteData } from './remote-data'; import { RequestService } from './request.service'; - /** * Check if one side of a RelationshipType is the ItemType with the given label * From 3c296620c69ed3ba42f47a30712f491975b62192 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 27 Jan 2021 10:43:14 +0100 Subject: [PATCH 06/20] v1.22.10 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d92afb4a40..ef5c3c5e68 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dspace-angular", - "version": "0.0.0", + "version": "1.22.10", "scripts": { "ng": "ng", "config:dev": "ts-node --project ./tsconfig.ts-node.json scripts/set-env.ts --dev", From 28ffe18b07067a7f259190da0b155d4e74495b17 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 27 Jan 2021 11:20:07 +0100 Subject: [PATCH 07/20] 75316: Build error fix --- .../relation-lookup-modal/relationship.effects.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts index 65e7cfb574..e68152f74d 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts @@ -33,7 +33,6 @@ import { RemoteData } from '../../../../../core/data/remote-data'; import { NotificationsService } from '../../../../notifications/notifications.service'; import { SelectableListService } from '../../../../object-list/selectable-list/selectable-list.service'; import { TranslateService } from '@ngx-translate/core'; -import { error } from 'util'; const DEBOUNCE_TIME = 500; From d69bde132750d56a166e8b48b604b42afffb9839 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 27 Jan 2021 11:24:40 +0100 Subject: [PATCH 08/20] 75316: TSlint error fix --- src/app/core/data/relationship-type.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/data/relationship-type.service.ts b/src/app/core/data/relationship-type.service.ts index b55a0ade8a..4dac044090 100644 --- a/src/app/core/data/relationship-type.service.ts +++ b/src/app/core/data/relationship-type.service.ts @@ -88,7 +88,7 @@ export class RelationshipTypeService extends DataService { map((types: RelationshipType[]) => { const match = types.find((type: RelationshipType) => hasValue(type)); if (hasValue(match)) { - return match + return match; } else { return null; } From f1407b7f5b1d66febc8a9f0feb2e0c1b289a0638 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Thu, 14 Jan 2021 16:20:04 +0100 Subject: [PATCH 09/20] 75832: Groups registry refactor --- .../group-form/group-form.component.ts | 8 +-- .../members-list/members-list.component.html | 42 ++++++++-------- .../members-list/members-list.component.ts | 50 +++++++++++++++---- .../subgroup-list/subgroups-list.component.ts | 2 +- .../groups-registry.component.html | 8 ++- .../groups-registry.component.ts | 21 +++----- src/app/core/eperson/eperson-data.service.ts | 6 +-- .../core/eperson/models/eperson-dto.model.ts | 4 ++ .../core/eperson/models/group-dto.model.ts | 15 +++++- src/app/core/eperson/models/group.model.ts | 10 +++- src/app/core/shared/dspace-object.model.ts | 2 +- src/assets/i18n/en.json5 | 2 - 12 files changed, 106 insertions(+), 64 deletions(-) diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts b/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts index 81e9513433..7984fc50d1 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts @@ -143,7 +143,9 @@ export class GroupFormComponent implements OnInit, OnDestroy { initialisePage() { this.subs.push(this.route.params.subscribe((params) => { - this.setActiveGroup(params.groupId); + if (params.groupId !== 'newGroup') { + this.setActiveGroup(params.groupId); + } })); this.canEdit$ = this.groupDataService.getActiveGroup().pipe( hasValueOperator(), @@ -225,14 +227,12 @@ export class GroupFormComponent implements OnInit, OnDestroy { { value: this.groupDescription.value } - ], + ] }, }; if (group === null) { - console.log('createNewGroup', values); this.createNewGroup(values); } else { - console.log('editGroup', group); this.editGroup(group); } } diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.html b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.html index 0ac67aff75..8e2d23f8d5 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.html +++ b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.html @@ -24,10 +24,10 @@ - @@ -42,23 +42,23 @@ - - {{ePerson.id}} - {{ePerson.name}} + + {{ePerson.eperson.id}} + {{ePerson.eperson.name}}
- -
@@ -70,7 +70,7 @@ - @@ -45,7 +45,6 @@ {{messagePrefix + 'table.id' | translate}} {{messagePrefix + 'table.name' | translate}} {{messagePrefix + 'table.members' | translate}} - {{messagePrefix + 'table.edit' | translate}} @@ -53,8 +52,7 @@ {{groupDto.group.id}} {{groupDto.group.name}} - {{(getMembers(groupDto.group) | async)?.payload?.totalElements + (getSubgroups(groupDto.group) | async)?.payload?.totalElements}} - + {{groupDto.epersons?.totalElements + groupDto.subgroups?.totalElements}}
diff --git a/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts b/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts index db5b1d3e3b..7a8e670267 100644 --- a/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts @@ -169,16 +169,16 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { /** * Delete Group */ - deleteGroup(group: Group) { - if (hasValue(group.id)) { - this.groupService.delete(group.id).pipe(getFirstCompletedRemoteData()) + deleteGroup(group: GroupDtoModel) { + if (hasValue(group.group.id)) { + this.groupService.delete(group.group.id).pipe(getFirstCompletedRemoteData()) .subscribe((rd: RemoteData) => { if (rd.hasSucceeded) { - this.notificationsService.success(this.translateService.get(this.messagePrefix + 'notification.deleted.success', { name: group.name })); + this.notificationsService.success(this.translateService.get(this.messagePrefix + 'notification.deleted.success', { name: group.group.name })); this.reset(); } else { this.notificationsService.error( - this.translateService.get(this.messagePrefix + 'notification.deleted.failure.title', { name: group.name }), + this.translateService.get(this.messagePrefix + 'notification.deleted.failure.title', { name: group.group.name }), this.translateService.get(this.messagePrefix + 'notification.deleted.failure.content', { cause: rd.errorMessage })); } }); @@ -194,7 +194,7 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { ).subscribe((href: string) => { this.requestService.setStaleByHrefSubstring(href); }); - } +} /** * Get the members (epersons embedded value of a group) @@ -233,15 +233,6 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { this.search({ query: '' }); } - /** - * Extract optional UUID from a group name => To be resolved to community or collection with link - * (Or will be resolved in backend and added to group object, tbd) //TODO - * @param groupName - */ - getOptionalComColFromName(groupName: string): string { - return this.groupService.getUUIDFromString(groupName); - } - /** * Unsub all subscriptions */ diff --git a/src/app/core/eperson/eperson-data.service.ts b/src/app/core/eperson/eperson-data.service.ts index 9d1be80366..79df246833 100644 --- a/src/app/core/eperson/eperson-data.service.ts +++ b/src/app/core/eperson/eperson-data.service.ts @@ -63,10 +63,10 @@ export class EPersonDataService extends DataService { * @param query Query of search * @param options Options of search request */ - public searchByScope(scope: string, query: string, options: FindListOptions = {}): Observable>> { + public searchByScope(scope: string, query: string, options: FindListOptions = {}, useCachedVersionIfAvailable?: boolean): Observable>> { switch (scope) { case 'metadata': - return this.getEpeopleByMetadata(query.trim(), options); + return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable); case 'email': return this.getEPersonByEmail(query.trim()).pipe( map((rd: RemoteData) => { @@ -100,7 +100,7 @@ export class EPersonDataService extends DataService { }) ); default: - return this.getEpeopleByMetadata(query.trim(), options); + return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable); } } diff --git a/src/app/core/eperson/models/eperson-dto.model.ts b/src/app/core/eperson/models/eperson-dto.model.ts index f491f6f8be..0e79902196 100644 --- a/src/app/core/eperson/models/eperson-dto.model.ts +++ b/src/app/core/eperson/models/eperson-dto.model.ts @@ -13,5 +13,9 @@ export class EpersonDtoModel { * Whether or not the linked EPerson is able to be deleted */ public ableToDelete: boolean; + /** + * Whether or not this EPerson is member of group on page it is being used on + */ + public memberOfGroup: boolean; } diff --git a/src/app/core/eperson/models/group-dto.model.ts b/src/app/core/eperson/models/group-dto.model.ts index db167dc6b2..47a70cf326 100644 --- a/src/app/core/eperson/models/group-dto.model.ts +++ b/src/app/core/eperson/models/group-dto.model.ts @@ -1,7 +1,9 @@ +import { PaginatedList } from '../../data/paginated-list.model'; +import { EPerson } from './eperson.model'; import { Group } from './group.model'; /** - * This class serves as a Data Transfer Model that contains the Group and whether or not it's able to be deleted + * This class serves as a Data Transfer Model that contains the Group, whether or not it's able to be deleted and its members */ export class GroupDtoModel { @@ -9,9 +11,20 @@ export class GroupDtoModel { * The Group linked to this object */ public group: Group; + /** * Whether or not the linked Group is able to be deleted */ public ableToDelete: boolean; + /** + * List of subgroups of this group + */ + public subgroups: PaginatedList; + + /** + * List of members of this group + */ + public epersons: PaginatedList; + } diff --git a/src/app/core/eperson/models/group.model.ts b/src/app/core/eperson/models/group.model.ts index 9e13627116..f147cc53a6 100644 --- a/src/app/core/eperson/models/group.model.ts +++ b/src/app/core/eperson/models/group.model.ts @@ -1,4 +1,4 @@ -import { autoserialize, deserialize, inheritSerialization } from 'cerialize'; +import { autoserialize, autoserializeAs, deserialize, inheritSerialization } from 'cerialize'; import { Observable } from 'rxjs'; import { link, typedObject } from '../../cache/builders/build-decorators'; import { PaginatedList } from '../../data/paginated-list.model'; @@ -10,12 +10,20 @@ import { HALLink } from '../../shared/hal-link.model'; import { EPerson } from './eperson.model'; import { EPERSON } from './eperson.resource-type'; import { GROUP } from './group.resource-type'; +import { excludeFromEquals } from '../../utilities/equals.decorators'; @typedObject @inheritSerialization(DSpaceObject) export class Group extends DSpaceObject { static type = GROUP; + /** + * A string representing the unique name of this Group + */ + @excludeFromEquals + @autoserializeAs('name') + protected _name: string; + /** * A string representing the unique handle of this Group */ diff --git a/src/app/core/shared/dspace-object.model.ts b/src/app/core/shared/dspace-object.model.ts index b88adfe861..9d1fba4f86 100644 --- a/src/app/core/shared/dspace-object.model.ts +++ b/src/app/core/shared/dspace-object.model.ts @@ -29,7 +29,7 @@ export class DSpaceObject extends ListableObject implements CacheableObject { @excludeFromEquals @deserializeAs('name') - private _name: string; + protected _name: string; /** * The human-readable identifier of this DSpaceObject diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 603acb3f48..5ddde2a307 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -308,8 +308,6 @@ "admin.access-control.groups.table.members": "Members", - "admin.access-control.groups.table.comcol": "Community / Collection", - "admin.access-control.groups.table.edit": "Edit", "admin.access-control.groups.table.edit.buttons.edit": "Edit \"{{name}}\"", From d574ffafa44138b63f9daaac4eb49bb354237440 Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Mon, 1 Feb 2021 19:10:15 +0100 Subject: [PATCH 10/20] 76634: Implement GoogleAnalyticsService --- src/app/app.component.ts | 9 ++++- .../google-analytics.service.spec.ts | 16 ++++++++ .../statistics/google-analytics.service.ts | 40 +++++++++++++++++++ src/config/global-config.interface.ts | 1 - src/environments/environment.common.ts | 2 - src/environments/mock-environment.ts | 2 - src/main.browser.ts | 16 -------- src/modules/app/browser-app.module.ts | 5 +++ 8 files changed, 68 insertions(+), 23 deletions(-) create mode 100644 src/app/statistics/google-analytics.service.spec.ts create mode 100644 src/app/statistics/google-analytics.service.ts diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 10cda90755..1ef5c868a6 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -33,6 +33,7 @@ import { models } from './core/core.module'; import { LocaleService } from './core/locale/locale.service'; import { hasValue } from './shared/empty.util'; import { KlaroService } from './shared/cookies/klaro.service'; +import {GoogleAnalyticsService} from './statistics/google-analytics.service'; @Component({ selector: 'ds-app', @@ -70,7 +71,8 @@ export class AppComponent implements OnInit, AfterViewInit { private menuService: MenuService, private windowService: HostWindowService, private localeService: LocaleService, - @Optional() private cookiesService: KlaroService + @Optional() private cookiesService: KlaroService, + @Optional() private googleAnalyticsService: GoogleAnalyticsService, ) { /* Use models object so all decorators are actually called */ @@ -84,7 +86,10 @@ export class AppComponent implements OnInit, AfterViewInit { // set the current language code this.localeService.setCurrentLanguageCode(); - angulartics2GoogleAnalytics.startTracking(); + // analytics + if (hasValue(googleAnalyticsService)) { + googleAnalyticsService.addTrackingIdToPage(); + } angulartics2DSpace.startTracking(); metadata.listenForRouteChange(); diff --git a/src/app/statistics/google-analytics.service.spec.ts b/src/app/statistics/google-analytics.service.spec.ts new file mode 100644 index 0000000000..3329024a18 --- /dev/null +++ b/src/app/statistics/google-analytics.service.spec.ts @@ -0,0 +1,16 @@ +import { TestBed } from '@angular/core/testing'; + +import { GoogleAnalyticsService } from './google-analytics.service'; + +describe('GoogleAnalyticsService', () => { + let service: GoogleAnalyticsService; + + beforeEach(() => { + TestBed.configureTestingModule({}); + service = TestBed.inject(GoogleAnalyticsService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); +}); diff --git a/src/app/statistics/google-analytics.service.ts b/src/app/statistics/google-analytics.service.ts new file mode 100644 index 0000000000..ec780bda2b --- /dev/null +++ b/src/app/statistics/google-analytics.service.ts @@ -0,0 +1,40 @@ +import { Injectable } from '@angular/core'; +import {Angulartics2GoogleAnalytics} from 'angulartics2/ga'; +import {ConfigurationDataService} from '../core/data/configuration-data.service'; +import {getFirstCompletedRemoteData} from '../core/shared/operators'; +import {isEmpty, isNotEmpty} from '../shared/empty.util'; + +@Injectable() +export class GoogleAnalyticsService { + + constructor( + private angulartics: Angulartics2GoogleAnalytics, + private configService: ConfigurationDataService, + ) { } + + addTrackingIdToPage() { + this.configService.findByPropertyName('google.analytics.key').pipe( + getFirstCompletedRemoteData(), + ).subscribe((remoteData) => { + // make sure we got a success response from the backend + if (!remoteData.hasSucceeded) { return; } + + const trackingId = remoteData.payload.values[0]; + + // make sure we received a tracking id + if (isEmpty(trackingId)) { return; } + + // add trackingId snippet to page + const keyScript = document.createElement('script'); + keyScript.innerHTML = `(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){ + (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o), + m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) + })(window,document,'script','https://www.google-analytics.com/analytics.js','ga'); + 'ga('create', '${trackingId}', 'auto');`; + document.body.appendChild(keyScript); + + // start tracking + this.angulartics.startTracking(); + }); + } +} diff --git a/src/config/global-config.interface.ts b/src/config/global-config.interface.ts index 0bc3d5eec4..5c6e56babb 100644 --- a/src/config/global-config.interface.ts +++ b/src/config/global-config.interface.ts @@ -23,7 +23,6 @@ export interface GlobalConfig extends Config { notifications: INotificationBoardOptions; submission: SubmissionConfig; universal: UniversalConfig; - gaTrackingId: string; logDirectory: string; debug: boolean; defaultLanguage: string; diff --git a/src/environments/environment.common.ts b/src/environments/environment.common.ts index d47c3cfcef..a0fe851cb4 100644 --- a/src/environments/environment.common.ts +++ b/src/environments/environment.common.ts @@ -132,8 +132,6 @@ export const environment: GlobalConfig = { async: true, time: false }, - // Google Analytics tracking id - gaTrackingId: '', // Log directory logDirectory: '.', // NOTE: will log all redux actions and transfers in console diff --git a/src/environments/mock-environment.ts b/src/environments/mock-environment.ts index b220c46083..ef3eb86cc2 100644 --- a/src/environments/mock-environment.ts +++ b/src/environments/mock-environment.ts @@ -110,8 +110,6 @@ export const environment: Partial = { async: true, time: false }, - // Google Analytics tracking id - gaTrackingId: '', // Log directory logDirectory: '.', // NOTE: will log all redux actions and transfers in console diff --git a/src/main.browser.ts b/src/main.browser.ts index 5149014d88..6e3a7ad602 100644 --- a/src/main.browser.ts +++ b/src/main.browser.ts @@ -25,25 +25,9 @@ export function main() { } }); - addGoogleAnalytics(); - return platformBrowserDynamic().bootstrapModule(BrowserAppModule, {preserveWhitespaces:true}); } -function addGoogleAnalytics() { - // Add google analytics if key is present in config - const trackingId = environment.gaTrackingId; - if (isNotEmpty(trackingId)) { - const keyScript = document.createElement('script'); - keyScript.innerHTML = `(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){ - (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o), - m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) - })(window,document,'script','https://www.google-analytics.com/analytics.js','ga');` - + 'ga(\'create\', \'' + environment.gaTrackingId + '\', \'auto\');'; - document.body.appendChild(keyScript); - } -} - // support async tag or hmr if (hasValue(environment.universal) && environment.universal.preboot === false) { bootloader(main); diff --git a/src/modules/app/browser-app.module.ts b/src/modules/app/browser-app.module.ts index 3aa6bf244b..e0bd7b5ca1 100644 --- a/src/modules/app/browser-app.module.ts +++ b/src/modules/app/browser-app.module.ts @@ -30,6 +30,7 @@ import { LocationToken } from '../../app/core/services/browser-hard-redirect.service'; import { LocaleService } from '../../app/core/locale/locale.service'; +import {GoogleAnalyticsService} from '../../app/statistics/google-analytics.service'; export const REQ_KEY = makeStateKey('req'); @@ -99,6 +100,10 @@ export function getRequest(transferState: TransferState): any { provide: HardRedirectService, useClass: BrowserHardRedirectService, }, + { + provide: GoogleAnalyticsService, + useClass: GoogleAnalyticsService, + }, { provide: LocationToken, useFactory: locationProvider, From 20f896e9397af62e8267220739225ca3e3156042 Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Tue, 2 Feb 2021 11:02:17 +0100 Subject: [PATCH 11/20] 76634: Inject document into GoogleAnalyticsService --- src/app/statistics/google-analytics.service.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/app/statistics/google-analytics.service.ts b/src/app/statistics/google-analytics.service.ts index ec780bda2b..20d0078225 100644 --- a/src/app/statistics/google-analytics.service.ts +++ b/src/app/statistics/google-analytics.service.ts @@ -1,8 +1,9 @@ -import { Injectable } from '@angular/core'; +import {Inject, Injectable} from '@angular/core'; import {Angulartics2GoogleAnalytics} from 'angulartics2/ga'; import {ConfigurationDataService} from '../core/data/configuration-data.service'; import {getFirstCompletedRemoteData} from '../core/shared/operators'; -import {isEmpty, isNotEmpty} from '../shared/empty.util'; +import {isEmpty} from '../shared/empty.util'; +import {DOCUMENT} from '@angular/common'; @Injectable() export class GoogleAnalyticsService { @@ -10,6 +11,7 @@ export class GoogleAnalyticsService { constructor( private angulartics: Angulartics2GoogleAnalytics, private configService: ConfigurationDataService, + @Inject(DOCUMENT) private document: Document ) { } addTrackingIdToPage() { @@ -25,13 +27,13 @@ export class GoogleAnalyticsService { if (isEmpty(trackingId)) { return; } // add trackingId snippet to page - const keyScript = document.createElement('script'); + const keyScript = this.document.createElement('script'); keyScript.innerHTML = `(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){ (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o), m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) })(window,document,'script','https://www.google-analytics.com/analytics.js','ga'); 'ga('create', '${trackingId}', 'auto');`; - document.body.appendChild(keyScript); + this.document.body.appendChild(keyScript); // start tracking this.angulartics.startTracking(); From ce5da5723bf2096ff6f74f18972781834fdc56d5 Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Tue, 2 Feb 2021 11:54:52 +0100 Subject: [PATCH 12/20] 76634: Write tests for GoogleAnalyticsService --- .../google-analytics.service.spec.ts | 120 +++++++++++++++++- 1 file changed, 116 insertions(+), 4 deletions(-) diff --git a/src/app/statistics/google-analytics.service.spec.ts b/src/app/statistics/google-analytics.service.spec.ts index 3329024a18..5a62b02334 100644 --- a/src/app/statistics/google-analytics.service.spec.ts +++ b/src/app/statistics/google-analytics.service.spec.ts @@ -1,16 +1,128 @@ -import { TestBed } from '@angular/core/testing'; - import { GoogleAnalyticsService } from './google-analytics.service'; +import {Angulartics2GoogleAnalytics} from 'angulartics2/ga'; +import {ConfigurationDataService} from '../core/data/configuration-data.service'; +import {createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$} from '../shared/remote-data.utils'; +import {ConfigurationProperty} from '../core/shared/configuration-property.model'; describe('GoogleAnalyticsService', () => { + const trackingIdProp = 'google.analytics.key'; + const trackingIdTestValue = 'mock-tracking-id'; + const innerHTMLTestValue = 'mock-script-inner-html'; let service: GoogleAnalyticsService; + let angularticsSpy: Angulartics2GoogleAnalytics; + let configSpy: ConfigurationDataService; + let scriptElementMock: any; + let innerHTMLSpy: any; + let bodyElementSpy: HTMLBodyElement; + let documentSpy: Document; + + const createConfigSuccessSpy = (...values: string[]) => jasmine.createSpyObj('configurationDataService', { + findByPropertyName: createSuccessfulRemoteDataObject$({ + ... new ConfigurationProperty(), + name: trackingIdProp, + values: values, + }), + }); beforeEach(() => { - TestBed.configureTestingModule({}); - service = TestBed.inject(GoogleAnalyticsService); + angularticsSpy = jasmine.createSpyObj('angulartics2GoogleAnalytics', [ + 'startTracking', + ]); + + configSpy = createConfigSuccessSpy(trackingIdTestValue); + + scriptElementMock = { + set innerHTML(newVal) { /* noop */ }, + get innerHTML() { return innerHTMLTestValue; } + }; + + innerHTMLSpy = spyOnProperty(scriptElementMock, 'innerHTML', 'set'); + + bodyElementSpy = jasmine.createSpyObj('body', { + appendChild: scriptElementMock, + }); + + documentSpy = jasmine.createSpyObj('document', { + createElement: scriptElementMock, + }, { + body: bodyElementSpy, + }); + + service = new GoogleAnalyticsService(angularticsSpy, configSpy, documentSpy); }); it('should be created', () => { expect(service).toBeTruthy(); }); + + describe('addTrackingIdToPage()', () => { + it(`should request the ${trackingIdProp} property`, () => { + service.addTrackingIdToPage(); + expect(configSpy.findByPropertyName).toHaveBeenCalledTimes(1); + expect(configSpy.findByPropertyName).toHaveBeenCalledWith(trackingIdProp); + }); + + describe('when the request fails', () => { + beforeEach(() => { + configSpy = jasmine.createSpyObj('configurationDataService', { + findByPropertyName: createFailedRemoteDataObject$(), + }); + + service = new GoogleAnalyticsService(angularticsSpy, configSpy, documentSpy); + }); + + it('should NOT add a script to the body', () => { + service.addTrackingIdToPage(); + expect(bodyElementSpy.appendChild).toHaveBeenCalledTimes(0); + }); + + it('should NOT start tracking', () => { + service.addTrackingIdToPage(); + expect(angularticsSpy.startTracking).toHaveBeenCalledTimes(0); + }); + }); + + describe('when the request succeeds', () => { + describe('when the tracking id is empty', () => { + beforeEach(() => { + configSpy = createConfigSuccessSpy(); + service = new GoogleAnalyticsService(angularticsSpy, configSpy, documentSpy); + }); + + it('should NOT add a script to the body', () => { + service.addTrackingIdToPage(); + expect(bodyElementSpy.appendChild).toHaveBeenCalledTimes(0); + }); + + it('should NOT start tracking', () => { + service.addTrackingIdToPage(); + expect(angularticsSpy.startTracking).toHaveBeenCalledTimes(0); + }); + }); + + describe('when the tracking id is non-empty', () => { + it('should create a script tag whose innerHTML contains the tracking id', () => { + service.addTrackingIdToPage(); + expect(documentSpy.createElement).toHaveBeenCalledTimes(1); + expect(documentSpy.createElement).toHaveBeenCalledWith('script'); + + // sanity check + expect(documentSpy.createElement('script')).toBe(scriptElementMock); + + expect(innerHTMLSpy).toHaveBeenCalledTimes(1); + expect(innerHTMLSpy.calls.argsFor(0)[0]).toContain(trackingIdTestValue); + }); + + it('should add a script to the body', () => { + service.addTrackingIdToPage(); + expect(bodyElementSpy.appendChild).toHaveBeenCalledTimes(1); + }); + + it('should start tracking', () => { + service.addTrackingIdToPage(); + expect(angularticsSpy.startTracking).toHaveBeenCalledTimes(1); + }); + }); + }); + }); }); From 3f6cf777b628addf0ff94b8867e9a65d113ffac1 Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Tue, 2 Feb 2021 12:28:26 +0100 Subject: [PATCH 13/20] 76634: Update tests for AppComponent --- src/app/app.component.spec.ts | 90 ++++++++++++------- .../statistics/google-analytics.service.ts | 2 +- 2 files changed, 61 insertions(+), 31 deletions(-) diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index 7eec1c0ff9..233f15ccea 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -32,6 +32,7 @@ import { storeModuleConfig } from './app.reducer'; import { LocaleService } from './core/locale/locale.service'; import { authReducer } from './core/auth/auth.reducer'; import { provideMockStore } from '@ngrx/store/testing'; +import {GoogleAnalyticsService} from './statistics/google-analytics.service'; let comp: AppComponent; let fixture: ComponentFixture; @@ -48,38 +49,40 @@ describe('App component', () => { }); } + const defaultTestBedConf = { + imports: [ + CommonModule, + StoreModule.forRoot(authReducer, storeModuleConfig), + TranslateModule.forRoot({ + loader: { + provide: TranslateLoader, + useClass: TranslateLoaderMock + } + }), + ], + declarations: [AppComponent], // declare the test component + providers: [ + { provide: NativeWindowService, useValue: new NativeWindowRef() }, + { provide: MetadataService, useValue: new MetadataServiceMock() }, + { provide: Angulartics2GoogleAnalytics, useValue: new AngularticsProviderMock() }, + { provide: Angulartics2DSpace, useValue: new AngularticsProviderMock() }, + { provide: AuthService, useValue: new AuthServiceMock() }, + { provide: Router, useValue: new RouterMock() }, + { provide: ActivatedRoute, useValue: new MockActivatedRoute() }, + { provide: MenuService, useValue: menuService }, + { provide: CSSVariableService, useClass: CSSVariableServiceStub }, + { provide: HostWindowService, useValue: new HostWindowServiceStub(800) }, + { provide: LocaleService, useValue: getMockLocaleService() }, + provideMockStore({ initialState }), + AppComponent, + RouteService + ], + schemas: [CUSTOM_ELEMENTS_SCHEMA] + }; + // waitForAsync beforeEach beforeEach(waitForAsync(() => { - return TestBed.configureTestingModule({ - imports: [ - CommonModule, - StoreModule.forRoot(authReducer, storeModuleConfig), - TranslateModule.forRoot({ - loader: { - provide: TranslateLoader, - useClass: TranslateLoaderMock - } - }), - ], - declarations: [AppComponent], // declare the test component - providers: [ - { provide: NativeWindowService, useValue: new NativeWindowRef() }, - { provide: MetadataService, useValue: new MetadataServiceMock() }, - { provide: Angulartics2GoogleAnalytics, useValue: new AngularticsProviderMock() }, - { provide: Angulartics2DSpace, useValue: new AngularticsProviderMock() }, - { provide: AuthService, useValue: new AuthServiceMock() }, - { provide: Router, useValue: new RouterMock() }, - { provide: ActivatedRoute, useValue: new MockActivatedRoute() }, - { provide: MenuService, useValue: menuService }, - { provide: CSSVariableService, useClass: CSSVariableServiceStub }, - { provide: HostWindowService, useValue: new HostWindowServiceStub(800) }, - { provide: LocaleService, useValue: getMockLocaleService() }, - provideMockStore({ initialState }), - AppComponent, - RouteService - ], - schemas: [CUSTOM_ELEMENTS_SCHEMA] - }); + return TestBed.configureTestingModule(defaultTestBedConf); })); // synchronous beforeEach @@ -113,4 +116,31 @@ describe('App component', () => { }); }); + + describe('when GoogleAnalyticsService is provided', () => { + let googleAnalyticsSpy; + + beforeEach(() => { + // NOTE: Cannot override providers once components have been compiled, so TestBed needs to be reset + TestBed.resetTestingModule(); + TestBed.configureTestingModule(defaultTestBedConf); + googleAnalyticsSpy = jasmine.createSpyObj('googleAnalyticsService', [ + 'addTrackingIdToPage', + ]); + TestBed.overrideProvider(GoogleAnalyticsService, {useValue: googleAnalyticsSpy}); + fixture = TestBed.createComponent(AppComponent); + comp = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should create component', () => { + expect(comp).toBeTruthy(); + }); + + describe('the constructor', () => { + it('should call googleAnalyticsService.addTrackingIdToPage()', () => { + expect(googleAnalyticsSpy.addTrackingIdToPage).toHaveBeenCalledTimes(1); + }); + }); + }); }); diff --git a/src/app/statistics/google-analytics.service.ts b/src/app/statistics/google-analytics.service.ts index 20d0078225..2392184b35 100644 --- a/src/app/statistics/google-analytics.service.ts +++ b/src/app/statistics/google-analytics.service.ts @@ -14,7 +14,7 @@ export class GoogleAnalyticsService { @Inject(DOCUMENT) private document: Document ) { } - addTrackingIdToPage() { + addTrackingIdToPage(): void { this.configService.findByPropertyName('google.analytics.key').pipe( getFirstCompletedRemoteData(), ).subscribe((remoteData) => { From 3a486eb81f0ba4caac29e5c335f5a0d323c1577c Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Tue, 2 Feb 2021 14:40:56 +0100 Subject: [PATCH 14/20] 76634: Add documentation --- src/app/statistics/google-analytics.service.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/app/statistics/google-analytics.service.ts b/src/app/statistics/google-analytics.service.ts index 2392184b35..1a35300315 100644 --- a/src/app/statistics/google-analytics.service.ts +++ b/src/app/statistics/google-analytics.service.ts @@ -5,6 +5,10 @@ import {getFirstCompletedRemoteData} from '../core/shared/operators'; import {isEmpty} from '../shared/empty.util'; import {DOCUMENT} from '@angular/common'; +/** + * Set up Google Analytics on the client side. + * See: {@link addTrackingIdToPage}. + */ @Injectable() export class GoogleAnalyticsService { @@ -14,6 +18,12 @@ export class GoogleAnalyticsService { @Inject(DOCUMENT) private document: Document ) { } + /** + * Call this method once when Angular initializes on the client side. + * It requests a Google Analytics tracking id from the rest backend + * (property: google.analytics.key), adds the tracking snippet to the + * page and starts tracking. + */ addTrackingIdToPage(): void { this.configService.findByPropertyName('google.analytics.key').pipe( getFirstCompletedRemoteData(), From ed98e7e39d37aa1ade95451edf7553eb4c1c5ba0 Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Tue, 2 Feb 2021 14:41:36 +0100 Subject: [PATCH 15/20] 76634: Bugfixes --- src/app/statistics/google-analytics.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/statistics/google-analytics.service.ts b/src/app/statistics/google-analytics.service.ts index 1a35300315..ce4073bea5 100644 --- a/src/app/statistics/google-analytics.service.ts +++ b/src/app/statistics/google-analytics.service.ts @@ -15,7 +15,7 @@ export class GoogleAnalyticsService { constructor( private angulartics: Angulartics2GoogleAnalytics, private configService: ConfigurationDataService, - @Inject(DOCUMENT) private document: Document + @Inject(DOCUMENT) private document: any, ) { } /** @@ -42,7 +42,7 @@ export class GoogleAnalyticsService { (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o), m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) })(window,document,'script','https://www.google-analytics.com/analytics.js','ga'); - 'ga('create', '${trackingId}', 'auto');`; + ga('create', '${trackingId}', 'auto');`; this.document.body.appendChild(keyScript); // start tracking From dde149aceacdd2b658c4632f72098a5b0b7120cd Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Thu, 4 Feb 2021 12:11:18 +0100 Subject: [PATCH 16/20] 75832: [Issue 962]: Fix unnecessary calls creating/deleting groups - feedback --- .../members-list/members-list.component.ts | 163 ++++++++++-------- src/assets/i18n/en.json5 | 2 + 2 files changed, 96 insertions(+), 69 deletions(-) diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts index 7a09f6ac8b..d01d117a86 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts @@ -2,8 +2,14 @@ import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { FormBuilder } from '@angular/forms'; import { Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; -import { Observable, of as observableOf, Subscription, BehaviorSubject, ObservedValueOf, combineLatest as observableCombineLatest} from 'rxjs'; -import { map, mergeMap, take } from 'rxjs/operators'; +import { + Observable, + of as observableOf, + Subscription, + BehaviorSubject, + combineLatest as observableCombineLatest, ObservedValueOf, +} from 'rxjs'; +import { map, mergeMap, switchMap, take } from 'rxjs/operators'; import {buildPaginatedList, PaginatedList} from '../../../../../core/data/paginated-list.model'; import { RemoteData } from '../../../../../core/data/remote-data'; import { EPersonDataService } from '../../../../../core/eperson/eperson-data.service'; @@ -13,7 +19,7 @@ import { Group } from '../../../../../core/eperson/models/group.model'; import { getRemoteDataPayload, getFirstSucceededRemoteData, - getFirstCompletedRemoteData + getFirstCompletedRemoteData, getAllCompletedRemoteData } from '../../../../../core/shared/operators'; import { NotificationsService } from '../../../../../shared/notifications/notifications.service'; import { PaginationComponentOptions } from '../../../../../shared/pagination/pagination-component-options.model'; @@ -23,9 +29,7 @@ import {EpersonDtoModel} from '../../../../../core/eperson/models/eperson-dto.mo * Keys to keep track of specific subscriptions */ enum SubKey { - Members, ActiveGroup, - SearchResults, MembersDTO, SearchResultsDTO, } @@ -45,12 +49,10 @@ export class MembersListComponent implements OnInit, OnDestroy { /** * EPeople being displayed in search result, initially all members, after search result of search */ - searchResults$: BehaviorSubject>> = new BehaviorSubject(undefined); ePeopleSearchDtos: BehaviorSubject> = new BehaviorSubject>(undefined); /** * List of EPeople members of currently active group being edited */ - members$: BehaviorSubject>> = new BehaviorSubject(undefined); ePeopleMembersOfGroupDtos: BehaviorSubject> = new BehaviorSubject>(undefined); /** @@ -135,17 +137,58 @@ export class MembersListComponent implements OnInit, OnDestroy { * @private */ private retrieveMembers(page: number) { - this.unsubFrom(SubKey.Members); - this.subs.set( - SubKey.Members, - this.ePersonDataService.findAllByHref(this.groupBeingEdited._links.epersons.href, { - currentPage: page, - elementsPerPage: this.config.pageSize - } - ).subscribe((rd: RemoteData>) => { - this.members$.next(rd); - if (rd.payload !== undefined) { - this.setEpersonDTOsFromResult(rd, this.ePeopleMembersOfGroupDtos, SubKey.MembersDTO); + this.unsubFrom(SubKey.MembersDTO); + this.subs.set(SubKey.MembersDTO, this.ePersonDataService.findAllByHref(this.groupBeingEdited._links.epersons.href, { + currentPage: page, + elementsPerPage: this.config.pageSize + }).pipe( + getAllCompletedRemoteData(), + map((rd: RemoteData) => { + if (rd.hasFailed) { + this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', {cause: rd.errorMessage})); + } else { + return rd; + } + }), + switchMap((epersonListRD: RemoteData>) => { + const dtos$ = observableCombineLatest(...epersonListRD.payload.page.map((member: EPerson) => { + const dto$: Observable = observableCombineLatest( + this.isMemberOfGroup(member), (isMember: ObservedValueOf>) => { + const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel(); + epersonDtoModel.eperson = member; + epersonDtoModel.memberOfGroup = isMember; + return epersonDtoModel; + }); + return dto$; + })); + return dtos$.pipe(map((dtos: EpersonDtoModel[]) => { + return buildPaginatedList(epersonListRD.payload.pageInfo, dtos); + })) + })) + .subscribe((paginatedListOfDTOs: PaginatedList) => { + this.ePeopleMembersOfGroupDtos.next(paginatedListOfDTOs); + })); + } + + /** + * Whether or not the given ePerson is a member of the group currently being edited + * @param possibleMember EPerson that is a possible member (being tested) of the group currently being edited + */ + isMemberOfGroup(possibleMember: EPerson): Observable { + return this.groupDataService.getActiveGroup().pipe(take(1), + mergeMap((group: Group) => { + if (group != null) { + return this.ePersonDataService.findAllByHref(group._links.epersons.href, { + currentPage: 1, + elementsPerPage: 9999 + }, false) + .pipe( + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + map((listEPeopleInGroup: PaginatedList) => listEPeopleInGroup.page.filter((ePersonInList: EPerson) => ePersonInList.id === possibleMember.id)), + map((epeople: EPerson[]) => epeople.length > 0)); + } else { + return observableOf(false); } })); } @@ -173,6 +216,7 @@ export class MembersListComponent implements OnInit, OnDestroy { if (activeGroup != null) { const response = this.groupDataService.deleteMemberFromGroup(activeGroup, ePerson.eperson); this.showNotifications('deleteMember', response, ePerson.eperson.name, activeGroup); + this.search({ scope: this.currentSearchScope, query: this.currentSearchQuery }); } else { this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup')); } @@ -195,29 +239,6 @@ export class MembersListComponent implements OnInit, OnDestroy { }); } - /** - * Whether or not the given ePerson is a member of the group currently being edited - * @param possibleMember EPerson that is a possible member (being tested) of the group currently being edited - */ - isMemberOfGroup(possibleMember: EPerson): Observable { - return this.groupDataService.getActiveGroup().pipe(take(1), - mergeMap((group: Group) => { - if (group != null) { - return this.ePersonDataService.findAllByHref(group._links.epersons.href, { - currentPage: 1, - elementsPerPage: 9999 - }) - .pipe( - getFirstSucceededRemoteData(), - getRemoteDataPayload(), - map((listEPeopleInGroup: PaginatedList) => listEPeopleInGroup.page.filter((ePersonInList: EPerson) => ePersonInList.id === possibleMember.id)), - map((epeople: EPerson[]) => epeople.length > 0)); - } else { - return observableOf(false); - } - })); - } - /** * Search in the EPeople by name, email or metadata * @param data Contains scope and query param @@ -237,34 +258,38 @@ export class MembersListComponent implements OnInit, OnDestroy { } this.searchDone = true; - this.unsubFrom(SubKey.SearchResults); - this.subs.set(SubKey.SearchResults, this.ePersonDataService.searchByScope(this.currentSearchScope, this.currentSearchQuery, { - currentPage: this.configSearch.currentPage, - elementsPerPage: this.configSearch.pageSize - }).subscribe((rd: RemoteData>) => { - this.searchResults$.next(rd); - if (rd.payload !== undefined) { - this.setEpersonDTOsFromResult(rd, this.ePeopleSearchDtos, SubKey.SearchResultsDTO); - } - })); - } - - private setEpersonDTOsFromResult(rd: RemoteData>, addTo: BehaviorSubject>, subkey) { - this.unsubFrom(subkey); - const dtos$ = observableCombineLatest(...rd.payload.page.map((member: EPerson) => { - const dto$: Observable = observableCombineLatest( - this.isMemberOfGroup(member), (isMember: ObservedValueOf>) => { - const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel(); - epersonDtoModel.eperson = member; - epersonDtoModel.memberOfGroup = isMember; - return epersonDtoModel; - }); - return dto$; - })); - this.subs.set(subkey,dtos$.pipe(map((dtos: EpersonDtoModel[]) => { - const paginatedListOfDTOs = buildPaginatedList(rd.payload.pageInfo, dtos); - addTo.next(paginatedListOfDTOs); - })).subscribe()); + this.unsubFrom(SubKey.SearchResultsDTO); + this.subs.set(SubKey.SearchResultsDTO, + this.ePersonDataService.searchByScope(this.currentSearchScope, this.currentSearchQuery, { + currentPage: this.configSearch.currentPage, + elementsPerPage: this.configSearch.pageSize + }, false).pipe( + getAllCompletedRemoteData(), + map((rd: RemoteData) => { + if (rd.hasFailed) { + this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', {cause: rd.errorMessage})); + } else { + return rd; + } + }), + switchMap((epersonListRD: RemoteData>) => { + const dtos$ = observableCombineLatest(...epersonListRD.payload.page.map((member: EPerson) => { + const dto$: Observable = observableCombineLatest( + this.isMemberOfGroup(member), (isMember: ObservedValueOf>) => { + const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel(); + epersonDtoModel.eperson = member; + epersonDtoModel.memberOfGroup = isMember; + return epersonDtoModel; + }); + return dto$; + })); + return dtos$.pipe(map((dtos: EpersonDtoModel[]) => { + return buildPaginatedList(epersonListRD.payload.pageInfo, dtos); + })) + })) + .subscribe((paginatedListOfDTOs: PaginatedList) => { + this.ePeopleSearchDtos.next(paginatedListOfDTOs); + })); } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 5ddde2a307..cb1453beed 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -402,6 +402,8 @@ "admin.access-control.groups.form.members-list.no-items": "No EPeople found in that search", + "admin.access-control.groups.form.subgroups-list.notification.failure": "Something went wrong: \"{{cause}}\"", + "admin.access-control.groups.form.subgroups-list.head": "Groups", "admin.access-control.groups.form.subgroups-list.search.head": "Add Subgroup", From 569f35437068b5bf9d93db93e4b4ced00a4df3d3 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Mon, 8 Feb 2021 15:00:44 +0100 Subject: [PATCH 17/20] 75832: [Issue 962]: Members total in Group registry fixed with GroupDTO --- .../group-form/members-list/members-list.component.ts | 4 ++-- .../group-registry/groups-registry.component.ts | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts index d01d117a86..f1d25725d6 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts @@ -163,7 +163,7 @@ export class MembersListComponent implements OnInit, OnDestroy { })); return dtos$.pipe(map((dtos: EpersonDtoModel[]) => { return buildPaginatedList(epersonListRD.payload.pageInfo, dtos); - })) + })); })) .subscribe((paginatedListOfDTOs: PaginatedList) => { this.ePeopleMembersOfGroupDtos.next(paginatedListOfDTOs); @@ -285,7 +285,7 @@ export class MembersListComponent implements OnInit, OnDestroy { })); return dtos$.pipe(map((dtos: EpersonDtoModel[]) => { return buildPaginatedList(epersonListRD.payload.pageInfo, dtos); - })) + })); })) .subscribe((paginatedListOfDTOs: PaginatedList) => { this.ePeopleSearchDtos.next(paginatedListOfDTOs); diff --git a/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts b/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts index 7a8e670267..06fe2d56be 100644 --- a/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts @@ -111,12 +111,17 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { return observableCombineLatest(groups.page.map((group: Group) => { return observableCombineLatest([ this.authorizationService.isAuthorized(FeatureID.CanDelete, hasValue(group) ? group.self : undefined), - this.hasLinkedDSO(group) + this.hasLinkedDSO(group), + this.getSubgroups(group), + this.getMembers(group) ]).pipe( - map(([isAuthorized, hasLinkedDSO]: boolean[]) => { + map(([isAuthorized, hasLinkedDSO, subgroups, members]: + [boolean, boolean, RemoteData>, RemoteData>]) => { const groupDtoModel: GroupDtoModel = new GroupDtoModel(); groupDtoModel.ableToDelete = isAuthorized && !hasLinkedDSO; groupDtoModel.group = group; + groupDtoModel.subgroups = subgroups.payload; + groupDtoModel.epersons = members.payload; return groupDtoModel; } ) From 6424654e6e6e610920228c6965a6c92b232b2642 Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Tue, 9 Feb 2021 14:49:26 +0100 Subject: [PATCH 18/20] Fix LGTM Alert for PR #1014 --- src/main.browser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.browser.ts b/src/main.browser.ts index 6e3a7ad602..1f399b858a 100644 --- a/src/main.browser.ts +++ b/src/main.browser.ts @@ -6,7 +6,7 @@ import { platformBrowserDynamic } from '@angular/platform-browser-dynamic'; import { bootloader } from '@angularclass/bootloader'; import { load as loadWebFont } from 'webfontloader'; -import { hasValue, isNotEmpty } from './app/shared/empty.util'; +import { hasValue } from './app/shared/empty.util'; import { BrowserAppModule } from './modules/app/browser-app.module'; From dd241c86b2e1d4b21c57a8127efcbdcfc11964f5 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Thu, 11 Feb 2021 12:36:32 +0100 Subject: [PATCH 19/20] 75832: [Issue 962]: Fix no change in search result if no results & fix 404 messages at delete in registry --- .../groups-registry.component.ts | 80 +++++++++---------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts b/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts index 06fe2d56be..305da75eeb 100644 --- a/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/groups-registry.component.ts @@ -26,7 +26,7 @@ import { DSpaceObject } from '../../../core/shared/dspace-object.model'; import { getAllSucceededRemoteDataPayload, getFirstCompletedRemoteData, - getAllSucceededRemoteData + getFirstSucceededRemoteData } from '../../../core/shared/operators'; import { PageInfo } from '../../../core/shared/page-info.model'; import { hasValue } from '../../../shared/empty.util'; @@ -55,15 +55,12 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { currentPage: 1 }); - /** - * A list of all the current Groups within the repository or the result of the search - */ - groups$: BehaviorSubject>> = new BehaviorSubject>>({} as any); /** * A BehaviorSubject with the list of GroupDtoModel objects made from the Groups in the repository or * as the result of the search */ groupsDto$: BehaviorSubject> = new BehaviorSubject>({} as any); + deletedGroupsIds: string[] = []; /** * An observable for the pageInfo, needed to pass to the pagination component @@ -104,35 +101,6 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { ngOnInit() { this.search({ query: this.currentSearchQuery }); - - this.subs.push(this.groups$.pipe( - getAllSucceededRemoteDataPayload(), - switchMap((groups: PaginatedList) => { - return observableCombineLatest(groups.page.map((group: Group) => { - return observableCombineLatest([ - this.authorizationService.isAuthorized(FeatureID.CanDelete, hasValue(group) ? group.self : undefined), - this.hasLinkedDSO(group), - this.getSubgroups(group), - this.getMembers(group) - ]).pipe( - map(([isAuthorized, hasLinkedDSO, subgroups, members]: - [boolean, boolean, RemoteData>, RemoteData>]) => { - const groupDtoModel: GroupDtoModel = new GroupDtoModel(); - groupDtoModel.ableToDelete = isAuthorized && !hasLinkedDSO; - groupDtoModel.group = group; - groupDtoModel.subgroups = subgroups.payload; - groupDtoModel.epersons = members.payload; - return groupDtoModel; - } - ) - ); - })).pipe(map((dtos: GroupDtoModel[]) => { - return buildPaginatedList(groups.pageInfo, dtos); - })); - })).subscribe((value: PaginatedList) => { - this.groupsDto$.next(value); - this.pageInfoState$.next(value.pageInfo); - })); } /** @@ -159,14 +127,42 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { this.searchSub.unsubscribe(); this.subs = this.subs.filter((sub: Subscription) => sub !== this.searchSub); } + this.searchSub = this.groupService.searchGroups(this.currentSearchQuery.trim(), { currentPage: this.config.currentPage, elementsPerPage: this.config.pageSize }).pipe( - getAllSucceededRemoteData() - ).subscribe((groupsRD: RemoteData>) => { - this.groups$.next(groupsRD); - this.pageInfoState$.next(groupsRD.payload.pageInfo); + getAllSucceededRemoteDataPayload(), + switchMap((groups: PaginatedList) => { + if (groups.page.length === 0) { + return observableOf(buildPaginatedList(groups.pageInfo, [])); + } + return observableCombineLatest(groups.page.map((group: Group) => { + if (!this.deletedGroupsIds.includes(group.id)) { + return observableCombineLatest([ + this.authorizationService.isAuthorized(FeatureID.CanDelete, hasValue(group) ? group.self : undefined), + this.hasLinkedDSO(group), + this.getSubgroups(group), + this.getMembers(group) + ]).pipe( + map(([isAuthorized, hasLinkedDSO, subgroups, members]: + [boolean, boolean, RemoteData>, RemoteData>]) => { + const groupDtoModel: GroupDtoModel = new GroupDtoModel(); + groupDtoModel.ableToDelete = isAuthorized && !hasLinkedDSO; + groupDtoModel.group = group; + groupDtoModel.subgroups = subgroups.payload; + groupDtoModel.epersons = members.payload; + return groupDtoModel; + } + ) + ); + } + })).pipe(map((dtos: GroupDtoModel[]) => { + return buildPaginatedList(groups.pageInfo, dtos); + })); + })).subscribe((value: PaginatedList) => { + this.groupsDto$.next(value); + this.pageInfoState$.next(value.pageInfo); }); this.subs.push(this.searchSub); } @@ -179,6 +175,7 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { this.groupService.delete(group.group.id).pipe(getFirstCompletedRemoteData()) .subscribe((rd: RemoteData) => { if (rd.hasSucceeded) { + this.deletedGroupsIds = [...this.deletedGroupsIds, group.group.id]; this.notificationsService.success(this.translateService.get(this.messagePrefix + 'notification.deleted.success', { name: group.group.name })); this.reset(); } else { @@ -199,14 +196,14 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { ).subscribe((href: string) => { this.requestService.setStaleByHrefSubstring(href); }); -} + } /** * Get the members (epersons embedded value of a group) * @param group */ getMembers(group: Group): Observable>> { - return this.ePersonDataService.findAllByHref(group._links.epersons.href); + return this.ePersonDataService.findAllByHref(group._links.epersons.href).pipe(getFirstSucceededRemoteData()); } /** @@ -214,7 +211,7 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { * @param group */ getSubgroups(group: Group): Observable>> { - return this.groupService.findAllByHref(group._links.subgroups.href); + return this.groupService.findAllByHref(group._links.subgroups.href).pipe(getFirstSucceededRemoteData()); } /** @@ -223,6 +220,7 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { */ hasLinkedDSO(group: Group): Observable { return this.dSpaceObjectDataService.findByHref(group._links.object.href).pipe( + getFirstSucceededRemoteData(), map((rd: RemoteData) => hasValue(rd) && hasValue(rd.payload)), catchError(() => observableOf(false)), ); From 216e09f889cec0bf3e45fcc44050444322ad23c5 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 11 Feb 2021 17:04:38 +0100 Subject: [PATCH 20/20] 75316: Feedback 2021-02-11 - import fixes --- package.json | 2 +- .../dynamic-lookup-relation-modal.component.ts | 10 ++-------- src/app/submission/submission.module.ts | 6 +++++- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index ef5c3c5e68..d92afb4a40 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dspace-angular", - "version": "1.22.10", + "version": "0.0.0", "scripts": { "ng": "ng", "config:dev": "ts-node --project ./tsconfig.ts-node.json scripts/set-env.ts --dev", diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts index 672ab852ee..83b756157c 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts @@ -1,5 +1,5 @@ import { Component, EventEmitter, NgZone, OnDestroy, OnInit, Output } from '@angular/core'; -import { combineLatest as observableCombineLatest, Observable, Subscription, zip } from 'rxjs'; +import { combineLatest as observableCombineLatest, Observable, Subscription } from 'rxjs'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; import { hasValue, isNotEmpty } from '../../../../empty.util'; import { map, skip, switchMap, take } from 'rxjs/operators'; @@ -11,11 +11,7 @@ import { ListableObject } from '../../../../object-collection/shared/listable-ob import { RelationshipOptions } from '../../models/relationship-options.model'; import { SearchResult } from '../../../../search/search-result.model'; import { Item } from '../../../../../core/shared/item.model'; -import { - getAllSucceededRemoteData, - getAllSucceededRemoteDataPayload, - getRemoteDataPayload -} from '../../../../../core/shared/operators'; +import { getAllSucceededRemoteDataPayload } from '../../../../../core/shared/operators'; import { AddRelationshipAction, RemoveRelationshipAction, UpdateRelationshipNameVariantAction } from './relationship.actions'; import { RelationshipService } from '../../../../../core/data/relationship.service'; import { RelationshipTypeService } from '../../../../../core/data/relationship-type.service'; @@ -23,8 +19,6 @@ import { Store } from '@ngrx/store'; import { AppState } from '../../../../../app.reducer'; import { Context } from '../../../../../core/shared/context.model'; import { LookupRelationService } from '../../../../../core/data/lookup-relation.service'; -import { RemoteData } from '../../../../../core/data/remote-data'; -import { PaginatedList } from '../../../../../core/data/paginated-list.model'; import { ExternalSource } from '../../../../../core/shared/external-source.model'; import { ExternalSourceService } from '../../../../../core/data/external-source.service'; import { Router } from '@angular/router'; diff --git a/src/app/submission/submission.module.ts b/src/app/submission/submission.module.ts index 951ddae4ac..32c11d6673 100644 --- a/src/app/submission/submission.module.ts +++ b/src/app/submission/submission.module.ts @@ -32,6 +32,8 @@ import { SubmissionImportExternalSearchbarComponent } from './import-external/im import { SubmissionImportExternalPreviewComponent } from './import-external/import-external-preview/submission-import-external-preview.component'; import { SubmissionImportExternalCollectionComponent } from './import-external/import-external-collection/submission-import-external-collection.component'; import { SubmissionSectionCcLicensesComponent } from './sections/cc-license/submission-section-cc-licenses.component'; +import { JournalEntitiesModule } from '../entity-groups/journal-entities/journal-entities.module'; +import { ResearchEntitiesModule } from '../entity-groups/research-entities/research-entities.module'; @NgModule({ imports: [ @@ -39,7 +41,9 @@ import { SubmissionSectionCcLicensesComponent } from './sections/cc-license/subm CoreModule.forRoot(), SharedModule, StoreModule.forFeature('submission', submissionReducers, storeModuleConfig as StoreConfig), - EffectsModule.forFeature(submissionEffects) + EffectsModule.forFeature(submissionEffects), + JournalEntitiesModule.withEntryComponents(), + ResearchEntitiesModule.withEntryComponents(), ], declarations: [ SubmissionSectionUploadAccessConditionsComponent,