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

Merge pull request #3715 from DSpace/backport-3677-to-dspace-8_x

Browse Source 
[Port dspace-8_x] Fix infinite loading on item pages and optimize menu resolver usage
This commit is contained in:
Tim Donohue
2024-12-05 16:48:52 -06:00
committed by GitHub
parent f47bf4665e 970544c130
commit 50885676d8
9 changed files with 87 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
cypress/e2e/item-edit.cy.ts
View File

@@ -9,11 +9,6 @@ beforeEach(() => {
// This page is restricted, so we will be shown the login form. Fill it out & submit.
cy.loginViaForm(Cypress.env('DSPACE_TEST_ADMIN_USER'), Cypress.env('DSPACE_TEST_ADMIN_PASSWORD'));
// We need to wait for the correction types allowed for the item to be loaded to be sure that each tab is fully loaded.
// This because the edit item page causes often tests to fails due to timeout.
cy.intercept('GET', 'server/api/config/correctiontypes/search/findByItem*').as('correctionTypes');
cy.wait('@correctionTypes');
});
describe('Edit Item > Edit Metadata tab', () => {

2
src/app/browse-by/browse-by-page-routes.ts
View File

@@ -1,6 +1,5 @@
import { Route } from '@angular/router';
import { dsoEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver';
import { browseByDSOBreadcrumbResolver } from './browse-by-dso-breadcrumb.resolver';
import { browseByGuard } from './browse-by-guard';
import { browseByI18nBreadcrumbResolver } from './browse-by-i18n-breadcrumb.resolver';
@@ -11,7 +10,6 @@ export const ROUTES: Route[] = [
path: '',
resolve: {
breadcrumb: browseByDSOBreadcrumbResolver,
menu: dsoEditMenuResolver,
},
children: [
{

4
src/app/collection-page/collection-page-routes.ts
View File

@@ -54,7 +54,6 @@ export const ROUTES: Route[] = [
resolve: {
dso: collectionPageResolver,
breadcrumb: collectionBreadcrumbResolver,
menu: dsoEditMenuResolver,
},
runGuardsAndResolvers: 'always',
children: [
@@ -83,6 +82,9 @@ export const ROUTES: Route[] = [
{
path: '',
component: ThemedCollectionPageComponent,
resolve: {
menu: dsoEditMenuResolver,
},
children: [
{
path: '',

4
src/app/community-page/community-page-routes.ts
View File

@@ -51,7 +51,6 @@ export const ROUTES: Route[] = [
resolve: {
dso: communityPageResolver,
breadcrumb: communityBreadcrumbResolver,
menu: dsoEditMenuResolver,
},
runGuardsAndResolvers: 'always',
children: [
@@ -70,6 +69,9 @@ export const ROUTES: Route[] = [
{
path: '',
component: ThemedCommunityPageComponent,
resolve: {
menu: dsoEditMenuResolver,
},
children: [
{
path: '',

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

@@ -65,6 +65,7 @@ describe('BaseDataService', () => {
let selfLink;
let linksToFollow;
let testScheduler;
let remoteDataTimestamp: number;
let remoteDataMocks: { [responseType: string]: RemoteData<any> };
let remoteDataPageMocks: { [responseType: string]: RemoteData<any> };
@@ -85,7 +86,9 @@ 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',
@@ -112,22 +115,22 @@ describe('BaseDataService', () => {
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),
};
remoteDataPageMocks = {
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, createPaginatedList([payload]), statusCodeSuccess),
SuccessStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.SuccessStale, undefined, createPaginatedList([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, createPaginatedList([payload]), statusCodeSuccess),
SuccessStale: new RemoteData(remoteDataTimestamp, msToLive, remoteDataTimestamp, RequestEntryState.SuccessStale, undefined, createPaginatedList([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(
@@ -361,11 +364,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({}, remoteDataPageMocks.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,
@@ -383,6 +390,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', {
@@ -411,17 +434,12 @@ describe('BaseDataService', () => {
it('should link all the followLinks of a cached object by calling addDependency', () => {
spyOn(objectCache, 'addDependency').and.callThrough();
testScheduler.run(({ cold, expectObservable, flush }) => {
spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d', {
spyOn(rdbService, 'buildSingle').and.returnValue(cold('a', {
a: remoteDataMocks.Success,
b: remoteDataMocks.RequestPending,
c: remoteDataMocks.ResponsePending,
d: remoteDataMocks.Success,
}));
const expected = '--b-c-d';
const expected = 'a';
const values = {
b: remoteDataMocks.RequestPending,
c: remoteDataMocks.ResponsePending,
d: remoteDataMocks.Success,
a: remoteDataMocks.Success,
};
expectObservable(service.findByHref(selfLink, false, false, ...linksToFollow)).toBe(expected, values);
@@ -570,11 +588,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({}, remoteDataPageMocks.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: remoteDataPageMocks.Success,
a: oldCachedSucceededData,
b: remoteDataPageMocks.RequestPending,
c: remoteDataPageMocks.ResponsePending,
d: remoteDataPageMocks.Success,
@@ -592,6 +614,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: remoteDataPageMocks.Success,
b: remoteDataPageMocks.SuccessStale,
}));
const expected = 'a-b';
const values = {
a: remoteDataPageMocks.Success,
b: remoteDataPageMocks.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

@@ -285,6 +285,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);
const response$: Observable<RemoteData<T>> = this.rdbService.buildSingle<T>(requestHref$, ...linksToFollow).pipe(
@@ -292,7 +293,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)),
);
@@ -338,6 +339,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);
const response$: Observable<RemoteData<PaginatedList<T>>> = this.rdbService.buildList<T>(requestHref$, ...linksToFollow).pipe(
@@ -345,7 +347,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

@@ -16,9 +16,9 @@ import {
Subscription,
} from 'rxjs';
import {
first,
map,
switchMap,
take,
tap,
} from 'rxjs/operators';
@@ -102,7 +102,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 {
@@ -187,7 +187,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-routes.ts
View File

@@ -27,7 +27,6 @@ export const ROUTES: Route[] = [
resolve: {
dso: itemPageResolver,
breadcrumb: itemBreadcrumbResolver,
menu: dsoEditMenuResolver,
},
runGuardsAndResolvers: 'always',
children: [
@@ -35,10 +34,16 @@ export const ROUTES: Route[] = [
path: '',
component: ThemedItemPageComponent,
pathMatch: 'full',
resolve: {
menu: dsoEditMenuResolver,
},
},
{
path: 'full',
component: ThemedFullItemPageComponent,
resolve: {
menu: dsoEditMenuResolver,
},
},
{
path: ITEM_EDIT_PATH,

2
src/app/shared/dso-page/dso-edit-menu-resolver.service.ts
View File

@@ -155,7 +155,7 @@ export class DSOEditMenuResolverService {
this.dsoVersioningModalService.getVersioningTooltipMessage(dso, 'item.page.version.hasDraft', 'item.page.version.create'),
this.authorizationService.isAuthorized(FeatureID.CanSynchronizeWithORCID, dso.self),
this.authorizationService.isAuthorized(FeatureID.CanClaimItem, dso.self),
this.correctionTypeDataService.findByItem(dso.uuid, false).pipe(
this.correctionTypeDataService.findByItem(dso.uuid, true).pipe(
getFirstCompletedRemoteData(),
getRemoteDataPayload()),
]).pipe(