Skip to content

Commit b8263a0

Browse files
committed
Request-a-copy: Error handling (review feedback)
* New accessExpired flag can be checked along with acceptRequest, so expired or not-granted links will show an error and draw 'normal' download links instead of doing a hard 4xx redirect * Old ItemWithSupplementaryData component removed * Notifications moved to sub-component of item page and display a warning on valid access token, or a danger alert if expired or not-granted, with an appropriate error message * Date formatted in messages as yyyy-MM-dd
1 parent 58cee5f commit b8263a0

15 files changed

Lines changed: 121 additions & 44 deletions

src/app/core/auth/access-token.resolver.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,15 @@ import {
44
Router,
55
} from '@angular/router';
66
import { Observable } from 'rxjs';
7-
import { tap } from 'rxjs/operators';
7+
import {
8+
map,
9+
tap,
10+
} from 'rxjs/operators';
811

912
import { getForbiddenRoute } from '../../app-routing-paths';
1013
import { hasValue } from '../../shared/empty.util';
1114
import { ItemRequestDataService } from '../data/item-request-data.service';
15+
import { RemoteData } from '../data/remote-data';
1216
import { redirectOn4xx } from '../shared/authorized.operators';
1317
import { ItemRequest } from '../shared/item-request.model';
1418
import {
@@ -43,6 +47,7 @@ export const accessTokenResolver: ResolveFn<ItemRequest> = (
4347
getFirstCompletedRemoteData(),
4448
// Handle authorization errors, not found errors and forbidden errors as normal
4549
redirectOn4xx(router, authService),
50+
map((rd: RemoteData<ItemRequest>) => rd),
4651
// Get payload of the item request
4752
getFirstSucceededRemoteDataPayload(),
4853
tap(request => {

src/app/core/shared/item-request.model.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ export class ItemRequest implements CacheableObject {
9191
@autoserialize
9292
accessExpiry: string;
9393

94+
@autoserialize
95+
accessExpired: boolean;
9496
/**
9597
* The {@link HALLink}s for this ItemRequest
9698
*/

src/app/core/shared/item-with-supplementary-data.model.ts

Lines changed: 0 additions & 20 deletions
This file was deleted.

src/app/item-page/media-viewer/media-viewer.component.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
OnDestroy,
77
OnInit,
88
} from '@angular/core';
9+
import { ActivatedRoute } from '@angular/router';
910
import { TranslateModule } from '@ngx-translate/core';
1011
import {
1112
BehaviorSubject,
@@ -25,7 +26,7 @@ import { RemoteData } from '../../core/data/remote-data';
2526
import { Bitstream } from '../../core/shared/bitstream.model';
2627
import { BitstreamFormat } from '../../core/shared/bitstream-format.model';
2728
import { Item } from '../../core/shared/item.model';
28-
import { ItemWithSupplementaryData } from '../../core/shared/item-with-supplementary-data.model';
29+
import { ItemRequest } from '../../core/shared/item-request.model';
2930
import { MediaViewerItem } from '../../core/shared/media-viewer-item.model';
3031
import { getFirstSucceededRemoteDataPayload } from '../../core/shared/operators';
3132
import { hasValue } from '../../shared/empty.util';
@@ -71,9 +72,12 @@ export class MediaViewerComponent implements OnDestroy, OnInit {
7172

7273
subs: Subscription[] = [];
7374

75+
itemRequest: ItemRequest;
76+
7477
constructor(
7578
protected bitstreamDataService: BitstreamDataService,
7679
protected changeDetectorRef: ChangeDetectorRef,
80+
protected route: ActivatedRoute,
7781
) {
7882
}
7983

@@ -85,6 +89,7 @@ export class MediaViewerComponent implements OnDestroy, OnInit {
8589
* This method loads all the Bitstreams and Thumbnails and converts it to {@link MediaViewerItem}s
8690
*/
8791
ngOnInit(): void {
92+
this.itemRequest = this.route.snapshot.data.itemRequest;
8893
const types: string[] = [
8994
...(this.mediaOptions.image ? ['image'] : []),
9095
...(this.mediaOptions.video ? ['audio', 'video'] : []),
@@ -170,9 +175,10 @@ export class MediaViewerComponent implements OnDestroy, OnInit {
170175
* Get access token, if this is accessed via a Request-a-Copy link
171176
*/
172177
get accessToken() {
173-
if (this.item instanceof ItemWithSupplementaryData && hasValue(this.item.itemRequest)) {
174-
return this.item.itemRequest.accessToken;
178+
if (hasValue(this.itemRequest) && this.itemRequest.accessToken && !this.itemRequest.accessExpired) {
179+
return this.itemRequest.accessToken;
175180
}
176181
return null;
177182
}
183+
178184
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<ng-container *ngVar="(itemRequest$ | async) as itemRequest">
2+
@if (hasValue(itemRequest)) {
3+
@if (!itemRequest.acceptRequest) {
4+
<!-- The request has NOT been accepted, display an error -->
5+
<div class="alert alert-danger wb-100 mb-2">
6+
<p><span role="img" class="request-a-copy-access-error-icon" [attr.aria-label]="'bitstream-request-a-copy.access-by-token.alt-text' | translate"><i class="fa-solid fa-lock"></i></span>{{'bitstream-request-a-copy.access-by-token.not-granted' | translate}}</p>
7+
<p>{{'bitstream-request-a-copy.access-by-token.re-request' | translate}}</p>
8+
</div>
9+
} @else if (itemRequest.accessExpired) {
10+
<!-- The request is accepted, but the access period has expired, display an error -->
11+
<div class="alert alert-danger wb-100 mb-2">
12+
<p><span role="img" class="request-a-copy-access-error-icon" [attr.aria-label]="'bitstream-request-a-copy.access-by-token.alt-text' | translate"><i class="fa-solid fa-lock"></i></span>{{'bitstream-request-a-copy.access-by-token.expired' | translate}} {{ formatDate(itemRequest.accessExpiry) }}</p>
13+
<p>{{'bitstream-request-a-copy.access-by-token.re-request' | translate}}</p>
14+
</div>
15+
} @else {
16+
<div class="alert alert-warning wb-100 mb-2">
17+
<p><span role="img" class="request-a-copy-access-icon" [attr.aria-label]="'bitstream-request-a-copy.access-by-token.alt-text' | translate"><i class="fa-solid fa-lock-open"></i></span>{{'bitstream-request-a-copy.access-by-token.warning' | translate}}</p>
18+
<!-- Only show the expiry date if it's not null, and doesn't start with the "FOREVER" year -->
19+
@if (hasValue(itemRequest.accessExpiry) && !itemRequest.accessExpiry.startsWith('+294276')) {
20+
<p>{{ 'bitstream-request-a-copy.access-by-token.expiry-label' | translate }} {{ formatDate(itemRequest.accessExpiry) }}</p>
21+
}
22+
</div>
23+
}
24+
}
25+
</ng-container>
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
.request-a-copy-access-icon {
2+
margin-right: 4px;
3+
color: var(--bs-success);
4+
}
5+
.request-a-copy-access-error-icon {
6+
margin-right: 4px;
7+
}

src/app/item-page/simple/access-by-token-notification/access-by-token-notification.component.spec.ts

Whitespace-only changes.
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { AsyncPipe } from '@angular/common';
2+
import {
3+
Component,
4+
OnInit,
5+
} from '@angular/core';
6+
import { ActivatedRoute } from '@angular/router';
7+
import { TranslateModule } from '@ngx-translate/core';
8+
import { Observable } from 'rxjs';
9+
import { map } from 'rxjs/operators';
10+
11+
import { ItemRequest } from '../../../core/shared/item-request.model';
12+
import {
13+
dateToString,
14+
stringToNgbDateStruct,
15+
} from '../../../shared/date.util';
16+
import {
17+
hasValue,
18+
isNotEmpty,
19+
} from '../../../shared/empty.util';
20+
import { VarDirective } from '../../../shared/utils/var.directive';
21+
22+
@Component({
23+
selector: 'ds-access-by-token-notification',
24+
templateUrl: './access-by-token-notification.component.html',
25+
styleUrls: ['./access-by-token-notification.component.scss'],
26+
imports: [
27+
AsyncPipe,
28+
TranslateModule,
29+
VarDirective,
30+
],
31+
standalone: true,
32+
})
33+
export class AccessByTokenNotificationComponent implements OnInit {
34+
35+
itemRequest$: Observable<ItemRequest>;
36+
protected readonly hasValue = hasValue;
37+
38+
constructor(protected route: ActivatedRoute) {
39+
}
40+
41+
ngOnInit() {
42+
this.itemRequest$ = this.route.data.pipe(
43+
map((data) => data.itemRequest as ItemRequest),
44+
);
45+
}
46+
47+
/**
48+
* Returns a date in simplified format (YYYY-MM-DD).
49+
*
50+
* @param date
51+
* @return a string with formatted date
52+
*/
53+
formatDate(date: string): string {
54+
return isNotEmpty(date) ? dateToString(stringToNgbDateStruct(date)) : '';
55+
}
56+
}

src/app/item-page/simple/item-page.component.html

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,9 @@
33
<div class="item-page" @fadeInOut>
44
@if (itemRD?.payload; as item) {
55
<div>
6-
<ng-container *ngVar="(itemRequest$ | async) as itemRequest">
7-
@if (hasValue(itemRequest)) {
8-
<div class="alert alert-warning wb-100 mb-2">
9-
<p><span role="img" class="request-a-copy-access-icon" [attr.aria-label]="'bitstream-request-a-copy.access-by-token.alt-text' | translate"><i class="fa-solid fa-lock-open"></i></span>{{'bitstream-request-a-copy.access-by-token.warning' | translate}}</p>
10-
<!-- Only show the expiry date if it's not null, and doesn't start with the "FOREVER" year -->
11-
@if (hasValue(itemRequest.accessExpiry) && !itemRequest.accessExpiry.startsWith('+294276')) {
12-
<p>{{ 'bitstream-request-a-copy.access-by-token.expiry-label' | translate }} {{ itemRequest.accessExpiry }}</p>
13-
}
14-
</div>
15-
}
16-
</ng-container>
6+
177
<ds-item-alerts [item]="item"></ds-item-alerts>
8+
<ds-access-by-token-notification></ds-access-by-token-notification>
189
<ds-qa-event-notification [item]="item"></ds-qa-event-notification>
1910
<ds-notify-requests-status [itemUuid]="item.uuid"></ds-notify-requests-status>
2011
<ds-item-versions-notice [item]="item"></ds-item-versions-notice>

src/app/item-page/simple/item-page.component.scss

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,4 @@
44
max-width: none;
55
}
66
}
7-
.request-a-copy-access-icon {
8-
margin-right: 4px;
9-
color: #26a269;
10-
}
7+

0 commit comments

Comments
 (0)