hazza/dspace-angular
1
0
Fork 1
mirror of https://github.com/DSpace/dspace-angular.git synced 2025-10-07 01:54:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3716 from alexandrevryghem/w2p-120109_fix-findByHref-and-findListByHref-skipping-their-response_contribute-7_x

Browse Source 
[Port dspace-7_x] Fix infinite loading on item pages and optimize menu resolver usage
This commit is contained in:
Tim Donohue
2024-12-18 11:17:35 -06:00
committed by GitHub
parent 36dd4762ed 6988df519e
commit 2ff5350302
6 changed files with 76 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/app/collection-page/collection-page-routing.module.ts
View File

@@ -55,7 +55,6 @@ import { CommunityBreadcrumbResolver } from '../core/breadcrumbs/community-bread
resolve: {
dso: CollectionPageResolver,
breadcrumb: CollectionBreadcrumbResolver,
menu: DSOEditMenuResolver
},
runGuardsAndResolvers: 'always',
children: [
@@ -85,6 +84,9 @@ import { CommunityBreadcrumbResolver } from '../core/breadcrumbs/community-bread
path: '',
component: ThemedCollectionPageComponent,
pathMatch: 'full',
resolve: {
menu: DSOEditMenuResolver,
},
}
],
data: {

4
src/app/community-page/community-page-routing.module.ts
View File

@@ -48,7 +48,6 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso
resolve: {
dso: CommunityPageResolver,
breadcrumb: CommunityBreadcrumbResolver,
menu: DSOEditMenuResolver
},
runGuardsAndResolvers: 'always',
children: [
@@ -68,6 +67,9 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso
path: '',
component: ThemedCommunityPageComponent,
pathMatch: 'full',
resolve: {
menu: DSOEditMenuResolver,
},
}
],
data: {

71
src/app/core/data/base/base-data.service.spec.ts
View File

@@ -50,6 +50,7 @@ describe('BaseDataService', () => {
let selfLink;
let linksToFollow;
let testScheduler;
let remoteDataTimestamp: number;
let remoteDataMocks;
function initTestService(): TestService {
@@ -86,20 +87,22 @@ describe('BaseDataService', () => {
expect(actual).toEqual(expected);
});
const timeStamp = new Date().getTime();
// The response's lastUpdated equals the time of 60 seconds after the test started, ensuring they are not perceived
// as cached values.
remoteDataTimestamp = new Date().getTime() + 60 * 1000;
const msToLive = 15 * 60 * 1000;
const payload = { foo: 'bar' };
const statusCodeSuccess = 200;
const statusCodeError = 404;
const errorMessage = 'not found';
remoteDataMocks = {
RequestPending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.RequestPending, undefined, undefined, undefined),
ResponsePending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.ResponsePending, undefined, undefined, undefined),
ResponsePendingStale: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.ResponsePendingStale, undefined, undefined, undefined),
Success: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.Success, undefined, payload, statusCodeSuccess),
SuccessStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.SuccessStale, undefined, payload, statusCodeSuccess),
Error: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.Error, errorMessage, undefined, statusCodeError),
ErrorStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.ErrorStale, errorMessage, undefined, statusCodeError),
RequestPending: new RemoteData(undefined, msToLive, remoteDataTimestamp, RequestEntryState.RequestPending, undefined, undefined, undefined),
ResponsePending: new RemoteData(undefined, msToLive, remoteDataTimestamp, RequestEntryState.ResponsePending, undefined, undefined, undefined),
ResponsePendingStale: new RemoteData(undefined, msToLive, remoteDataTimestamp, RequestEntryState.ResponsePendingStale, undefined, undefined, undefined),
Success: new RemoteData(remoteDataTimestamp, msToLive, remoteDataTimestamp, RequestEntryState.Success, undefined, payload, statusCodeSuccess),
SuccessStale: new RemoteData(remoteDataTimestamp, msToLive, remoteDataTimestamp, RequestEntryState.SuccessStale, undefined, payload, statusCodeSuccess),
Error: new RemoteData(remoteDataTimestamp, msToLive, remoteDataTimestamp, RequestEntryState.Error, errorMessage, undefined, statusCodeError),
ErrorStale: new RemoteData(remoteDataTimestamp, msToLive, remoteDataTimestamp, RequestEntryState.ErrorStale, errorMessage, undefined, statusCodeError),
};
return new TestService(
@@ -333,11 +336,15 @@ describe('BaseDataService', () => {
spyOn(service as any, 'reRequestStaleRemoteData').and.callFake(() => (source) => source);
});
it(`should not emit a cached completed RemoteData, but only start emitting after the state first changes to RequestPending`, () => {
it('should not emit a cached completed RemoteData', () => {
// Old cached value from 1 minute before the test started
const oldCachedSucceededData: RemoteData<any> = Object.assign({}, remoteDataMocks.Success, {
timeCompleted: remoteDataTimestamp - 2 * 60 * 1000,
lastUpdated: remoteDataTimestamp - 2 * 60 * 1000,
} as RemoteData<any>);
testScheduler.run(({ cold, expectObservable }) => {
spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e', {
a: remoteDataMocks.Success,
a: oldCachedSucceededData,
b: remoteDataMocks.RequestPending,
c: remoteDataMocks.ResponsePending,
d: remoteDataMocks.Success,
@@ -355,6 +362,22 @@ describe('BaseDataService', () => {
});
});
it('should emit the first completed RemoteData since the request was made', () => {
testScheduler.run(({ cold, expectObservable }) => {
spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b', {
a: remoteDataMocks.Success,
b: remoteDataMocks.SuccessStale,
}));
const expected = 'a-b';
const values = {
a: remoteDataMocks.Success,
b: remoteDataMocks.SuccessStale,
};
expectObservable(service.findByHref(selfLink, false, true, ...linksToFollow)).toBe(expected, values);
});
});
it(`should not emit a cached stale RemoteData, but only start emitting after the state first changes to RequestPending`, () => {
testScheduler.run(({ cold, expectObservable }) => {
spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e-f-g', {
@@ -521,11 +544,15 @@ describe('BaseDataService', () => {
spyOn(service as any, 'reRequestStaleRemoteData').and.callFake(() => (source) => source);
});
it(`should not emit a cached completed RemoteData, but only start emitting after the state first changes to RequestPending`, () => {
it('should not emit a cached completed RemoteData', () => {
testScheduler.run(({ cold, expectObservable }) => {
// Old cached value from 1 minute before the test started
const oldCachedSucceededData: RemoteData<any> = Object.assign({}, remoteDataMocks.Success, {
timeCompleted: remoteDataTimestamp - 2 * 60 * 1000,
lastUpdated: remoteDataTimestamp - 2 * 60 * 1000,
} as RemoteData<any>);
spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e', {
a: remoteDataMocks.Success,
a: oldCachedSucceededData,
b: remoteDataMocks.RequestPending,
c: remoteDataMocks.ResponsePending,
d: remoteDataMocks.Success,
@@ -543,6 +570,22 @@ describe('BaseDataService', () => {
});
});
it('should emit the first completed RemoteData since the request was made', () => {
testScheduler.run(({ cold, expectObservable }) => {
spyOn(rdbService, 'buildList').and.returnValue(cold('a-b', {
a: remoteDataMocks.Success,
b: remoteDataMocks.SuccessStale,
}));
const expected = 'a-b';
const values = {
a: remoteDataMocks.Success,
b: remoteDataMocks.SuccessStale,
};
expectObservable(service.findListByHref(selfLink, findListOptions, false, true, ...linksToFollow)).toBe(expected, values);
});
});
it(`should not emit a cached stale RemoteData, but only start emitting after the state first changes to RequestPending`, () => {
testScheduler.run(({ cold, expectObservable }) => {
spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e-f-g', {

6
src/app/core/data/base/base-data.service.ts
View File

@@ -266,6 +266,7 @@ export class BaseDataService<T extends CacheableObject> implements HALDataServic
map((href: string) => this.buildHrefFromFindOptions(href, {}, [], ...linksToFollow)),
);
const startTime: number = new Date().getTime();
this.createAndSendGetRequest(requestHref$, useCachedVersionIfAvailable);
return this.rdbService.buildSingle<T>(requestHref$, ...linksToFollow).pipe(
@@ -273,7 +274,7 @@ export class BaseDataService<T extends CacheableObject> implements HALDataServic
// call it isn't immediately returned, but we wait until the remote data for the new request
// is created. If useCachedVersionIfAvailable is false it also ensures you don't get a
// cached completed object
skipWhile((rd: RemoteData<T>) => rd.isStale || (!useCachedVersionIfAvailable && rd.hasCompleted)),
skipWhile((rd: RemoteData<T>) => rd.isStale || (!useCachedVersionIfAvailable && rd.lastUpdated < startTime)),
this.reRequestStaleRemoteData(reRequestOnStale, () =>
this.findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)),
);
@@ -300,6 +301,7 @@ export class BaseDataService<T extends CacheableObject> implements HALDataServic
map((href: string) => this.buildHrefFromFindOptions(href, options, [], ...linksToFollow)),
);
const startTime: number = new Date().getTime();
this.createAndSendGetRequest(requestHref$, useCachedVersionIfAvailable);
return this.rdbService.buildList<T>(requestHref$, ...linksToFollow).pipe(
@@ -307,7 +309,7 @@ export class BaseDataService<T extends CacheableObject> implements HALDataServic
// call it isn't immediately returned, but we wait until the remote data for the new request
// is created. If useCachedVersionIfAvailable is false it also ensures you don't get a
// cached completed object
skipWhile((rd: RemoteData<PaginatedList<T>>) => rd.isStale || (!useCachedVersionIfAvailable && rd.hasCompleted)),
skipWhile((rd: RemoteData<PaginatedList<T>>) => rd.isStale || (!useCachedVersionIfAvailable && rd.lastUpdated < startTime)),
this.reRequestStaleRemoteData(reRequestOnStale, () =>
this.findListByHref(href$, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)),
);

6
src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts
View File

@@ -6,7 +6,7 @@ import { ObjectUpdatesService } from '../../../core/data/object-updates/object-u
import { ActivatedRoute, Router, Data } from '@angular/router';
import { NotificationsService } from '../../../shared/notifications/notifications.service';
import { TranslateService } from '@ngx-translate/core';
import { first, map, switchMap, tap } from 'rxjs/operators';
import { take, map, switchMap, tap } from 'rxjs/operators';
import { RemoteData } from '../../../core/data/remote-data';
import { AbstractTrackableComponent } from '../../../shared/trackable/abstract-trackable.component';
import { environment } from '../../../../environments/environment';
@@ -82,7 +82,7 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
super.ngOnInit();
this.discardTimeOut = environment.item.edit.undoTimeout;
this.hasChanges().pipe(first()).subscribe((hasChanges) => {
this.hasChanges().pipe(take(1)).subscribe((hasChanges) => {
if (!hasChanges) {
this.initializeOriginalFields();
} else {
@@ -167,7 +167,7 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
*/
private checkLastModified() {
const currentVersion = this.item.lastModified;
this.objectUpdatesService.getLastModified(this.url).pipe(first()).subscribe(
this.objectUpdatesService.getLastModified(this.url).pipe(take(1)).subscribe(
(updateVersion: Date) => {
if (updateVersion.getDate() !== currentVersion.getDate()) {
this.notificationsService.warning(this.getNotificationTitle('outdated'), this.getNotificationContent('outdated'));

7
src/app/item-page/item-page-routing.module.ts
View File

@@ -28,7 +28,6 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver';
resolve: {
dso: ItemPageResolver,
breadcrumb: ItemBreadcrumbResolver,
menu: DSOEditMenuResolver
},
runGuardsAndResolvers: 'always',
children: [
@@ -36,10 +35,16 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver';
path: '',
component: ThemedItemPageComponent,
pathMatch: 'full',
resolve: {
menu: DSOEditMenuResolver,
},
},
{
path: 'full',
component: ThemedFullItemPageComponent,
resolve: {
menu: DSOEditMenuResolver,
},
},
{
path: ITEM_EDIT_PATH,