Skip to content

Commit e964863

Browse files
authored
Merge pull request #5221 from DSpace/backport-5028-to-dspace-9_x
[Port dspace-9_x] Reduce bitstream authorization requests for item page
2 parents a436108 + e52e93c commit e964863

2 files changed

Lines changed: 38 additions & 22 deletions

File tree

src/app/shared/file-download-link/file-download-link.component.spec.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,11 @@ describe('FileDownloadLinkComponent', () => {
113113
component.item = item;
114114
fixture.detectChanges();
115115
});
116+
it('should return canDownload truthy', () => {
117+
expect(component.canDownload$).toBeObservable(cold('-a', { a: true }));
118+
});
116119
it('should return the bitstreamPath based on the input bitstream', () => {
117120
expect(component.bitstreamPath$).toBeObservable(cold('-a', { a: { routerLink: new URLCombiner(getBitstreamModuleRoute(), bitstream.uuid, 'download').toString(), queryParams: {} } }));
118-
expect(component.canDownload$).toBeObservable(cold('--a', { a: true }));
119121

120122
});
121123
it('should init the component', () => {
@@ -147,9 +149,11 @@ describe('FileDownloadLinkComponent', () => {
147149
component.bitstream = bitstream;
148150
fixture.detectChanges();
149151
});
152+
it('should return canDownload falsy', () => {
153+
expect(component.canDownload$).toBeObservable(cold('-a', { a: false }));
154+
});
150155
it('should return the bitstreamPath based on the input bitstream', () => {
151-
expect(component.bitstreamPath$).toBeObservable(cold('-a', { a: { routerLink: new URLCombiner(getItemModuleRoute(), item.uuid, 'request-a-copy').toString(), queryParams: { bitstream: bitstream.uuid } } }));
152-
expect(component.canDownload$).toBeObservable(cold('--a', { a: false }));
156+
expect(component.bitstreamPath$).toBeObservable(cold('--a', { a: { routerLink: new URLCombiner(getItemModuleRoute(), item.uuid, 'request-a-copy').toString(), queryParams: { bitstream: bitstream.uuid } } }));
153157

154158
});
155159
it('should init the component', () => {
@@ -176,9 +180,11 @@ describe('FileDownloadLinkComponent', () => {
176180
component.item = item;
177181
fixture.detectChanges();
178182
});
183+
it('should return canDownload falsy', () => {
184+
expect(component.canDownload$).toBeObservable(cold('-a', { a: false }));
185+
});
179186
it('should return the bitstreamPath based on the input bitstream', () => {
180-
expect(component.bitstreamPath$).toBeObservable(cold('-a', { a: { routerLink: new URLCombiner(getBitstreamModuleRoute(), bitstream.uuid, 'download').toString(), queryParams: {} } }));
181-
expect(component.canDownload$).toBeObservable(cold('--a', { a: false }));
187+
expect(component.bitstreamPath$).toBeObservable(cold('--a', { a: { routerLink: new URLCombiner(getBitstreamModuleRoute(), bitstream.uuid, 'download').toString(), queryParams: {} } }));
182188

183189
});
184190
it('should init the component and show the locked icon', () => {
@@ -205,9 +211,11 @@ describe('FileDownloadLinkComponent', () => {
205211
component.item = item;
206212
fixture.detectChanges();
207213
});
214+
it('should return canDownload falsy', () => {
215+
expect(component.canDownload$).toBeObservable(cold('-a', { a: false }));
216+
});
208217
it('should return the bitstreamPath based on the access token and request-a-copy path', () => {
209-
expect(component.bitstreamPath$).toBeObservable(cold('-a', { a: { routerLink: new URLCombiner(getBitstreamModuleRoute(), bitstream.uuid, 'download').toString(), queryParams: { accessToken: 'abc123' } } }));
210-
expect(component.canDownload$).toBeObservable(cold('--a', { a: false }));
218+
expect(component.bitstreamPath$).toBeObservable(cold('--a', { a: { routerLink: new URLCombiner(getBitstreamModuleRoute(), bitstream.uuid, 'download').toString(), queryParams: { accessToken: 'abc123' } } }));
211219

212220
});
213221
it('should init the component and show an open lock', () => {

src/app/shared/file-download-link/file-download-link.component.ts

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ import {
2121
Observable,
2222
of,
2323
} from 'rxjs';
24-
import { map } from 'rxjs/operators';
24+
import {
25+
map,
26+
switchMap,
27+
} from 'rxjs/operators';
2528

2629
import {
2730
getBitstreamDownloadRoute,
@@ -112,13 +115,23 @@ export class FileDownloadLinkComponent implements OnInit {
112115
if (this.enableRequestACopy) {
113116
// Obtain item request data from the route snapshot
114117
this.itemRequest = this.route.snapshot.data.itemRequest;
115-
// Set up observables to test access rights to a normal bitstream download, a valid token download, and the request-a-copy feature
118+
// Set up observable to evaluate access rights for a normal download
116119
this.canDownload$ = this.authorizationService.isAuthorized(FeatureID.CanDownload, isNotEmpty(this.bitstream) ? this.bitstream.self : undefined);
117-
this.canDownloadWithToken$ = of((this.itemRequest && this.itemRequest.acceptRequest && !this.itemRequest.accessExpired) ? (this.itemRequest.allfiles !== false || this.itemRequest.bitstreamId === this.bitstream.uuid) : false);
118-
this.canRequestACopy$ = this.authorizationService.isAuthorized(FeatureID.CanRequestACopy, isNotEmpty(this.bitstream) ? this.bitstream.self : undefined);
119-
// Set up observable to determine the path to the bitstream based on the user's access rights and features as above
120-
this.bitstreamPath$ = observableCombineLatest([this.canDownload$, this.canDownloadWithToken$, this.canRequestACopy$]).pipe(
121-
map(([canDownload, canDownloadWithToken, canRequestACopy]) => this.getBitstreamPath(canDownload, canDownloadWithToken, canRequestACopy)),
120+
// Only set up and execute other observables if canDownload emits false
121+
this.bitstreamPath$ = this.canDownload$.pipe(
122+
switchMap(canDownload => {
123+
if (canDownload) {
124+
return of(this.getBitstreamDownloadPath());
125+
}
126+
// Set up and combine observables to evaluate access rights to a valid token download and the request-a-copy feature
127+
this.canDownloadWithToken$ = of((this.itemRequest && this.itemRequest.acceptRequest && !this.itemRequest.accessExpired) ? (this.itemRequest.allfiles !== false || this.itemRequest.bitstreamId === this.bitstream.uuid) : false);
128+
this.canRequestACopy$ = this.authorizationService.isAuthorized(FeatureID.CanRequestACopy, isNotEmpty(this.bitstream) ? this.bitstream.self : undefined);
129+
// Set up canDownload observable so the template can read the state
130+
this.canDownload$ = of(false);
131+
return observableCombineLatest([this.canDownloadWithToken$, this.canRequestACopy$]).pipe(
132+
map(([canDownloadWithToken, canRequestACopy]) => this.getBitstreamPathForRequestACopy(canDownloadWithToken, canRequestACopy)),
133+
);
134+
}),
122135
);
123136
} else {
124137
this.bitstreamPath$ = of(this.getBitstreamDownloadPath());
@@ -130,21 +143,16 @@ export class FileDownloadLinkComponent implements OnInit {
130143
* Return a path to the bitstream based on what kind of access and authorization the user has, and whether
131144
* they may request a copy
132145
*
133-
* @param canDownload user can download normally
134146
* @param canDownloadWithToken user can download using a token granted by a request approver
135147
* @param canRequestACopy user can request approval to access a copy
136148
*/
137-
getBitstreamPath(canDownload: boolean, canDownloadWithToken, canRequestACopy: boolean) {
138-
// No matter what, if the user can download with their own authZ, allow it
139-
if (canDownload) {
140-
return this.getBitstreamDownloadPath();
141-
}
142-
// Otherwise, if they access token is valid, use this
149+
getBitstreamPathForRequestACopy(canDownloadWithToken: boolean, canRequestACopy: boolean) {
150+
// if the access token is valid, use this
143151
if (canDownloadWithToken) {
144152
return this.getAccessByTokenBitstreamPath(this.itemRequest);
145153
}
146154
// If the user can't download, but can request a copy, show the request a copy link
147-
if (!canDownload && canRequestACopy && hasValue(this.item)) {
155+
if (canRequestACopy && hasValue(this.item)) {
148156
return getBitstreamRequestACopyRoute(this.item, this.bitstream);
149157
}
150158
// By default, return the plain path

0 commit comments

Comments
 (0)