From 430ee3846a2446e16914764e71c1d0c5e739ae2c Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 23 Nov 2023 21:04:02 +0100 Subject: [PATCH 01/23] Fixed ePerson link on edit group page --- .../group-form/members-list/members-list.component.html | 6 ++---- .../members-list/members-list.component.spec.ts | 3 --- .../group-form/members-list/members-list.component.ts | 3 +++ src/app/core/eperson/eperson-data.service.ts | 9 +-------- .../reviewers-list/reviewers-list.component.spec.ts | 3 --- 5 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html index c0c77f44eb..1e61f2cdb2 100644 --- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html +++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html @@ -24,8 +24,7 @@ {{eperson.id}} - + {{ dsoNameService.getName(eperson) }} @@ -106,8 +105,7 @@ {{eperson.id}} - + {{ dsoNameService.getName(eperson) }} diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts index 5d97dcade8..99ee9827e8 100644 --- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts +++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts @@ -68,9 +68,6 @@ describe('MembersListComponent', () => { clearLinkRequests() { // empty }, - getEPeoplePageRouterLink(): string { - return '/access-control/epeople'; - } }; groupsDataServiceStub = { activeGroup: activeGroup, diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts index feb90b52b3..6129d4d02d 100644 --- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts +++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts @@ -23,6 +23,7 @@ import { NotificationsService } from '../../../../shared/notifications/notificat import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model'; import { PaginationService } from '../../../../core/pagination/pagination.service'; import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service'; +import { getEPersonEditRoute } from '../../../access-control-routing-paths'; /** * Keys to keep track of specific subscriptions @@ -131,6 +132,8 @@ export class MembersListComponent implements OnInit, OnDestroy { // current active group being edited groupBeingEdited: Group; + readonly getEPersonEditRoute = getEPersonEditRoute; + constructor( protected groupDataService: GroupDataService, public ePersonDataService: EPersonDataService, diff --git a/src/app/core/eperson/eperson-data.service.ts b/src/app/core/eperson/eperson-data.service.ts index fb8b8bc9b0..a85d471e7d 100644 --- a/src/app/core/eperson/eperson-data.service.ts +++ b/src/app/core/eperson/eperson-data.service.ts @@ -34,7 +34,7 @@ import { PatchData, PatchDataImpl } from '../data/base/patch-data'; import { DeleteData, DeleteDataImpl } from '../data/base/delete-data'; import { RestRequestMethod } from '../data/rest-request-method'; import { dataService } from '../data/base/data-service.decorator'; -import { getEPersonEditRoute, getEPersonsRoute } from '../../access-control/access-control-routing-paths'; +import { getEPersonEditRoute } from '../../access-control/access-control-routing-paths'; const ePeopleRegistryStateSelector = (state: AppState) => state.epeopleRegistry; const editEPersonSelector = createSelector(ePeopleRegistryStateSelector, (ePeopleRegistryState: EPeopleRegistryState) => ePeopleRegistryState.editEPerson); @@ -313,13 +313,6 @@ export class EPersonDataService extends IdentifiableDataService impleme return getEPersonEditRoute(ePerson.id); } - /** - * Get EPeople admin page - */ - public getEPeoplePageRouterLink(): string { - return getEPersonsRoute(); - } - /** * Create a new EPerson using a token * @param eperson diff --git a/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.spec.ts b/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.spec.ts index bcbeef5683..91e200e85d 100644 --- a/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.spec.ts +++ b/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.spec.ts @@ -72,9 +72,6 @@ describe('ReviewersListComponent', () => { clearLinkRequests() { // empty }, - getEPeoplePageRouterLink(): string { - return '/access-control/epeople'; - } }; groupsDataServiceStub = { activeGroup: activeGroup, From 09fc44a539477df9e67809921d9ddf1b22198adc Mon Sep 17 00:00:00 2001 From: Thomas Misilo Date: Tue, 5 Dec 2023 10:02:17 -0600 Subject: [PATCH 02/23] Decrease min-height for Login Dropdown Menu Fixes #2690 This is noticable when you have disabled local authentication, and only have a singular remote authentication such as Shibboleth or Orcid. --- src/app/shared/auth-nav-menu/auth-nav-menu.component.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.scss b/src/app/shared/auth-nav-menu/auth-nav-menu.component.scss index 40e9113b00..3f6b2b009c 100644 --- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.scss +++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.scss @@ -4,7 +4,7 @@ } .loginDropdownMenu { - min-height: 260px; + min-height: 75px; } .dropdown-item.active, .dropdown-item:active, From 7bedf7fc59983983bfdd1061db8d0ae4a5822f12 Mon Sep 17 00:00:00 2001 From: Alan Orth Date: Wed, 20 Dec 2023 17:45:35 +0300 Subject: [PATCH 03/23] src/app/community-list-page: remove newlines in links The newlines cause whitespace to be enapsulated in the rendered link. --- .../community-list/community-list.component.html | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/app/community-list-page/community-list/community-list.component.html b/src/app/community-list-page/community-list/community-list.component.html index de67607bb4..eed9ca64ca 100644 --- a/src/app/community-list-page/community-list/community-list.component.html +++ b/src/app/community-list-page/community-list/community-list.component.html @@ -35,9 +35,7 @@
- - {{ dsoNameService.getName(node.payload) }} - + {{ dsoNameService.getName(node.payload) }}   {{node.payload.archivedItemsCount}} @@ -73,9 +71,7 @@
- - {{ dsoNameService.getName(node.payload) }} - + {{ dsoNameService.getName(node.payload) }}
From 7207bbbd0e647693148438f49cb7bdbd7ca3fe86 Mon Sep 17 00:00:00 2001 From: Alan Orth Date: Wed, 20 Dec 2023 22:32:22 +0300 Subject: [PATCH 04/23] src/app/item-page: remove newlines in links The newlines cause whitespace to be enapsulated in the rendered link. --- .../metadata-values/metadata-values.component.html | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/app/item-page/field-components/metadata-values/metadata-values.component.html b/src/app/item-page/field-components/metadata-values/metadata-values.component.html index 61088edd16..a598815d4c 100644 --- a/src/app/item-page/field-components/metadata-values/metadata-values.component.html +++ b/src/app/item-page/field-components/metadata-values/metadata-values.component.html @@ -32,7 +32,5 @@ - {{value}} - + [queryParams]="getQueryParams(value)">{{value}} From bd6c99da2e68653d29373a9b41a7190cd7256c78 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 29 Nov 2023 14:25:36 +0100 Subject: [PATCH 05/23] ensure HALEndpointService doesn't use stale requests (cherry picked from commit 38752d9d71b2441b83bbcb616347df66b7fb5e75) --- src/app/core/server-check/server-check.guard.spec.ts | 10 ++++++---- src/app/core/server-check/server-check.guard.ts | 6 ++---- src/app/core/shared/hal-endpoint.service.ts | 12 ++++++++---- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/app/core/server-check/server-check.guard.spec.ts b/src/app/core/server-check/server-check.guard.spec.ts index 044609ef42..f65a7deca7 100644 --- a/src/app/core/server-check/server-check.guard.spec.ts +++ b/src/app/core/server-check/server-check.guard.spec.ts @@ -9,7 +9,7 @@ import SpyObj = jasmine.SpyObj; describe('ServerCheckGuard', () => { let guard: ServerCheckGuard; let router: Router; - const eventSubject = new ReplaySubject(1); + let eventSubject: ReplaySubject; let rootDataServiceStub: SpyObj; let testScheduler: TestScheduler; let redirectUrlTree: UrlTree; @@ -24,6 +24,7 @@ describe('ServerCheckGuard', () => { findRoot: jasmine.createSpy('findRoot') }); redirectUrlTree = new UrlTree(); + eventSubject = new ReplaySubject(1); router = { events: eventSubject.asObservable(), navigateByUrl: jasmine.createSpy('navigateByUrl'), @@ -64,10 +65,10 @@ describe('ServerCheckGuard', () => { }); describe(`listenForRouteChanges`, () => { - it(`should retrieve the root endpoint, without using the cache, when the method is first called`, () => { + it(`should invalidate the root cache, when the method is first called`, () => { testScheduler.run(() => { guard.listenForRouteChanges(); - expect(rootDataServiceStub.findRoot).toHaveBeenCalledWith(false); + expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalledTimes(1); }); }); @@ -80,7 +81,8 @@ describe('ServerCheckGuard', () => { eventSubject.next(new NavigationEnd(2,'', '')); eventSubject.next(new NavigationStart(3,'')); }); - expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalledTimes(3); + // once when the method is first called, and then 3 times for NavigationStart events + expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalledTimes(1 + 3); }); }); }); diff --git a/src/app/core/server-check/server-check.guard.ts b/src/app/core/server-check/server-check.guard.ts index 65ca2b0c49..79c34c3659 100644 --- a/src/app/core/server-check/server-check.guard.ts +++ b/src/app/core/server-check/server-check.guard.ts @@ -53,10 +53,8 @@ export class ServerCheckGuard implements CanActivateChild { */ listenForRouteChanges(): void { // we'll always be too late for the first NavigationStart event with the router subscribe below, - // so this statement is for the very first route operation. A `find` without using the cache, - // rather than an invalidateRootCache, because invalidating as the app is bootstrapping can - // break other features - this.rootDataService.findRoot(false); + // so this statement is for the very first route operation. + this.rootDataService.invalidateRootCache(); this.router.events.pipe( filter(event => event instanceof NavigationStart), diff --git a/src/app/core/shared/hal-endpoint.service.ts b/src/app/core/shared/hal-endpoint.service.ts index 8b6316a6ce..98ab6a16ea 100644 --- a/src/app/core/shared/hal-endpoint.service.ts +++ b/src/app/core/shared/hal-endpoint.service.ts @@ -1,5 +1,5 @@ import { Observable } from 'rxjs'; -import { distinctUntilChanged, map, startWith, switchMap, take } from 'rxjs/operators'; +import { distinctUntilChanged, map, startWith, switchMap, take, skipWhile } from 'rxjs/operators'; import { RequestService } from '../data/request.service'; import { EndpointMapRequest } from '../data/request.models'; import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; @@ -9,7 +9,7 @@ import { EndpointMap } from '../cache/response.models'; import { getFirstCompletedRemoteData } from './operators'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RemoteData } from '../data/remote-data'; -import { UnCacheableObject } from './uncacheable-object.model'; +import { CacheableObject } from '../cache/cacheable-object.model'; @Injectable() export class HALEndpointService { @@ -33,9 +33,13 @@ export class HALEndpointService { this.requestService.send(request, true); - return this.rdbService.buildFromHref(href).pipe( + return this.rdbService.buildFromHref(href).pipe( + // This skip ensures that if a stale object is present in the cache when you do a + // call it isn't immediately returned, but we wait until the remote data for the new request + // is created. + skipWhile((rd: RemoteData) => rd.isStale), getFirstCompletedRemoteData(), - map((response: RemoteData) => { + map((response: RemoteData) => { if (hasValue(response.payload)) { return response.payload._links; } else { From d109a5aeb2e8bb3bbee79029e840bc46f9293e08 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Fri, 1 Dec 2023 16:19:12 +0100 Subject: [PATCH 06/23] also skip loading hal requests (cherry picked from commit c8ac260b783d6bd9426325f7e2893e4b4f8d6f0d) --- src/app/core/shared/hal-endpoint.service.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/app/core/shared/hal-endpoint.service.ts b/src/app/core/shared/hal-endpoint.service.ts index 98ab6a16ea..5cdd7ccfad 100644 --- a/src/app/core/shared/hal-endpoint.service.ts +++ b/src/app/core/shared/hal-endpoint.service.ts @@ -6,7 +6,6 @@ import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; import { Injectable } from '@angular/core'; import { EndpointMap } from '../cache/response.models'; -import { getFirstCompletedRemoteData } from './operators'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RemoteData } from '../data/remote-data'; import { CacheableObject } from '../cache/cacheable-object.model'; @@ -37,8 +36,8 @@ export class HALEndpointService { // This skip ensures that if a stale object is present in the cache when you do a // call it isn't immediately returned, but we wait until the remote data for the new request // is created. - skipWhile((rd: RemoteData) => rd.isStale), - getFirstCompletedRemoteData(), + skipWhile((rd: RemoteData) => rd.isLoading || rd.isStale), + take(1), map((response: RemoteData) => { if (hasValue(response.payload)) { return response.payload._links; From 33b7c39dd13758c3b02ed2333bc9c58cc49e2716 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 6 Dec 2023 11:14:41 +0100 Subject: [PATCH 07/23] add ResponsePendingStale state (cherry picked from commit 790e7171992f2cefc6999306703e52586204986b) --- .../core/data/base/base-data.service.spec.ts | 98 ++++---- src/app/core/data/base/base-data.service.ts | 4 +- .../data/request-entry-state.model.spec.ts | 186 ++++++++++++++ .../core/data/request-entry-state.model.ts | 25 +- src/app/core/data/request.reducer.spec.ts | 230 +++++++++++++++--- src/app/core/data/request.reducer.ts | 55 +++-- src/app/core/data/request.service.ts | 2 +- .../core/shared/hal-endpoint.service.spec.ts | 176 ++++++++++++-- src/app/core/shared/hal-endpoint.service.ts | 25 +- 9 files changed, 660 insertions(+), 141 deletions(-) create mode 100644 src/app/core/data/request-entry-state.model.spec.ts 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 098f075c10..75662a691f 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -95,6 +95,7 @@ describe('BaseDataService', () => { 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), @@ -303,19 +304,21 @@ describe('BaseDataService', () => { 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', { - a: remoteDataMocks.SuccessStale, - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e-f-g', { + a: remoteDataMocks.ResponsePendingStale, + b: remoteDataMocks.SuccessStale, + c: remoteDataMocks.ErrorStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, })); - const expected = '--b-c-d-e'; + const expected = '------d-e-f-g'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, }; expectObservable(service.findByHref(selfLink, true, true, ...linksToFollow)).toBe(expected, values); @@ -354,19 +357,21 @@ describe('BaseDataService', () => { 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', { - a: remoteDataMocks.SuccessStale, - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e-f-g', { + a: remoteDataMocks.ResponsePendingStale, + b: remoteDataMocks.SuccessStale, + c: remoteDataMocks.ErrorStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, })); - const expected = '--b-c-d-e'; + const expected = '------d-e-f-g'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, }; expectObservable(service.findByHref(selfLink, false, true, ...linksToFollow)).toBe(expected, values); @@ -487,19 +492,21 @@ describe('BaseDataService', () => { 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', { - a: remoteDataMocks.SuccessStale, - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e-f-g', { + a: remoteDataMocks.ResponsePendingStale, + b: remoteDataMocks.SuccessStale, + c: remoteDataMocks.ErrorStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, })); - const expected = '--b-c-d-e'; + const expected = '------d-e-f-g'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, }; expectObservable(service.findListByHref(selfLink, findListOptions, true, true, ...linksToFollow)).toBe(expected, values); @@ -538,21 +545,24 @@ describe('BaseDataService', () => { 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', { - a: remoteDataMocks.SuccessStale, - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e-f-g', { + a: remoteDataMocks.ResponsePendingStale, + b: remoteDataMocks.SuccessStale, + c: remoteDataMocks.ErrorStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, })); - const expected = '--b-c-d-e'; + const expected = '------d-e-f-g'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, }; + expectObservable(service.findListByHref(selfLink, findListOptions, false, true, ...linksToFollow)).toBe(expected, values); }); }); diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index edd6d9e2a4..c7cd5b0a70 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -273,7 +273,7 @@ export class BaseDataService 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) => useCachedVersionIfAvailable ? rd.isStale : rd.hasCompleted), + skipWhile((rd: RemoteData) => rd.isStale || (!useCachedVersionIfAvailable && rd.hasCompleted)), this.reRequestStaleRemoteData(reRequestOnStale, () => this.findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), ); @@ -307,7 +307,7 @@ export class BaseDataService 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>) => useCachedVersionIfAvailable ? rd.isStale : rd.hasCompleted), + skipWhile((rd: RemoteData>) => rd.isStale || (!useCachedVersionIfAvailable && rd.hasCompleted)), this.reRequestStaleRemoteData(reRequestOnStale, () => this.findListByHref(href$, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), ); diff --git a/src/app/core/data/request-entry-state.model.spec.ts b/src/app/core/data/request-entry-state.model.spec.ts new file mode 100644 index 0000000000..7daa655566 --- /dev/null +++ b/src/app/core/data/request-entry-state.model.spec.ts @@ -0,0 +1,186 @@ +import { + isRequestPending, + isError, + isSuccess, + isErrorStale, + isSuccessStale, + isResponsePending, + isResponsePendingStale, + isLoading, + isStale, + hasFailed, + hasSucceeded, + hasCompleted, + RequestEntryState +} from './request-entry-state.model'; + +describe(`isRequestPending`, () => { + it(`should only return true if the given state is RequestPending`, () => { + expect(isRequestPending(RequestEntryState.RequestPending)).toBeTrue(); + + expect(isRequestPending(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isRequestPending(RequestEntryState.Error)).toBeFalse(); + expect(isRequestPending(RequestEntryState.Success)).toBeFalse(); + expect(isRequestPending(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isRequestPending(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isRequestPending(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isError`, () => { + it(`should only return true if the given state is Error`, () => { + expect(isError(RequestEntryState.Error)).toBeTrue(); + + expect(isError(RequestEntryState.RequestPending)).toBeFalse(); + expect(isError(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isError(RequestEntryState.Success)).toBeFalse(); + expect(isError(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isError(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isError(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isSuccess`, () => { + it(`should only return true if the given state is Success`, () => { + expect(isSuccess(RequestEntryState.Success)).toBeTrue(); + + expect(isSuccess(RequestEntryState.RequestPending)).toBeFalse(); + expect(isSuccess(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isSuccess(RequestEntryState.Error)).toBeFalse(); + expect(isSuccess(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isSuccess(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isSuccess(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isErrorStale`, () => { + it(`should only return true if the given state is ErrorStale`, () => { + expect(isErrorStale(RequestEntryState.ErrorStale)).toBeTrue(); + + expect(isErrorStale(RequestEntryState.RequestPending)).toBeFalse(); + expect(isErrorStale(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isErrorStale(RequestEntryState.Error)).toBeFalse(); + expect(isErrorStale(RequestEntryState.Success)).toBeFalse(); + expect(isErrorStale(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isErrorStale(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isSuccessStale`, () => { + it(`should only return true if the given state is SuccessStale`, () => { + expect(isSuccessStale(RequestEntryState.SuccessStale)).toBeTrue(); + + expect(isSuccessStale(RequestEntryState.RequestPending)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.Error)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.Success)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.ErrorStale)).toBeFalse(); + }); +}); + +describe(`isResponsePending`, () => { + it(`should only return true if the given state is ResponsePending`, () => { + expect(isResponsePending(RequestEntryState.ResponsePending)).toBeTrue(); + + expect(isResponsePending(RequestEntryState.RequestPending)).toBeFalse(); + expect(isResponsePending(RequestEntryState.Error)).toBeFalse(); + expect(isResponsePending(RequestEntryState.Success)).toBeFalse(); + expect(isResponsePending(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isResponsePending(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isResponsePending(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isResponsePendingStale`, () => { + it(`should only return true if the given state is requestPending`, () => { + expect(isResponsePendingStale(RequestEntryState.ResponsePendingStale)).toBeTrue(); + + expect(isResponsePendingStale(RequestEntryState.RequestPending)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.Error)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.Success)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isLoading`, () => { + it(`should only return true if the given state is RequestPending, ResponsePending or ResponsePendingStale`, () => { + expect(isLoading(RequestEntryState.RequestPending)).toBeTrue(); + expect(isLoading(RequestEntryState.ResponsePending)).toBeTrue(); + expect(isLoading(RequestEntryState.ResponsePendingStale)).toBeTrue(); + + expect(isLoading(RequestEntryState.Error)).toBeFalse(); + expect(isLoading(RequestEntryState.Success)).toBeFalse(); + expect(isLoading(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isLoading(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`hasFailed`, () => { + describe(`when the state is loading`, () => { + it(`should return undefined`, () => { + expect(hasFailed(RequestEntryState.RequestPending)).toBeUndefined(); + expect(hasFailed(RequestEntryState.ResponsePending)).toBeUndefined(); + expect(hasFailed(RequestEntryState.ResponsePendingStale)).toBeUndefined(); + }); + }); + + describe(`when the state has completed`, () => { + it(`should only return true if the given state is Error or ErrorStale`, () => { + expect(hasFailed(RequestEntryState.Error)).toBeTrue(); + expect(hasFailed(RequestEntryState.ErrorStale)).toBeTrue(); + + expect(hasFailed(RequestEntryState.Success)).toBeFalse(); + expect(hasFailed(RequestEntryState.SuccessStale)).toBeFalse(); + }); + }); +}); + +describe(`hasSucceeded`, () => { + describe(`when the state is loading`, () => { + it(`should return undefined`, () => { + expect(hasSucceeded(RequestEntryState.RequestPending)).toBeUndefined(); + expect(hasSucceeded(RequestEntryState.ResponsePending)).toBeUndefined(); + expect(hasSucceeded(RequestEntryState.ResponsePendingStale)).toBeUndefined(); + }); + }); + + describe(`when the state has completed`, () => { + it(`should only return true if the given state is Error or ErrorStale`, () => { + expect(hasSucceeded(RequestEntryState.Success)).toBeTrue(); + expect(hasSucceeded(RequestEntryState.SuccessStale)).toBeTrue(); + + expect(hasSucceeded(RequestEntryState.Error)).toBeFalse(); + expect(hasSucceeded(RequestEntryState.ErrorStale)).toBeFalse(); + }); + }); +}); + + +describe(`hasCompleted`, () => { + it(`should only return true if the given state is Error, Success, ErrorStale or SuccessStale`, () => { + expect(hasCompleted(RequestEntryState.Error)).toBeTrue(); + expect(hasCompleted(RequestEntryState.Success)).toBeTrue(); + expect(hasCompleted(RequestEntryState.ErrorStale)).toBeTrue(); + expect(hasCompleted(RequestEntryState.SuccessStale)).toBeTrue(); + + expect(hasCompleted(RequestEntryState.RequestPending)).toBeFalse(); + expect(hasCompleted(RequestEntryState.ResponsePending)).toBeFalse(); + expect(hasCompleted(RequestEntryState.ResponsePendingStale)).toBeFalse(); + }); +}); + +describe(`isStale`, () => { + it(`should only return true if the given state is ResponsePendingStale, SuccessStale or ErrorStale`, () => { + expect(isStale(RequestEntryState.ResponsePendingStale)).toBeTrue(); + expect(isStale(RequestEntryState.SuccessStale)).toBeTrue(); + expect(isStale(RequestEntryState.ErrorStale)).toBeTrue(); + + expect(isStale(RequestEntryState.RequestPending)).toBeFalse(); + expect(isStale(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isStale(RequestEntryState.Error)).toBeFalse(); + expect(isStale(RequestEntryState.Success)).toBeFalse(); + }); +}); diff --git a/src/app/core/data/request-entry-state.model.ts b/src/app/core/data/request-entry-state.model.ts index a813b6e743..3aeace39d2 100644 --- a/src/app/core/data/request-entry-state.model.ts +++ b/src/app/core/data/request-entry-state.model.ts @@ -3,8 +3,9 @@ export enum RequestEntryState { ResponsePending = 'ResponsePending', Error = 'Error', Success = 'Success', + ResponsePendingStale = 'ResponsePendingStale', ErrorStale = 'ErrorStale', - SuccessStale = 'SuccessStale' + SuccessStale = 'SuccessStale', } /** @@ -42,12 +43,21 @@ export const isSuccessStale = (state: RequestEntryState) => */ export const isResponsePending = (state: RequestEntryState) => state === RequestEntryState.ResponsePending; + /** - * Returns true if the given state is RequestPending or ResponsePending, - * false otherwise + * Returns true if the given state is ResponsePendingStale, false otherwise + */ +export const isResponsePendingStale = (state: RequestEntryState) => + state === RequestEntryState.ResponsePendingStale; + +/** + * Returns true if the given state is RequestPending, RequestPendingStale, ResponsePending, or + * ResponsePendingStale, false otherwise */ export const isLoading = (state: RequestEntryState) => - isRequestPending(state) || isResponsePending(state); + isRequestPending(state) || + isResponsePending(state) || + isResponsePendingStale(state); /** * If isLoading is true for the given state, this method returns undefined, we can't know yet. @@ -82,7 +92,10 @@ export const hasCompleted = (state: RequestEntryState) => !isLoading(state); /** - * Returns true if the given state is SuccessStale or ErrorStale, false otherwise + * Returns true if the given state is isRequestPendingStale, isResponsePendingStale, SuccessStale or + * ErrorStale, false otherwise */ export const isStale = (state: RequestEntryState) => - isSuccessStale(state) || isErrorStale(state); + isResponsePendingStale(state) || + isSuccessStale(state) || + isErrorStale(state); diff --git a/src/app/core/data/request.reducer.spec.ts b/src/app/core/data/request.reducer.spec.ts index 05f074a96a..86b9c4cd5d 100644 --- a/src/app/core/data/request.reducer.spec.ts +++ b/src/app/core/data/request.reducer.spec.ts @@ -48,9 +48,16 @@ describe('requestReducer', () => { lastUpdated: 0 } }; + const testResponsePendingState = { + [id1]: { + state: RequestEntryState.ResponsePending, + lastUpdated: 0 + } + }; deepFreeze(testInitState); deepFreeze(testSuccessState); deepFreeze(testErrorState); + deepFreeze(testResponsePendingState); it('should return the current state when no valid actions have been made', () => { const action = new NullAction(); @@ -91,29 +98,94 @@ describe('requestReducer', () => { expect(newState[id1].response).toEqual(undefined); }); - it('should set state to Success for the given RestRequest in the state, in response to a SUCCESS action', () => { - const state = testInitState; + describe(`in response to a SUCCESS action`, () => { + let startState; + describe(`when the entry isn't stale`, () => { + beforeEach(() => { + startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePending + }) + }); + deepFreeze(startState); + }); + it('should set state to Success for the given RestRequest in the state', () => { + const action = new RequestSuccessAction(id1, 200); + const newState = requestReducer(startState, action); - const action = new RequestSuccessAction(id1, 200); - const newState = requestReducer(state, action); + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].state).toEqual(RequestEntryState.Success); + expect(newState[id1].response.statusCode).toEqual(200); + }); + }); + + describe(`when the entry is stale`, () => { + beforeEach(() => { + startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePendingStale + }) + }); + deepFreeze(startState); + }); + it('should set state to SuccessStale for the given RestRequest in the state', () => { + const action = new RequestSuccessAction(id1, 200); + const newState = requestReducer(startState, action); + + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].state).toEqual(RequestEntryState.SuccessStale); + expect(newState[id1].response.statusCode).toEqual(200); + }); + }); - expect(newState[id1].request.uuid).toEqual(id1); - expect(newState[id1].request.href).toEqual(link1); - expect(newState[id1].state).toEqual(RequestEntryState.Success); - expect(newState[id1].response.statusCode).toEqual(200); }); - it('should set state to Error for the given RestRequest in the state, in response to an ERROR action', () => { - const state = testInitState; + describe(`in response to an ERROR action`, () => { + let startState; + describe(`when the entry isn't stale`, () => { + beforeEach(() => { + startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePending + }) + }); + deepFreeze(startState); + }); + it('should set state to Error for the given RestRequest in the state', () => { + const action = new RequestErrorAction(id1, 404, 'Not Found'); + const newState = requestReducer(startState, action); - const action = new RequestErrorAction(id1, 404, 'Not Found'); - const newState = requestReducer(state, action); + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].state).toEqual(RequestEntryState.Error); + expect(newState[id1].response.statusCode).toEqual(404); + expect(newState[id1].response.errorMessage).toEqual('Not Found'); + }); + }); - expect(newState[id1].request.uuid).toEqual(id1); - expect(newState[id1].request.href).toEqual(link1); - expect(newState[id1].state).toEqual(RequestEntryState.Error); - expect(newState[id1].response.statusCode).toEqual(404); - expect(newState[id1].response.errorMessage).toEqual('Not Found'); + describe(`when the entry is stale`, () => { + beforeEach(() => { + startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePendingStale + }) + }); + deepFreeze(startState); + }); + it('should set state to ErrorStale for the given RestRequest in the state', () => { + const action = new RequestErrorAction(id1, 404, 'Not Found'); + const newState = requestReducer(startState, action); + + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].state).toEqual(RequestEntryState.ErrorStale); + expect(newState[id1].response.statusCode).toEqual(404); + expect(newState[id1].response.errorMessage).toEqual('Not Found'); + }); + + }); }); it('should update the response\'s timeCompleted for the given RestRequest in the state, in response to a RESET_TIMESTAMPS action', () => { @@ -145,28 +217,112 @@ describe('requestReducer', () => { expect(newState[id1]).toBeNull(); }); - describe(`for an entry with state: Success`, () => { - it(`should set the state to SuccessStale, in response to a STALE action`, () => { - const state = testSuccessState; + describe(`in response to a STALE action`, () => { + describe(`when the entry has been removed`, () => { + it(`shouldn't do anything`, () => { + const startState = { + [id1]: null + }; + deepFreeze(startState); - const action = new RequestStaleAction(id1); - const newState = requestReducer(state, action); + const action = new RequestStaleAction(id1); + const newState = requestReducer(startState, action); - expect(newState[id1].state).toEqual(RequestEntryState.SuccessStale); - expect(newState[id1].lastUpdated).toBe(action.lastUpdated); + expect(newState[id1]).toBeNull(); + }); + }); + + describe(`for stale entries`, () => { + it(`shouldn't do anything`, () => { + const rpsStartState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePendingStale + }) + }); + deepFreeze(rpsStartState); + + const action = new RequestStaleAction(id1); + let newState = requestReducer(rpsStartState, action); + + expect(newState[id1].state).toEqual(rpsStartState[id1].state); + expect(newState[id1].lastUpdated).toBe(rpsStartState[id1].lastUpdated); + + const ssStartState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.SuccessStale + }) + }); + + newState = requestReducer(ssStartState, action); + + expect(newState[id1].state).toEqual(ssStartState[id1].state); + expect(newState[id1].lastUpdated).toBe(ssStartState[id1].lastUpdated); + + const esStartState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ErrorStale + }) + }); + + newState = requestReducer(esStartState, action); + + expect(newState[id1].state).toEqual(esStartState[id1].state); + expect(newState[id1].lastUpdated).toBe(esStartState[id1].lastUpdated); + + }); + }); + + describe(`for and entry with state: RequestPending`, () => { + it(`shouldn't do anything`, () => { + const startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.RequestPending + }) + }); + + const action = new RequestStaleAction(id1); + const newState = requestReducer(startState, action); + + expect(newState[id1].state).toEqual(startState[id1].state); + expect(newState[id1].lastUpdated).toBe(startState[id1].lastUpdated); + + }); + }); + + describe(`for an entry with state: ResponsePending`, () => { + it(`should set the state to ResponsePendingStale`, () => { + const state = testResponsePendingState; + + const action = new RequestStaleAction(id1); + const newState = requestReducer(state, action); + + expect(newState[id1].state).toEqual(RequestEntryState.ResponsePendingStale); + expect(newState[id1].lastUpdated).toBe(action.lastUpdated); + }); + }); + + describe(`for an entry with state: Success`, () => { + it(`should set the state to SuccessStale`, () => { + const state = testSuccessState; + + const action = new RequestStaleAction(id1); + const newState = requestReducer(state, action); + + expect(newState[id1].state).toEqual(RequestEntryState.SuccessStale); + expect(newState[id1].lastUpdated).toBe(action.lastUpdated); + }); + }); + + describe(`for an entry with state: Error`, () => { + it(`should set the state to ErrorStale`, () => { + const state = testErrorState; + + const action = new RequestStaleAction(id1); + const newState = requestReducer(state, action); + + expect(newState[id1].state).toEqual(RequestEntryState.ErrorStale); + expect(newState[id1].lastUpdated).toBe(action.lastUpdated); + }); }); }); - - describe(`for an entry with state: Error`, () => { - it(`should set the state to ErrorStale, in response to a STALE action`, () => { - const state = testErrorState; - - const action = new RequestStaleAction(id1); - const newState = requestReducer(state, action); - - expect(newState[id1].state).toEqual(RequestEntryState.ErrorStale); - expect(newState[id1].lastUpdated).toBe(action.lastUpdated); - }); - }); - }); diff --git a/src/app/core/data/request.reducer.ts b/src/app/core/data/request.reducer.ts index 9bf17faf8d..9cf4fee0e2 100644 --- a/src/app/core/data/request.reducer.ts +++ b/src/app/core/data/request.reducer.ts @@ -11,7 +11,13 @@ import { ResetResponseTimestampsAction } from './request.actions'; import { isNull } from '../../shared/empty.util'; -import { hasSucceeded, isStale, RequestEntryState } from './request-entry-state.model'; +import { + hasSucceeded, + isStale, + RequestEntryState, + isRequestPending, + isResponsePending +} from './request-entry-state.model'; import { RequestState } from './request-state.model'; // Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) @@ -91,14 +97,17 @@ function executeRequest(storeState: RequestState, action: RequestExecuteAction): * the new storeState, with the response added to the request */ function completeSuccessRequest(storeState: RequestState, action: RequestSuccessAction): RequestState { - if (isNull(storeState[action.payload.uuid])) { + const prevEntry = storeState[action.payload.uuid]; + if (isNull(prevEntry)) { // after a request has been removed it's possible pending changes still come in. // Don't store them return storeState; } else { return Object.assign({}, storeState, { - [action.payload.uuid]: Object.assign({}, storeState[action.payload.uuid], { - state: RequestEntryState.Success, + [action.payload.uuid]: Object.assign({}, prevEntry, { + // If a response comes in for a request that's already stale, still store it otherwise + // components that are waiting for it might freeze + state: isStale(prevEntry.state) ? RequestEntryState.SuccessStale : RequestEntryState.Success, response: { timeCompleted: action.payload.timeCompleted, lastUpdated: action.payload.timeCompleted, @@ -124,14 +133,17 @@ function completeSuccessRequest(storeState: RequestState, action: RequestSuccess * the new storeState, with the response added to the request */ function completeFailedRequest(storeState: RequestState, action: RequestErrorAction): RequestState { - if (isNull(storeState[action.payload.uuid])) { + const prevEntry = storeState[action.payload.uuid]; + if (isNull(prevEntry)) { // after a request has been removed it's possible pending changes still come in. // Don't store them return storeState; } else { return Object.assign({}, storeState, { - [action.payload.uuid]: Object.assign({}, storeState[action.payload.uuid], { - state: RequestEntryState.Error, + [action.payload.uuid]: Object.assign({}, prevEntry, { + // If a response comes in for a request that's already stale, still store it otherwise + // components that are waiting for it might freeze + state: isStale(prevEntry.state) ? RequestEntryState.ErrorStale : RequestEntryState.Error, response: { timeCompleted: action.payload.timeCompleted, lastUpdated: action.payload.timeCompleted, @@ -155,22 +167,27 @@ function completeFailedRequest(storeState: RequestState, action: RequestErrorAct * the new storeState, set to stale */ function expireRequest(storeState: RequestState, action: RequestStaleAction): RequestState { - if (isNull(storeState[action.payload.uuid])) { - // after a request has been removed it's possible pending changes still come in. - // Don't store them + const prevEntry = storeState[action.payload.uuid]; + if (isNull(prevEntry) || isStale(prevEntry.state) || isRequestPending(prevEntry.state)) { + // No need to do anything if the entry doesn't exist, is already stale, or if the request is + // still pending, because that means it still needs to be sent to the server. Any response + // is guaranteed to have been generated after the request was set to stale. return storeState; } else { - const prevEntry = storeState[action.payload.uuid]; - if (isStale(prevEntry.state)) { - return storeState; + let nextRequestEntryState: RequestEntryState; + if (isResponsePending(prevEntry.state)) { + nextRequestEntryState = RequestEntryState.ResponsePendingStale; + } else if (hasSucceeded(prevEntry.state)) { + nextRequestEntryState = RequestEntryState.SuccessStale; } else { - return Object.assign({}, storeState, { - [action.payload.uuid]: Object.assign({}, prevEntry, { - state: hasSucceeded(prevEntry.state) ? RequestEntryState.SuccessStale : RequestEntryState.ErrorStale, - lastUpdated: action.lastUpdated - }) - }); + nextRequestEntryState = RequestEntryState.ErrorStale; } + return Object.assign({}, storeState, { + [action.payload.uuid]: Object.assign({}, prevEntry, { + state: nextRequestEntryState, + lastUpdated: action.lastUpdated + }) + }); } } diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index ec633370ce..9f43c3f599 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -164,7 +164,7 @@ export class RequestService { this.getByHref(request.href).pipe( take(1)) .subscribe((re: RequestEntry) => { - isPending = (hasValue(re) && isLoading(re.state)); + isPending = (hasValue(re) && isLoading(re.state) && !isStale(re.state)); }); return isPending; } diff --git a/src/app/core/shared/hal-endpoint.service.spec.ts b/src/app/core/shared/hal-endpoint.service.spec.ts index 56e890b318..b81d0806df 100644 --- a/src/app/core/shared/hal-endpoint.service.spec.ts +++ b/src/app/core/shared/hal-endpoint.service.spec.ts @@ -1,4 +1,3 @@ -import { cold, hot } from 'jasmine-marbles'; import { getMockRequestService } from '../../shared/mocks/request.service.mock'; import { RequestService } from '../data/request.service'; import { HALEndpointService } from './hal-endpoint.service'; @@ -7,12 +6,17 @@ import { combineLatest as observableCombineLatest, of as observableOf } from 'rx import { environment } from '../../../environments/environment'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { TestScheduler } from 'rxjs/testing'; +import { RemoteData } from '../data/remote-data'; +import { RequestEntryState } from '../data/request-entry-state.model'; describe('HALEndpointService', () => { let service: HALEndpointService; let requestService: RequestService; let rdbService: RemoteDataBuildService; let envConfig; + let testScheduler; + let remoteDataMocks; const endpointMap = { test: { href: 'https://rest.api/test' @@ -68,7 +72,30 @@ describe('HALEndpointService', () => { }; const linkPath = 'test'; + const timeStamp = new Date().getTime(); + const msToLive = 15 * 60 * 1000; + const payload = { + _links: endpointMaps[one] + }; + 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), + }; + beforeEach(() => { + testScheduler = new TestScheduler((actual, expected) => { + // asserting the two objects are equal + // e.g. using chai. + expect(actual).toEqual(expected); + }); requestService = getMockRequestService(); rdbService = jasmine.createSpyObj('rdbService', { buildFromHref: createSuccessfulRemoteDataObject$({ @@ -111,20 +138,28 @@ describe('HALEndpointService', () => { }); it(`should return the endpoint URL for the service's linkPath`, () => { - spyOn(service as any, 'getEndpointAt').and - .returnValue(hot('a-', { a: 'https://rest.api/test' })); - const result = service.getEndpoint(linkPath); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getEndpointAt').and + .returnValue(cold('a-', { a: 'https://rest.api/test' })); + const result = service.getEndpoint(linkPath); - const expected = cold('(b|)', { b: endpointMap.test.href }); - expect(result).toBeObservable(expected); + const expected = '(b|)'; + const values = { + b: endpointMap.test.href + }; + expectObservable(result).toBe(expected, values); + }); }); it('should return undefined for a linkPath that isn\'t in the endpoint map', () => { - spyOn(service as any, 'getEndpointAt').and - .returnValue(hot('a-', { a: undefined })); - const result = service.getEndpoint('unknown'); - const expected = cold('(b|)', { b: undefined }); - expect(result).toBeObservable(expected); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getEndpointAt').and + .returnValue(cold('a-', { a: undefined })); + const result = service.getEndpoint('unknown'); + const expected = '(b|)'; + const values = { b: undefined }; + expectObservable(result).toBe(expected, values); + }); }); }); @@ -183,29 +218,118 @@ describe('HALEndpointService', () => { }); it('should return undefined as long as getRootEndpointMap hasn\'t fired', () => { - spyOn(service as any, 'getRootEndpointMap').and - .returnValue(hot('----')); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getRootEndpointMap').and + .returnValue(cold('----')); - const result = service.isEnabledOnRestApi(linkPath); - const expected = cold('b---', { b: undefined }); - expect(result).toBeObservable(expected); + const result = service.isEnabledOnRestApi(linkPath); + const expected = 'b---'; + const values = { b: undefined }; + expectObservable(result).toBe(expected, values); + }); }); it('should return true if the service\'s linkPath is in the endpoint map', () => { - spyOn(service as any, 'getRootEndpointMap').and - .returnValue(hot('--a-', { a: endpointMap })); - const result = service.isEnabledOnRestApi(linkPath); - const expected = cold('b-c-', { b: undefined, c: true }); - expect(result).toBeObservable(expected); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getRootEndpointMap').and + .returnValue(cold('--a-', { a: endpointMap })); + const result = service.isEnabledOnRestApi(linkPath); + const expected = 'b-c-'; + const values = { b: undefined, c: true }; + expectObservable(result).toBe(expected, values); + }); }); it('should return false if the service\'s linkPath isn\'t in the endpoint map', () => { - spyOn(service as any, 'getRootEndpointMap').and - .returnValue(hot('--a-', { a: endpointMap })); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getRootEndpointMap').and + .returnValue(cold('--a-', { a: endpointMap })); - const result = service.isEnabledOnRestApi('unknown'); - const expected = cold('b-c-', { b: undefined, c: false }); - expect(result).toBeObservable(expected); + const result = service.isEnabledOnRestApi('unknown'); + const expected = 'b-c-'; + const values = { b: undefined, c: false }; + expectObservable(result).toBe(expected, values); + }); + }); + + }); + + describe(`getEndpointMapAt`, () => { + const href = 'https://rest.api/some/sub/path'; + + it(`should call requestService.send with a new EndpointMapRequest for the given href. useCachedVersionIfAvailable should be true`, () => { + testScheduler.run(() => { + (service as any).getEndpointMapAt(href); + }); + const expected = new EndpointMapRequest(requestService.generateRequestId(), href); + expect(requestService.send).toHaveBeenCalledWith(expected, true); + }); + + it(`should call rdbService.buildFromHref with the given href`, () => { + testScheduler.run(() => { + (service as any).getEndpointMapAt(href); + }); + expect(rdbService.buildFromHref).toHaveBeenCalledWith(href); + }); + + describe(`when the RemoteData returned from rdbService is stale`, () => { + it(`should re-request it`, () => { + spyOn(service as any, 'getEndpointMapAt').and.callThrough(); + testScheduler.run(({ cold }) => { + (rdbService.buildFromHref as jasmine.Spy).and.returnValue(cold('a', { a: remoteDataMocks.ResponsePendingStale })); + // we need to subscribe to the result, to ensure the "tap" that does the re-request can fire + (service as any).getEndpointMapAt(href).subscribe(); + }); + expect((service as any).getEndpointMapAt).toHaveBeenCalledTimes(2); + }); + }); + + describe(`when the RemoteData returned from rdbService isn't stale`, () => { + it(`should not re-request it`, () => { + spyOn(service as any, 'getEndpointMapAt').and.callThrough(); + testScheduler.run(({ cold }) => { + (rdbService.buildFromHref as jasmine.Spy).and.returnValue(cold('a', { a: remoteDataMocks.ResponsePending })); + // we need to subscribe to the result, to ensure the "tap" that does the re-request can fire + (service as any).getEndpointMapAt(href).subscribe(); + }); + expect((service as any).getEndpointMapAt).toHaveBeenCalledTimes(1); + }); + }); + + it(`should emit exactly once, returning the endpoint map in the response, when the RemoteData completes`, () => { + testScheduler.run(({ cold, expectObservable }) => { + (rdbService.buildFromHref as jasmine.Spy).and.returnValue(cold('a-b-c-d-e-f-g-h-i-j-k-l', { + a: remoteDataMocks.RequestPending, + b: remoteDataMocks.ResponsePending, + c: remoteDataMocks.ResponsePendingStale, + d: remoteDataMocks.SuccessStale, + e: remoteDataMocks.RequestPending, + f: remoteDataMocks.ResponsePending, + g: remoteDataMocks.Success, + h: remoteDataMocks.SuccessStale, + i: remoteDataMocks.RequestPending, + k: remoteDataMocks.ResponsePending, + l: remoteDataMocks.Error, + })); + const expected = '------------(g|)'; + const values = { + g: endpointMaps[one] + }; + expectObservable((service as any).getEndpointMapAt(one)).toBe(expected, values); + }); + }); + + it(`should emit undefined when the response doesn't have a payload`, () => { + testScheduler.run(({ cold, expectObservable }) => { + (rdbService.buildFromHref as jasmine.Spy).and.returnValue(cold('a', { + a: remoteDataMocks.Error, + })); + const expected = '(a|)'; + const values = { + g: undefined + }; + expectObservable((service as any).getEndpointMapAt(href)).toBe(expected, values); + }); }); }); diff --git a/src/app/core/shared/hal-endpoint.service.ts b/src/app/core/shared/hal-endpoint.service.ts index 5cdd7ccfad..07754616c7 100644 --- a/src/app/core/shared/hal-endpoint.service.ts +++ b/src/app/core/shared/hal-endpoint.service.ts @@ -1,11 +1,19 @@ import { Observable } from 'rxjs'; -import { distinctUntilChanged, map, startWith, switchMap, take, skipWhile } from 'rxjs/operators'; +import { + distinctUntilChanged, + map, + startWith, + switchMap, + take, + tap, filter +} from 'rxjs/operators'; import { RequestService } from '../data/request.service'; import { EndpointMapRequest } from '../data/request.models'; import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; import { Injectable } from '@angular/core'; import { EndpointMap } from '../cache/response.models'; +import { getFirstCompletedRemoteData } from './operators'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RemoteData } from '../data/remote-data'; import { CacheableObject } from '../cache/cacheable-object.model'; @@ -33,11 +41,16 @@ export class HALEndpointService { this.requestService.send(request, true); return this.rdbService.buildFromHref(href).pipe( - // This skip ensures that if a stale object is present in the cache when you do a - // call it isn't immediately returned, but we wait until the remote data for the new request - // is created. - skipWhile((rd: RemoteData) => rd.isLoading || rd.isStale), - take(1), + // Re-request stale responses + tap((rd: RemoteData) => { + if (hasValue(rd) && rd.isStale) { + this.getEndpointMapAt(href); + } + }), + // Filter out all stale responses. We're only interested in a single, non-stale, + // completed RemoteData + filter((rd: RemoteData) => !rd.isStale), + getFirstCompletedRemoteData(), map((response: RemoteData) => { if (hasValue(response.payload)) { return response.payload._links; From 92a74cee5aa2e4a9085a7a3d7694edf4855d6281 Mon Sep 17 00:00:00 2001 From: Sascha Szott Date: Wed, 10 Jan 2024 16:54:18 +0100 Subject: [PATCH 08/23] use localized error message --- src/app/core/auth/auth.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 6604936cde..8b08b4f32d 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -119,7 +119,7 @@ export class AuthService { if (hasValue(rd.payload) && rd.payload.authenticated) { return rd.payload; } else { - throw (new Error('Invalid email or password')); + throw (new Error('auth.errors.invalid-user')); } })); From 7fbb692e94b6bcf9f16c8b2ec6880831175b936e Mon Sep 17 00:00:00 2001 From: Dawnkai Date: Fri, 27 Oct 2023 14:32:50 +0200 Subject: [PATCH 09/23] Deque Analysis Color Contrast fixes --- .../dso-edit-metadata-value.component.scss | 16 +++ src/app/shared/alert/alert.component.scss | 3 + .../notification/notification.component.scss | 6 + .../search-range-filter.component.scss | 8 +- .../submission-form-footer.component.scss | 15 +++ src/styles/_bootstrap_variables.scss | 5 +- src/styles/_custom_variables.scss | 15 ++- src/styles/_global-styles.scss | 107 ++++++++++++++++-- 8 files changed, 161 insertions(+), 14 deletions(-) diff --git a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.scss b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.scss index 4a207ee1a4..e3bda1697a 100644 --- a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.scss +++ b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.scss @@ -14,3 +14,19 @@ .cdk-drag-placeholder { opacity: 0; } + +.btn.btn-outline-warning:not(:disabled):hover { + background-color: #7f480c; +} +.btn.btn-success:not(:disabled):focus, .btn.btn-outline-success:not(:disabled):focus{ + outline: 2px solid rgba(43, 99, 47, 1) +} +.btn.btn-danger:not(:disabled):focus, .btn.btn-outline-danger:not(:disabled):focus{ + outline: 2px solid rgba(190, 114, 114, 1); +} +.btn.btn-warning:not(:disabled):focus, .btn.btn-outline-warning:not(:disabled):focus { + outline: 2px solid rgba(88, 87, 65, 1); +} +.btn.btn-primary:not(:disabled):focus, .btn.btn-outline-primary:not(:disabled):focus { + outline: 2px solid rgba(130, 135, 139, 1); +} diff --git a/src/app/shared/alert/alert.component.scss b/src/app/shared/alert/alert.component.scss index 1a70081367..7b225a47b5 100644 --- a/src/app/shared/alert/alert.component.scss +++ b/src/app/shared/alert/alert.component.scss @@ -1,3 +1,6 @@ .close:focus { outline: none !important; } +button.close { + opacity: 0.6; +} diff --git a/src/app/shared/notifications/notification/notification.component.scss b/src/app/shared/notifications/notification/notification.component.scss index 06c46b0f5d..aa0720afb7 100644 --- a/src/app/shared/notifications/notification/notification.component.scss +++ b/src/app/shared/notifications/notification/notification.component.scss @@ -30,3 +30,9 @@ .alert-warning .notification-progress-loader span { background: var(--ds-notification-bg-warning); } +.alert-success{ + color: var(--ds-notification-success-color); +} +.alert-danger { + color: var(--ds-notification-danger-color); +} diff --git a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss index b1cfa841f8..8d207dd023 100644 --- a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss +++ b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss @@ -26,7 +26,7 @@ } .noUi-connect { - background: var(--ds-slider-color); + background: var(--ds-range-filter-connect-color); } .noUi-target { @@ -37,4 +37,10 @@ margin: 0 calc(-1 * (var(--ds-slider-handle-width) / 2)); width: calc(100% + var(--ds-slider-handle-width)); } + .noUi-handle { + border-color: var(--ds-range-filter-border-color); + &::before, &::after { + background-color: var(--ds-range-filter-border-color); + } + } } diff --git a/src/app/submission/form/footer/submission-form-footer.component.scss b/src/app/submission/form/footer/submission-form-footer.component.scss index e69de29bb2..a435e5c393 100644 --- a/src/app/submission/form/footer/submission-form-footer.component.scss +++ b/src/app/submission/form/footer/submission-form-footer.component.scss @@ -0,0 +1,15 @@ +:host{ + .btn:focus, .btn-outline-primary:not(:disabled):not(.disabled):active:focus, .btn-outline-primary:not(:disabled):not(.disabled).active:focus, .show > .btn-outline-primary.dropdown-toggle:focus, .custom-control-input:focus ~ .custom-control-label::before{ + box-shadow: inset 0 3px 5px rgba(0, 0, 0, 0.25), 0 0 0 0.2rem rgba(27, 41, 55, 0.6); + } + + .btn.btn-success:focus{ + box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.15), 0 1px 1px rgba(0, 0, 0, 0.075), 0 0 0 0.2rem rgba(0, 100, 0, 0.7) + } + .btn.btn-danger:focus{ + box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.15), 0 1px 1px rgba(0, 0, 0, 0.075), 0 0 0 0.2rem rgba(200, 0, 0, 0.7) + } + .btn.btn-warning:focus{ + box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.15), 0 1px 1px rgba(0, 0, 0, 0.075), 0 0 0 0.2rem rgba(89, 81, 0, 0.7) + } +} diff --git a/src/styles/_bootstrap_variables.scss b/src/styles/_bootstrap_variables.scss index 5dfc03fbd3..cc3d09369b 100644 --- a/src/styles/_bootstrap_variables.scss +++ b/src/styles/_bootstrap_variables.scss @@ -26,10 +26,9 @@ $green: #94BA65 !default; $cyan: #006666 !default; $yellow: #ec9433 !default; $red: #CF4444 !default; +$teal: #1F7293 !default; $dark: darken($blue, 17%) !default; - - $theme-colors: ( primary: $blue, secondary: $gray-700, @@ -41,7 +40,7 @@ $theme-colors: ( dark: $dark ) !default; /* Fonts */ -$link-color: map-get($theme-colors, info) !default; +$link-color: $teal !default; $navbar-dark-color: rgba(white, .5) !default; $navbar-light-color: rgba(black, .5) !default; diff --git a/src/styles/_custom_variables.scss b/src/styles/_custom_variables.scss index 778ef6e9e3..0e356a5a37 100644 --- a/src/styles/_custom_variables.scss +++ b/src/styles/_custom_variables.scss @@ -61,6 +61,19 @@ --ds-notification-bg-danger: #{darken(adjust-hue($danger, -10), 10%)}; --ds-notification-bg-info: #{darken(adjust-hue($info, -10), 10%)}; --ds-notification-bg-warning: #{darken(adjust-hue($warning, -10), 10%)}; + --ds-notification-success-color: #307B23; + --ds-notification-danger-color: #9a6e6e; + --ds-text-warning-color: #cf822c; + --ds-text-success-color: #74a030; + --ds-range-filter-border-color: #949494; + --ds-range-filter-connect-color: #63852e; + + --ds-badge-archived-background-color: #2a701e; + --ds-badge-archived-color: #000; + --ds-button-success-background-color: #358726; + --ds-button-success-background-hover-color: #{darken(#358726, 10%)}; + --ds-button-warning-background-color: #{darken($yellow, 20%)}; + --ds-button-warning-background-hover-color: #{darken($yellow, 30%)}; --ds-fa-fixed-width: #{$fa-fixed-width}; --ds-icon-padding: #{$icon-padding}; @@ -81,7 +94,7 @@ --ds-home-news-background-color: #{$gray-200}; --ds-breadcrumb-bg: #{$gray-200} !important; - --ds-breadcrumb-link-color: #{$cyan}; + --ds-breadcrumb-link-color: #{darken($cyan, 10%)}; --ds-breadcrumb-link-active-color: #{darken($cyan, 30%)}; --ds-breadcrumb-max-length: 200px; diff --git a/src/styles/_global-styles.scss b/src/styles/_global-styles.scss index 00fcb0f86f..16a4799e1e 100644 --- a/src/styles/_global-styles.scss +++ b/src/styles/_global-styles.scss @@ -234,7 +234,8 @@ ds-dynamic-form-control-container.d-none { } .badge-archived { - background-color: #{map-get($theme-colors, success)}; + background-color: var(--ds-badge-archived-background-color); + color: var(--ds-badge-archived-color) } .badge-workflow { @@ -269,27 +270,115 @@ ul.dso-edit-menu-dropdown > li .nav-item.nav-link { } .pt-0\.5 { - padding-top: 0.125rem !important; + padding-top: 0.125rem !important; } .pr-0\.5 { - padding-right: 0.125rem !important; + padding-right: 0.125rem !important; } .pb-0\.5 { - padding-bottom: 0.125rem !important; + padding-bottom: 0.125rem !important; } .pl-0\.5 { - padding-left: 0.125rem !important; + padding-left: 0.125rem !important; } .px-0\.5 { - padding-left: 0.125rem !important; - padding-right: 0.125rem !important; + padding-left: 0.125rem !important; + padding-right: 0.125rem !important; } .py-0\.5 { - padding-top: 0.125rem !important; - padding-bottom: 0.125rem !important; + padding-top: 0.125rem !important; + padding-bottom: 0.125rem !important; +} + +.btn.btn-success{ + background-color: var(--ds-button-success-background-color); + border-color: var(--ds-button-success-background-color); + &:hover{ + background-color: var(--ds-button-success-background-hover-color); + border-color: var(--ds-button-success-background-hover-color); + } +} +.btn.btn-outline-success{ + border-color: var(--ds-button-success-background-color); + color: var(--ds-button-success-background-color); + &:hover{ + background-color: var(--ds-button-success-background-hover-color); + color: $white; + } +} +.btn.btn-outline-warning{ + border-color:var(--ds-button-warning-background-color); + color:var(--ds-button-warning-background-color); + &:hover{ + background-color:var(--ds-button-warning-background-hover-color); + color: $white; + } + &:disabled{ + background-color: transparent; + &:hover{ + color:var(--ds-button-warning-background-color); + } + } +} + + +.btn:focus, .btn-outline-primary:not(:disabled):not(.disabled):active:focus, .btn-outline-primary:not(:disabled):not(.disabled).active:focus, .show > .btn-outline-primary.dropdown-toggle:focus, .custom-control-input:focus ~ .custom-control-label::before, .form-control:focus, .page-link:focus{ + box-shadow: inset 0 3px 5px rgba(0, 0, 0, 0.25), 0 0 0 0.2rem rgba(27, 41, 55, 0.5); +} + +.btn.btn-success:focus, .btn.btn-outline-success:focus{ + outline: 2px solid rgba(76, 145, 76, 1); + outline-offset: 2px; + box-shadow: none; +} +.btn.btn-danger:focus, .btn.btn-outline-danger:focus{ + outline: 2px solid rgba(205, 126, 126, 1); + outline-offset: 2px; + box-shadow: none; +} +.btn.btn-warning:focus, .btn.btn-outline-warning:focus { + outline: 2px solid rgba(88, 87, 65, 1); + outline-offset: 2px; + box-shadow: none; +} +.btn.btn-primary:focus, .btn.btn-outline-primary:focus { + box-shadow: none; + outline: 2px solid rgba(134, 137, 139, 1); + outline-offset: 2px; +} +.btn.btn-secondary:focus, .btn.btn-outline-secondary:focus { + box-shadow: none; + outline: 2px solid rgba(141, 148, 155, 1); + outline-offset: 2px; +} +.btn.btn-warning{ + background-color:var(--ds-button-warning-background-color); + &:hover{ + background-color:var(--ds-button-warning-background-hover-color); + } + &:disabled{ + background-color: transparent; + } +} +dynamic-ng-bootstrap-checkbox .custom-control-label::before { + border-color: #858c91; +} +.text-warning { + color: var(--ds-text-warning-color) !important; +} +.text-success { + color: var(--ds-text-success-color) !important; +} +ngb-accordion { + a.close { + opacity: 0.75; + } + a.close:not(:disabled):not(.disabled):hover { + opacity: 0.9; + } } From c20b0a7c1156a844ac09c32d24464a61f1a69c7c Mon Sep 17 00:00:00 2001 From: Maciej Kleban Date: Fri, 17 Nov 2023 17:11:08 +0100 Subject: [PATCH 10/23] Replace hard-coded colors with bootstrap variants * Replace custom variables with Bootstrap variants * Replace custom button colors with Bootstrap variants * Remove custom colors and replace them with bootstrap variants * Fix checkbox offset styles --------- Co-authored-by: Maciej Kleban --- .../dso-edit-metadata-value.component.scss | 16 -- src/app/shared/alert/alert.component.scss | 7 +- .../notification/notification.component.scss | 9 +- .../search-range-filter.component.scss | 6 +- .../submission-form-footer.component.scss | 15 -- src/styles/_custom_variables.scss | 16 +- src/styles/_global-styles.scss | 220 +++++++++++------- .../_theme_sass_variable_overrides.scss | 2 +- 8 files changed, 143 insertions(+), 148 deletions(-) diff --git a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.scss b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.scss index e3bda1697a..4a207ee1a4 100644 --- a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.scss +++ b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.scss @@ -14,19 +14,3 @@ .cdk-drag-placeholder { opacity: 0; } - -.btn.btn-outline-warning:not(:disabled):hover { - background-color: #7f480c; -} -.btn.btn-success:not(:disabled):focus, .btn.btn-outline-success:not(:disabled):focus{ - outline: 2px solid rgba(43, 99, 47, 1) -} -.btn.btn-danger:not(:disabled):focus, .btn.btn-outline-danger:not(:disabled):focus{ - outline: 2px solid rgba(190, 114, 114, 1); -} -.btn.btn-warning:not(:disabled):focus, .btn.btn-outline-warning:not(:disabled):focus { - outline: 2px solid rgba(88, 87, 65, 1); -} -.btn.btn-primary:not(:disabled):focus, .btn.btn-outline-primary:not(:disabled):focus { - outline: 2px solid rgba(130, 135, 139, 1); -} diff --git a/src/app/shared/alert/alert.component.scss b/src/app/shared/alert/alert.component.scss index 7b225a47b5..907edae24b 100644 --- a/src/app/shared/alert/alert.component.scss +++ b/src/app/shared/alert/alert.component.scss @@ -1,5 +1,8 @@ -.close:focus { - outline: none !important; +.close { + opacity: 0.75; + &:focus { + outline: none !important; + } } button.close { opacity: 0.6; diff --git a/src/app/shared/notifications/notification/notification.component.scss b/src/app/shared/notifications/notification/notification.component.scss index aa0720afb7..ecfc25fee0 100644 --- a/src/app/shared/notifications/notification/notification.component.scss +++ b/src/app/shared/notifications/notification/notification.component.scss @@ -5,7 +5,8 @@ } .close { - outline: none !important + outline: none !important; + opacity: 0.8; } .notification-icon { @@ -30,9 +31,3 @@ .alert-warning .notification-progress-loader span { background: var(--ds-notification-bg-warning); } -.alert-success{ - color: var(--ds-notification-success-color); -} -.alert-danger { - color: var(--ds-notification-danger-color); -} diff --git a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss index 8d207dd023..1240f35678 100644 --- a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss +++ b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss @@ -26,7 +26,7 @@ } .noUi-connect { - background: var(--ds-range-filter-connect-color); + background: var(--ds-slider-color); } .noUi-target { @@ -38,9 +38,9 @@ width: calc(100% + var(--ds-slider-handle-width)); } .noUi-handle { - border-color: var(--ds-range-filter-border-color); + border-color: var(--ds-slider-handle-color); &::before, &::after { - background-color: var(--ds-range-filter-border-color); + background-color: var(--ds-slider-handle-color); } } } diff --git a/src/app/submission/form/footer/submission-form-footer.component.scss b/src/app/submission/form/footer/submission-form-footer.component.scss index a435e5c393..e69de29bb2 100644 --- a/src/app/submission/form/footer/submission-form-footer.component.scss +++ b/src/app/submission/form/footer/submission-form-footer.component.scss @@ -1,15 +0,0 @@ -:host{ - .btn:focus, .btn-outline-primary:not(:disabled):not(.disabled):active:focus, .btn-outline-primary:not(:disabled):not(.disabled).active:focus, .show > .btn-outline-primary.dropdown-toggle:focus, .custom-control-input:focus ~ .custom-control-label::before{ - box-shadow: inset 0 3px 5px rgba(0, 0, 0, 0.25), 0 0 0 0.2rem rgba(27, 41, 55, 0.6); - } - - .btn.btn-success:focus{ - box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.15), 0 1px 1px rgba(0, 0, 0, 0.075), 0 0 0 0.2rem rgba(0, 100, 0, 0.7) - } - .btn.btn-danger:focus{ - box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.15), 0 1px 1px rgba(0, 0, 0, 0.075), 0 0 0 0.2rem rgba(200, 0, 0, 0.7) - } - .btn.btn-warning:focus{ - box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.15), 0 1px 1px rgba(0, 0, 0, 0.075), 0 0 0 0.2rem rgba(89, 81, 0, 0.7) - } -} diff --git a/src/styles/_custom_variables.scss b/src/styles/_custom_variables.scss index 0e356a5a37..37af68509e 100644 --- a/src/styles/_custom_variables.scss +++ b/src/styles/_custom_variables.scss @@ -61,19 +61,6 @@ --ds-notification-bg-danger: #{darken(adjust-hue($danger, -10), 10%)}; --ds-notification-bg-info: #{darken(adjust-hue($info, -10), 10%)}; --ds-notification-bg-warning: #{darken(adjust-hue($warning, -10), 10%)}; - --ds-notification-success-color: #307B23; - --ds-notification-danger-color: #9a6e6e; - --ds-text-warning-color: #cf822c; - --ds-text-success-color: #74a030; - --ds-range-filter-border-color: #949494; - --ds-range-filter-connect-color: #63852e; - - --ds-badge-archived-background-color: #2a701e; - --ds-badge-archived-color: #000; - --ds-button-success-background-color: #358726; - --ds-button-success-background-hover-color: #{darken(#358726, 10%)}; - --ds-button-warning-background-color: #{darken($yellow, 20%)}; - --ds-button-warning-background-hover-color: #{darken($yellow, 30%)}; --ds-fa-fixed-width: #{$fa-fixed-width}; --ds-icon-padding: #{$icon-padding}; @@ -98,8 +85,9 @@ --ds-breadcrumb-link-active-color: #{darken($cyan, 30%)}; --ds-breadcrumb-max-length: 200px; - --ds-slider-color: #{$green}; + --ds-slider-color: #{darken($green, 20%)}; --ds-slider-handle-width: 18px; + --ds-slider-handle-color: #{darken($blue, 17%)}; --ds-search-form-scope-max-width: 150px; diff --git a/src/styles/_global-styles.scss b/src/styles/_global-styles.scss index 16a4799e1e..27d11250b4 100644 --- a/src/styles/_global-styles.scss +++ b/src/styles/_global-styles.scss @@ -222,51 +222,50 @@ ds-dynamic-form-control-container.d-none { } .badge-validation { - background-color: #{map-get($theme-colors, warning)}; + background-color: #{map-get($theme-colors, warning)}; } .badge-waiting-controller { - background-color: #{map-get($theme-colors, info)}; + background-color: #{map-get($theme-colors, info)}; } .badge-workspace { - background-color: #{map-get($theme-colors, primary)}; + background-color: #{map-get($theme-colors, primary)}; } .badge-archived { - background-color: var(--ds-badge-archived-background-color); - color: var(--ds-badge-archived-color) + background-color: darken($green, 25); } .badge-workflow { - background-color: #{map-get($theme-colors, info)}; + background-color: #{map-get($theme-colors, info)}; } .badge-item-type { - background-color: #{map-get($theme-colors, info)}; + background-color: #{map-get($theme-colors, info)}; } .visually-hidden { - position: absolute !important; - width: 1px !important; - height: 1px !important; - padding: 0 !important; - margin: -1px !important; - overflow: hidden !important; - clip: rect(0, 0, 0, 0) !important; - white-space: nowrap !important; - border: 0 !important; + position: absolute !important; + width: 1px !important; + height: 1px !important; + padding: 0 !important; + margin: -1px !important; + overflow: hidden !important; + clip: rect(0, 0, 0, 0) !important; + white-space: nowrap !important; + border: 0 !important; } -ul.dso-edit-menu-dropdown > li .nav-item.nav-link { - // ensure that links in DSO edit menu dropdowns are unstyled (li elements are styled instead to support icons) - padding: 0; - display: inline; +ul.dso-edit-menu-dropdown>li .nav-item.nav-link { + // ensure that links in DSO edit menu dropdowns are unstyled (li elements are styled instead to support icons) + padding: 0; + display: inline; } .table th, .table td { - vertical-align: middle; + vertical-align: middle; } .pt-0\.5 { @@ -295,85 +294,107 @@ ul.dso-edit-menu-dropdown > li .nav-item.nav-link { padding-bottom: 0.125rem !important; } -.btn.btn-success{ - background-color: var(--ds-button-success-background-color); - border-color: var(--ds-button-success-background-color); - &:hover{ - background-color: var(--ds-button-success-background-hover-color); - border-color: var(--ds-button-success-background-hover-color); - } -} -.btn.btn-outline-success{ - border-color: var(--ds-button-success-background-color); - color: var(--ds-button-success-background-color); - &:hover{ - background-color: var(--ds-button-success-background-hover-color); - color: $white; - } -} -.btn.btn-outline-warning{ - border-color:var(--ds-button-warning-background-color); - color:var(--ds-button-warning-background-color); - &:hover{ - background-color:var(--ds-button-warning-background-hover-color); - color: $white; - } - &:disabled{ - background-color: transparent; - &:hover{ - color:var(--ds-button-warning-background-color); - } - } +.btn { + &:focus { + outline-offset: 2px !important; + outline-style: solid !important; + outline-width: 2px !important; + box-shadow: none !important; + } + &:disabled { + opacity: 0.7; + } + &.btn-success { + background-color: darken($success, 20%); + border-color: darken($success, 20%); + &:hover { + background-color: darken($success, 30%); + border-color: darken($success, 30%); + } + &:focus { + outline-color: darken($success, 20%); + } + } + &.btn-outline-success { + border-color: darken($success, 20%); + color: darken($success, 20%); + + &:hover { + background-color: darken($success, 30%); + color: $white; + } + &:focus { + outline-color: darken($success, 20%); + } + } + &.btn-warning { + background-color: darken($warning, 20%); + &:hover { + background-color: darken($warning, 30%); + } + &:disabled { + background-color: transparent; + } + &:focus { + outline-color: darken($warning, 22%); + } + } + + &.btn-outline-warning { + border-color: darken($warning, 20%); + color: darken($warning, 20%); + &:hover { + background-color: darken($warning, 30%); + color: $white; + } + &:disabled { + background-color: transparent; + + &:hover { + color: darken($warning, 20%); + } + } + :focus { + outline-color: darken($warning, 22%); + } + &:not(:disabled):hover { + background-color: darken($warning, 22%); + } + } + + &.btn-secondary { + &:focus { + outline-color: darken($secondary, 20%); + } + } + + &.btn-danger:focus, &.btn-outline-danger:focus { + outline-color: darken($danger, 20%); + } + + &.btn-primary:focus, &.btn-outline-primary:focus { + outline-color: darken($primary, 5%); + } } - -.btn:focus, .btn-outline-primary:not(:disabled):not(.disabled):active:focus, .btn-outline-primary:not(:disabled):not(.disabled).active:focus, .show > .btn-outline-primary.dropdown-toggle:focus, .custom-control-input:focus ~ .custom-control-label::before, .form-control:focus, .page-link:focus{ - box-shadow: inset 0 3px 5px rgba(0, 0, 0, 0.25), 0 0 0 0.2rem rgba(27, 41, 55, 0.5); +dynamic-ng-bootstrap-checkbox .custom-control-input:focus ~ .custom-control-label::before { + outline: 2px solid $gray-700 !important; + box-shadow: none !important; + outline-offset: 2px !important; } -.btn.btn-success:focus, .btn.btn-outline-success:focus{ - outline: 2px solid rgba(76, 145, 76, 1); - outline-offset: 2px; - box-shadow: none; -} -.btn.btn-danger:focus, .btn.btn-outline-danger:focus{ - outline: 2px solid rgba(205, 126, 126, 1); - outline-offset: 2px; - box-shadow: none; -} -.btn.btn-warning:focus, .btn.btn-outline-warning:focus { - outline: 2px solid rgba(88, 87, 65, 1); - outline-offset: 2px; - box-shadow: none; -} -.btn.btn-primary:focus, .btn.btn-outline-primary:focus { - box-shadow: none; - outline: 2px solid rgba(134, 137, 139, 1); - outline-offset: 2px; -} -.btn.btn-secondary:focus, .btn.btn-outline-secondary:focus { - box-shadow: none; - outline: 2px solid rgba(141, 148, 155, 1); - outline-offset: 2px; -} -.btn.btn-warning{ - background-color:var(--ds-button-warning-background-color); - &:hover{ - background-color:var(--ds-button-warning-background-hover-color); - } - &:disabled{ - background-color: transparent; - } -} dynamic-ng-bootstrap-checkbox .custom-control-label::before { - border-color: #858c91; + border-color: $gray-700; } + .text-warning { - color: var(--ds-text-warning-color) !important; + color: darken($warning, 10%) !important; } + .text-success { - color: var(--ds-text-success-color) !important; + color: darken($success, 11%) !important; } + ngb-accordion { a.close { opacity: 0.75; @@ -382,3 +403,22 @@ ngb-accordion { opacity: 0.9; } } + +.form-control, .page-link { + &:disabled::placeholder { + color: lighten($gray-700, 10%); + } + &:focus { + box-shadow: none; + outline: 2px solid lighten($gray-700, 10%); + outline-offset: 2px !important; + } +} + +.alert-success { + color: darken($success, 22%); +} + +.alert-danger { + color: darken($danger, 22%); +} diff --git a/src/themes/dspace/styles/_theme_sass_variable_overrides.scss b/src/themes/dspace/styles/_theme_sass_variable_overrides.scss index b5799c9749..aaa27f6f9e 100644 --- a/src/themes/dspace/styles/_theme_sass_variable_overrides.scss +++ b/src/themes/dspace/styles/_theme_sass_variable_overrides.scss @@ -12,7 +12,7 @@ $navbar-dark-color: #FFFFFF; /* Reassign color vars to semantic color scheme */ $blue: #2b4e72 !default; $green: #92C642 !default; -$cyan: #207698 !default; +$cyan: #1e6f90 !default; $yellow: #ec9433 !default; $red: #CF4444 !default; $dark: #43515f !default; From 53d521a87e7e1962875f29f9f2f868b717f825ef Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Wed, 30 Aug 2023 16:00:10 -0500 Subject: [PATCH 11/23] Add new e2e accessibility tests & update some existing ones --- cypress/e2e/login-modal.cy.ts | 21 +++++++++++++++++---- cypress/e2e/my-dspace.cy.ts | 16 +++++----------- cypress/e2e/search-page.cy.ts | 5 ++--- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/cypress/e2e/login-modal.cy.ts b/cypress/e2e/login-modal.cy.ts index d29c13c2f9..d4559dc6bf 100644 --- a/cypress/e2e/login-modal.cy.ts +++ b/cypress/e2e/login-modal.cy.ts @@ -109,6 +109,9 @@ describe('Login Modal', () => { cy.get('ds-themed-navbar [data-test="register"]').click(); cy.location('pathname').should('eq', '/register'); cy.get('ds-register-email').should('exist'); + + // Test accessibility of this page + testA11y('ds-register-email'); }); it('should allow forgot password', () => { @@ -123,16 +126,26 @@ describe('Login Modal', () => { cy.get('ds-themed-navbar [data-test="forgot"]').click(); cy.location('pathname').should('eq', '/forgot'); cy.get('ds-forgot-email').should('exist'); + + // Test accessibility of this page + testA11y('ds-forgot-email'); }); - it('should pass accessibility tests', () => { + it('should pass accessibility tests in menus', () => { cy.visit('/'); + // Open login menu & verify accessibility page.openLoginMenu(); - cy.get('ds-log-in').should('exist'); - - // Analyze for accessibility issues testA11y('ds-log-in'); + + // Now login + page.submitLoginAndPasswordByPressingButton(TEST_ADMIN_USER, TEST_ADMIN_PASSWORD); + cy.get('ds-log-in').should('not.exist'); + + // Open user menu, verify user menu accesibility + page.openUserMenu(); + cy.get('ds-user-menu').should('be.visible'); + testA11y('ds-user-menu'); }); }); diff --git a/cypress/e2e/my-dspace.cy.ts b/cypress/e2e/my-dspace.cy.ts index 13f4a1b547..4b27fe13de 100644 --- a/cypress/e2e/my-dspace.cy.ts +++ b/cypress/e2e/my-dspace.cy.ts @@ -1,4 +1,3 @@ -import { Options } from 'cypress-axe'; import { TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD, TEST_SUBMIT_COLLECTION_NAME } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; @@ -35,16 +34,8 @@ describe('My DSpace page', () => { cy.get('ds-object-detail').should('be.visible'); - // Analyze for accessibility issues - testA11y('ds-my-dspace-page', - { - rules: { - // Search filters fail these two "moderate" impact rules - 'heading-order': { enabled: false }, - 'landmark-unique': { enabled: false } - } - } as Options - ); + // Analyze for accessibility issues + testA11y('ds-my-dspace-page'); }); // NOTE: Deleting existing submissions is exercised by submission.spec.ts @@ -136,6 +127,9 @@ describe('My DSpace page', () => { // The external import searchbox should be visible cy.get('ds-submission-import-external-searchbar').should('be.visible'); + + // Test for accessibility issues + testA11y('ds-submission-import-external'); }); }); diff --git a/cypress/e2e/search-page.cy.ts b/cypress/e2e/search-page.cy.ts index 755f8eaac6..aca3d23d04 100644 --- a/cypress/e2e/search-page.cy.ts +++ b/cypress/e2e/search-page.cy.ts @@ -46,9 +46,8 @@ describe('Search Page', () => { testA11y('ds-search-page', { rules: { - // Search filters fail these two "moderate" impact rules - 'heading-order': { enabled: false }, - 'landmark-unique': { enabled: false } + // Card titles fail this test currently + 'heading-order': { enabled: false } } } as Options ); From aeea1cd592c9dfe7bb1f98f096c6eb0f3d6d3cf2 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 1 Sep 2023 09:47:32 -0500 Subject: [PATCH 12/23] Fix ARIA labels on submission form relationship button and dynamic dropdowns --- .../ds-dynamic-form-control-container.component.html | 2 ++ .../dynamic-scrollable-dropdown.component.html | 7 +++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html index 9e1f1d48aa..606db29db3 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html @@ -45,7 +45,9 @@ diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.html index 1ac38e9943..3be79b20f3 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.html @@ -2,14 +2,15 @@
+ [attr.aria-owns]="'combobox_' + id + '_listbox'" + [attr.aria-expanded]="sdRef.isOpen()" + [attr.aria-activedescendant]="(currentValue | async) ? 'combobox_' + id + '_selected' : null">
Date: Fri, 1 Sep 2023 09:48:17 -0500 Subject: [PATCH 13/23] Fix ARIA labels and tabindex on date picker in submission form. Tabindex is unnecessary & throws accessibility errors from AXE tools --- .../models/date-picker/date-picker.component.html | 6 +++--- .../shared/form/number-picker/number-picker.component.html | 5 +++-- .../form/number-picker/number-picker.component.spec.ts | 1 - 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html index 26803f3c67..3bf02c4abd 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html @@ -4,7 +4,7 @@ {{model.placeholder}} * - Increment + Increment {{placeholder}}
diff --git a/src/app/shared/form/number-picker/number-picker.component.spec.ts b/src/app/shared/form/number-picker/number-picker.component.spec.ts index d4484dbfa3..bf33a6bdd0 100644 --- a/src/app/shared/form/number-picker/number-picker.component.spec.ts +++ b/src/app/shared/form/number-picker/number-picker.component.spec.ts @@ -42,7 +42,6 @@ describe('NumberPickerComponent test suite', () => { beforeEach(() => { html = ` Date: Fri, 1 Sep 2023 11:29:05 -0500 Subject: [PATCH 14/23] Refactor e2e test infrastruction to allow easier way to lookup REST API info and generate CSRF tokens --- cypress/plugins/index.ts | 24 +++++ cypress/support/commands.ts | 177 +++++++++++++++--------------------- cypress/support/e2e.ts | 51 ++++++++--- 3 files changed, 139 insertions(+), 113 deletions(-) diff --git a/cypress/plugins/index.ts b/cypress/plugins/index.ts index ead38afb92..cc3dccba38 100644 --- a/cypress/plugins/index.ts +++ b/cypress/plugins/index.ts @@ -1,5 +1,11 @@ const fs = require('fs'); +// These two global variables are used to store information about the REST API used +// by these e2e tests. They are filled out prior to running any tests in the before() +// method of e2e.ts. They can then be accessed by any tests via the getters below. +let REST_BASE_URL: string; +let REST_DOMAIN: string; + // Plugins enable you to tap into, modify, or extend the internal behavior of Cypress // For more info, visit https://on.cypress.io/plugins-api module.exports = (on, config) => { @@ -30,6 +36,24 @@ module.exports = (on, config) => { } return null; + }, + // Save value of REST Base URL, looked up before all tests. + // This allows other tests to use it easily via getRestBaseURL() below. + saveRestBaseURL(url: string) { + return (REST_BASE_URL = url); + }, + // Retrieve currently saved value of REST Base URL + getRestBaseURL() { + return REST_BASE_URL ; + }, + // Save value of REST Domain, looked up before all tests. + // This allows other tests to use it easily via getRestBaseDomain() below. + saveRestBaseDomain(domain: string) { + return (REST_DOMAIN = domain); + }, + // Retrieve currently saved value of REST Domain + getRestBaseDomain() { + return REST_DOMAIN ; } }); }; diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 92f0b1aeeb..7da454e2d0 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -5,11 +5,7 @@ import { AuthTokenInfo, TOKENITEM } from 'src/app/core/auth/models/auth-token-info.model'; import { DSPACE_XSRF_COOKIE, XSRF_REQUEST_HEADER } from 'src/app/core/xsrf/xsrf.constants'; - -// NOTE: FALLBACK_TEST_REST_BASE_URL is only used if Cypress cannot read the REST API BaseURL -// from the Angular UI's config.json. See 'login()'. -export const FALLBACK_TEST_REST_BASE_URL = 'http://localhost:8080/server'; -export const FALLBACK_TEST_REST_DOMAIN = 'localhost'; +import { v4 as uuidv4 } from 'uuid'; // Declare Cypress namespace to help with Intellisense & code completion in IDEs // ALL custom commands MUST be listed here for code completion to work @@ -41,6 +37,13 @@ declare global { * @param dsoType type of DSpace Object (e.g. "item", "collection", "community") */ generateViewEvent(uuid: string, dsoType: string): typeof generateViewEvent; + + /** + * Create a new CSRF token and add to required Cookie. CSRF Token is returned + * in chainable in order to allow it to be sent also in required CSRF header. + * @returns Chainable reference to allow CSRF token to also be sent in header. + */ + createCSRFCookie(): Chainable; } } } @@ -54,59 +57,32 @@ declare global { * @param password password to login as */ function login(email: string, password: string): void { - // Cypress doesn't have access to the running application in Node.js. - // So, it's not possible to inject or load the AppConfig or environment of the Angular UI. - // Instead, we'll read our running application's config.json, which contains the configs & - // is regenerated at runtime each time the Angular UI application starts up. - cy.task('readUIConfig').then((str: string) => { - // Parse config into a JSON object - const config = JSON.parse(str); + // Create a fake CSRF cookie/token to use in POST + cy.createCSRFCookie().then((csrfToken: string) => { + // get our REST API's base URL, also needed for POST + cy.task('getRestBaseURL').then((baseRestUrl: string) => { + // Now, send login POST request including that CSRF token + cy.request({ + method: 'POST', + url: baseRestUrl + '/api/authn/login', + headers: { [XSRF_REQUEST_HEADER]: csrfToken}, + form: true, // indicates the body should be form urlencoded + body: { user: email, password: password } + }).then((resp) => { + // We expect a successful login + expect(resp.status).to.eq(200); + // We expect to have a valid authorization header returned (with our auth token) + expect(resp.headers).to.have.property('authorization'); - // Find the URL of our REST API. Have a fallback ready, just in case 'rest.baseUrl' cannot be found. - let baseRestUrl = FALLBACK_TEST_REST_BASE_URL; - if (!config.rest.baseUrl) { - console.warn("Could not load 'rest.baseUrl' from config.json. Falling back to " + FALLBACK_TEST_REST_BASE_URL); - } else { - //console.log("Found 'rest.baseUrl' in config.json. Using this REST API for login: ".concat(config.rest.baseUrl)); - baseRestUrl = config.rest.baseUrl; - } + // Initialize our AuthTokenInfo object from the authorization header. + const authheader = resp.headers.authorization as string; + const authinfo: AuthTokenInfo = new AuthTokenInfo(authheader); - // Now find domain of our REST API, again with a fallback. - let baseDomain = FALLBACK_TEST_REST_DOMAIN; - if (!config.rest.host) { - console.warn("Could not load 'rest.host' from config.json. Falling back to " + FALLBACK_TEST_REST_DOMAIN); - } else { - baseDomain = config.rest.host; - } - - // Create a fake CSRF Token. Set it in the required server-side cookie - const csrfToken = 'fakeLoginCSRFToken'; - cy.setCookie(DSPACE_XSRF_COOKIE, csrfToken, { 'domain': baseDomain }); - - // Now, send login POST request including that CSRF token - cy.request({ - method: 'POST', - url: baseRestUrl + '/api/authn/login', - headers: { [XSRF_REQUEST_HEADER]: csrfToken}, - form: true, // indicates the body should be form urlencoded - body: { user: email, password: password } - }).then((resp) => { - // We expect a successful login - expect(resp.status).to.eq(200); - // We expect to have a valid authorization header returned (with our auth token) - expect(resp.headers).to.have.property('authorization'); - - // Initialize our AuthTokenInfo object from the authorization header. - const authheader = resp.headers.authorization as string; - const authinfo: AuthTokenInfo = new AuthTokenInfo(authheader); - - // Save our AuthTokenInfo object to our dsAuthInfo UI cookie - // This ensures the UI will recognize we are logged in on next "visit()" - cy.setCookie(TOKENITEM, JSON.stringify(authinfo)); + // Save our AuthTokenInfo object to our dsAuthInfo UI cookie + // This ensures the UI will recognize we are logged in on next "visit()" + cy.setCookie(TOKENITEM, JSON.stringify(authinfo)); + }); }); - - // Remove cookie with fake CSRF token, as it's no longer needed - cy.clearCookie(DSPACE_XSRF_COOKIE); }); } // Add as a Cypress command (i.e. assign to 'cy.login') @@ -141,56 +117,53 @@ Cypress.Commands.add('loginViaForm', loginViaForm); * @param dsoType type of DSpace Object (e.g. "item", "collection", "community") */ function generateViewEvent(uuid: string, dsoType: string): void { - // Cypress doesn't have access to the running application in Node.js. - // So, it's not possible to inject or load the AppConfig or environment of the Angular UI. - // Instead, we'll read our running application's config.json, which contains the configs & - // is regenerated at runtime each time the Angular UI application starts up. - cy.task('readUIConfig').then((str: string) => { - // Parse config into a JSON object - const config = JSON.parse(str); - - // Find the URL of our REST API. Have a fallback ready, just in case 'rest.baseUrl' cannot be found. - let baseRestUrl = FALLBACK_TEST_REST_BASE_URL; - if (!config.rest.baseUrl) { - console.warn("Could not load 'rest.baseUrl' from config.json. Falling back to " + FALLBACK_TEST_REST_BASE_URL); - } else { - baseRestUrl = config.rest.baseUrl; - } - - // Now find domain of our REST API, again with a fallback. - let baseDomain = FALLBACK_TEST_REST_DOMAIN; - if (!config.rest.host) { - console.warn("Could not load 'rest.host' from config.json. Falling back to " + FALLBACK_TEST_REST_DOMAIN); - } else { - baseDomain = config.rest.host; - } - - // Create a fake CSRF Token. Set it in the required server-side cookie - const csrfToken = 'fakeGenerateViewEventCSRFToken'; - cy.setCookie(DSPACE_XSRF_COOKIE, csrfToken, { 'domain': baseDomain }); - - // Now, send 'statistics/viewevents' POST request including that fake CSRF token in required header - cy.request({ - method: 'POST', - url: baseRestUrl + '/api/statistics/viewevents', - headers: { - [XSRF_REQUEST_HEADER] : csrfToken, - // use a known public IP address to avoid being seen as a "bot" - 'X-Forwarded-For': '1.1.1.1', - // Use a user-agent of a Firefox browser on Windows. This again avoids being seen as a "bot" - 'user-agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/119.0', - }, - //form: true, // indicates the body should be form urlencoded - body: { targetId: uuid, targetType: dsoType }, - }).then((resp) => { - // We expect a 201 (which means statistics event was created) - expect(resp.status).to.eq(201); + // Create a fake CSRF cookie/token to use in POST + cy.createCSRFCookie().then((csrfToken: string) => { + // get our REST API's base URL, also needed for POST + cy.task('getRestBaseURL').then((baseRestUrl: string) => { + // Now, send 'statistics/viewevents' POST request including that fake CSRF token in required header + cy.request({ + method: 'POST', + url: baseRestUrl + '/api/statistics/viewevents', + headers: { + [XSRF_REQUEST_HEADER] : csrfToken, + // use a known public IP address to avoid being seen as a "bot" + 'X-Forwarded-For': '1.1.1.1', + // Use a user-agent of a Firefox browser on Windows. This again avoids being seen as a "bot" + 'user-agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/119.0', + }, + //form: true, // indicates the body should be form urlencoded + body: { targetId: uuid, targetType: dsoType }, + }).then((resp) => { + // We expect a 201 (which means statistics event was created) + expect(resp.status).to.eq(201); + }); }); - - // Remove cookie with fake CSRF token, as it's no longer needed - cy.clearCookie(DSPACE_XSRF_COOKIE); }); } // Add as a Cypress command (i.e. assign to 'cy.generateViewEvent') Cypress.Commands.add('generateViewEvent', generateViewEvent); + +/** + * Can be used by tests to generate a random XSRF/CSRF token and save it to + * the required XSRF/CSRF cookie for usage when sending POST requests or similar. + * The generated CSRF token is returned in a Chainable to allow it to be also sent + * in the CSRF HTTP Header. + * @returns a Cypress Chainable which can be used to get the generated CSRF Token + */ +function createCSRFCookie(): Cypress.Chainable { + // Generate a new token which is a random UUID + const csrfToken: string = uuidv4(); + + // Save it to our required cookie + cy.task('getRestBaseDomain').then((baseDomain: string) => { + // Create a fake CSRF Token. Set it in the required server-side cookie + cy.setCookie(DSPACE_XSRF_COOKIE, csrfToken, { 'domain': baseDomain }); + }); + + // return the generated token wrapped in a chainable + return cy.wrap(csrfToken); +} +// Add as a Cypress command (i.e. assign to 'cy.createCSRFCookie') +Cypress.Commands.add('createCSRFCookie', createCSRFCookie); diff --git a/cypress/support/e2e.ts b/cypress/support/e2e.ts index dd7ee1824c..5e4fddb805 100644 --- a/cypress/support/e2e.ts +++ b/cypress/support/e2e.ts @@ -19,24 +19,50 @@ import './commands'; // Import Cypress Axe tools for all tests // https://github.com/component-driven/cypress-axe import 'cypress-axe'; +import { DSPACE_XSRF_COOKIE } from 'src/app/core/xsrf/xsrf.constants'; + + +// Runs once before all tests +before(() => { + // Cypress doesn't have access to the running application in Node.js. + // So, it's not possible to inject or load the AppConfig or environment of the Angular UI. + // Instead, we'll read our running application's config.json, which contains the configs & + // is regenerated at runtime each time the Angular UI application starts up. + cy.task('readUIConfig').then((str: string) => { + // Parse config into a JSON object + const config = JSON.parse(str); + + // Find URL of our REST API & save to global variable via task + let baseRestUrl = FALLBACK_TEST_REST_BASE_URL; + if (!config.rest.baseUrl) { + console.warn("Could not load 'rest.baseUrl' from config.json. Falling back to " + FALLBACK_TEST_REST_BASE_URL); + } else { + baseRestUrl = config.rest.baseUrl; + } + cy.task('saveRestBaseURL', baseRestUrl); + + // Find domain of our REST API & save to global variable via task. + let baseDomain = FALLBACK_TEST_REST_DOMAIN; + if (!config.rest.host) { + console.warn("Could not load 'rest.host' from config.json. Falling back to " + FALLBACK_TEST_REST_DOMAIN); + } else { + baseDomain = config.rest.host; + } + cy.task('saveRestBaseDomain', baseDomain); + + }); +}); // Runs once before the first test in each "block" beforeEach(() => { // Pre-agree to all Klaro cookies by setting the klaro-anonymous cookie // This just ensures it doesn't get in the way of matching other objects in the page. cy.setCookie('klaro-anonymous', '{%22authentication%22:true%2C%22preferences%22:true%2C%22acknowledgement%22:true%2C%22google-analytics%22:true%2C%22google-recaptcha%22:true}'); + + // Remove any CSRF cookies saved from prior tests + cy.clearCookie(DSPACE_XSRF_COOKIE); }); -// For better stability between tests, we visit "about:blank" (i.e. blank page) after each test. -// This ensures any remaining/outstanding XHR requests are killed, so they don't affect the next test. -// Borrowed from: https://glebbahmutov.com/blog/visit-blank-page-between-tests/ -/*afterEach(() => { - cy.window().then((win) => { - win.location.href = 'about:blank'; - }); -});*/ - - // Global constants used in tests // May be overridden in our cypress.json config file using specified environment variables. // Default values listed here are all valid for the Demo Entities Data set available at @@ -57,7 +83,10 @@ export const TEST_SUBMIT_COLLECTION_NAME = Cypress.env('DSPACE_TEST_SUBMIT_COLLE export const TEST_SUBMIT_COLLECTION_UUID = Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_UUID') || '9d8334e9-25d3-4a67-9cea-3dffdef80144'; export const TEST_SUBMIT_USER = Cypress.env('DSPACE_TEST_SUBMIT_USER') || 'dspacedemo+submit@gmail.com'; export const TEST_SUBMIT_USER_PASSWORD = Cypress.env('DSPACE_TEST_SUBMIT_USER_PASSWORD') || 'dspace'; - +// NOTE: FALLBACK_TEST_REST_BASE_URL is only used if Cypress cannot read the REST API BaseURL +// from the Angular UI's config.json. See 'before()' above. +const FALLBACK_TEST_REST_BASE_URL = 'http://localhost:8080/server'; +const FALLBACK_TEST_REST_DOMAIN = 'localhost'; // USEFUL REGEX for testing From 8d61fa6ac3cb65e72bfd2d9afb3a80908e4b8774 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 5 Sep 2023 14:47:59 -0500 Subject: [PATCH 15/23] Improve accessibility testing on Submission page --- cypress/e2e/submission.cy.ts | 87 +++++++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 2 deletions(-) diff --git a/cypress/e2e/submission.cy.ts b/cypress/e2e/submission.cy.ts index ed10b2d13a..43a063035d 100644 --- a/cypress/e2e/submission.cy.ts +++ b/cypress/e2e/submission.cy.ts @@ -1,8 +1,10 @@ -import { TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD, TEST_SUBMIT_COLLECTION_NAME, TEST_SUBMIT_COLLECTION_UUID } from 'cypress/support/e2e'; +import { testA11y } from 'cypress/support/utils'; +import { TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD, TEST_SUBMIT_COLLECTION_NAME, TEST_SUBMIT_COLLECTION_UUID, TEST_ADMIN_USER, TEST_ADMIN_PASSWORD } from 'cypress/support/e2e'; +import { Options } from 'cypress-axe'; describe('New Submission page', () => { - // NOTE: We already test that new submissions can be started from MyDSpace in my-dspace.spec.ts + // NOTE: We already test that new Item submissions can be started from MyDSpace in my-dspace.spec.ts it('should create a new submission when using /submit path & pass accessibility', () => { // Test that calling /submit with collection & entityType will create a new submission cy.visit('/submit?collection='.concat(TEST_SUBMIT_COLLECTION_UUID).concat('&entityType=none')); @@ -25,6 +27,22 @@ describe('New Submission page', () => { cy.get('div#section_upload').should('be.visible'); cy.get('div#section_license').should('be.visible'); + // Test entire page for accessibility + testA11y('ds-submission-edit', + { + rules: { + // Author & Subject fields have invalid "aria-multiline" attrs, see #1272 + 'aria-allowed-attr': { enabled: false }, + // All panels are accordians & fail "aria-required-children" and "nested-interactive". Seem to require updating ng-bootstrap and #2216 + 'aria-required-children': { enabled: false }, + 'nested-interactive': { enabled: false }, + // All select boxes fail to have a name / aria-label. This is a bug in ng-dynamic-forms and may require #2216 + 'select-name': { enabled: false }, + } + + } as Options + ); + // Discard button should work // Clicking it will display a confirmation, which we will confirm with another click cy.get('button#discard').click(); @@ -131,4 +149,69 @@ describe('New Submission page', () => { cy.get('ds-notification div.alert-success').should('be.visible'); }); + it('is possible to submit a new "Person" and that form passes accessibility', () => { + // To submit a different entity type, we'll start from MyDSpace + cy.visit('/mydspace'); + + // This page is restricted, so we will be shown the login form. Fill it out & submit. + // NOTE: At this time, we MUST login as admin to submit Person objects + cy.loginViaForm(TEST_ADMIN_USER, TEST_ADMIN_PASSWORD); + + // Open the New Submission dropdown + cy.get('button[data-test="submission-dropdown"]').click(); + // Click on the "Person" type in that dropdown + cy.get('#entityControlsDropdownMenu button[title="Person"]').click(); + + // This should display the (popup window) + cy.get('ds-create-item-parent-selector').should('be.visible'); + + // Click on the first Collection button to seelect a collection (actual collection doesn't matter) + cy.get('ds-authorized-collection-selector button:first-child').click(); + + // New URL should include /workspaceitems, as we've started a new submission + cy.url().should('include', '/workspaceitems'); + + // The Submission edit form tag should be visible + cy.get('ds-submission-edit').should('be.visible'); + + // 3 sections should be visible by default + cy.get('div#section_personStep').should('be.visible'); + cy.get('div#section_upload').should('be.visible'); + cy.get('div#section_license').should('be.visible'); + + // Test entire page for accessibility + testA11y('ds-submission-edit', + { + rules: { + // All panels are accordians & fail "aria-required-children" and "nested-interactive". Seem to require updating ng-bootstrap and #2216 + 'aria-required-children': { enabled: false }, + 'nested-interactive': { enabled: false }, + } + + } as Options + ); + + // Click the lookup button next to "Publication" field + cy.get('button[data-test="lookup-button"]').click(); + + // A popup modal window should be visible + cy.get('ds-dynamic-lookup-relation-modal').should('be.visible'); + + // Popup modal should also pass accessibility tests + //testA11y('ds-dynamic-lookup-relation-modal'); + testA11y({ + include: ['ds-dynamic-lookup-relation-modal'], + exclude: [ + ['ul.nav-tabs'] // Tabs at top of model have several issues which seem to be caused by ng-bootstrap + ], + }); + + // Close popup window + cy.get('ds-dynamic-lookup-relation-modal button.close').click(); + + // Back on the form, click the discard button to remove new submission + // Clicking it will display a confirmation, which we will confirm with another click + cy.get('button#discard').click(); + cy.get('button#discard_submit').click(); + }); }); From e47593b303f269a18b2a40b7920daa85a6c23123 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 8 Sep 2023 16:14:24 -0500 Subject: [PATCH 16/23] Fix circular dependency issue by using Cypres env variables directly instead of global constants --- cypress.config.ts | 5 +++-- cypress/e2e/breadcrumbs.cy.ts | 3 +-- cypress/e2e/collection-page.cy.ts | 3 +-- cypress/e2e/collection-statistics.cy.ts | 8 ++++---- cypress/e2e/community-page.cy.ts | 3 +-- cypress/e2e/community-statistics.cy.ts | 8 ++++---- cypress/e2e/homepage-statistics.cy.ts | 6 +++--- cypress/e2e/item-page.cy.ts | 5 ++--- cypress/e2e/item-statistics.cy.ts | 8 ++++---- cypress/e2e/login-modal.cy.ts | 11 +++++------ cypress/e2e/my-dspace.cy.ts | 15 +++++++-------- cypress/e2e/search-navbar.cy.ts | 4 +--- cypress/e2e/search-page.cy.ts | 10 ++++++---- cypress/e2e/submission.cy.ts | 18 +++++++++--------- cypress/support/e2e.ts | 20 -------------------- 15 files changed, 51 insertions(+), 76 deletions(-) diff --git a/cypress.config.ts b/cypress.config.ts index 91eeb9838b..b1da2b4793 100644 --- a/cypress.config.ts +++ b/cypress.config.ts @@ -9,8 +9,9 @@ export default defineConfig({ openMode: 0, }, env: { - // Global constants used in DSpace e2e tests (see also ./cypress/support/e2e.ts) - // May be overridden in our cypress.json config file using specified environment variables. + // Global DSpace environment variables used in all our Cypress e2e tests + // May be modified in this config, or overridden in a variety of ways. + // See Cypress environment variable docs: https://docs.cypress.io/guides/guides/environment-variables // Default values listed here are all valid for the Demo Entities Data set available at // https://github.com/DSpace-Labs/AIP-Files/releases/tag/demo-entities-data // (This is the data set used in our CI environment) diff --git a/cypress/e2e/breadcrumbs.cy.ts b/cypress/e2e/breadcrumbs.cy.ts index ea6acdafcd..0cddbc723c 100644 --- a/cypress/e2e/breadcrumbs.cy.ts +++ b/cypress/e2e/breadcrumbs.cy.ts @@ -1,10 +1,9 @@ -import { TEST_ENTITY_PUBLICATION } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; describe('Breadcrumbs', () => { it('should pass accessibility tests', () => { // Visit an Item, as those have more breadcrumbs - cy.visit('/entities/publication/'.concat(TEST_ENTITY_PUBLICATION)); + cy.visit('/entities/publication/'.concat(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION'))); // Wait for breadcrumbs to be visible cy.get('ds-breadcrumbs').should('be.visible'); diff --git a/cypress/e2e/collection-page.cy.ts b/cypress/e2e/collection-page.cy.ts index a034b4361d..55c10cc6e2 100644 --- a/cypress/e2e/collection-page.cy.ts +++ b/cypress/e2e/collection-page.cy.ts @@ -1,10 +1,9 @@ -import { TEST_COLLECTION } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; describe('Collection Page', () => { it('should pass accessibility tests', () => { - cy.visit('/collections/'.concat(TEST_COLLECTION)); + cy.visit('/collections/'.concat(Cypress.env('DSPACE_TEST_COLLECTION'))); // tag must be loaded cy.get('ds-collection-page').should('be.visible'); diff --git a/cypress/e2e/collection-statistics.cy.ts b/cypress/e2e/collection-statistics.cy.ts index 6df4e9a454..43bf67ce51 100644 --- a/cypress/e2e/collection-statistics.cy.ts +++ b/cypress/e2e/collection-statistics.cy.ts @@ -1,11 +1,11 @@ -import { REGEX_MATCH_NON_EMPTY_TEXT, TEST_COLLECTION } from 'cypress/support/e2e'; +import { REGEX_MATCH_NON_EMPTY_TEXT } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; describe('Collection Statistics Page', () => { - const COLLECTIONSTATISTICSPAGE = '/statistics/collections/'.concat(TEST_COLLECTION); + const COLLECTIONSTATISTICSPAGE = '/statistics/collections/'.concat(Cypress.env('DSPACE_TEST_COLLECTION')); it('should load if you click on "Statistics" from a Collection page', () => { - cy.visit('/collections/'.concat(TEST_COLLECTION)); + cy.visit('/collections/'.concat(Cypress.env('DSPACE_TEST_COLLECTION'))); cy.get('ds-navbar ds-link-menu-item a[title="Statistics"]').click(); cy.location('pathname').should('eq', COLLECTIONSTATISTICSPAGE); }); @@ -18,7 +18,7 @@ describe('Collection Statistics Page', () => { it('should contain a "Total visits per month" section', () => { cy.visit(COLLECTIONSTATISTICSPAGE); // Check just for existence because this table is empty in CI environment as it's historical data - cy.get('.'.concat(TEST_COLLECTION).concat('_TotalVisitsPerMonth')).should('exist'); + cy.get('.'.concat(Cypress.env('DSPACE_TEST_COLLECTION')).concat('_TotalVisitsPerMonth')).should('exist'); }); it('should pass accessibility tests', () => { diff --git a/cypress/e2e/community-page.cy.ts b/cypress/e2e/community-page.cy.ts index 6c628e21ce..4a2e2f9baa 100644 --- a/cypress/e2e/community-page.cy.ts +++ b/cypress/e2e/community-page.cy.ts @@ -1,10 +1,9 @@ -import { TEST_COMMUNITY } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; describe('Community Page', () => { it('should pass accessibility tests', () => { - cy.visit('/communities/'.concat(TEST_COMMUNITY)); + cy.visit('/communities/'.concat(Cypress.env('DSPACE_TEST_COMMUNITY'))); // tag must be loaded cy.get('ds-community-page').should('be.visible'); diff --git a/cypress/e2e/community-statistics.cy.ts b/cypress/e2e/community-statistics.cy.ts index 710450e797..ca306eff5c 100644 --- a/cypress/e2e/community-statistics.cy.ts +++ b/cypress/e2e/community-statistics.cy.ts @@ -1,11 +1,11 @@ -import { REGEX_MATCH_NON_EMPTY_TEXT, TEST_COMMUNITY } from 'cypress/support/e2e'; +import { REGEX_MATCH_NON_EMPTY_TEXT } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; describe('Community Statistics Page', () => { - const COMMUNITYSTATISTICSPAGE = '/statistics/communities/'.concat(TEST_COMMUNITY); + const COMMUNITYSTATISTICSPAGE = '/statistics/communities/'.concat(Cypress.env('DSPACE_TEST_COMMUNITY')); it('should load if you click on "Statistics" from a Community page', () => { - cy.visit('/communities/'.concat(TEST_COMMUNITY)); + cy.visit('/communities/'.concat(Cypress.env('DSPACE_TEST_COMMUNITY'))); cy.get('ds-navbar ds-link-menu-item a[title="Statistics"]').click(); cy.location('pathname').should('eq', COMMUNITYSTATISTICSPAGE); }); @@ -18,7 +18,7 @@ describe('Community Statistics Page', () => { it('should contain a "Total visits per month" section', () => { cy.visit(COMMUNITYSTATISTICSPAGE); // Check just for existence because this table is empty in CI environment as it's historical data - cy.get('.'.concat(TEST_COMMUNITY).concat('_TotalVisitsPerMonth')).should('exist'); + cy.get('.'.concat(Cypress.env('DSPACE_TEST_COMMUNITY')).concat('_TotalVisitsPerMonth')).should('exist'); }); it('should pass accessibility tests', () => { diff --git a/cypress/e2e/homepage-statistics.cy.ts b/cypress/e2e/homepage-statistics.cy.ts index 2a1ab9785a..ff7dbeb852 100644 --- a/cypress/e2e/homepage-statistics.cy.ts +++ b/cypress/e2e/homepage-statistics.cy.ts @@ -1,4 +1,4 @@ -import { REGEX_MATCH_NON_EMPTY_TEXT, TEST_ENTITY_PUBLICATION } from 'cypress/support/e2e'; +import { REGEX_MATCH_NON_EMPTY_TEXT } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; import '../support/commands'; @@ -11,8 +11,8 @@ describe('Site Statistics Page', () => { it('should pass accessibility tests', () => { // generate 2 view events on an Item's page - cy.generateViewEvent(TEST_ENTITY_PUBLICATION, 'item'); - cy.generateViewEvent(TEST_ENTITY_PUBLICATION, 'item'); + cy.generateViewEvent(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION'), 'item'); + cy.generateViewEvent(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION'), 'item'); cy.visit('/statistics'); diff --git a/cypress/e2e/item-page.cy.ts b/cypress/e2e/item-page.cy.ts index 9dba6eb8ce..a6a208e9f4 100644 --- a/cypress/e2e/item-page.cy.ts +++ b/cypress/e2e/item-page.cy.ts @@ -1,9 +1,8 @@ -import { TEST_ENTITY_PUBLICATION } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; describe('Item Page', () => { - const ITEMPAGE = '/items/'.concat(TEST_ENTITY_PUBLICATION); - const ENTITYPAGE = '/entities/publication/'.concat(TEST_ENTITY_PUBLICATION); + const ITEMPAGE = '/items/'.concat(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION')); + const ENTITYPAGE = '/entities/publication/'.concat(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION')); // Test that entities will redirect to /entities/[type]/[uuid] when accessed via /items/[uuid] it('should redirect to the entity page when navigating to an item page', () => { diff --git a/cypress/e2e/item-statistics.cy.ts b/cypress/e2e/item-statistics.cy.ts index 9b90cb24af..b856744cba 100644 --- a/cypress/e2e/item-statistics.cy.ts +++ b/cypress/e2e/item-statistics.cy.ts @@ -1,11 +1,11 @@ -import { REGEX_MATCH_NON_EMPTY_TEXT, TEST_ENTITY_PUBLICATION } from 'cypress/support/e2e'; +import { REGEX_MATCH_NON_EMPTY_TEXT } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; describe('Item Statistics Page', () => { - const ITEMSTATISTICSPAGE = '/statistics/items/'.concat(TEST_ENTITY_PUBLICATION); + const ITEMSTATISTICSPAGE = '/statistics/items/'.concat(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION')); it('should load if you click on "Statistics" from an Item/Entity page', () => { - cy.visit('/entities/publication/'.concat(TEST_ENTITY_PUBLICATION)); + cy.visit('/entities/publication/'.concat(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION'))); cy.get('ds-navbar ds-link-menu-item a[title="Statistics"]').click(); cy.location('pathname').should('eq', ITEMSTATISTICSPAGE); }); @@ -24,7 +24,7 @@ describe('Item Statistics Page', () => { it('should contain a "Total visits per month" section', () => { cy.visit(ITEMSTATISTICSPAGE); // Check just for existence because this table is empty in CI environment as it's historical data - cy.get('.'.concat(TEST_ENTITY_PUBLICATION).concat('_TotalVisitsPerMonth')).should('exist'); + cy.get('.'.concat(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION')).concat('_TotalVisitsPerMonth')).should('exist'); }); it('should pass accessibility tests', () => { diff --git a/cypress/e2e/login-modal.cy.ts b/cypress/e2e/login-modal.cy.ts index d4559dc6bf..c56b98fd26 100644 --- a/cypress/e2e/login-modal.cy.ts +++ b/cypress/e2e/login-modal.cy.ts @@ -1,4 +1,3 @@ -import { TEST_ADMIN_PASSWORD, TEST_ADMIN_USER, TEST_ENTITY_PUBLICATION } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; const page = { @@ -37,7 +36,7 @@ const page = { describe('Login Modal', () => { it('should login when clicking button & stay on same page', () => { - const ENTITYPAGE = '/entities/publication/'.concat(TEST_ENTITY_PUBLICATION); + const ENTITYPAGE = '/entities/publication/'.concat(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION')); cy.visit(ENTITYPAGE); // Login menu should exist @@ -47,7 +46,7 @@ describe('Login Modal', () => { page.openLoginMenu(); cy.get('.form-login').should('be.visible'); - page.submitLoginAndPasswordByPressingButton(TEST_ADMIN_USER, TEST_ADMIN_PASSWORD); + page.submitLoginAndPasswordByPressingButton(Cypress.env('DSPACE_TEST_ADMIN_USER'), Cypress.env('DSPACE_TEST_ADMIN_PASSWORD')); cy.get('ds-log-in').should('not.exist'); // Verify we are still on the same page @@ -67,7 +66,7 @@ describe('Login Modal', () => { cy.get('.form-login').should('be.visible'); // Login, and the tag should no longer exist - page.submitLoginAndPasswordByPressingEnter(TEST_ADMIN_USER, TEST_ADMIN_PASSWORD); + page.submitLoginAndPasswordByPressingEnter(Cypress.env('DSPACE_TEST_ADMIN_USER'), Cypress.env('DSPACE_TEST_ADMIN_PASSWORD')); cy.get('.form-login').should('not.exist'); // Verify we are still on homepage @@ -81,7 +80,7 @@ describe('Login Modal', () => { it('should support logout', () => { // First authenticate & access homepage - cy.login(TEST_ADMIN_USER, TEST_ADMIN_PASSWORD); + cy.login(Cypress.env('DSPACE_TEST_ADMIN_USER'), Cypress.env('DSPACE_TEST_ADMIN_PASSWORD')); cy.visit('/'); // Verify ds-log-in tag doesn't exist, but ds-log-out tag does exist @@ -140,7 +139,7 @@ describe('Login Modal', () => { testA11y('ds-log-in'); // Now login - page.submitLoginAndPasswordByPressingButton(TEST_ADMIN_USER, TEST_ADMIN_PASSWORD); + page.submitLoginAndPasswordByPressingButton(Cypress.env('DSPACE_TEST_ADMIN_USER'), Cypress.env('DSPACE_TEST_ADMIN_PASSWORD')); cy.get('ds-log-in').should('not.exist'); // Open user menu, verify user menu accesibility diff --git a/cypress/e2e/my-dspace.cy.ts b/cypress/e2e/my-dspace.cy.ts index 4b27fe13de..c48656ffcc 100644 --- a/cypress/e2e/my-dspace.cy.ts +++ b/cypress/e2e/my-dspace.cy.ts @@ -1,4 +1,3 @@ -import { TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD, TEST_SUBMIT_COLLECTION_NAME } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; describe('My DSpace page', () => { @@ -6,7 +5,7 @@ describe('My DSpace page', () => { cy.visit('/mydspace'); // This page is restricted, so we will be shown the login form. Fill it out & submit. - cy.loginViaForm(TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD); + cy.loginViaForm(Cypress.env('DSPACE_TEST_SUBMIT_USER'), Cypress.env('DSPACE_TEST_SUBMIT_USER_PASSWORD')); cy.get('ds-my-dspace-page').should('be.visible'); @@ -25,7 +24,7 @@ describe('My DSpace page', () => { cy.visit('/mydspace'); // This page is restricted, so we will be shown the login form. Fill it out & submit. - cy.loginViaForm(TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD); + cy.loginViaForm(Cypress.env('DSPACE_TEST_SUBMIT_USER'), Cypress.env('DSPACE_TEST_SUBMIT_USER_PASSWORD')); cy.get('ds-my-dspace-page').should('be.visible'); @@ -43,7 +42,7 @@ describe('My DSpace page', () => { cy.visit('/mydspace'); // This page is restricted, so we will be shown the login form. Fill it out & submit. - cy.loginViaForm(TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD); + cy.loginViaForm(Cypress.env('DSPACE_TEST_SUBMIT_USER'), Cypress.env('DSPACE_TEST_SUBMIT_USER_PASSWORD')); // Open the New Submission dropdown cy.get('button[data-test="submission-dropdown"]').click(); @@ -54,10 +53,10 @@ describe('My DSpace page', () => { cy.get('ds-create-item-parent-selector').should('be.visible'); // Type in a known Collection name in the search box - cy.get('ds-authorized-collection-selector input[type="search"]').type(TEST_SUBMIT_COLLECTION_NAME); + cy.get('ds-authorized-collection-selector input[type="search"]').type(Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_NAME')); // Click on the button matching that known Collection name - cy.get('ds-authorized-collection-selector button[title="'.concat(TEST_SUBMIT_COLLECTION_NAME).concat('"]')).click(); + cy.get('ds-authorized-collection-selector button[title="'.concat(Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_NAME')).concat('"]')).click(); // New URL should include /workspaceitems, as we've started a new submission cy.url().should('include', '/workspaceitems'); @@ -66,7 +65,7 @@ describe('My DSpace page', () => { cy.get('ds-submission-edit').should('be.visible'); // A Collection menu button should exist & its value should be the selected collection - cy.get('#collectionControlsMenuButton span').should('have.text', TEST_SUBMIT_COLLECTION_NAME); + cy.get('#collectionControlsMenuButton span').should('have.text', Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_NAME')); // Now that we've created a submission, we'll test that we can go back and Edit it. // Get our Submission URL, to parse out the ID of this new submission @@ -115,7 +114,7 @@ describe('My DSpace page', () => { cy.visit('/mydspace'); // This page is restricted, so we will be shown the login form. Fill it out & submit. - cy.loginViaForm(TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD); + cy.loginViaForm(Cypress.env('DSPACE_TEST_SUBMIT_USER'), Cypress.env('DSPACE_TEST_SUBMIT_USER_PASSWORD')); // Open the New Import dropdown cy.get('button[data-test="import-dropdown"]').click(); diff --git a/cypress/e2e/search-navbar.cy.ts b/cypress/e2e/search-navbar.cy.ts index 648db17fe6..9dd93c7a2d 100644 --- a/cypress/e2e/search-navbar.cy.ts +++ b/cypress/e2e/search-navbar.cy.ts @@ -1,5 +1,3 @@ -import { TEST_SEARCH_TERM } from 'cypress/support/e2e'; - const page = { fillOutQueryInNavBar(query) { // Click the magnifying glass @@ -17,7 +15,7 @@ const page = { describe('Search from Navigation Bar', () => { // NOTE: these tests currently assume this query will return results! - const query = TEST_SEARCH_TERM; + const query = Cypress.env('DSPACE_TEST_SEARCH_TERM'); it('should go to search page with correct query if submitted (from home)', () => { cy.visit('/'); diff --git a/cypress/e2e/search-page.cy.ts b/cypress/e2e/search-page.cy.ts index aca3d23d04..429f4e6da4 100644 --- a/cypress/e2e/search-page.cy.ts +++ b/cypress/e2e/search-page.cy.ts @@ -1,8 +1,10 @@ import { Options } from 'cypress-axe'; -import { TEST_SEARCH_TERM } from 'cypress/support/e2e'; import { testA11y } from 'cypress/support/utils'; describe('Search Page', () => { + // NOTE: these tests currently assume this query will return results! + const query = Cypress.env('DSPACE_TEST_SEARCH_TERM'); + it('should redirect to the correct url when query was set and submit button was triggered', () => { const queryString = 'Another interesting query string'; cy.visit('/search'); @@ -13,8 +15,8 @@ describe('Search Page', () => { }); it('should load results and pass accessibility tests', () => { - cy.visit('/search?query='.concat(TEST_SEARCH_TERM)); - cy.get('[data-test="search-box"]').should('have.value', TEST_SEARCH_TERM); + cy.visit('/search?query='.concat(query)); + cy.get('[data-test="search-box"]').should('have.value', query); // tag must be loaded cy.get('ds-search-page').should('be.visible'); @@ -31,7 +33,7 @@ describe('Search Page', () => { }); it('should have a working grid view that passes accessibility tests', () => { - cy.visit('/search?query='.concat(TEST_SEARCH_TERM)); + cy.visit('/search?query='.concat(query)); // Click button in sidebar to display grid view cy.get('ds-search-sidebar [data-test="grid-view"]').click(); diff --git a/cypress/e2e/submission.cy.ts b/cypress/e2e/submission.cy.ts index 43a063035d..2b482d8515 100644 --- a/cypress/e2e/submission.cy.ts +++ b/cypress/e2e/submission.cy.ts @@ -1,5 +1,5 @@ import { testA11y } from 'cypress/support/utils'; -import { TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD, TEST_SUBMIT_COLLECTION_NAME, TEST_SUBMIT_COLLECTION_UUID, TEST_ADMIN_USER, TEST_ADMIN_PASSWORD } from 'cypress/support/e2e'; +//import { TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD, TEST_SUBMIT_COLLECTION_NAME, TEST_SUBMIT_COLLECTION_UUID, TEST_ADMIN_USER, TEST_ADMIN_PASSWORD } from 'cypress/support/e2e'; import { Options } from 'cypress-axe'; describe('New Submission page', () => { @@ -7,10 +7,10 @@ describe('New Submission page', () => { // NOTE: We already test that new Item submissions can be started from MyDSpace in my-dspace.spec.ts it('should create a new submission when using /submit path & pass accessibility', () => { // Test that calling /submit with collection & entityType will create a new submission - cy.visit('/submit?collection='.concat(TEST_SUBMIT_COLLECTION_UUID).concat('&entityType=none')); + cy.visit('/submit?collection='.concat(Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_UUID')).concat('&entityType=none')); // This page is restricted, so we will be shown the login form. Fill it out & submit. - cy.loginViaForm(TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD); + cy.loginViaForm(Cypress.env('DSPACE_TEST_SUBMIT_USER'), Cypress.env('DSPACE_TEST_SUBMIT_USER_PASSWORD')); // Should redirect to /workspaceitems, as we've started a new submission cy.url().should('include', '/workspaceitems'); @@ -19,7 +19,7 @@ describe('New Submission page', () => { cy.get('ds-submission-edit').should('be.visible'); // A Collection menu button should exist & it's value should be the selected collection - cy.get('#collectionControlsMenuButton span').should('have.text', TEST_SUBMIT_COLLECTION_NAME); + cy.get('#collectionControlsMenuButton span').should('have.text', Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_NAME')); // 4 sections should be visible by default cy.get('div#section_traditionalpageone').should('be.visible'); @@ -51,10 +51,10 @@ describe('New Submission page', () => { it('should block submission & show errors if required fields are missing', () => { // Create a new submission - cy.visit('/submit?collection='.concat(TEST_SUBMIT_COLLECTION_UUID).concat('&entityType=none')); + cy.visit('/submit?collection='.concat(Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_UUID')).concat('&entityType=none')); // This page is restricted, so we will be shown the login form. Fill it out & submit. - cy.loginViaForm(TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD); + cy.loginViaForm(Cypress.env('DSPACE_TEST_SUBMIT_USER'), Cypress.env('DSPACE_TEST_SUBMIT_USER_PASSWORD')); // Attempt an immediate deposit without filling out any fields cy.get('button#deposit').click(); @@ -111,10 +111,10 @@ describe('New Submission page', () => { it('should allow for deposit if all required fields completed & file uploaded', () => { // Create a new submission - cy.visit('/submit?collection='.concat(TEST_SUBMIT_COLLECTION_UUID).concat('&entityType=none')); + cy.visit('/submit?collection='.concat(Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_UUID')).concat('&entityType=none')); // This page is restricted, so we will be shown the login form. Fill it out & submit. - cy.loginViaForm(TEST_SUBMIT_USER, TEST_SUBMIT_USER_PASSWORD); + cy.loginViaForm(Cypress.env('DSPACE_TEST_SUBMIT_USER'), Cypress.env('DSPACE_TEST_SUBMIT_USER_PASSWORD')); // Fill out all required fields (Title, Date) cy.get('input#dc_title').type('DSpace logo uploaded via e2e tests'); @@ -155,7 +155,7 @@ describe('New Submission page', () => { // This page is restricted, so we will be shown the login form. Fill it out & submit. // NOTE: At this time, we MUST login as admin to submit Person objects - cy.loginViaForm(TEST_ADMIN_USER, TEST_ADMIN_PASSWORD); + cy.loginViaForm(Cypress.env('DSPACE_TEST_ADMIN_USER'), Cypress.env('DSPACE_TEST_ADMIN_PASSWORD')); // Open the New Submission dropdown cy.get('button[data-test="submission-dropdown"]').click(); diff --git a/cypress/support/e2e.ts b/cypress/support/e2e.ts index 5e4fddb805..f6c6865052 100644 --- a/cypress/support/e2e.ts +++ b/cypress/support/e2e.ts @@ -63,26 +63,6 @@ beforeEach(() => { cy.clearCookie(DSPACE_XSRF_COOKIE); }); -// Global constants used in tests -// May be overridden in our cypress.json config file using specified environment variables. -// Default values listed here are all valid for the Demo Entities Data set available at -// https://github.com/DSpace-Labs/AIP-Files/releases/tag/demo-entities-data -// (This is the data set used in our CI environment) - -// Admin account used for administrative tests -export const TEST_ADMIN_USER = Cypress.env('DSPACE_TEST_ADMIN_USER') || 'dspacedemo+admin@gmail.com'; -export const TEST_ADMIN_PASSWORD = Cypress.env('DSPACE_TEST_ADMIN_PASSWORD') || 'dspace'; -// Community/collection/publication used for view/edit tests -export const TEST_COLLECTION = Cypress.env('DSPACE_TEST_COLLECTION') || '282164f5-d325-4740-8dd1-fa4d6d3e7200'; -export const TEST_COMMUNITY = Cypress.env('DSPACE_TEST_COMMUNITY') || '0958c910-2037-42a9-81c7-dca80e3892b4'; -export const TEST_ENTITY_PUBLICATION = Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION') || 'e98b0f27-5c19-49a0-960d-eb6ad5287067'; -// Search term (should return results) used in search tests -export const TEST_SEARCH_TERM = Cypress.env('DSPACE_TEST_SEARCH_TERM') || 'test'; -// Collection used for submission tests -export const TEST_SUBMIT_COLLECTION_NAME = Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_NAME') || 'Sample Collection'; -export const TEST_SUBMIT_COLLECTION_UUID = Cypress.env('DSPACE_TEST_SUBMIT_COLLECTION_UUID') || '9d8334e9-25d3-4a67-9cea-3dffdef80144'; -export const TEST_SUBMIT_USER = Cypress.env('DSPACE_TEST_SUBMIT_USER') || 'dspacedemo+submit@gmail.com'; -export const TEST_SUBMIT_USER_PASSWORD = Cypress.env('DSPACE_TEST_SUBMIT_USER_PASSWORD') || 'dspace'; // NOTE: FALLBACK_TEST_REST_BASE_URL is only used if Cypress cannot read the REST API BaseURL // from the Angular UI's config.json. See 'before()' above. const FALLBACK_TEST_REST_BASE_URL = 'http://localhost:8080/server'; From 6c76d8c1d77c5466042edc5301e81742546640b4 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 8 Dec 2023 14:54:15 -0600 Subject: [PATCH 17/23] Add Edit Community accessibility tests & minor cleanup --- cypress/e2e/community-edit.cy.ts | 98 +++++++++++++++++++ cypress/e2e/community-page.cy.ts | 2 +- cypress/e2e/submission.cy.ts | 12 ++- .../edit-comcol-page.component.html | 6 +- 4 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 cypress/e2e/community-edit.cy.ts diff --git a/cypress/e2e/community-edit.cy.ts b/cypress/e2e/community-edit.cy.ts new file mode 100644 index 0000000000..06d78701f0 --- /dev/null +++ b/cypress/e2e/community-edit.cy.ts @@ -0,0 +1,98 @@ +import { testA11y } from 'cypress/support/utils'; + +const COMMUNITY_EDIT_PAGE = '/communities/'.concat(Cypress.env('DSPACE_TEST_COMMUNITY')).concat('/edit'); + +beforeEach(() => { + // All tests start with visiting the Edit Community Page + cy.visit(COMMUNITY_EDIT_PAGE); + + // 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')); +}); + +describe('Edit Community > Edit Metadata tab', () => { + it('should pass accessibility tests', () => { + // tag must be loaded + cy.get('ds-edit-community').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-edit-community'); + }); +}); + +describe('Edit Community > Assign Roles tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="roles"]').click(); + + // tag must be loaded + cy.get('ds-community-roles').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-community-roles'); + }); +}); + +describe('Edit Community > Curate tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="curate"]').click(); + + // tag must be loaded + cy.get('ds-community-curate').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-community-curate'); + }); +}); + +describe('Edit Community > Access Control tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="access-control"]').click(); + + // tag must be loaded + cy.get('ds-community-access-control').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-community-access-control'); + }); +}); + +describe('Edit Community > Authorizations tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="authorizations"]').click(); + + // tag must be loaded + cy.get('ds-community-authorizations').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-community-authorizations'); + }); + + it('should have an edit policy page which also passes accessibility tests', () => { + cy.visit(COMMUNITY_EDIT_PAGE.concat('/authorizations')); + + cy.get('button[title="Edit policy"]').click(); + + // tag must be loaded + cy.get('ds-resource-policy-edit').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-resource-policy-edit'); + }); +}); + +describe('Edit Community > Delete page', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="delete-button"]').click(); + + // tag must be loaded + cy.get('ds-delete-community').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-delete-community'); + }); +}); diff --git a/cypress/e2e/community-page.cy.ts b/cypress/e2e/community-page.cy.ts index 4a2e2f9baa..386bb592a0 100644 --- a/cypress/e2e/community-page.cy.ts +++ b/cypress/e2e/community-page.cy.ts @@ -9,6 +9,6 @@ describe('Community Page', () => { cy.get('ds-community-page').should('be.visible'); // Analyze for accessibility issues - testA11y('ds-community-page',); + testA11y('ds-community-page'); }); }); diff --git a/cypress/e2e/submission.cy.ts b/cypress/e2e/submission.cy.ts index 2b482d8515..97676ca25c 100644 --- a/cypress/e2e/submission.cy.ts +++ b/cypress/e2e/submission.cy.ts @@ -31,12 +31,15 @@ describe('New Submission page', () => { testA11y('ds-submission-edit', { rules: { - // Author & Subject fields have invalid "aria-multiline" attrs, see #1272 + // Author & Subject fields have invalid "aria-multiline" attrs. + // See https://github.com/DSpace/dspace-angular/issues/1272 'aria-allowed-attr': { enabled: false }, - // All panels are accordians & fail "aria-required-children" and "nested-interactive". Seem to require updating ng-bootstrap and #2216 + // All panels are accordians & fail "aria-required-children" and "nested-interactive". + // Seem to require updating ng-bootstrap and https://github.com/DSpace/dspace-angular/issues/2216 'aria-required-children': { enabled: false }, 'nested-interactive': { enabled: false }, - // All select boxes fail to have a name / aria-label. This is a bug in ng-dynamic-forms and may require #2216 + // All select boxes fail to have a name / aria-label. + // This is a bug in ng-dynamic-forms and may require https://github.com/DSpace/dspace-angular/issues/2216 'select-name': { enabled: false }, } @@ -183,7 +186,8 @@ describe('New Submission page', () => { testA11y('ds-submission-edit', { rules: { - // All panels are accordians & fail "aria-required-children" and "nested-interactive". Seem to require updating ng-bootstrap and #2216 + // All panels are accordians & fail "aria-required-children" and "nested-interactive". + // Seem to require updating ng-bootstrap and https://github.com/DSpace/dspace-angular/issues/2216 'aria-required-children': { enabled: false }, 'nested-interactive': { enabled: false }, } diff --git a/src/app/shared/comcol/comcol-forms/edit-comcol-page/edit-comcol-page.component.html b/src/app/shared/comcol/comcol-forms/edit-comcol-page/edit-comcol-page.component.html index c397966fba..d7f15ce06b 100644 --- a/src/app/shared/comcol/comcol-forms/edit-comcol-page/edit-comcol-page.component.html +++ b/src/app/shared/comcol/comcol-forms/edit-comcol-page/edit-comcol-page.component.html @@ -5,7 +5,8 @@

{{ type + '.edit.head' | translate }}

+ [routerLink]="((type === 'community') ? '/communities/' : '/collections/') + (dsoRD$ | async)?.payload.uuid + '/delete'" + data-test="delete-button"> {{type + '.edit.delete' | translate}}
@@ -14,7 +15,8 @@
  • + [routerLink]="['./' + page]" + [attr.data-test]="page"> {{ type + '.edit.tabs.' + page + '.head' | translate}}
    • From 9894d315a09d116fcdc536ef23a7db1e5828dc59 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 8 Dec 2023 16:09:02 -0600 Subject: [PATCH 18/23] Add Edit Collection accessibility Testing --- cypress/e2e/collection-edit.cy.ts | 86 +++++++++++++++++++++++++++++++ cypress/e2e/community-edit.cy.ts | 12 ----- 2 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 cypress/e2e/collection-edit.cy.ts diff --git a/cypress/e2e/collection-edit.cy.ts b/cypress/e2e/collection-edit.cy.ts new file mode 100644 index 0000000000..f6089acf70 --- /dev/null +++ b/cypress/e2e/collection-edit.cy.ts @@ -0,0 +1,86 @@ +import { testA11y } from 'cypress/support/utils'; + +const COLLECTION_EDIT_PAGE = '/collections/'.concat(Cypress.env('DSPACE_TEST_COLLECTION')).concat('/edit'); + +beforeEach(() => { + // All tests start with visiting the Edit Collection Page + cy.visit(COLLECTION_EDIT_PAGE); + + // 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')); +}); + +describe('Edit Collection > Edit Metadata tab', () => { + it('should pass accessibility tests', () => { + // tag must be loaded + cy.get('ds-edit-collection').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-edit-collection'); + }); +}); + +describe('Edit Collection > Assign Roles tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="roles"]').click(); + + // tag must be loaded + cy.get('ds-collection-roles').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-collection-roles'); + }); +}); + +describe('Edit Collection > Curate tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="curate"]').click(); + + // tag must be loaded + cy.get('ds-collection-curate').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-collection-curate'); + }); +}); + +describe('Edit Collection > Access Control tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="access-control"]').click(); + + // tag must be loaded + cy.get('ds-collection-access-control').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-collection-access-control'); + }); +}); + +describe('Edit Collection > Authorizations tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="authorizations"]').click(); + + // tag must be loaded + cy.get('ds-collection-authorizations').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-collection-authorizations'); + }); +}); + +describe('Edit Collection > Delete page', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="delete-button"]').click(); + + // tag must be loaded + cy.get('ds-delete-collection').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-delete-collection'); + }); +}); diff --git a/cypress/e2e/community-edit.cy.ts b/cypress/e2e/community-edit.cy.ts index 06d78701f0..8fc1a7733e 100644 --- a/cypress/e2e/community-edit.cy.ts +++ b/cypress/e2e/community-edit.cy.ts @@ -70,18 +70,6 @@ describe('Edit Community > Authorizations tab', () => { // Analyze for accessibility issues testA11y('ds-community-authorizations'); }); - - it('should have an edit policy page which also passes accessibility tests', () => { - cy.visit(COMMUNITY_EDIT_PAGE.concat('/authorizations')); - - cy.get('button[title="Edit policy"]').click(); - - // tag must be loaded - cy.get('ds-resource-policy-edit').should('be.visible'); - - // Analyze for accessibility issues - testA11y('ds-resource-policy-edit'); - }); }); describe('Edit Community > Delete page', () => { From 00cb2f9e8a2635eaa8e5bc4c5a4234cb723f86b0 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 8 Dec 2023 16:30:22 -0600 Subject: [PATCH 19/23] Add accessibility tests (and minor fixes) for Edit Collection's Content Source tab and Item Mapper tab --- cypress/e2e/collection-edit.cy.ts | 42 +++++++++++++++++++ .../collection-item-mapper.component.html | 4 +- .../collection-source-controls.component.html | 2 +- .../collection-source.component.html | 2 +- 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/cypress/e2e/collection-edit.cy.ts b/cypress/e2e/collection-edit.cy.ts index f6089acf70..3e7ecf6141 100644 --- a/cypress/e2e/collection-edit.cy.ts +++ b/cypress/e2e/collection-edit.cy.ts @@ -33,6 +33,25 @@ describe('Edit Collection > Assign Roles tab', () => { }); }); +describe('Edit Collection > Content Source tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="source"]').click(); + + // tag must be loaded + cy.get('ds-collection-source').should('be.visible'); + + // Check the external source checkbox (to display all fields on the page) + cy.get('#externalSourceCheck').check(); + + // Wait for the source controls to appear + cy.get('ds-collection-source-controls').should('be.visible'); + + // Analyze entire page for accessibility issues + testA11y('ds-collection-source'); + }); +}); + describe('Edit Collection > Curate tab', () => { it('should pass accessibility tests', () => { @@ -72,6 +91,29 @@ describe('Edit Collection > Authorizations tab', () => { }); }); +describe('Edit Collection > Item Mapper tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="mapper"]').click(); + + // tag must be loaded + cy.get('ds-collection-item-mapper').should('be.visible'); + + // Analyze entire page for accessibility issues + testA11y('ds-collection-item-mapper'); + + // Click on the "Map new Items" tab + cy.get('li[data-test="mapTab"] a').click(); + + // Make sure search form is now visible + cy.get('ds-search-form').should('be.visible'); + + // Analyze entire page (again) for accessibility issues + testA11y('ds-collection-item-mapper'); + }); +}); + + describe('Edit Collection > Delete page', () => { it('should pass accessibility tests', () => { diff --git a/src/app/collection-page/collection-item-mapper/collection-item-mapper.component.html b/src/app/collection-page/collection-item-mapper/collection-item-mapper.component.html index 649aa9b43d..77f85d5f78 100644 --- a/src/app/collection-page/collection-item-mapper/collection-item-mapper.component.html +++ b/src/app/collection-page/collection-item-mapper/collection-item-mapper.component.html @@ -6,7 +6,7 @@

    {{'collection.edit.item-mapper.description' | translate}}

      -
    • +
    • {{'collection.edit.item-mapper.tabs.browse' | translate}}
      @@ -23,7 +23,7 @@
      • -
    • +
    • {{'collection.edit.item-mapper.tabs.map' | translate}}
      diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html index 4d7b3e657e..521c771cfa 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html @@ -1,6 +1,6 @@
      -

      {{ 'collection.source.controls.head' | translate }}

      +

      {{ 'collection.source.controls.head' | translate }}

      {{'collection.source.controls.harvest.status' | translate}} {{contentSource?.harvestStatus}} diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html index 41e66610a8..b0c155ac95 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html @@ -26,7 +26,7 @@ for="externalSourceCheck">{{ 'collection.edit.tabs.source.external' | translate }}
      -

      {{ 'collection.edit.tabs.source.form.head' | translate }}

      +

      {{ 'collection.edit.tabs.source.form.head' | translate }}

      Date: Wed, 13 Dec 2023 14:56:52 -0600 Subject: [PATCH 20/23] Add Item Edit accessibility tests. Switch Item to use to one with bitstreams. Minor updates to HTML to pass accessibility tests. --- cypress.config.ts | 2 +- cypress/e2e/item-edit.cy.ts | 135 ++++++++++++++++++ .../dso-edit-metadata-value.component.html | 10 +- .../dso-edit-metadata-value.component.spec.ts | 2 +- .../edit-item-page.component.html | 1 + .../item-collection-mapper.component.html | 4 +- 6 files changed, 145 insertions(+), 9 deletions(-) create mode 100644 cypress/e2e/item-edit.cy.ts diff --git a/cypress.config.ts b/cypress.config.ts index b1da2b4793..4143327870 100644 --- a/cypress.config.ts +++ b/cypress.config.ts @@ -22,7 +22,7 @@ export default defineConfig({ // Community/collection/publication used for view/edit tests DSPACE_TEST_COMMUNITY: '0958c910-2037-42a9-81c7-dca80e3892b4', DSPACE_TEST_COLLECTION: '282164f5-d325-4740-8dd1-fa4d6d3e7200', - DSPACE_TEST_ENTITY_PUBLICATION: 'e98b0f27-5c19-49a0-960d-eb6ad5287067', + DSPACE_TEST_ENTITY_PUBLICATION: '6160810f-1e53-40db-81ef-f6621a727398', // Search term (should return results) used in search tests DSPACE_TEST_SEARCH_TERM: 'test', // Collection used for submission tests diff --git a/cypress/e2e/item-edit.cy.ts b/cypress/e2e/item-edit.cy.ts new file mode 100644 index 0000000000..b4c01a1a94 --- /dev/null +++ b/cypress/e2e/item-edit.cy.ts @@ -0,0 +1,135 @@ +import { Options } from 'cypress-axe'; +import { testA11y } from 'cypress/support/utils'; + +const ITEM_EDIT_PAGE = '/items/'.concat(Cypress.env('DSPACE_TEST_ENTITY_PUBLICATION')).concat('/edit'); + +beforeEach(() => { + // All tests start with visiting the Edit Item Page + cy.visit(ITEM_EDIT_PAGE); + + // 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')); +}); + +describe('Edit Item > Edit Metadata tab', () => { + it('should pass accessibility tests', () => { + cy.get('a[data-test="metadata"]').click(); + + // tag must be loaded + cy.get('ds-edit-item-page').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-edit-item-page'); + }); +}); + +describe('Edit Item > Status tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="status"]').click(); + + // tag must be loaded + cy.get('ds-item-status').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-item-status'); + }); +}); + +describe('Edit Item > Bitstreams tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="bitstreams"]').click(); + + // tag must be loaded + cy.get('ds-item-bitstreams').should('be.visible'); + + // Table of item bitstreams must also be loaded + cy.get('div.item-bitstreams').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-item-bitstreams', + { + rules: { + // Currently Bitstreams page loads a pagination component per Bundle + // and they all use the same 'id="p-dad"'. + 'duplicate-id': { enabled: false }, + } + } as Options + ); + }); +}); + +describe('Edit Item > Curate tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="curate"]').click(); + + // tag must be loaded + cy.get('ds-item-curate').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-item-curate'); + }); +}); + +describe('Edit Item > Relationships tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="relationships"]').click(); + + // tag must be loaded + cy.get('ds-item-relationships').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-item-relationships'); + }); +}); + +describe('Edit Item > Version History tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="versionhistory"]').click(); + + // tag must be loaded + cy.get('ds-item-version-history').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-item-version-history'); + }); +}); + +describe('Edit Item > Access Control tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="access-control"]').click(); + + // tag must be loaded + cy.get('ds-item-access-control').should('be.visible'); + + // Analyze for accessibility issues + testA11y('ds-item-access-control'); + }); +}); + +describe('Edit Item > Collection Mapper tab', () => { + + it('should pass accessibility tests', () => { + cy.get('a[data-test="mapper"]').click(); + + // tag must be loaded + cy.get('ds-item-collection-mapper').should('be.visible'); + + // Analyze entire page for accessibility issues + testA11y('ds-item-collection-mapper'); + + // Click on the "Map new collections" tab + cy.get('li[data-test="mapTab"] a').click(); + + // Make sure search form is now visible + cy.get('ds-search-form').should('be.visible'); + + // Analyze entire page (again) for accessibility issues + testA11y('ds-item-collection-mapper'); + }); +}); diff --git a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.html b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.html index 2701ee24ab..47449bda5c 100644 --- a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.html +++ b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.html @@ -21,25 +21,25 @@
      - - - -
      - - Decrement {{placeholder}} + {{'form.number-picker.decrement' | translate: {field: name} }}
      diff --git a/src/app/shared/form/number-picker/number-picker.component.spec.ts b/src/app/shared/form/number-picker/number-picker.component.spec.ts index bf33a6bdd0..b6d623d70b 100644 --- a/src/app/shared/form/number-picker/number-picker.component.spec.ts +++ b/src/app/shared/form/number-picker/number-picker.component.spec.ts @@ -7,6 +7,7 @@ import { By } from '@angular/platform-browser'; import { NumberPickerComponent } from './number-picker.component'; import { FormsModule, ReactiveFormsModule } from '@angular/forms'; import { createTestComponent } from '../../testing/utils.test'; +import { TranslateModule } from '@ngx-translate/core'; describe('NumberPickerComponent test suite', () => { @@ -23,7 +24,8 @@ describe('NumberPickerComponent test suite', () => { imports: [ FormsModule, ReactiveFormsModule, - NgbModule + NgbModule, + TranslateModule.forRoot() ], declarations: [ NumberPickerComponent, diff --git a/src/app/shared/form/number-picker/number-picker.component.ts b/src/app/shared/form/number-picker/number-picker.component.ts index 40562dd61c..8f2de5ee54 100644 --- a/src/app/shared/form/number-picker/number-picker.component.ts +++ b/src/app/shared/form/number-picker/number-picker.component.ts @@ -1,5 +1,6 @@ import { ChangeDetectorRef, Component, EventEmitter, Input, OnInit, Output, SimpleChanges, } from '@angular/core'; import { ControlValueAccessor, UntypedFormBuilder, NG_VALUE_ACCESSOR } from '@angular/forms'; +import { TranslateService } from '@ngx-translate/core'; import { isEmpty } from '../../empty.util'; @Component({ @@ -31,7 +32,9 @@ export class NumberPickerComponent implements OnInit, ControlValueAccessor { startValue: number; - constructor(private fb: UntypedFormBuilder, private cd: ChangeDetectorRef) { + constructor(private fb: UntypedFormBuilder, + private cd: ChangeDetectorRef, + private translate: TranslateService) { } ngOnInit() { diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index deea56b1d2..d7c50b1a60 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1770,6 +1770,10 @@ "form.repeatable.sort.tip": "Drop the item in the new position", + "form.number-picker.decrement": "Decrement {{field}}", + + "form.number-picker.increment": "Increment {{field}}", + "grant-deny-request-copy.deny": "Don't send copy", "grant-deny-request-copy.email.back": "Back", From a5a89a4b985897a02a9ab4110ca0a02b8afdc6ad Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Wed, 17 Jan 2024 12:05:00 -0600 Subject: [PATCH 23/23] Add environment variable for Person collection name. Use it in submission e2e tests. --- cypress.config.ts | 4 +++- cypress/e2e/submission.cy.ts | 10 ++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/cypress.config.ts b/cypress.config.ts index 4143327870..458b035a48 100644 --- a/cypress.config.ts +++ b/cypress.config.ts @@ -25,9 +25,11 @@ export default defineConfig({ DSPACE_TEST_ENTITY_PUBLICATION: '6160810f-1e53-40db-81ef-f6621a727398', // Search term (should return results) used in search tests DSPACE_TEST_SEARCH_TERM: 'test', - // Collection used for submission tests + // Main Collection used for submission tests. Should be able to accept normal Item objects DSPACE_TEST_SUBMIT_COLLECTION_NAME: 'Sample Collection', DSPACE_TEST_SUBMIT_COLLECTION_UUID: '9d8334e9-25d3-4a67-9cea-3dffdef80144', + // Collection used for Person entity submission tests. MUST be configured with EntityType=Person. + DSPACE_TEST_SUBMIT_PERSON_COLLECTION_NAME: 'People', // Account used to test basic submission process DSPACE_TEST_SUBMIT_USER: 'dspacedemo+submit@gmail.com', DSPACE_TEST_SUBMIT_USER_PASSWORD: 'dspace', diff --git a/cypress/e2e/submission.cy.ts b/cypress/e2e/submission.cy.ts index 97676ca25c..4402410f23 100644 --- a/cypress/e2e/submission.cy.ts +++ b/cypress/e2e/submission.cy.ts @@ -168,8 +168,11 @@ describe('New Submission page', () => { // This should display the (popup window) cy.get('ds-create-item-parent-selector').should('be.visible'); - // Click on the first Collection button to seelect a collection (actual collection doesn't matter) - cy.get('ds-authorized-collection-selector button:first-child').click(); + // Type in a known Collection name in the search box + cy.get('ds-authorized-collection-selector input[type="search"]').type(Cypress.env('DSPACE_TEST_SUBMIT_PERSON_COLLECTION_NAME')); + + // Click on the button matching that known Collection name + cy.get('ds-authorized-collection-selector button[title="'.concat(Cypress.env('DSPACE_TEST_SUBMIT_PERSON_COLLECTION_NAME')).concat('"]')).click(); // New URL should include /workspaceitems, as we've started a new submission cy.url().should('include', '/workspaceitems'); @@ -177,6 +180,9 @@ describe('New Submission page', () => { // The Submission edit form tag should be visible cy.get('ds-submission-edit').should('be.visible'); + // A Collection menu button should exist & its value should be the selected collection + cy.get('#collectionControlsMenuButton span').should('have.text', Cypress.env('DSPACE_TEST_SUBMIT_PERSON_COLLECTION_NAME')); + // 3 sections should be visible by default cy.get('div#section_personStep').should('be.visible'); cy.get('div#section_upload').should('be.visible');