From 9e2a682a516d4804ed545a32c0bfba12d0e6e094 Mon Sep 17 00:00:00 2001 From: nikunj59 Date: Fri, 24 Jun 2022 19:26:30 +0530 Subject: [PATCH 01/57] CST-6153 changes for current password field introduce --- ...ofile-page-security-form.component.spec.ts | 13 +++++++++++ .../profile-page-security-form.component.ts | 21 +++++++++++++----- .../profile-page/profile-page.component.html | 1 + .../profile-page.component.spec.ts | 9 ++++++-- .../profile-page/profile-page.component.ts | 22 ++++++++++++++----- src/assets/i18n/en.json5 | 2 ++ 6 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.spec.ts b/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.spec.ts index 213a83b86e..ba758b1203 100644 --- a/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.spec.ts +++ b/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.spec.ts @@ -74,6 +74,19 @@ describe('ProfilePageSecurityFormComponent', () => { expect(component.passwordValue.emit).toHaveBeenCalledWith('new-password'); })); + + it('should emit the value on password change with current password for profile-page', fakeAsync(() => { + spyOn(component.passwordValue, 'emit'); + spyOn(component.currentPasswordValue, 'emit'); + component.FORM_PREFIX = 'profile.security.form.'; + component.ngOnInit(); + component.formGroup.patchValue({password: 'new-password'}); + component.formGroup.patchValue({'current-password': 'current-password'}); + tick(300); + + expect(component.passwordValue.emit).toHaveBeenCalledWith('new-password'); + expect(component.currentPasswordValue.emit).toHaveBeenCalledWith('current-password'); + })) }); }); }); diff --git a/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.ts b/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.ts index 4f310204e3..ab39ca1929 100644 --- a/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.ts +++ b/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.ts @@ -27,6 +27,10 @@ export class ProfilePageSecurityFormComponent implements OnInit { * Emits the value of the password */ @Output() passwordValue = new EventEmitter(); + /** + * Emits the value of the current-password + */ + @Output() currentPasswordValue = new EventEmitter(); /** * The form's input models @@ -69,6 +73,14 @@ export class ProfilePageSecurityFormComponent implements OnInit { } ngOnInit(): void { + if (this.FORM_PREFIX === 'profile.security.form.') { + this.formModel.unshift(new DynamicInputModel({ + id: 'current-password', + name: 'current-password', + inputType: 'password', + required: true + })); + } if (this.passwordCanBeEmpty) { this.formGroup = this.formService.createFormGroup(this.formModel, {validators: [this.checkPasswordsEqual, this.checkPasswordLength]}); @@ -85,11 +97,7 @@ export class ProfilePageSecurityFormComponent implements OnInit { this.subs.push(this.formGroup.statusChanges.pipe( debounceTime(300), map((status: string) => { - if (status !== 'VALID') { - return true; - } else { - return false; - } + return status !== 'VALID'; })).subscribe((status) => this.isInvalid.emit(status)) ); @@ -97,6 +105,9 @@ export class ProfilePageSecurityFormComponent implements OnInit { debounceTime(300), ).subscribe((valueChange) => { this.passwordValue.emit(valueChange.password); + if (this.FORM_PREFIX === 'profile.security.form.') { + this.currentPasswordValue.emit(valueChange['current-password']); + } })); } diff --git a/src/app/profile-page/profile-page.component.html b/src/app/profile-page/profile-page.component.html index 6e22f73a75..b6f53b08bf 100644 --- a/src/app/profile-page/profile-page.component.html +++ b/src/app/profile-page/profile-page.component.html @@ -24,6 +24,7 @@ [FORM_PREFIX]="'profile.security.form.'" (isInvalid)="setInvalid($event)" (passwordValue)="setPasswordValue($event)" + (currentPasswordValue)="setCurrentPasswordValue($event)" > diff --git a/src/app/profile-page/profile-page.component.spec.ts b/src/app/profile-page/profile-page.component.spec.ts index 6893ac2437..71537e9c32 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -180,7 +180,7 @@ describe('ProfilePageComponent', () => { beforeEach(() => { component.setPasswordValue(''); - + component.setCurrentPasswordValue('current-password'); result = component.updateSecurity(); }); @@ -199,6 +199,7 @@ describe('ProfilePageComponent', () => { beforeEach(() => { component.setPasswordValue('test'); component.setInvalid(true); + component.setCurrentPasswordValue('current-password'); result = component.updateSecurity(); }); @@ -215,8 +216,12 @@ describe('ProfilePageComponent', () => { beforeEach(() => { component.setPasswordValue('testest'); component.setInvalid(false); + component.setCurrentPasswordValue('current-password'); - operations = [{ op: 'add', path: '/password', value: 'testest' }]; + operations = [ + { op: 'add', path: '/password', value: 'testest' }, + { op: 'add', path: '/challenge', value: 'current-password' } + ]; result = component.updateSecurity(); }); diff --git a/src/app/profile-page/profile-page.component.ts b/src/app/profile-page/profile-page.component.ts index 5629a1ae18..15811ee77f 100644 --- a/src/app/profile-page/profile-page.component.ts +++ b/src/app/profile-page/profile-page.component.ts @@ -67,6 +67,10 @@ export class ProfilePageComponent implements OnInit { * The password filled in, in the security form */ private password: string; + /** + * The current-password filled in, in the security form + */ + private currentPassword: string; /** * The authenticated user @@ -138,15 +142,15 @@ export class ProfilePageComponent implements OnInit { */ updateSecurity() { const passEntered = isNotEmpty(this.password); - if (this.invalidSecurity) { this.notificationsService.error(this.translate.instant(this.PASSWORD_NOTIFICATIONS_PREFIX + 'error.general')); } if (!this.invalidSecurity && passEntered) { - const operation = {op: 'add', path: '/password', value: this.password} as Operation; - this.epersonService.patch(this.currentUser, [operation]).pipe( - getFirstCompletedRemoteData() - ).subscribe((response: RemoteData) => { + const operations = [ + { op: 'add', path: '/password', value: this.password }, + { op: 'add', path: '/challenge', value: this.currentPassword } + ] as Operation[]; + this.epersonService.patch(this.currentUser, operations).pipe(getFirstCompletedRemoteData()).subscribe((response: RemoteData) => { if (response.hasSucceeded) { this.notificationsService.success( this.translate.instant(this.PASSWORD_NOTIFICATIONS_PREFIX + 'success.title'), @@ -170,6 +174,14 @@ export class ProfilePageComponent implements OnInit { this.password = $event; } + /** + * Set the current-password value based on the value emitted from the security form + * @param $event + */ + setCurrentPasswordValue($event: string) { + this.currentPassword = $event; + } + /** * Submit of the security form that triggers the updateProfile method */ diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 3d5f15b4f2..860db5aac6 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3046,6 +3046,8 @@ "profile.security.form.label.passwordrepeat": "Retype to confirm", + "profile.security.form.label.current-password": "Current password", + "profile.security.form.notifications.success.content": "Your changes to the password were saved.", "profile.security.form.notifications.success.title": "Password saved", From 36560e0e6500d646c2ec1608f7b6a23800c22ce6 Mon Sep 17 00:00:00 2001 From: nikunj59 Date: Mon, 27 Jun 2022 20:24:46 +0530 Subject: [PATCH 02/57] CST-6153 added test case --- .../profile-page.component.spec.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/app/profile-page/profile-page.component.spec.ts b/src/app/profile-page/profile-page.component.spec.ts index 71537e9c32..31332a5fd7 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -233,6 +233,29 @@ describe('ProfilePageComponent', () => { expect(epersonService.patch).toHaveBeenCalledWith(user, operations); }); }); + + describe('when password is filled in, and is valid but return 403', () => { + let result; + let operations; + + it('should return call epersonService.patch', (done) => { + epersonService.patch.and.returnValue(observableOf(Object.assign(new RestResponse(false, 403, 'Error')))); + component.setPasswordValue('testest'); + component.setInvalid(false); + component.setCurrentPasswordValue('current-password'); + operations = [ + { op: 'add', path: '/password', value: 'testest' }, + { op: 'add', path: '/challenge', value: 'current-password' } + ]; + result = component.updateSecurity(); + epersonService.patch(user, operations).subscribe((response) => { + expect(response.statusCode).toEqual(403); + done(); + }); + expect(epersonService.patch).toHaveBeenCalledWith(user, operations); + expect(result).toEqual(true); + }); + }); }); describe('canChangePassword$', () => { From 10bbb01a442d03725b5825b9372a834ba809bd25 Mon Sep 17 00:00:00 2001 From: nikunj59 Date: Thu, 30 Jun 2022 14:24:51 +0530 Subject: [PATCH 03/57] CST-6153 added error msg for required field of security form --- .../profile-page-security-form.component.spec.ts | 2 +- src/assets/i18n/en.json5 | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.spec.ts b/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.spec.ts index ba758b1203..88f50e3dea 100644 --- a/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.spec.ts +++ b/src/app/profile-page/profile-page-security-form/profile-page-security-form.component.spec.ts @@ -86,7 +86,7 @@ describe('ProfilePageSecurityFormComponent', () => { expect(component.passwordValue.emit).toHaveBeenCalledWith('new-password'); expect(component.currentPasswordValue.emit).toHaveBeenCalledWith('current-password'); - })) + })); }); }); }); diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 1abcd551a6..4a6fb33e25 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3060,6 +3060,8 @@ "profile.security.form.notifications.error.not-same": "The provided passwords are not the same.", + "profile.security.form.notifications.error.general": "Please fill required fields of security form.", + "profile.title": "Update Profile", "profile.card.researcher": "Researcher Profile", From 2d7b5768bf438cc4598599453dedfd8cca779e5e Mon Sep 17 00:00:00 2001 From: Nikunj Sharma Date: Tue, 9 Aug 2022 13:25:03 +0530 Subject: [PATCH 04/57] CST-6153 changes for challange sign update --- src/app/profile-page/profile-page.component.spec.ts | 6 ++---- src/app/profile-page/profile-page.component.ts | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/app/profile-page/profile-page.component.spec.ts b/src/app/profile-page/profile-page.component.spec.ts index 31332a5fd7..66351144d5 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -219,8 +219,7 @@ describe('ProfilePageComponent', () => { component.setCurrentPasswordValue('current-password'); operations = [ - { op: 'add', path: '/password', value: 'testest' }, - { op: 'add', path: '/challenge', value: 'current-password' } + { "op": "add", "path": "/password", "value": { "password": "testest", "challenge": "current-password" } } ]; result = component.updateSecurity(); }); @@ -244,8 +243,7 @@ describe('ProfilePageComponent', () => { component.setInvalid(false); component.setCurrentPasswordValue('current-password'); operations = [ - { op: 'add', path: '/password', value: 'testest' }, - { op: 'add', path: '/challenge', value: 'current-password' } + { "op": "add", "path": "/password", "value": {"password": "testest", "challenge": "current-password" }} ]; result = component.updateSecurity(); epersonService.patch(user, operations).subscribe((response) => { diff --git a/src/app/profile-page/profile-page.component.ts b/src/app/profile-page/profile-page.component.ts index 15811ee77f..c171502d33 100644 --- a/src/app/profile-page/profile-page.component.ts +++ b/src/app/profile-page/profile-page.component.ts @@ -147,8 +147,7 @@ export class ProfilePageComponent implements OnInit { } if (!this.invalidSecurity && passEntered) { const operations = [ - { op: 'add', path: '/password', value: this.password }, - { op: 'add', path: '/challenge', value: this.currentPassword } + { "op": "add", "path": "/password", "value": { "password": this.password, "challenge": this.currentPassword } } ] as Operation[]; this.epersonService.patch(this.currentUser, operations).pipe(getFirstCompletedRemoteData()).subscribe((response: RemoteData) => { if (response.hasSucceeded) { From 66380857c9cd2ff45c694d1699b626be360e6fdd Mon Sep 17 00:00:00 2001 From: Nikunj Sharma Date: Tue, 6 Sep 2022 17:02:25 +0530 Subject: [PATCH 05/57] CST-6153 updated lint error --- src/app/profile-page/profile-page.component.spec.ts | 4 ++-- src/app/profile-page/profile-page.component.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/profile-page/profile-page.component.spec.ts b/src/app/profile-page/profile-page.component.spec.ts index 66351144d5..c1bfe8e6bb 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -219,7 +219,7 @@ describe('ProfilePageComponent', () => { component.setCurrentPasswordValue('current-password'); operations = [ - { "op": "add", "path": "/password", "value": { "password": "testest", "challenge": "current-password" } } + { 'op': 'add', 'path': '/password', 'value': { 'password': 'testest', 'challenge': 'current-password' } } ]; result = component.updateSecurity(); }); @@ -243,7 +243,7 @@ describe('ProfilePageComponent', () => { component.setInvalid(false); component.setCurrentPasswordValue('current-password'); operations = [ - { "op": "add", "path": "/password", "value": {"password": "testest", "challenge": "current-password" }} + { 'op': 'add', 'path': '/password', 'value': {'password': 'testest', 'challenge': 'current-password' }} ]; result = component.updateSecurity(); epersonService.patch(user, operations).subscribe((response) => { diff --git a/src/app/profile-page/profile-page.component.ts b/src/app/profile-page/profile-page.component.ts index c171502d33..5432a0303c 100644 --- a/src/app/profile-page/profile-page.component.ts +++ b/src/app/profile-page/profile-page.component.ts @@ -147,7 +147,7 @@ export class ProfilePageComponent implements OnInit { } if (!this.invalidSecurity && passEntered) { const operations = [ - { "op": "add", "path": "/password", "value": { "password": this.password, "challenge": this.currentPassword } } + { 'op': 'add', 'path': '/password', 'value': { 'password': this.password, 'challenge': this.currentPassword } } ] as Operation[]; this.epersonService.patch(this.currentUser, operations).pipe(getFirstCompletedRemoteData()).subscribe((response: RemoteData) => { if (response.hasSucceeded) { From d40f163c4916c4fdad7c859980ef195f6976bc97 Mon Sep 17 00:00:00 2001 From: Yury Bondarenko Date: Thu, 8 Sep 2022 13:25:41 +0200 Subject: [PATCH 06/57] 94273: Implement dependencies between requests When an object is invalidated, its dependent requests are invalidated as well --- src/app/core/cache/object-cache.actions.ts | 48 ++++++- .../core/cache/object-cache.reducer.spec.ts | 28 +++- src/app/core/cache/object-cache.reducer.ts | 66 ++++++++- .../core/cache/object-cache.service.spec.ts | 133 +++++++++++++++++- src/app/core/cache/object-cache.service.ts | 107 ++++++++++++-- src/app/core/data/data.service.spec.ts | 58 +++++++- src/app/core/data/data.service.ts | 45 +++++- .../core/eperson/group-data.service.spec.ts | 29 ++-- .../shared/mocks/object-cache.service.mock.ts | 2 + 9 files changed, 468 insertions(+), 48 deletions(-) diff --git a/src/app/core/cache/object-cache.actions.ts b/src/app/core/cache/object-cache.actions.ts index 88b4730b3f..8c83f6104e 100644 --- a/src/app/core/cache/object-cache.actions.ts +++ b/src/app/core/cache/object-cache.actions.ts @@ -13,7 +13,9 @@ export const ObjectCacheActionTypes = { REMOVE: type('dspace/core/cache/object/REMOVE'), RESET_TIMESTAMPS: type('dspace/core/cache/object/RESET_TIMESTAMPS'), ADD_PATCH: type('dspace/core/cache/object/ADD_PATCH'), - APPLY_PATCH: type('dspace/core/cache/object/APPLY_PATCH') + APPLY_PATCH: type('dspace/core/cache/object/APPLY_PATCH'), + ADD_DEPENDENTS: type('dspace/core/cache/object/ADD_DEPENDENTS'), + REMOVE_DEPENDENTS: type('dspace/core/cache/object/REMOVE_DEPENDENTS') }; /** @@ -126,13 +128,49 @@ export class ApplyPatchObjectCacheAction implements Action { } } +export class AddDependentsObjectCacheAction implements Action { + type = ObjectCacheActionTypes.ADD_DEPENDENTS; + payload: { + href: string; + dependentRequestUUIDs: string[]; + }; + + /** + * Create a new AddDependencyObjectCacheAction + * + * @param href the self link of a cached object + * @param dependentRequestUUIDs the UUID of the request that depends on this object + */ + constructor(href: string, dependentRequestUUIDs: string[]) { + this.payload = { + href, + dependentRequestUUIDs, + }; + } +} + +export class RemoveDependentsObjectCacheAction implements Action { + type = ObjectCacheActionTypes.REMOVE_DEPENDENTS; + payload: string; + + /** + * Create a new AddDependencyObjectCacheAction + * + * @param href the self link of a cached object for which to remove all dependent request UUIDs + */ + constructor(href: string) { + this.payload = href; + } +} /** * A type to encompass all ObjectCacheActions */ export type ObjectCacheAction = AddToObjectCacheAction - | RemoveFromObjectCacheAction - | ResetObjectCacheTimestampsAction - | AddPatchObjectCacheAction - | ApplyPatchObjectCacheAction; + | RemoveFromObjectCacheAction + | ResetObjectCacheTimestampsAction + | AddPatchObjectCacheAction + | ApplyPatchObjectCacheAction + | AddDependentsObjectCacheAction + | RemoveDependentsObjectCacheAction; diff --git a/src/app/core/cache/object-cache.reducer.spec.ts b/src/app/core/cache/object-cache.reducer.spec.ts index 82e2da58b1..e346a00b61 100644 --- a/src/app/core/cache/object-cache.reducer.spec.ts +++ b/src/app/core/cache/object-cache.reducer.spec.ts @@ -2,11 +2,13 @@ import * as deepFreeze from 'deep-freeze'; import { Operation } from 'fast-json-patch'; import { Item } from '../shared/item.model'; import { + AddDependentsObjectCacheAction, AddPatchObjectCacheAction, AddToObjectCacheAction, ApplyPatchObjectCacheAction, + RemoveDependentsObjectCacheAction, RemoveFromObjectCacheAction, - ResetObjectCacheTimestampsAction + ResetObjectCacheTimestampsAction, } from './object-cache.actions'; import { objectCacheReducer } from './object-cache.reducer'; @@ -42,20 +44,22 @@ describe('objectCacheReducer', () => { timeCompleted: new Date().getTime(), msToLive: 900000, requestUUIDs: [requestUUID1], + dependentRequestUUIDs: [], patches: [], isDirty: false, }, [selfLink2]: { data: { type: Item.type, - self: requestUUID2, + self: selfLink2, foo: 'baz', - _links: { self: { href: requestUUID2 } } + _links: { self: { href: selfLink2 } } }, alternativeLinks: [altLink3, altLink4], timeCompleted: new Date().getTime(), msToLive: 900000, - requestUUIDs: [selfLink2], + requestUUIDs: [requestUUID2], + dependentRequestUUIDs: [requestUUID1], patches: [], isDirty: false } @@ -189,4 +193,20 @@ describe('objectCacheReducer', () => { expect((newState[selfLink1].data as any).name).toEqual(newName); }); + it('should add dependent requests on ADD_DEPENDENTS', () => { + let newState = objectCacheReducer(testState, new AddDependentsObjectCacheAction(selfLink1, ['new', 'newer', 'newest'])); + expect(newState[selfLink1].dependentRequestUUIDs).toEqual(['new', 'newer', 'newest']); + + newState = objectCacheReducer(newState, new AddDependentsObjectCacheAction(selfLink2, ['more'])); + expect(newState[selfLink2].dependentRequestUUIDs).toEqual([requestUUID1, 'more']); + }); + + it('should clear dependent requests on REMOVE_DEPENDENTS', () => { + let newState = objectCacheReducer(testState, new RemoveDependentsObjectCacheAction(selfLink1)); + expect(newState[selfLink1].dependentRequestUUIDs).toEqual([]); + + newState = objectCacheReducer(newState, new RemoveDependentsObjectCacheAction(selfLink2)); + expect(newState[selfLink2].dependentRequestUUIDs).toEqual([]); + }); + }); diff --git a/src/app/core/cache/object-cache.reducer.ts b/src/app/core/cache/object-cache.reducer.ts index 1a42408f72..dc3f50db68 100644 --- a/src/app/core/cache/object-cache.reducer.ts +++ b/src/app/core/cache/object-cache.reducer.ts @@ -1,12 +1,13 @@ /* eslint-disable max-classes-per-file */ import { + AddDependentsObjectCacheAction, AddPatchObjectCacheAction, AddToObjectCacheAction, ApplyPatchObjectCacheAction, ObjectCacheAction, - ObjectCacheActionTypes, + ObjectCacheActionTypes, RemoveDependentsObjectCacheAction, RemoveFromObjectCacheAction, - ResetObjectCacheTimestampsAction + ResetObjectCacheTimestampsAction, } from './object-cache.actions'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { CacheEntry } from './cache-entry'; @@ -69,6 +70,12 @@ export class ObjectCacheEntry implements CacheEntry { */ requestUUIDs: string[]; + /** + * A list of UUIDs for the requests that depend on this object. + * When this object is invalidated, these requests will be invalidated as well. + */ + dependentRequestUUIDs: string[]; + /** * An array of patches that were made on the client side to this entry, but haven't been sent to the server yet */ @@ -134,6 +141,14 @@ export function objectCacheReducer(state = initialState, action: ObjectCacheActi return applyPatchObjectCache(state, action as ApplyPatchObjectCacheAction); } + case ObjectCacheActionTypes.ADD_DEPENDENTS: { + return addDependentsObjectCacheState(state, action as AddDependentsObjectCacheAction); + } + + case ObjectCacheActionTypes.REMOVE_DEPENDENTS: { + return removeDependentsObjectCacheState(state, action as RemoveDependentsObjectCacheAction); + } + default: { return state; } @@ -159,6 +174,7 @@ function addToObjectCache(state: ObjectCacheState, action: AddToObjectCacheActio timeCompleted: action.payload.timeCompleted, msToLive: action.payload.msToLive, requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])], + dependentRequestUUIDs: existing.dependentRequestUUIDs || [], isDirty: isNotEmpty(existing.patches), patches: existing.patches || [], alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks] @@ -252,3 +268,49 @@ function applyPatchObjectCache(state: ObjectCacheState, action: ApplyPatchObject } return newState; } + +/** + * Add a list of dependent request UUIDs to a cached object, used when defining new dependencies + * + * @param state the current state + * @param action an AddDependentsObjectCacheAction + * @return the new state, with the dependent requests of the cached object updated + */ +function addDependentsObjectCacheState(state: ObjectCacheState, action: AddDependentsObjectCacheAction): ObjectCacheState { + const href = action.payload.href; + const newState = Object.assign({}, state); + + if (hasValue(newState[href])) { + newState[href] = Object.assign({}, newState[href], { + dependentRequestUUIDs: [ + ...new Set([ + ...newState[href]?.dependentRequestUUIDs || [], + ...action.payload.dependentRequestUUIDs, + ]) + ] + }); + } + + return newState; +} + + +/** + * Remove all dependent request UUIDs from a cached object, used to clear out-of-date depedencies + * + * @param state the current state + * @param action an AddDependentsObjectCacheAction + * @return the new state, with the dependent requests of the cached object updated + */ +function removeDependentsObjectCacheState(state: ObjectCacheState, action: RemoveDependentsObjectCacheAction): ObjectCacheState { + const href = action.payload; + const newState = Object.assign({}, state); + + if (hasValue(newState[href])) { + newState[href] = Object.assign({}, newState[href], { + dependentRequestUUIDs: [] + }); + } + + return newState; +} diff --git a/src/app/core/cache/object-cache.service.spec.ts b/src/app/core/cache/object-cache.service.spec.ts index f18c262524..6af797be29 100644 --- a/src/app/core/cache/object-cache.service.spec.ts +++ b/src/app/core/cache/object-cache.service.spec.ts @@ -11,10 +11,12 @@ import { coreReducers} from '../core.reducers'; import { RestRequestMethod } from '../data/rest-request-method'; import { Item } from '../shared/item.model'; import { + AddDependentsObjectCacheAction, + RemoveDependentsObjectCacheAction, AddPatchObjectCacheAction, AddToObjectCacheAction, ApplyPatchObjectCacheAction, - RemoveFromObjectCacheAction + RemoveFromObjectCacheAction, } from './object-cache.actions'; import { Patch } from './object-cache.reducer'; import { ObjectCacheService } from './object-cache.service'; @@ -25,6 +27,7 @@ import { storeModuleConfig } from '../../app.reducer'; import { TestColdObservable } from 'jasmine-marbles/src/test-observables'; import { IndexName } from '../index/index-name.model'; import { CoreState } from '../core-state.model'; +import { TestScheduler } from 'rxjs/testing'; describe('ObjectCacheService', () => { let service: ObjectCacheService; @@ -38,6 +41,7 @@ describe('ObjectCacheService', () => { let altLink1; let altLink2; let requestUUID; + let requestUUID2; let alternativeLink; let timestamp; let timestamp2; @@ -55,6 +59,7 @@ describe('ObjectCacheService', () => { altLink1 = 'https://alternative.link/endpoint/1234'; altLink2 = 'https://alternative.link/endpoint/5678'; requestUUID = '4d3a4ce8-a375-4b98-859b-39f0a014d736'; + requestUUID2 = 'c0f486c1-c4d3-4a03-b293-ca5b71ff0054'; alternativeLink = 'https://rest.api/endpoint/5e4f8a5-be98-4c51-9fd8-6bfedcbd59b7/item'; timestamp = new Date().getTime(); timestamp2 = new Date().getTime() - 200; @@ -71,13 +76,17 @@ describe('ObjectCacheService', () => { data: objectToCache, timeCompleted: timestamp, msToLive: msToLive, - alternativeLinks: [altLink1, altLink2] + alternativeLinks: [altLink1, altLink2], + requestUUIDs: [requestUUID], + dependentRequestUUIDs: [], }; cacheEntry2 = { data: objectToCache, timeCompleted: timestamp2, msToLive: msToLive2, - alternativeLinks: [altLink2] + alternativeLinks: [altLink2], + requestUUIDs: [requestUUID2], + dependentRequestUUIDs: [], }; invalidCacheEntry = Object.assign({}, cacheEntry, { msToLive: -1 }); operations = [{ op: 'replace', path: '/name', value: 'random string' } as Operation]; @@ -343,4 +352,122 @@ describe('ObjectCacheService', () => { expect(store.dispatch).toHaveBeenCalledWith(new ApplyPatchObjectCacheAction(selfLink)); }); }); + + describe('request dependencies', () => { + beforeEach(() => { + const state = Object.assign({}, initialState, { + core: Object.assign({}, initialState.core, { + 'cache/object': { + ['objectWithoutDependents']: { + dependentRequestUUIDs: [], + }, + ['objectWithDependents']: { + dependentRequestUUIDs: [requestUUID], + }, + [selfLink]: cacheEntry, + }, + 'index': { + 'object/alt-link-to-self-link': { + [anotherLink]: selfLink, + ['objectWithoutDependentsAlt']: 'objectWithoutDependents', + ['objectWithDependentsAlt']: 'objectWithDependents', + } + } + }) + }); + mockStore.setState(state); + }); + + describe('addDependency', () => { + it('should dispatch an ADD_DEPENDENTS action', () => { + service.addDependency(selfLink, 'objectWithoutDependents'); + expect(store.dispatch).toHaveBeenCalledOnceWith(new AddDependentsObjectCacheAction('objectWithoutDependents', [requestUUID])); + }); + + it('should resolve alt links', () => { + service.addDependency(anotherLink, 'objectWithoutDependentsAlt'); + expect(store.dispatch).toHaveBeenCalledOnceWith(new AddDependentsObjectCacheAction('objectWithoutDependents', [requestUUID])); + }); + + it('should not dispatch if either href cannot be resolved to a cached self link', () => { + service.addDependency(selfLink, 'unknown'); + service.addDependency('unknown', 'objectWithoutDependents'); + service.addDependency('nothing', 'matches'); + expect(store.dispatch).not.toHaveBeenCalled(); + }); + + it('should not dispatch if either href is undefined', () => { + service.addDependency(selfLink, undefined); + service.addDependency(undefined, 'objectWithoutDependents'); + service.addDependency(undefined, undefined); + expect(store.dispatch).not.toHaveBeenCalled(); + }); + + it('should not dispatch if the dependency exists already', () => { + service.addDependency(selfLink, 'objectWithDependents'); + expect(store.dispatch).not.toHaveBeenCalled(); + }); + + it('should work with observable hrefs', () => { + service.addDependency(observableOf(selfLink), observableOf('objectWithoutDependents')); + expect(store.dispatch).toHaveBeenCalledOnceWith(new AddDependentsObjectCacheAction('objectWithoutDependents', [requestUUID])); + }); + + it('should only dispatch once for the first value of either observable href', () => { + const testScheduler = new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); + + testScheduler.run(({ cold: tsCold, flush }) => { + const href$ = tsCold('--y-n-n', { + y: selfLink, + n: 'NOPE' + }); + const dependsOnHref$ = tsCold('-y-n-n', { + y: 'objectWithoutDependents', + n: 'NOPE' + }); + + service.addDependency(href$, dependsOnHref$); + flush(); + + expect(store.dispatch).toHaveBeenCalledOnceWith(new AddDependentsObjectCacheAction('objectWithoutDependents', [requestUUID])); + }); + }); + + it('should not dispatch if either of the hrefs emits undefined', () => { + const testScheduler = new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); + + testScheduler.run(({ cold: tsCold, flush }) => { + const undefined$ = tsCold('--u'); + + service.addDependency(selfLink, undefined$); + service.addDependency(undefined$, 'objectWithoutDependents'); + service.addDependency(undefined$, undefined$); + flush(); + + expect(store.dispatch).not.toHaveBeenCalled(); + }); + }); + }); + + describe('removeDependents', () => { + it('should dispatch a REMOVE_DEPENDENTS action', () => { + service.removeDependents('objectWithDependents'); + expect(store.dispatch).toHaveBeenCalledOnceWith(new RemoveDependentsObjectCacheAction('objectWithDependents')); + }); + + it('should resolve alt links', () => { + service.removeDependents('objectWithDependentsAlt'); + expect(store.dispatch).toHaveBeenCalledOnceWith(new RemoveDependentsObjectCacheAction('objectWithDependents')); + }); + + it('should not dispatch if the href cannot be resolved to a cached self link', () => { + service.removeDependents('unknown'); + expect(store.dispatch).not.toHaveBeenCalled(); + }); + }); + }); }); diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index cdf87e5c1a..9ca0216210 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -4,23 +4,15 @@ import { applyPatch, Operation } from 'fast-json-patch'; import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs'; import { distinctUntilChanged, filter, map, mergeMap, switchMap, take } from 'rxjs/operators'; -import { hasValue, isNotEmpty, isEmpty } from '../../shared/empty.util'; +import { hasNoValue, hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; import { CoreState } from '../core-state.model'; import { coreSelector } from '../core.selectors'; import { RestRequestMethod } from '../data/rest-request-method'; -import { - selfLinkFromAlternativeLinkSelector, - selfLinkFromUuidSelector -} from '../index/index.selectors'; +import { selfLinkFromAlternativeLinkSelector, selfLinkFromUuidSelector } from '../index/index.selectors'; import { GenericConstructor } from '../shared/generic-constructor'; import { getClassForType } from './builders/build-decorators'; import { LinkService } from './builders/link.service'; -import { - AddPatchObjectCacheAction, - AddToObjectCacheAction, - ApplyPatchObjectCacheAction, - RemoveFromObjectCacheAction -} from './object-cache.actions'; +import { AddDependentsObjectCacheAction, AddPatchObjectCacheAction, AddToObjectCacheAction, ApplyPatchObjectCacheAction, RemoveDependentsObjectCacheAction, RemoveFromObjectCacheAction } from './object-cache.actions'; import { ObjectCacheEntry, ObjectCacheState } from './object-cache.reducer'; import { AddToSSBAction } from './server-sync-buffer.actions'; @@ -339,4 +331,97 @@ export class ObjectCacheService { this.store.dispatch(new ApplyPatchObjectCacheAction(selfLink)); } + /** + * Add a new dependency between two cached objects. + * When {@link dependsOnHref$} is invalidated, {@link href$} will be invalidated as well. + * + * This method should be called _after_ requests have been sent; + * it will only work if both objects are already present in the cache. + * + * If either object is undefined, the dependency will not be added + * + * @param href$ the href of an object to add a dependency to + * @param dependsOnHref$ the href of the new dependency + */ + addDependency(href$: string | Observable, dependsOnHref$: string | Observable) { + if (hasNoValue(href$) || hasNoValue(dependsOnHref$)) { + return; + } + + if (typeof href$ === 'string') { + href$ = observableOf(href$); + } + if (typeof dependsOnHref$ === 'string') { + dependsOnHref$ = observableOf(dependsOnHref$); + } + + observableCombineLatest([ + href$, + dependsOnHref$.pipe( + switchMap(dependsOnHref => this.resolveSelfLink(dependsOnHref)) + ), + ]).pipe( + switchMap(([href, dependsOnSelfLink]: [string, string]) => { + const dependsOnSelfLink$ = observableOf(dependsOnSelfLink); + + return observableCombineLatest([ + dependsOnSelfLink$, + dependsOnSelfLink$.pipe( + switchMap(selfLink => this.getBySelfLink(selfLink)), + map(oce => oce?.dependentRequestUUIDs || []), + ), + this.getByHref(href).pipe( + // only add the latest request to keep dependency index from growing indefinitely + map((entry: ObjectCacheEntry) => entry?.requestUUIDs?.[0]), + ) + ]); + }), + take(1), + ).subscribe(([dependsOnSelfLink, currentDependents, newDependent]: [string, string[], string]) => { + // don't dispatch if either href is invalid or if the new dependency already exists + if (hasValue(dependsOnSelfLink) && hasValue(newDependent) && !currentDependents.includes(newDependent)) { + this.store.dispatch(new AddDependentsObjectCacheAction(dependsOnSelfLink, [newDependent])); + } + }); + } + + /** + * Clear all dependent requests associated with a cache entry. + * + * @href the href of a cached object + */ + removeDependents(href: string) { + this.resolveSelfLink(href).pipe( + take(1), + ).subscribe((selfLink: string) => { + if (hasValue(selfLink)) { + this.store.dispatch(new RemoveDependentsObjectCacheAction(selfLink)); + } + }); + } + + + /** + * Resolve the self link of an existing cached object from an arbitrary href + * + * @param href any href + * @return an observable of the self link corresponding to the given href. + * Will emit the given href if it was a self link, another href + * if the given href was an alt link, or undefined if there is no + * cached object for this href. + */ + private resolveSelfLink(href: string): Observable { + return this.getBySelfLink(href).pipe( + switchMap((oce: ObjectCacheEntry) => { + if (isNotEmpty(oce)) { + return [href]; + } else { + return this.store.pipe( + select(selfLinkFromAlternativeLinkSelector(href)), + ); + } + }), + ); + } + } diff --git a/src/app/core/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index b8c41dee9d..cdac1f2855 100644 --- a/src/app/core/data/data.service.spec.ts +++ b/src/app/core/data/data.service.spec.ts @@ -2,7 +2,7 @@ import { HttpClient } from '@angular/common/http'; import { Store } from '@ngrx/store'; import { compare, Operation } from 'fast-json-patch'; -import { Observable, of as observableOf } from 'rxjs'; +import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { followLink } from '../../shared/utils/follow-link-config.model'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; @@ -13,6 +13,7 @@ import { HALEndpointService } from '../shared/hal-endpoint.service'; import { Item } from '../shared/item.model'; import { createFailedRemoteDataObject, + createFailedRemoteDataObject$, createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$, } from '../../shared/remote-data.utils'; @@ -96,7 +97,13 @@ describe('DataService', () => { }, getByHref: () => { /* empty */ - } + }, + addDependency: () => { + /* empty */ + }, + removeDependents: () => { + /* empty */ + }, } as any; store = {} as Store; selfLink = 'https://rest.api/endpoint/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; @@ -849,7 +856,8 @@ describe('DataService', () => { beforeEach(() => { getByHrefSpy = spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ - requestUUIDs: ['request1', 'request2', 'request3'] + requestUUIDs: ['request1', 'request2', 'request3'], + dependentRequestUUIDs: [] })); }); @@ -898,9 +906,9 @@ describe('DataService', () => { it('should only fire for the current state of the object (instead of tracking it)', () => { testScheduler.run(({ cold, flush }) => { getByHrefSpy.and.returnValue(cold('a---b---c---', { - a: { requestUUIDs: ['request1'] }, // this is the state at the moment we're invalidating the cache - b: { requestUUIDs: ['request2'] }, // we shouldn't keep tracking the state - c: { requestUUIDs: ['request3'] }, // because we may invalidate when we shouldn't + a: { requestUUIDs: ['request1'], dependentRequestUUIDs: [] }, // this is the state at the moment we're invalidating the cache + b: { requestUUIDs: ['request2'], dependentRequestUUIDs: [] }, // we shouldn't keep tracking the state + c: { requestUUIDs: ['request3'], dependentRequestUUIDs: [] }, // because we may invalidate when we shouldn't })); service.invalidateByHref('some-href'); @@ -970,4 +978,42 @@ describe('DataService', () => { }); }); }); + + describe('addDependency', () => { + let addDependencySpy; + + beforeEach(() => { + addDependencySpy = spyOn(objectCache, 'addDependency'); + }); + + it('should call objectCache.addDependency with the object\'s self link', () => { + addDependencySpy.and.callFake((href$: Observable, dependsOn$: Observable) => { + observableCombineLatest([href$, dependsOn$]).subscribe(([href, dependsOn]) => { + expect(href).toBe('object-href'); + expect(dependsOn).toBe('dependsOnHref'); + }); + }); + + (service as any).addDependency( + createSuccessfulRemoteDataObject$({ _links: { self: { href: 'object-href' } } }), + observableOf('dependsOnHref') + ); + expect(addDependencySpy).toHaveBeenCalled(); + }); + + it('should call objectCache.addDependency without an href if request failed', () => { + addDependencySpy.and.callFake((href$: Observable, dependsOn$: Observable) => { + observableCombineLatest([href$, dependsOn$]).subscribe(([href, dependsOn]) => { + expect(href).toBe(undefined); + expect(dependsOn).toBe('dependsOnHref'); + }); + }); + + (service as any).addDependency( + createFailedRemoteDataObject$('something went wrong'), + observableOf('dependsOnHref') + ); + expect(addDependencySpy).toHaveBeenCalled(); + }); + }); }); diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index c17fccb198..6f260bae25 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -27,7 +27,7 @@ import { ObjectCacheService } from '../cache/object-cache.service'; import { DSpaceSerializer } from '../dspace-rest/dspace.serializer'; import { DSpaceObject } from '../shared/dspace-object.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; -import { getFirstSucceededRemoteData, getRemoteDataPayload } from '../shared/operators'; +import { getFirstSucceededRemoteData, getRemoteDataPayload, getFirstCompletedRemoteData } from '../shared/operators'; import { URLCombiner } from '../url-combiner/url-combiner'; import { ChangeAnalyzer } from './change-analyzer'; import { PaginatedList } from './paginated-list.model'; @@ -576,6 +576,35 @@ export abstract class DataService implements UpdateDa return result$; } + /** + * Shorthand method to add a dependency to a cached object + * ``` + * const out$ = this.findByHref(...); // or another method that sends a request + * this.addDependency(out$, dependsOnHref); + * ``` + * When {@link dependsOnHref$} is invalidated, {@link object$} will be invalidated as well. + * + * + * @param object$ the cached object + * @param dependsOnHref$ the href of the object it should depend on + */ + protected addDependency(object$: Observable>>, dependsOnHref$: string | Observable) { + this.objectCache.addDependency( + object$.pipe( + getFirstCompletedRemoteData(), + switchMap((rd: RemoteData) => { + if (rd.hasSucceeded) { + return [rd.payload._links.self.href]; + } else { + // undefined href will be skipped in objectCache.addDependency + return [undefined]; + } + }), + ), + dependsOnHref$ + ); + } + /** * Invalidate an existing DSpaceObject by marking all requests it is included in as stale * @param objectId The id of the object to be invalidated @@ -597,11 +626,17 @@ export abstract class DataService implements UpdateDa this.objectCache.getByHref(href).pipe( take(1), - switchMap((oce: ObjectCacheEntry) => observableFrom(oce.requestUUIDs).pipe( - mergeMap((requestUUID: string) => this.requestService.setStaleByUUID(requestUUID)), - toArray(), - )), + switchMap((oce: ObjectCacheEntry) => { + return observableFrom([ + ...oce.requestUUIDs, + ...oce.dependentRequestUUIDs + ]).pipe( + mergeMap((requestUUID: string) => this.requestService.setStaleByUUID(requestUUID)), + toArray(), + ); + }), ).subscribe(() => { + this.objectCache.removeDependents(href); done$.next(true); done$.complete(); }); diff --git a/src/app/core/eperson/group-data.service.spec.ts b/src/app/core/eperson/group-data.service.spec.ts index c1b9e59d5b..8f5e92aaf6 100644 --- a/src/app/core/eperson/group-data.service.spec.ts +++ b/src/app/core/eperson/group-data.service.spec.ts @@ -27,11 +27,10 @@ import { createPaginatedList, createRequestEntry$ } from '../../shared/testing/u import { CoreState } from '../core-state.model'; import { FindListOptions } from '../data/find-list-options.model'; import { DataService } from '../data/data.service'; -import { ObjectCacheService } from '../cache/object-cache.service'; -import { getMockLinkService } from '../../shared/mocks/link-service.mock'; import { DataServiceStub } from '../../shared/testing/data-service.stub'; import { of as observableOf } from 'rxjs'; import { ObjectCacheEntry } from '../cache/object-cache.reducer'; +import { getMockObjectCacheService } from '../../shared/mocks/object-cache.service.mock'; describe('GroupDataService', () => { let service: GroupDataService; @@ -44,7 +43,7 @@ describe('GroupDataService', () => { let groups$; let halService; let rdbService; - let objectCache: ObjectCacheService; + let objectCache; let dataService: DataServiceStub; function init() { @@ -54,7 +53,9 @@ describe('GroupDataService', () => { groups$ = createSuccessfulRemoteDataObject$(createPaginatedList(groups)); rdbService = getMockRemoteDataBuildServiceHrefMap(undefined, { 'https://dspace.4science.it/dspace-spring-rest/api/eperson/groups': groups$ }); halService = new HALEndpointServiceStub(restEndpointURL); - objectCache = new ObjectCacheService(store, getMockLinkService()); + + objectCache = getMockObjectCacheService(); + dataService = new DataServiceStub(); TestBed.configureTestingModule({ imports: [ @@ -122,8 +123,9 @@ describe('GroupDataService', () => { describe('addSubGroupToGroup', () => { beforeEach(() => { - spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ - requestUUIDs: ['request1', 'request2'] + objectCache.getByHref.and.returnValue(observableOf({ + requestUUIDs: ['request1', 'request2'], + dependentRequestUUIDs: [], } as ObjectCacheEntry)); spyOn(dataService, 'invalidateByHref'); service.addSubGroupToGroup(GroupMock, GroupMock2).subscribe(); @@ -151,8 +153,9 @@ describe('GroupDataService', () => { describe('deleteSubGroupFromGroup', () => { beforeEach(() => { - spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ - requestUUIDs: ['request1', 'request2'] + objectCache.getByHref.and.returnValue(observableOf({ + requestUUIDs: ['request1', 'request2'], + dependentRequestUUIDs: [], } as ObjectCacheEntry)); spyOn(dataService, 'invalidateByHref'); service.deleteSubGroupFromGroup(GroupMock, GroupMock2).subscribe(); @@ -176,8 +179,9 @@ describe('GroupDataService', () => { describe('addMemberToGroup', () => { beforeEach(() => { - spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ - requestUUIDs: ['request1', 'request2'] + objectCache.getByHref.and.returnValue(observableOf({ + requestUUIDs: ['request1', 'request2'], + dependentRequestUUIDs: [], } as ObjectCacheEntry)); spyOn(dataService, 'invalidateByHref'); service.addMemberToGroup(GroupMock, EPersonMock2).subscribe(); @@ -206,8 +210,9 @@ describe('GroupDataService', () => { describe('deleteMemberFromGroup', () => { beforeEach(() => { - spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ - requestUUIDs: ['request1', 'request2'] + objectCache.getByHref.and.returnValue(observableOf({ + requestUUIDs: ['request1', 'request2'], + dependentRequestUUIDs: [], } as ObjectCacheEntry)); spyOn(dataService, 'invalidateByHref'); service.deleteMemberFromGroup(GroupMock, EPersonMock).subscribe(); diff --git a/src/app/shared/mocks/object-cache.service.mock.ts b/src/app/shared/mocks/object-cache.service.mock.ts index 515c2abe86..8cceaefc3d 100644 --- a/src/app/shared/mocks/object-cache.service.mock.ts +++ b/src/app/shared/mocks/object-cache.service.mock.ts @@ -13,6 +13,8 @@ export function getMockObjectCacheService(): ObjectCacheService { 'hasByUUID', 'hasByHref', 'getRequestUUIDBySelfLink', + 'addDependency', + 'removeDependents', ]); } From 7c13db2f89701039a3aa2d699268389a91062155 Mon Sep 17 00:00:00 2001 From: Yury Bondarenko Date: Thu, 8 Sep 2022 13:26:01 +0200 Subject: [PATCH 07/57] 94273: Invalidate object on successful patch --- src/app/core/data/data.service.spec.ts | 24 ++++++++++++++++++++---- src/app/core/data/data.service.ts | 2 +- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/app/core/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index cdac1f2855..77d5771b91 100644 --- a/src/app/core/data/data.service.spec.ts +++ b/src/app/core/data/data.service.spec.ts @@ -351,7 +351,12 @@ describe('DataService', () => { describe('patch', () => { const dso = { - uuid: 'dso-uuid' + uuid: 'dso-uuid', + _links: { + self: { + href: 'dso-href', + } + } }; const operations = [ Object.assign({ @@ -361,12 +366,23 @@ describe('DataService', () => { }) as Operation ]; - beforeEach(() => { + it('should send a PatchRequest', () => { service.patch(dso, operations); + expect(requestService.send).toHaveBeenCalledWith(jasmine.any(PatchRequest)); }); - it('should send a PatchRequest', () => { - expect(requestService.send).toHaveBeenCalledWith(jasmine.any(PatchRequest)); + it('should invalidate the cached object if successfully patched', () => { + spyOn(rdbService, 'buildFromRequestUUIDAndAwait'); + spyOn(service, 'invalidateByHref'); + + service.patch(dso, operations); + + expect(rdbService.buildFromRequestUUIDAndAwait).toHaveBeenCalled(); + expect((rdbService.buildFromRequestUUIDAndAwait as jasmine.Spy).calls.argsFor(0)[0]).toBe(requestService.generateRequestId()); + const callback = (rdbService.buildFromRequestUUIDAndAwait as jasmine.Spy).calls.argsFor(0)[1]; + callback(); + + expect(service.invalidateByHref).toHaveBeenCalledWith('dso-href'); }); }); diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 6f260bae25..b69d219c2e 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -484,7 +484,7 @@ export abstract class DataService implements UpdateDa this.requestService.send(request); }); - return this.rdbService.buildFromRequestUUID(requestId); + return this.rdbService.buildFromRequestUUIDAndAwait(requestId, () => this.invalidateByHref(object._links.self.href)); } createPatchFromCache(object: T): Observable { From 293ba8408e63d04e595695f01056156eef3f749a Mon Sep 17 00:00:00 2001 From: Yury Bondarenko Date: Thu, 8 Sep 2022 13:27:34 +0200 Subject: [PATCH 08/57] 94273: Make isAuthorized depend on the target object This ensures that a successful ItemDataService.setWithdrawn call invalidates all related authorizations --- .../authorization-data.service.spec.ts | 46 ++++++++++++++++++- .../authorization-data.service.ts | 24 ++++++++-- 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/app/core/data/feature-authorization/authorization-data.service.spec.ts b/src/app/core/data/feature-authorization/authorization-data.service.spec.ts index df46d3f0a1..255ce8f741 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.spec.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.spec.ts @@ -3,7 +3,7 @@ import { SiteDataService } from '../site-data.service'; import { AuthService } from '../../auth/auth.service'; import { Site } from '../../shared/site.model'; import { EPerson } from '../../eperson/models/eperson.model'; -import { of as observableOf } from 'rxjs'; +import { of as observableOf, combineLatest as observableCombineLatest, Observable } from 'rxjs'; import { FeatureID } from './feature-id'; import { hasValue } from '../../../shared/empty.util'; import { RequestParam } from '../../cache/models/request-param.model'; @@ -17,6 +17,7 @@ describe('AuthorizationDataService', () => { let service: AuthorizationDataService; let siteService: SiteDataService; let authService: AuthService; + let objectCache; let site: Site; let ePerson: EPerson; @@ -43,7 +44,11 @@ describe('AuthorizationDataService', () => { isAuthenticated: () => observableOf(true), getAuthenticatedUserFromStore: () => observableOf(ePerson) } as AuthService; - service = new AuthorizationDataService(requestService, undefined, undefined, undefined, undefined, undefined, undefined, undefined, authService, siteService); + objectCache = jasmine.createSpyObj('objectCache', { + addDependency: undefined, + removeDependents: undefined, + }); + service = new AuthorizationDataService(requestService, undefined, undefined, objectCache, undefined, undefined, undefined, undefined, authService, siteService); } beforeEach(() => { @@ -110,6 +115,43 @@ describe('AuthorizationDataService', () => { expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, ePersonUuid, FeatureID.LoginOnBehalfOf), true, true); }); }); + + describe('dependencies', () => { + let addDependencySpy; + + beforeEach(() => { + (service.searchBy as any).and.returnValue(observableOf('searchBy RD$')); + addDependencySpy = spyOn(service as any, 'addDependency'); + }); + + it('should add a dependency on the objectUrl', (done) => { + addDependencySpy.and.callFake((href$: Observable, dependsOn$: Observable) => { + observableCombineLatest([href$, dependsOn$]).subscribe(([href, dependsOn]) => { + expect(href).toBe('searchBy RD$'); + expect(dependsOn).toBe('object-href'); + }); + }); + + service.searchByObject(FeatureID.AdministratorOf, 'object-href').subscribe(() => { + expect(addDependencySpy).toHaveBeenCalled(); + done(); + }); + }); + + it('should add a dependency on the Site object if no objectUrl is given', (done) => { + addDependencySpy.and.callFake((object$: Observable, dependsOn$: Observable) => { + observableCombineLatest([object$, dependsOn$]).subscribe(([object, dependsOn]) => { + expect(object).toBe('searchBy RD$'); + expect(dependsOn).toBe('test-site-href'); + }); + }); + + service.searchByObject(FeatureID.AdministratorOf).subscribe(() => { + expect(addDependencySpy).toHaveBeenCalled(); + done(); + }); + }); + }); }); describe('isAuthorized', () => { diff --git a/src/app/core/data/feature-authorization/authorization-data.service.ts b/src/app/core/data/feature-authorization/authorization-data.service.ts index f27919844d..008e69b119 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -18,10 +18,10 @@ import { followLink, FollowLinkConfig } from '../../../shared/utils/follow-link- import { RemoteData } from '../remote-data'; import { PaginatedList } from '../paginated-list.model'; import { catchError, map, switchMap } from 'rxjs/operators'; -import { hasValue, isNotEmpty } from '../../../shared/empty.util'; +import { hasNoValue, hasValue, isNotEmpty } from '../../../shared/empty.util'; import { RequestParam } from '../../cache/models/request-param.model'; import { AuthorizationSearchParams } from './authorization-search-params'; -import { addSiteObjectUrlIfEmpty, oneAuthorizationMatchesFeature } from './authorization-utils'; +import { oneAuthorizationMatchesFeature } from './authorization-utils'; import { FeatureID } from './feature-id'; import { getFirstCompletedRemoteData } from '../../shared/operators'; import { CoreState } from '../../core-state.model'; @@ -102,12 +102,28 @@ export class AuthorizationDataService extends DataService { * {@link HALLink}s should be automatically resolved */ searchByObject(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string, options: FindListOptions = {}, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig[]): Observable>> { - return observableOf(new AuthorizationSearchParams(objectUrl, ePersonUuid, featureId)).pipe( - addSiteObjectUrlIfEmpty(this.siteService), + const objectUrl$ = observableOf(objectUrl).pipe( + switchMap((url) => { + if (hasNoValue(url)) { + return this.siteService.find().pipe( + map((site) => site.self) + ); + } else { + return observableOf(url); + } + }), + ); + + const out$ = objectUrl$.pipe( + map((url: string) => new AuthorizationSearchParams(url, ePersonUuid, featureId)), switchMap((params: AuthorizationSearchParams) => { return this.searchBy(this.searchByObjectPath, this.createSearchOptions(params.objectUrl, options, params.ePersonUuid, params.featureId), useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); }) ); + + this.addDependency(out$, objectUrl$); + + return out$; } /** From 70c6eac88d3de14cd70b4a9a83616f63785ebf57 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 15 Sep 2022 14:30:59 +0200 Subject: [PATCH 09/57] [CST-6753] Change google-analytics.service in order to use GoogleTagManager instead of GoogleAnalytics --- .../google-analytics.service.spec.ts | 25 +++++++++++-------- .../statistics/google-analytics.service.ts | 18 +++++++------ src/modules/app/server-app.module.ts | 5 ++-- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/app/statistics/google-analytics.service.spec.ts b/src/app/statistics/google-analytics.service.spec.ts index 0c6bc2bc51..24c5345260 100644 --- a/src/app/statistics/google-analytics.service.spec.ts +++ b/src/app/statistics/google-analytics.service.spec.ts @@ -1,20 +1,19 @@ import { GoogleAnalyticsService } from './google-analytics.service'; -import { Angulartics2GoogleAnalytics } from 'angulartics2'; +import { Angulartics2GoogleTagManager } from 'angulartics2'; import { ConfigurationDataService } from '../core/data/configuration-data.service'; -import { - createFailedRemoteDataObject$, - createSuccessfulRemoteDataObject$ -} from '../shared/remote-data.utils'; +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'; + const srcTestValue = 'mock-script-src'; let service: GoogleAnalyticsService; - let angularticsSpy: Angulartics2GoogleAnalytics; + let angularticsSpy: Angulartics2GoogleTagManager; let configSpy: ConfigurationDataService; let scriptElementMock: any; + let srcSpy: any; let innerHTMLSpy: any; let bodyElementSpy: HTMLBodyElement; let documentSpy: Document; @@ -28,18 +27,21 @@ describe('GoogleAnalyticsService', () => { }); beforeEach(() => { - angularticsSpy = jasmine.createSpyObj('angulartics2GoogleAnalytics', [ + angularticsSpy = jasmine.createSpyObj('Angulartics2GoogleTagManager', [ 'startTracking', ]); configSpy = createConfigSuccessSpy(trackingIdTestValue); scriptElementMock = { + set src(newVal) { /* noop */ }, + get src() { return innerHTMLTestValue; }, set innerHTML(newVal) { /* noop */ }, - get innerHTML() { return innerHTMLTestValue; } + get innerHTML() { return srcTestValue; } }; innerHTMLSpy = spyOnProperty(scriptElementMock, 'innerHTML', 'set'); + srcSpy = spyOnProperty(scriptElementMock, 'src', 'set'); bodyElementSpy = jasmine.createSpyObj('body', { appendChild: scriptElementMock, @@ -106,19 +108,22 @@ describe('GoogleAnalyticsService', () => { 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).toHaveBeenCalledTimes(2); expect(documentSpy.createElement).toHaveBeenCalledWith('script'); // sanity check expect(documentSpy.createElement('script')).toBe(scriptElementMock); + expect(srcSpy).toHaveBeenCalledTimes(1); + expect(srcSpy.calls.argsFor(0)[0]).toContain(trackingIdTestValue); + 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); + expect(bodyElementSpy.appendChild).toHaveBeenCalledTimes(2); }); it('should start tracking', () => { diff --git a/src/app/statistics/google-analytics.service.ts b/src/app/statistics/google-analytics.service.ts index 0b52f54c4f..6bdff53d52 100644 --- a/src/app/statistics/google-analytics.service.ts +++ b/src/app/statistics/google-analytics.service.ts @@ -1,5 +1,5 @@ import { Inject, Injectable } from '@angular/core'; -import { Angulartics2GoogleAnalytics } from 'angulartics2'; +import { Angulartics2GoogleTagManager } from 'angulartics2'; import { ConfigurationDataService } from '../core/data/configuration-data.service'; import { getFirstCompletedRemoteData } from '../core/shared/operators'; import { isEmpty } from '../shared/empty.util'; @@ -13,7 +13,8 @@ import { DOCUMENT } from '@angular/common'; export class GoogleAnalyticsService { constructor( - private angulartics: Angulartics2GoogleAnalytics, + // private angulartics: Angulartics2GoogleAnalytics, + private angulartics: Angulartics2GoogleTagManager, private configService: ConfigurationDataService, @Inject(DOCUMENT) private document: any, ) { } @@ -36,15 +37,16 @@ export class GoogleAnalyticsService { // make sure we received a tracking id if (isEmpty(trackingId)) { return; } - // add trackingId snippet to page + // add GTag snippet to page 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');`; + keyScript.src = `https://www.googletagmanager.com/gtag/js?id=${trackingId}`; this.document.body.appendChild(keyScript); + const libScript = this.document.createElement('script'); + libScript.innerHTML = `window.dataLayer = window.dataLayer || [];function gtag(){window.dataLayer.push(arguments);} + gtag('js', new Date());gtag('config', '${trackingId}');`; + this.document.body.appendChild(libScript); + // start tracking this.angulartics.startTracking(); }); diff --git a/src/modules/app/server-app.module.ts b/src/modules/app/server-app.module.ts index 35fa050d6f..8565db3e23 100644 --- a/src/modules/app/server-app.module.ts +++ b/src/modules/app/server-app.module.ts @@ -6,8 +6,7 @@ import { ServerModule, ServerTransferStateModule } from '@angular/platform-serve import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; -import { Angulartics2 } from 'angulartics2'; -import { Angulartics2GoogleAnalytics } from 'angulartics2'; +import { Angulartics2, Angulartics2GoogleTagManager } from 'angulartics2'; import { AppComponent } from '../../app/app.component'; @@ -60,7 +59,7 @@ export function createTranslateLoader(transferState: TransferState) { useClass: Angulartics2Mock }, { - provide: Angulartics2GoogleAnalytics, + provide: Angulartics2GoogleTagManager, useClass: AngularticsProviderMock }, { From 1df41deb8f071b58beaeb4ee88b3f1397d6851c0 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 15 Sep 2022 14:36:09 +0200 Subject: [PATCH 10/57] [CST-6753] Remove unused occurrences of GoogleAnalytics --- src/app/app.component.spec.ts | 4 +--- src/app/root/root.component.spec.ts | 2 -- src/app/root/root.component.ts | 2 -- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index 8bec7edc80..422ead99e1 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -1,10 +1,9 @@ import { Store, StoreModule } from '@ngrx/store'; import { ComponentFixture, inject, TestBed, waitForAsync } from '@angular/core/testing'; import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; -import { CommonModule, DOCUMENT } from '@angular/common'; +import { CommonModule } from '@angular/common'; import { ActivatedRoute, Router } from '@angular/router'; import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; -import { Angulartics2GoogleAnalytics } from 'angulartics2'; // Load the implementations that should be tested import { AppComponent } from './app.component'; @@ -73,7 +72,6 @@ describe('App 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() }, diff --git a/src/app/root/root.component.spec.ts b/src/app/root/root.component.spec.ts index 64a15a3087..504bc34e34 100644 --- a/src/app/root/root.component.spec.ts +++ b/src/app/root/root.component.spec.ts @@ -9,7 +9,6 @@ import { TranslateLoaderMock } from '../shared/mocks/translate-loader.mock'; import { NativeWindowRef, NativeWindowService } from '../core/services/window.service'; import { MetadataService } from '../core/metadata/metadata.service'; import { MetadataServiceMock } from '../shared/mocks/metadata-service.mock'; -import { Angulartics2GoogleAnalytics } from 'angulartics2'; import { AngularticsProviderMock } from '../shared/mocks/angulartics-provider.service.mock'; import { Angulartics2DSpace } from '../statistics/angulartics/dspace-provider'; import { AuthService } from '../core/auth/auth.service'; @@ -50,7 +49,6 @@ describe('RootComponent', () => { 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() }, diff --git a/src/app/root/root.component.ts b/src/app/root/root.component.ts index 3dc555919c..472ba440c9 100644 --- a/src/app/root/root.component.ts +++ b/src/app/root/root.component.ts @@ -5,7 +5,6 @@ import { Router } from '@angular/router'; import { combineLatest as combineLatestObservable, Observable, of } from 'rxjs'; import { Store } from '@ngrx/store'; import { TranslateService } from '@ngx-translate/core'; -import { Angulartics2GoogleAnalytics } from 'angulartics2'; import { MetadataService } from '../core/metadata/metadata.service'; import { HostWindowState } from '../shared/search/host-window.reducer'; @@ -51,7 +50,6 @@ export class RootComponent implements OnInit { private translate: TranslateService, private store: Store, private metadata: MetadataService, - private angulartics2GoogleAnalytics: Angulartics2GoogleAnalytics, private angulartics2DSpace: Angulartics2DSpace, private authService: AuthService, private router: Router, From dd2317699acb7ec50db084647e3c5b13c3ff94d2 Mon Sep 17 00:00:00 2001 From: Jens Vannerum Date: Mon, 19 Sep 2022 10:11:40 +0200 Subject: [PATCH 11/57] make search-settings.component themed --- .../themed-search-settings.component.ts | 33 +++++++++++++++++++ src/app/shared/search/search.module.ts | 2 ++ .../search-settings.component.html | 0 .../search-settings.component.scss | 0 .../search-settings.component.ts | 31 +++++++++++++++++ src/themes/custom/theme.module.ts | 4 ++- 6 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 src/app/shared/search/search-settings/themed-search-settings.component.ts create mode 100644 src/themes/custom/app/shared/search-settings/search-settings.component.html create mode 100644 src/themes/custom/app/shared/search-settings/search-settings.component.scss create mode 100644 src/themes/custom/app/shared/search-settings/search-settings.component.ts diff --git a/src/app/shared/search/search-settings/themed-search-settings.component.ts b/src/app/shared/search/search-settings/themed-search-settings.component.ts new file mode 100644 index 0000000000..b1f1d08264 --- /dev/null +++ b/src/app/shared/search/search-settings/themed-search-settings.component.ts @@ -0,0 +1,33 @@ +import { Component, Input } from '@angular/core'; +import { ThemedComponent } from '../../theme-support/themed.component'; +import { SearchSettingsComponent } from './search-settings.component'; +import { SortOptions } from '../../../core/cache/models/sort-options.model'; + +/** + * Themed wrapper for SearchSettingsComponent + */ +@Component({ + selector: 'ds-themed-search-settings', + styleUrls: [], + templateUrl: '../../theme-support/themed.component.html', +}) +export class ThemedSearchSettingsComponent extends ThemedComponent { + @Input() currentSortOption: SortOptions; + @Input() sortOptionsList: SortOptions[]; + + + protected inAndOutputNames: (keyof SearchSettingsComponent & keyof this)[] = [ + 'currentSortOption', 'sortOptionsList']; + + protected getComponentName(): string { + return 'SearchSettingsComponent'; + } + + protected importThemedComponent(themeName: string): Promise { + return import(`../../../../themes/${themeName}/app/shared/search/search-settings/search-settings.component`); + } + + protected importUnthemedComponent(): Promise { + return import('./search-settings.component'); + } +} diff --git a/src/app/shared/search/search.module.ts b/src/app/shared/search/search.module.ts index 668d260c23..51ba17c6cf 100644 --- a/src/app/shared/search/search.module.ts +++ b/src/app/shared/search/search.module.ts @@ -28,12 +28,14 @@ import { MissingTranslationHelper } from '../translate/missing-translation.helpe import { SharedModule } from '../shared.module'; import { SearchResultsComponent } from './search-results/search-results.component'; import { SearchComponent } from './search.component'; +import { ThemedSearchSettingsComponent } from './search-settings/themed-search-settings.component'; const COMPONENTS = [ SearchComponent, SearchResultsComponent, SearchSidebarComponent, SearchSettingsComponent, + ThemedSearchSettingsComponent, SearchFiltersComponent, SearchFilterComponent, SearchFacetFilterComponent, diff --git a/src/themes/custom/app/shared/search-settings/search-settings.component.html b/src/themes/custom/app/shared/search-settings/search-settings.component.html new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/themes/custom/app/shared/search-settings/search-settings.component.scss b/src/themes/custom/app/shared/search-settings/search-settings.component.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/themes/custom/app/shared/search-settings/search-settings.component.ts b/src/themes/custom/app/shared/search-settings/search-settings.component.ts new file mode 100644 index 0000000000..db27ac95f4 --- /dev/null +++ b/src/themes/custom/app/shared/search-settings/search-settings.component.ts @@ -0,0 +1,31 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE_ATMIRE and NOTICE_ATMIRE files at the root of the source + * tree and available online at + * + * https://www.atmire.com/software-license/ + */ +import { Component } from '@angular/core'; +import { + SearchSettingsComponent as BaseComponent, +} from '../../../../../app/shared/search/search-settings/search-settings.component'; +import { SEARCH_CONFIG_SERVICE } from '../../../../../app/my-dspace-page/my-dspace-page.component'; +import { SearchConfigurationService } from '../../../../../app/core/shared/search/search-configuration.service'; + + +@Component({ + selector: 'ds-search-settings', + // styleUrls: ['./search-settings.component.scss'], + styleUrls: ['../../../../../app/shared/search/search-settings/search-settings.component.scss'], + // templateUrl: './search-settings.component.html', + templateUrl: '../../../../../app/shared/search/search-settings/search-settings.component.html', + providers: [ + { + provide: SEARCH_CONFIG_SERVICE, + useClass: SearchConfigurationService + } + ] + +}) + +export class SearchSettingsComponent extends BaseComponent {} diff --git a/src/themes/custom/theme.module.ts b/src/themes/custom/theme.module.ts index e2e97b9087..3457259792 100644 --- a/src/themes/custom/theme.module.ts +++ b/src/themes/custom/theme.module.ts @@ -84,6 +84,7 @@ import { SearchModule } from '../../app/shared/search/search.module'; import { ResourcePoliciesModule } from '../../app/shared/resource-policies/resource-policies.module'; import { ComcolModule } from '../../app/shared/comcol/comcol.module'; import { FeedbackComponent } from './app/info/feedback/feedback.component'; +import { SearchSettingsComponent } from './app/shared/search-settings/search-settings.component'; const DECLARATIONS = [ FileSectionComponent, @@ -126,7 +127,8 @@ const DECLARATIONS = [ NavbarComponent, HeaderNavbarWrapperComponent, BreadcrumbsComponent, - FeedbackComponent + FeedbackComponent, + SearchSettingsComponent ]; @NgModule({ From a17ee028dfebdcce95f115a275d3b906215ee5e3 Mon Sep 17 00:00:00 2001 From: Jens Vannerum Date: Mon, 19 Sep 2022 11:56:04 +0200 Subject: [PATCH 12/57] make search-settings.component themed --- .../shared/search/search-sidebar/search-sidebar.component.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/shared/search/search-sidebar/search-sidebar.component.html b/src/app/shared/search/search-sidebar/search-sidebar.component.html index e17fe941ba..863bd2d71f 100644 --- a/src/app/shared/search/search-sidebar/search-sidebar.component.html +++ b/src/app/shared/search/search-sidebar/search-sidebar.component.html @@ -21,7 +21,8 @@ [currentConfiguration]="configuration" [refreshFilters]="refreshFilters" [inPlaceSearch]="inPlaceSearch"> - + From f84e4d6146c000ffe8da36f0541ffd6347af0e97 Mon Sep 17 00:00:00 2001 From: Jens Vannerum Date: Mon, 19 Sep 2022 12:35:21 +0200 Subject: [PATCH 13/57] 89685: Resolve wrong directory --- .../search-settings/search-settings.component.html | 0 .../search-settings/search-settings.component.scss | 0 .../search-settings/search-settings.component.ts | 10 +++++----- src/themes/custom/lazy-theme.module.ts | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) rename src/themes/custom/app/shared/{ => search}/search-settings/search-settings.component.html (100%) rename src/themes/custom/app/shared/{ => search}/search-settings/search-settings.component.scss (100%) rename src/themes/custom/app/shared/{ => search}/search-settings/search-settings.component.ts (56%) diff --git a/src/themes/custom/app/shared/search-settings/search-settings.component.html b/src/themes/custom/app/shared/search/search-settings/search-settings.component.html similarity index 100% rename from src/themes/custom/app/shared/search-settings/search-settings.component.html rename to src/themes/custom/app/shared/search/search-settings/search-settings.component.html diff --git a/src/themes/custom/app/shared/search-settings/search-settings.component.scss b/src/themes/custom/app/shared/search/search-settings/search-settings.component.scss similarity index 100% rename from src/themes/custom/app/shared/search-settings/search-settings.component.scss rename to src/themes/custom/app/shared/search/search-settings/search-settings.component.scss diff --git a/src/themes/custom/app/shared/search-settings/search-settings.component.ts b/src/themes/custom/app/shared/search/search-settings/search-settings.component.ts similarity index 56% rename from src/themes/custom/app/shared/search-settings/search-settings.component.ts rename to src/themes/custom/app/shared/search/search-settings/search-settings.component.ts index db27ac95f4..e17c2425b5 100644 --- a/src/themes/custom/app/shared/search-settings/search-settings.component.ts +++ b/src/themes/custom/app/shared/search/search-settings/search-settings.component.ts @@ -8,17 +8,17 @@ import { Component } from '@angular/core'; import { SearchSettingsComponent as BaseComponent, -} from '../../../../../app/shared/search/search-settings/search-settings.component'; -import { SEARCH_CONFIG_SERVICE } from '../../../../../app/my-dspace-page/my-dspace-page.component'; -import { SearchConfigurationService } from '../../../../../app/core/shared/search/search-configuration.service'; +} from '../../../../../../app/shared/search/search-settings/search-settings.component'; +import { SEARCH_CONFIG_SERVICE } from '../../../../../../app/my-dspace-page/my-dspace-page.component'; +import { SearchConfigurationService } from '../../../../../../app/core/shared/search/search-configuration.service'; @Component({ selector: 'ds-search-settings', // styleUrls: ['./search-settings.component.scss'], - styleUrls: ['../../../../../app/shared/search/search-settings/search-settings.component.scss'], + styleUrls: ['../../../../../../app/shared/search/search-settings/search-settings.component.scss'], // templateUrl: './search-settings.component.html', - templateUrl: '../../../../../app/shared/search/search-settings/search-settings.component.html', + templateUrl: '../../../../../../app/shared/search/search-settings/search-settings.component.html', providers: [ { provide: SEARCH_CONFIG_SERVICE, diff --git a/src/themes/custom/lazy-theme.module.ts b/src/themes/custom/lazy-theme.module.ts index 7015270363..c25460b576 100644 --- a/src/themes/custom/lazy-theme.module.ts +++ b/src/themes/custom/lazy-theme.module.ts @@ -99,7 +99,7 @@ import { import { LoadingComponent } from './app/shared/loading/loading.component'; import { SearchResultsComponent } from './app/shared/search/search-results/search-results.component'; import { AdminSidebarComponent } from './app/admin/admin-sidebar/admin-sidebar.component'; -import { SearchSettingsComponent } from './app/shared/search-settings/search-settings.component'; +import { SearchSettingsComponent } from './app/shared/search/search-settings/search-settings.component'; const DECLARATIONS = [ FileSectionComponent, From f787491af4b2f516d7e31bdd14b8c094ecd383e3 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Tue, 20 Sep 2022 11:33:37 +0200 Subject: [PATCH 14/57] [CST-6153] Fix issue with forgot password page --- src/app/core/eperson/eperson-data.service.spec.ts | 2 +- src/app/core/eperson/eperson-data.service.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/app/core/eperson/eperson-data.service.spec.ts b/src/app/core/eperson/eperson-data.service.spec.ts index ce0b1f3ee4..d8a4a546e6 100644 --- a/src/app/core/eperson/eperson-data.service.spec.ts +++ b/src/app/core/eperson/eperson-data.service.spec.ts @@ -307,7 +307,7 @@ describe('EPersonDataService', () => { it('should sent a patch request with an uuid, token and new password to the epersons endpoint', () => { service.patchPasswordWithToken('test-uuid', 'test-token', 'test-password'); - const operation = Object.assign({ op: 'add', path: '/password', value: 'test-password' }); + const operation = Object.assign({ op: 'add', path: '/password', value: { password: 'test-password' } }); const expected = new PatchRequest(requestService.generateRequestId(), epersonsEndpoint + '/test-uuid?token=test-token', [operation]); expect(requestService.send).toHaveBeenCalledWith(expected); diff --git a/src/app/core/eperson/eperson-data.service.ts b/src/app/core/eperson/eperson-data.service.ts index 8f9312d732..9eab566bf4 100644 --- a/src/app/core/eperson/eperson-data.service.ts +++ b/src/app/core/eperson/eperson-data.service.ts @@ -3,7 +3,10 @@ import { createSelector, select, Store } from '@ngrx/store'; import { Operation } from 'fast-json-patch'; import { Observable } from 'rxjs'; import { find, map, take } from 'rxjs/operators'; -import { EPeopleRegistryCancelEPersonAction, EPeopleRegistryEditEPersonAction } from '../../access-control/epeople-registry/epeople-registry.actions'; +import { + EPeopleRegistryCancelEPersonAction, + EPeopleRegistryEditEPersonAction +} from '../../access-control/epeople-registry/epeople-registry.actions'; import { EPeopleRegistryState } from '../../access-control/epeople-registry/epeople-registry.reducers'; import { AppState } from '../../app.reducer'; import { hasNoValue, hasValue } from '../../shared/empty.util'; @@ -318,7 +321,7 @@ export class EPersonDataService extends IdentifiableDataService impleme patchPasswordWithToken(uuid: string, token: string, password: string): Observable> { const requestId = this.requestService.generateRequestId(); - const operation = Object.assign({ op: 'add', path: '/password', value: password }); + const operation = Object.assign({ op: 'add', path: '/password', value: { 'password': password } }); const hrefObs = this.halService.getEndpoint(this.linkPath).pipe( map((endpoint: string) => this.getIDHref(endpoint, uuid)), From 5099d0b18a4fdd7f0df9a8bae31edb4d558f5210 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Tue, 20 Sep 2022 11:41:09 +0200 Subject: [PATCH 15/57] [CST-6153] Show i18n message instead of server error response --- src/app/profile-page/profile-page.component.ts | 3 ++- src/assets/i18n/en.json5 | 8 +++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/app/profile-page/profile-page.component.ts b/src/app/profile-page/profile-page.component.ts index 5432a0303c..63a3f3f00f 100644 --- a/src/app/profile-page/profile-page.component.ts +++ b/src/app/profile-page/profile-page.component.ts @@ -157,7 +157,8 @@ export class ProfilePageComponent implements OnInit { ); } else { this.notificationsService.error( - this.translate.instant(this.PASSWORD_NOTIFICATIONS_PREFIX + 'error.title'), response.errorMessage + this.translate.instant(this.PASSWORD_NOTIFICATIONS_PREFIX + 'error.title'), + this.translate.instant(this.PASSWORD_NOTIFICATIONS_PREFIX + 'error.change-failed') ); } }); diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 3707903823..0c43ccdc73 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -640,7 +640,7 @@ "bitstream-request-a-copy.alert.canDownload2": "here", "bitstream-request-a-copy.header": "Request a copy of the file", - + "bitstream-request-a-copy.intro": "Enter the following information to request a copy for the following item: ", "bitstream-request-a-copy.intro.bitstream.one": "Requesting the following file: ", @@ -2984,7 +2984,7 @@ "process.detail.create" : "Create similar process", "process.detail.actions": "Actions", - + "process.detail.delete.button": "Delete process", "process.detail.delete.header": "Delete process", @@ -3037,7 +3037,7 @@ "process.bulk.delete.success": "{{count}} process(es) have been succesfully deleted", - + "profile.breadcrumbs": "Update Profile", @@ -3093,6 +3093,8 @@ "profile.security.form.notifications.error.title": "Error changing passwords", + "profile.security.form.notifications.error.change-failed": "An error occurred while trying to change the password. Please check if the current password is correct.", + "profile.security.form.notifications.error.not-long-enough": "The password has to be at least 6 characters long.", "profile.security.form.notifications.error.not-same": "The provided passwords are not the same.", From 2642ed35b78af773324541d2703dba2dc2cb6eff Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 21 Sep 2022 17:47:58 +0200 Subject: [PATCH 16/57] [CST-6153] Use new params name --- src/app/core/eperson/eperson-data.service.spec.ts | 2 +- src/app/core/eperson/eperson-data.service.ts | 2 +- src/app/profile-page/profile-page.component.spec.ts | 4 ++-- src/app/profile-page/profile-page.component.ts | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/app/core/eperson/eperson-data.service.spec.ts b/src/app/core/eperson/eperson-data.service.spec.ts index d8a4a546e6..b4b939eebf 100644 --- a/src/app/core/eperson/eperson-data.service.spec.ts +++ b/src/app/core/eperson/eperson-data.service.spec.ts @@ -307,7 +307,7 @@ describe('EPersonDataService', () => { it('should sent a patch request with an uuid, token and new password to the epersons endpoint', () => { service.patchPasswordWithToken('test-uuid', 'test-token', 'test-password'); - const operation = Object.assign({ op: 'add', path: '/password', value: { password: 'test-password' } }); + const operation = Object.assign({ op: 'add', path: '/password', value: { new_password: 'test-password' } }); const expected = new PatchRequest(requestService.generateRequestId(), epersonsEndpoint + '/test-uuid?token=test-token', [operation]); expect(requestService.send).toHaveBeenCalledWith(expected); diff --git a/src/app/core/eperson/eperson-data.service.ts b/src/app/core/eperson/eperson-data.service.ts index 9eab566bf4..d30030365c 100644 --- a/src/app/core/eperson/eperson-data.service.ts +++ b/src/app/core/eperson/eperson-data.service.ts @@ -321,7 +321,7 @@ export class EPersonDataService extends IdentifiableDataService impleme patchPasswordWithToken(uuid: string, token: string, password: string): Observable> { const requestId = this.requestService.generateRequestId(); - const operation = Object.assign({ op: 'add', path: '/password', value: { 'password': password } }); + const operation = Object.assign({ op: 'add', path: '/password', value: { 'new_password': password } }); const hrefObs = this.halService.getEndpoint(this.linkPath).pipe( map((endpoint: string) => this.getIDHref(endpoint, uuid)), diff --git a/src/app/profile-page/profile-page.component.spec.ts b/src/app/profile-page/profile-page.component.spec.ts index c1bfe8e6bb..709ed56790 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -219,7 +219,7 @@ describe('ProfilePageComponent', () => { component.setCurrentPasswordValue('current-password'); operations = [ - { 'op': 'add', 'path': '/password', 'value': { 'password': 'testest', 'challenge': 'current-password' } } + { 'op': 'add', 'path': '/password', 'value': { 'new_password': 'testest', 'current_password': 'current-password' } } ]; result = component.updateSecurity(); }); @@ -243,7 +243,7 @@ describe('ProfilePageComponent', () => { component.setInvalid(false); component.setCurrentPasswordValue('current-password'); operations = [ - { 'op': 'add', 'path': '/password', 'value': {'password': 'testest', 'challenge': 'current-password' }} + { 'op': 'add', 'path': '/password', 'value': {'new_password': 'testest', 'current_password': 'current-password' }} ]; result = component.updateSecurity(); epersonService.patch(user, operations).subscribe((response) => { diff --git a/src/app/profile-page/profile-page.component.ts b/src/app/profile-page/profile-page.component.ts index 63a3f3f00f..109b2663f4 100644 --- a/src/app/profile-page/profile-page.component.ts +++ b/src/app/profile-page/profile-page.component.ts @@ -147,7 +147,7 @@ export class ProfilePageComponent implements OnInit { } if (!this.invalidSecurity && passEntered) { const operations = [ - { 'op': 'add', 'path': '/password', 'value': { 'password': this.password, 'challenge': this.currentPassword } } + { 'op': 'add', 'path': '/password', 'value': { 'new_password': this.password, 'current_password': this.currentPassword } } ] as Operation[]; this.epersonService.patch(this.currentUser, operations).pipe(getFirstCompletedRemoteData()).subscribe((response: RemoteData) => { if (response.hasSucceeded) { From 68def61f20ab8655b2c5720a36025ea84bcbb0ee Mon Sep 17 00:00:00 2001 From: Yury Bondarenko Date: Wed, 21 Sep 2022 19:20:49 +0200 Subject: [PATCH 17/57] Test invalidation of dependent requests --- src/app/core/data/base/base-data.service.spec.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/app/core/data/base/base-data.service.spec.ts b/src/app/core/data/base/base-data.service.spec.ts index acda298d3b..17532f477a 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -566,7 +566,7 @@ describe('BaseDataService', () => { beforeEach(() => { getByHrefSpy = spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ requestUUIDs: ['request1', 'request2', 'request3'], - dependentRequestUUIDs: [] + dependentRequestUUIDs: ['request4', 'request5'] })); }); @@ -578,6 +578,8 @@ describe('BaseDataService', () => { expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request4'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request5'); done(); }); }); @@ -590,6 +592,8 @@ describe('BaseDataService', () => { expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request4'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request5'); })); it('should return an Observable that only emits true once all requests are stale', () => { @@ -599,9 +603,13 @@ describe('BaseDataService', () => { case 'request1': return cold('--(t|)', BOOLEAN); case 'request2': - return cold('----(t|)', BOOLEAN); - case 'request3': return cold('------(t|)', BOOLEAN); + case 'request3': + return cold('---(t|)', BOOLEAN); + case 'request4': + return cold('-(t|)', BOOLEAN); + case 'request5': + return cold('----(t|)', BOOLEAN); } }); From d4107e3f9aea1761b5c7fb01e14b42f4b0bbf142 Mon Sep 17 00:00:00 2001 From: Michael Spalti Date: Wed, 21 Sep 2022 12:13:10 -0700 Subject: [PATCH 18/57] Initial update for multiple bundle check. --- .../mirador-viewer.component.spec.ts | 5 ++ .../mirador-viewer.component.ts | 10 ++- .../mirador-viewer/mirador-viewer.service.ts | 74 +++++++++++++------ 3 files changed, 63 insertions(+), 26 deletions(-) diff --git a/src/app/item-page/mirador-viewer/mirador-viewer.component.spec.ts b/src/app/item-page/mirador-viewer/mirador-viewer.component.spec.ts index 645d2af91d..40ad0fd5d0 100644 --- a/src/app/item-page/mirador-viewer/mirador-viewer.component.spec.ts +++ b/src/app/item-page/mirador-viewer/mirador-viewer.component.spec.ts @@ -12,6 +12,7 @@ import { createPaginatedList } from '../../shared/testing/utils.test'; import { of as observableOf } from 'rxjs'; import { MiradorViewerService } from './mirador-viewer.service'; import { HostWindowService } from '../../shared/host-window.service'; +import { BundleDataService } from '../../core/data/bundle-data.service'; function getItem(metadata: MetadataMap) { @@ -46,6 +47,7 @@ describe('MiradorViewerComponent with search', () => { declarations: [MiradorViewerComponent], providers: [ { provide: BitstreamDataService, useValue: {} }, + { provide: BundleDataService, useValue: {} }, { provide: HostWindowService, useValue: mockHostWindowService } ], schemas: [NO_ERRORS_SCHEMA] @@ -108,6 +110,7 @@ describe('MiradorViewerComponent with multiple images', () => { declarations: [MiradorViewerComponent], providers: [ { provide: BitstreamDataService, useValue: {} }, + { provide: BundleDataService, useValue: {} }, { provide: HostWindowService, useValue: mockHostWindowService } ], schemas: [NO_ERRORS_SCHEMA] @@ -167,6 +170,7 @@ describe('MiradorViewerComponent with a single image', () => { declarations: [MiradorViewerComponent], providers: [ { provide: BitstreamDataService, useValue: {} }, + { provide: BundleDataService, useValue: {} }, { provide: HostWindowService, useValue: mockHostWindowService } ], schemas: [NO_ERRORS_SCHEMA] @@ -225,6 +229,7 @@ describe('MiradorViewerComponent in development mode', () => { set: { providers: [ { provide: MiradorViewerService, useValue: viewerService }, + { provide: BundleDataService, useValue: {} }, { provide: HostWindowService, useValue: mockHostWindowService } ] } diff --git a/src/app/item-page/mirador-viewer/mirador-viewer.component.ts b/src/app/item-page/mirador-viewer/mirador-viewer.component.ts index 8876d2cea0..bcc50c5ed4 100644 --- a/src/app/item-page/mirador-viewer/mirador-viewer.component.ts +++ b/src/app/item-page/mirador-viewer/mirador-viewer.component.ts @@ -8,6 +8,7 @@ import { map, take } from 'rxjs/operators'; import { isPlatformBrowser } from '@angular/common'; import { MiradorViewerService } from './mirador-viewer.service'; import { HostWindowService, WidthCategory } from '../../shared/host-window.service'; +import { BundleDataService } from '../../core/data/bundle-data.service'; @Component({ selector: 'ds-mirador-viewer', @@ -55,6 +56,7 @@ export class MiradorViewerComponent implements OnInit { constructor(private sanitizer: DomSanitizer, private viewerService: MiradorViewerService, private bitstreamDataService: BitstreamDataService, + private bundleDataService: BundleDataService, private hostWindowService: HostWindowService, @Inject(PLATFORM_ID) private platformId: any) { } @@ -120,8 +122,12 @@ export class MiradorViewerComponent implements OnInit { }) ); } else { - // Sets the multi value based on the image count. - this.iframeViewerUrl = this.viewerService.getImageCount(this.object, this.bitstreamDataService).pipe( + // Sets the multi value based on the image count. Any count greater than 1 + // will add the right thumbnail navigation panel to the viewer. + this.iframeViewerUrl = this.viewerService.getImageCount( + this.object, + this.bitstreamDataService, + this.bundleDataService).pipe( map(c => { if (c > 1) { this.multi = true; diff --git a/src/app/item-page/mirador-viewer/mirador-viewer.service.ts b/src/app/item-page/mirador-viewer/mirador-viewer.service.ts index 4bb095b89f..639c8b3d4e 100644 --- a/src/app/item-page/mirador-viewer/mirador-viewer.service.ts +++ b/src/app/item-page/mirador-viewer/mirador-viewer.service.ts @@ -2,13 +2,15 @@ import { Injectable, isDevMode } from '@angular/core'; import { Observable } from 'rxjs'; import { Item } from '../../core/shared/item.model'; import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../../core/shared/operators'; -import { last, map, switchMap } from 'rxjs/operators'; +import { filter, last, map, switchMap } from 'rxjs/operators'; import { RemoteData } from '../../core/data/remote-data'; import { PaginatedList } from '../../core/data/paginated-list.model'; import { Bitstream } from '../../core/shared/bitstream.model'; import { BitstreamFormat } from '../../core/shared/bitstream-format.model'; import { BitstreamDataService } from '../../core/data/bitstream-data.service'; import { followLink, FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; +import { Bundle } from '../../core/shared/bundle.model'; +import { BundleDataService } from '../../core/data/bundle-data.service'; @Injectable() export class MiradorViewerService { @@ -26,32 +28,56 @@ export class MiradorViewerService { } /** - * Returns observable of the number of images in the ORIGINAL bundle + * Returns observable of the number of images found in eligible IIIF bundles. This checks + * only the first 5 bitstreams in each bundle since any count greater than one is + * enough to set the IIIF viewer to use the "multi" image layout. * @param item * @param bitstreamDataService + * @param bundleDataService */ - getImageCount(item: Item, bitstreamDataService: BitstreamDataService): Observable { + getImageCount(item: Item, bitstreamDataService: BitstreamDataService, bundleDataService: BundleDataService): + Observable { let count = 0; - return bitstreamDataService.findAllByItemAndBundleName(item, 'ORIGINAL', { - currentPage: 1, - elementsPerPage: 10 - }, true, true, ...this.LINKS_TO_FOLLOW) - .pipe( - getFirstCompletedRemoteData(), - map((bitstreamsRD: RemoteData>) => bitstreamsRD.payload), - map((paginatedList: PaginatedList) => paginatedList.page), - switchMap((bitstreams: Bitstream[]) => bitstreams), - switchMap((bitstream: Bitstream) => bitstream.format.pipe( - getFirstSucceededRemoteDataPayload(), - map((format: BitstreamFormat) => format) - )), - map((format: BitstreamFormat) => { - if (format.mimetype.includes('image')) { - count++; - } - return count; - }), - last() - ); + return bundleDataService.findAllByItem(item).pipe( + getFirstCompletedRemoteData(), + map((bundlesRD: RemoteData>) => bundlesRD.payload), + map((paginatedList: PaginatedList) => paginatedList.page), + switchMap((bundles: Bundle[]) => bundles), + filter((b: Bundle) => this.isIiifBundle(b.name)), + switchMap((bundle: Bundle) => { + return bitstreamDataService.findAllByItemAndBundleName(item, bundle.name, { + currentPage: 1, + elementsPerPage: 5 + }, true, true, ...this.LINKS_TO_FOLLOW).pipe( + getFirstCompletedRemoteData(), + map((bitstreamsRD: RemoteData>) => bitstreamsRD.payload), + map((paginatedList: PaginatedList) => paginatedList.page), + switchMap((bitstreams: Bitstream[]) => bitstreams), + switchMap((bitstream: Bitstream) => bitstream.format.pipe( + getFirstSucceededRemoteDataPayload(), + map((format: BitstreamFormat) => format) + )), + map((format: BitstreamFormat) => { + if (format.mimetype.includes('image')) { + count++; + } + return count; + }), + last() + ); + })); } + + isIiifBundle(bundleName: string): boolean { + return !( + bundleName === 'OtherContent' || + bundleName === 'LICENSE' || + bundleName === 'THUMBNAIL' || + bundleName === 'TEXT' || + bundleName === 'METADATA' || + bundleName === 'CC-LICENSE' || + bundleName === 'BRANDED_PREVIEW' + ); + } + } From cc3f570da7357c4cb9d5a3484119c9a6fa8aaad1 Mon Sep 17 00:00:00 2001 From: Michael Spalti Date: Wed, 21 Sep 2022 12:16:22 -0700 Subject: [PATCH 19/57] Updated comments. --- .../mirador-viewer/mirador-viewer.component.ts | 12 ++++++------ .../mirador-viewer/mirador-viewer.service.ts | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/app/item-page/mirador-viewer/mirador-viewer.component.ts b/src/app/item-page/mirador-viewer/mirador-viewer.component.ts index bcc50c5ed4..fee8046272 100644 --- a/src/app/item-page/mirador-viewer/mirador-viewer.component.ts +++ b/src/app/item-page/mirador-viewer/mirador-viewer.component.ts @@ -109,10 +109,10 @@ export class MiradorViewerComponent implements OnInit { this.notMobile = !(category === WidthCategory.XS || category === WidthCategory.SM); }); - // We need to set the multi property to true if the - // item is searchable or when the ORIGINAL bundle contains more - // than 1 image. (The multi property determines whether the - // Mirador side thumbnail navigation panel is shown.) + // Set the multi property. The default mirador configuration adds a right + // thumbnail navigation panel to the viewer when multi is 'true'. + + // Set the multi property to 'true' if the item is searchable. if (this.searchable) { this.multi = true; const observable = of(''); @@ -122,8 +122,8 @@ export class MiradorViewerComponent implements OnInit { }) ); } else { - // Sets the multi value based on the image count. Any count greater than 1 - // will add the right thumbnail navigation panel to the viewer. + // Set the multi property based on the image count in IIIF-eligible bundles. + // Any count greater than 1 sets the value to 'true'. this.iframeViewerUrl = this.viewerService.getImageCount( this.object, this.bitstreamDataService, diff --git a/src/app/item-page/mirador-viewer/mirador-viewer.service.ts b/src/app/item-page/mirador-viewer/mirador-viewer.service.ts index 639c8b3d4e..68be928cfa 100644 --- a/src/app/item-page/mirador-viewer/mirador-viewer.service.ts +++ b/src/app/item-page/mirador-viewer/mirador-viewer.service.ts @@ -28,12 +28,12 @@ export class MiradorViewerService { } /** - * Returns observable of the number of images found in eligible IIIF bundles. This checks - * only the first 5 bitstreams in each bundle since any count greater than one is - * enough to set the IIIF viewer to use the "multi" image layout. + * Returns observable of the number of images found in eligible IIIF bundles. Checks + * the mimetype of the first 5 bitstreams in each bundle. * @param item * @param bitstreamDataService * @param bundleDataService + * @returns the total image count */ getImageCount(item: Item, bitstreamDataService: BitstreamDataService, bundleDataService: BundleDataService): Observable { From 9de6f38ea62a0d1aff76d7b5a4a0988e2460f632 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Fri, 23 Sep 2022 10:37:52 +0200 Subject: [PATCH 20/57] [CST-6153] fix merge --- src/assets/i18n/en.json5 | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index bcb5615d1a..df5fd27062 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3091,8 +3091,6 @@ "profile.security.form.notifications.error.change-failed": "An error occurred while trying to change the password. Please check if the current password is correct.", - "profile.security.form.notifications.error.not-long-enough": "The password has to be at least 6 characters long.", - "profile.security.form.notifications.error.not-same": "The provided passwords are not the same.", "profile.security.form.notifications.error.general": "Please fill required fields of security form.", From 2f71dc358b60bfc78d02e079649e361dc64df1bd Mon Sep 17 00:00:00 2001 From: Samuel Cambien Date: Fri, 23 Sep 2022 10:27:27 +0200 Subject: [PATCH 21/57] issue #1404, issue #1762, issue #1763 Add support for line breaks, markdown and mathjax in metadata --- package.json | 3 + .../edit-in-place-field.component.html | 2 +- .../metadata-values.component.html | 19 +- .../metadata-values.component.spec.ts | 2 +- .../metadata-values.component.ts | 19 +- ...item-page-abstract-field.component.spec.ts | 16 +- .../item-page-abstract-field.component.ts | 1 + .../item-page-field.component.html | 7 +- .../item-page-field.component.spec.ts | 82 +++++- .../item-page-field.component.ts | 5 + src/app/shared/shared.module.ts | 4 +- .../truncatable-part.component.html | 2 +- src/app/shared/utils/markdown.pipe.spec.ts | 66 +++++ src/app/shared/utils/markdown.pipe.ts | 70 +++++ src/config/app-config.interface.ts | 2 + src/config/default-app-config.ts | 4 + src/environments/environment.test.ts | 4 +- src/styles/_global-styles.scss | 4 + yarn.lock | 277 +++++++++++++++++- 19 files changed, 562 insertions(+), 27 deletions(-) create mode 100644 src/app/shared/utils/markdown.pipe.spec.ts create mode 100644 src/app/shared/utils/markdown.pipe.ts diff --git a/package.json b/package.json index 278afdf6c3..5c53b41c7f 100644 --- a/package.json +++ b/package.json @@ -97,6 +97,8 @@ "jwt-decode": "^3.1.2", "klaro": "^0.7.10", "lodash": "^4.17.21", + "markdown-it": "^13.0.1", + "markdown-it-mathjax3": "^4.3.1", "mirador": "^3.3.0", "mirador-dl-plugin": "^0.13.0", "mirador-share-plugin": "^0.11.0", @@ -114,6 +116,7 @@ "postcss-cli": "^8.3.0", "reflect-metadata": "^0.1.13", "rxjs": "^6.6.3", + "sanitize-html": "^2.7.2", "sortablejs": "1.13.0", "tslib": "^2.0.0", "url-parse": "^1.5.3", diff --git a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html index f5543af971..46299c1b08 100644 --- a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html +++ b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html @@ -26,7 +26,7 @@
- {{metadata?.value}} + {{metadata?.value}}