Skip to content

Commit 04aefdc

Browse files
milanmajchrakclaude
andcommitted
MENDELU/Add "Allow external URLs" toggle to collection Content Source
Collection > Edit > Content Source now shows an "Allow external URLs" checkbox when the harvest type is "Harvest metadata and bitstreams", together with a short description of what it does and what the risk is when it is ticked (dataquest-dev/dspace-customers#860). The checkbox is a plain template control rather than a dynamic form control, so it reverts correctly on Discard/Reinstate - the form's patchValue only names the three existing containers. The warning renders whenever the harvest type matches, not only when the box is ticked, because the administrator needs to read it before ticking; the alert styling appears only in the risky state. Requires the matching backend change; the REST field is allow_external_urls. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 0deac48 commit 04aefdc

6 files changed

Lines changed: 120 additions & 0 deletions

File tree

src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,26 @@ <h3>{{ 'collection.edit.tabs.source.form.head' | translate }}</h3>
5252
(cancel)="onCancel()"></ds-form>
5353
}
5454
</div>
55+
@if (contentSource && (contentSource?.harvestType === harvestTypeMetadataAndBitstreams)) {
56+
<div class="container mt-2">
57+
<div class="row">
58+
<div class="col-12">
59+
<div class="form-check">
60+
<input type="checkbox" class="form-check-input" id="allowExternalUrlsCheck"
61+
[checked]="contentSource.allowExternalUrls" (change)="changeAllowExternalUrls()"
62+
[attr.aria-describedby]="'allowExternalUrlsWarning'">
63+
<label class="form-check-label"
64+
for="allowExternalUrlsCheck">{{ 'collection.edit.tabs.source.allow-external-urls' | translate }}</label>
65+
</div>
66+
<!-- Warning styling only in the risky state; no role="alert", this is static help text -->
67+
<div id="allowExternalUrlsWarning" class="mt-2"
68+
[ngClass]="contentSource.allowExternalUrls ? 'alert alert-warning' : 'form-text'">
69+
{{ 'collection.edit.tabs.source.allow-external-urls.warning' | translate }}
70+
</div>
71+
</div>
72+
</div>
73+
</div>
74+
}
5575
@if ((contentSource?.harvestType !== harvestTypeNone)) {
5676
<div class="container mt-2">
5777
<div class="row">

src/app/collection-page/edit-collection-page/collection-source/collection-source.component.spec.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,72 @@ describe('CollectionSourceComponent', () => {
214214
});
215215
});
216216

217+
describe('when selecting the allow external URLs checkbox', () => {
218+
let input;
219+
220+
beforeEach(() => {
221+
comp.contentSource.harvestType = ContentSourceHarvestType.MetadataAndBitstreams;
222+
fixture.detectChanges();
223+
input = fixture.debugElement.query(By.css('#allowExternalUrlsCheck')).nativeElement;
224+
input.click();
225+
fixture.detectChanges();
226+
});
227+
228+
it('should enable allowExternalUrls', () => {
229+
expect(comp.contentSource.allowExternalUrls).toBeTrue();
230+
});
231+
232+
it('should send a field update', () => {
233+
expect(objectUpdatesService.saveAddFieldUpdate).toHaveBeenCalledWith(router.url, comp.contentSource);
234+
});
235+
});
236+
237+
describe('the allow external URLs checkbox and warning', () => {
238+
it('should be hidden when only metadata is harvested', () => {
239+
comp.contentSource.harvestType = ContentSourceHarvestType.Metadata;
240+
fixture.detectChanges();
241+
expect(fixture.debugElement.query(By.css('#allowExternalUrlsCheck'))).toBeNull();
242+
expect(fixture.debugElement.query(By.css('#allowExternalUrlsWarning'))).toBeNull();
243+
});
244+
245+
it('should be hidden when only references to bitstreams are harvested', () => {
246+
comp.contentSource.harvestType = ContentSourceHarvestType.MetadataAndRef;
247+
fixture.detectChanges();
248+
expect(fixture.debugElement.query(By.css('#allowExternalUrlsCheck'))).toBeNull();
249+
expect(fixture.debugElement.query(By.css('#allowExternalUrlsWarning'))).toBeNull();
250+
});
251+
252+
it('should be shown when bitstreams are harvested', () => {
253+
comp.contentSource.harvestType = ContentSourceHarvestType.MetadataAndBitstreams;
254+
fixture.detectChanges();
255+
expect(fixture.debugElement.query(By.css('#allowExternalUrlsCheck'))).not.toBeNull();
256+
expect(fixture.debugElement.query(By.css('#allowExternalUrlsWarning'))).not.toBeNull();
257+
});
258+
259+
it('should describe the checkbox with the warning, so screen readers announce the risk', () => {
260+
comp.contentSource.harvestType = ContentSourceHarvestType.MetadataAndBitstreams;
261+
fixture.detectChanges();
262+
const checkbox = fixture.debugElement.query(By.css('#allowExternalUrlsCheck')).nativeElement;
263+
expect(checkbox.getAttribute('aria-describedby')).toEqual('allowExternalUrlsWarning');
264+
});
265+
266+
it('should show the text unstyled before the checkbox is ticked', () => {
267+
comp.contentSource.harvestType = ContentSourceHarvestType.MetadataAndBitstreams;
268+
fixture.detectChanges();
269+
expect(fixture.debugElement.query(By.css('#allowExternalUrlsCheck')).nativeElement.checked).toBeFalse();
270+
const warning = fixture.debugElement.query(By.css('#allowExternalUrlsWarning')).nativeElement;
271+
expect(warning.classList.contains('alert-warning')).toBeFalse();
272+
});
273+
274+
it('should style the text as a warning once the checkbox is ticked', () => {
275+
comp.contentSource.harvestType = ContentSourceHarvestType.MetadataAndBitstreams;
276+
comp.contentSource.allowExternalUrls = true;
277+
fixture.detectChanges();
278+
const warning = fixture.debugElement.query(By.css('#allowExternalUrlsWarning')).nativeElement;
279+
expect(warning.classList.contains('alert-warning')).toBeTrue();
280+
});
281+
});
282+
217283
describe('isValid', () => {
218284
it('should return true when ContentSource is disabled but the form invalid', () => {
219285
spyOnProperty(comp.formGroup, 'valid').and.returnValue(false);

src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
AsyncPipe,
33
Location,
4+
NgClass,
45
} from '@angular/common';
56
import {
67
Component,
@@ -79,6 +80,7 @@ import { CollectionSourceControlsComponent } from './collection-source-controls/
7980
BtnDisabledDirective,
8081
CollectionSourceControlsComponent,
8182
FormComponent,
83+
NgClass,
8284
ThemedLoadingComponent,
8385
TranslateModule,
8486
],
@@ -254,6 +256,11 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem
254256
*/
255257
harvestTypeNone = ContentSourceHarvestType.None;
256258

259+
/**
260+
* The content harvesting type that downloads bitstreams, the only one the external URL flag applies to
261+
*/
262+
harvestTypeMetadataAndBitstreams = ContentSourceHarvestType.MetadataAndBitstreams;
263+
257264
/**
258265
* The previously selected harvesting type
259266
* Used for switching between ContentSourceHarvestType.None and the previously selected value when enabling / disabling harvesting
@@ -465,6 +472,16 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem
465472
this.updateContentSource(false);
466473
}
467474

475+
/**
476+
* Switch the allowExternalUrls flag on or off and fire a field update
477+
* Deliberately not a dynamic form control: the patchValue() in initializeOriginalContentSource() only names the
478+
* three form containers, so a control here would not revert on Discard/Reinstate
479+
*/
480+
changeAllowExternalUrls() {
481+
this.contentSource.allowExternalUrls = !this.contentSource.allowExternalUrls;
482+
this.saveFieldUpdate();
483+
}
484+
468485
/**
469486
* Loop over all inputs and update the Content Source with their value
470487
* @param updateHarvestType When set to false, the harvestType of the contentSource will be ignored in the update

src/app/core/shared/content-source.model.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,13 @@ export class ContentSource extends CacheableObject {
7373
@autoserializeAs('harvest_type')
7474
harvestType = ContentSourceHarvestType.None;
7575

76+
/**
77+
* Whether bitstreams may be fetched from hosts other than the OAI provider
78+
* Initialised to false on purpose: cerialize omits undefined values, so without it the key would never reach the backend
79+
*/
80+
@autoserializeAs('allow_external_urls')
81+
allowExternalUrls = false;
82+
7683
/**
7784
* The available metadata configurations
7885
*/

src/assets/i18n/cs.json5

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,6 +1916,12 @@
19161916
// "collection.edit.tabs.roles.title": "Collection Edit - Roles",
19171917
"collection.edit.tabs.roles.title": "Upravit kolekci - Role",
19181918

1919+
// "collection.edit.tabs.source.allow-external-urls": "Allow external URLs",
1920+
"collection.edit.tabs.source.allow-external-urls": "Povolit externí URL",
1921+
1922+
// "collection.edit.tabs.source.allow-external-urls.warning": "When off, files are fetched only from the same scheme and host as the OAI provider above, and a record pointing elsewhere is skipped in full. When on, your server fetches from any address the provider names — that provider, not you, decides what is downloaded and republished here, so enable it only for providers you trust. Private and internal addresses stay blocked either way.",
1923+
"collection.edit.tabs.source.allow-external-urls.warning": "Když je vypnuto, stahují se soubory jen ze stejného schématu a hostitele, jaké má adresa poskytovatele OAI výše; záznam odkazující jinam se přeskočí celý. Když je zapnuto, stahuje váš server z libovolné adresy, kterou poskytovatel uvede — o tom, co se zde stáhne a znovu zveřejní, tak rozhoduje on, a ne vy. Zapínejte jen u poskytovatelů, kterým důvěřujete. Privátní a interní adresy zůstávají blokované tak či tak.",
1924+
19191925
// "collection.edit.tabs.source.external": "This collection harvests its content from an external source",
19201926
"collection.edit.tabs.source.external": "Tato kolekce získává svůj obsah z externího zdroje",
19211927

src/assets/i18n/en.json5

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,6 +1277,10 @@
12771277

12781278
"collection.edit.tabs.roles.title": "Collection Edit - Roles",
12791279

1280+
"collection.edit.tabs.source.allow-external-urls": "Allow external URLs",
1281+
1282+
"collection.edit.tabs.source.allow-external-urls.warning": "When off, files are fetched only from the same scheme and host as the OAI provider above, and a record pointing elsewhere is skipped in full. When on, your server fetches from any address the provider names — that provider, not you, decides what is downloaded and republished here, so enable it only for providers you trust. Private and internal addresses stay blocked either way.",
1283+
12801284
"collection.edit.tabs.source.external": "This collection harvests its content from an external source",
12811285

12821286
"collection.edit.tabs.source.form.errors.oaiSource.required": "You must provide a set id of the target collection.",

0 commit comments

Comments
 (0)