From 038e31ccd1a771e3c720a0dbd264f7ffe4720125 Mon Sep 17 00:00:00 2001 From: "max.nuding" Date: Wed, 15 Nov 2023 10:51:01 +0100 Subject: [PATCH 001/356] Submission form now displays custom messages for regex validated fields if they exist --- .../shared/form/builder/parsers/concat-field-parser.ts | 4 +++- .../shared/form/builder/parsers/dropdown-field-parser.ts | 4 +++- src/app/shared/form/builder/parsers/field-parser.ts | 9 +++++++-- src/app/shared/form/builder/parsers/name-field-parser.ts | 6 ++++-- src/app/shared/form/builder/parsers/parser-factory.ts | 2 ++ .../shared/form/builder/parsers/series-field-parser.ts | 6 ++++-- 6 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/app/shared/form/builder/parsers/concat-field-parser.ts b/src/app/shared/form/builder/parsers/concat-field-parser.ts index e86de70c816..c18fa8234e4 100644 --- a/src/app/shared/form/builder/parsers/concat-field-parser.ts +++ b/src/app/shared/form/builder/parsers/concat-field-parser.ts @@ -19,6 +19,7 @@ import { SUBMISSION_ID } from './field-parser'; import { DsDynamicInputModel, DsDynamicInputModelConfig } from '../ds-dynamic-form-ui/models/ds-dynamic-input.model'; +import { TranslateService } from '@ngx-translate/core'; export class ConcatFieldParser extends FieldParser { @@ -27,10 +28,11 @@ export class ConcatFieldParser extends FieldParser { @Inject(CONFIG_DATA) configData: FormFieldModel, @Inject(INIT_FORM_VALUES) initFormValues, @Inject(PARSER_OPTIONS) parserOptions: ParserOptions, + translate: TranslateService, protected separator: string, protected firstPlaceholder: string = null, protected secondPlaceholder: string = null) { - super(submissionId, configData, initFormValues, parserOptions); + super(submissionId, configData, initFormValues, parserOptions, translate); this.separator = separator; this.firstPlaceholder = firstPlaceholder; diff --git a/src/app/shared/form/builder/parsers/dropdown-field-parser.ts b/src/app/shared/form/builder/parsers/dropdown-field-parser.ts index 3e5ec0b9daa..a4bfb810f34 100644 --- a/src/app/shared/form/builder/parsers/dropdown-field-parser.ts +++ b/src/app/shared/form/builder/parsers/dropdown-field-parser.ts @@ -15,6 +15,7 @@ import { import { isNotEmpty } from '../../../empty.util'; import { FormFieldMetadataValueObject } from '../models/form-field-metadata-value.model'; import { ParserOptions } from './parser-options'; +import { TranslateService } from '@ngx-translate/core'; export class DropdownFieldParser extends FieldParser { @@ -23,8 +24,9 @@ export class DropdownFieldParser extends FieldParser { @Inject(CONFIG_DATA) configData: FormFieldModel, @Inject(INIT_FORM_VALUES) initFormValues, @Inject(PARSER_OPTIONS) parserOptions: ParserOptions, + translate: TranslateService ) { - super(submissionId, configData, initFormValues, parserOptions); + super(submissionId, configData, initFormValues, parserOptions, translate); } public modelFactory(fieldValue?: FormFieldMetadataValueObject | any, label?: boolean): any { diff --git a/src/app/shared/form/builder/parsers/field-parser.ts b/src/app/shared/form/builder/parsers/field-parser.ts index 7ea55d44549..3c4d425df36 100644 --- a/src/app/shared/form/builder/parsers/field-parser.ts +++ b/src/app/shared/form/builder/parsers/field-parser.ts @@ -25,6 +25,7 @@ import { VocabularyOptions } from '../../../../core/submission/vocabularies/mode import { ParserType } from './parser-type'; import { isNgbDateStruct } from '../../../date.util'; import { SubmissionScopeType } from '../../../../core/submission/submission-scope-type'; +import { TranslateService } from '@ngx-translate/core'; export const SUBMISSION_ID: InjectionToken = new InjectionToken('submissionId'); export const CONFIG_DATA: InjectionToken = new InjectionToken('configData'); @@ -50,7 +51,8 @@ export abstract class FieldParser { @Inject(SUBMISSION_ID) protected submissionId: string, @Inject(CONFIG_DATA) protected configData: FormFieldModel, @Inject(INIT_FORM_VALUES) protected initFormValues: any, - @Inject(PARSER_OPTIONS) protected parserOptions: ParserOptions + @Inject(PARSER_OPTIONS) protected parserOptions: ParserOptions, + protected translate: TranslateService ) { } @@ -395,11 +397,14 @@ export abstract class FieldParser { } else { regex = new RegExp(this.configData.input.regex); } + const baseTranslationKey = 'error.validation.pattern'; + const fieldranslationKey = `${baseTranslationKey}.${controlModel.id}`; + const fieldTranslationExists = this.translate.instant(fieldranslationKey) !== fieldranslationKey; controlModel.validators = Object.assign({}, controlModel.validators, { pattern: regex }); controlModel.errorMessages = Object.assign( {}, controlModel.errorMessages, - { pattern: 'error.validation.pattern' }); + { pattern: fieldTranslationExists ? fieldranslationKey : baseTranslationKey }); } protected markAsRequired(controlModel) { diff --git a/src/app/shared/form/builder/parsers/name-field-parser.ts b/src/app/shared/form/builder/parsers/name-field-parser.ts index e5ecb034ea4..469b32be929 100644 --- a/src/app/shared/form/builder/parsers/name-field-parser.ts +++ b/src/app/shared/form/builder/parsers/name-field-parser.ts @@ -1,4 +1,5 @@ import { Inject } from '@angular/core'; +import { TranslateService } from '@ngx-translate/core'; import { FormFieldModel } from '../models/form-field.model'; import { ConcatFieldParser } from './concat-field-parser'; import { CONFIG_DATA, INIT_FORM_VALUES, PARSER_OPTIONS, SUBMISSION_ID } from './field-parser'; @@ -10,8 +11,9 @@ export class NameFieldParser extends ConcatFieldParser { @Inject(SUBMISSION_ID) submissionId: string, @Inject(CONFIG_DATA) configData: FormFieldModel, @Inject(INIT_FORM_VALUES) initFormValues, - @Inject(PARSER_OPTIONS) parserOptions: ParserOptions + @Inject(PARSER_OPTIONS) parserOptions: ParserOptions, + translate: TranslateService ) { - super(submissionId, configData, initFormValues, parserOptions, ',', 'form.last-name', 'form.first-name'); + super(submissionId, configData, initFormValues, parserOptions, translate, ',', 'form.last-name', 'form.first-name'); } } diff --git a/src/app/shared/form/builder/parsers/parser-factory.ts b/src/app/shared/form/builder/parsers/parser-factory.ts index 26a9cb0f289..97fc36cbb48 100644 --- a/src/app/shared/form/builder/parsers/parser-factory.ts +++ b/src/app/shared/form/builder/parsers/parser-factory.ts @@ -19,12 +19,14 @@ import { SeriesFieldParser } from './series-field-parser'; import { TagFieldParser } from './tag-field-parser'; import { TextareaFieldParser } from './textarea-field-parser'; import { DisabledFieldParser } from './disabled-field-parser'; +import { TranslateService } from '@ngx-translate/core'; const fieldParserDeps = [ SUBMISSION_ID, CONFIG_DATA, INIT_FORM_VALUES, PARSER_OPTIONS, + TranslateService ]; /** diff --git a/src/app/shared/form/builder/parsers/series-field-parser.ts b/src/app/shared/form/builder/parsers/series-field-parser.ts index 36ee9c36c1a..589a6dc04ca 100644 --- a/src/app/shared/form/builder/parsers/series-field-parser.ts +++ b/src/app/shared/form/builder/parsers/series-field-parser.ts @@ -1,4 +1,5 @@ import { Inject } from '@angular/core'; +import { TranslateService } from '@ngx-translate/core'; import { FormFieldModel } from '../models/form-field.model'; import { ConcatFieldParser } from './concat-field-parser'; import { CONFIG_DATA, INIT_FORM_VALUES, PARSER_OPTIONS, SUBMISSION_ID } from './field-parser'; @@ -10,8 +11,9 @@ export class SeriesFieldParser extends ConcatFieldParser { @Inject(SUBMISSION_ID) submissionId: string, @Inject(CONFIG_DATA) configData: FormFieldModel, @Inject(INIT_FORM_VALUES) initFormValues, - @Inject(PARSER_OPTIONS) parserOptions: ParserOptions + @Inject(PARSER_OPTIONS) parserOptions: ParserOptions, + translate: TranslateService ) { - super(submissionId, configData, initFormValues, parserOptions, ';'); + super(submissionId, configData, initFormValues, parserOptions, translate, ';'); } } From c93a64db838ddc4dce5f73aa9e28f28c457fb14d Mon Sep 17 00:00:00 2001 From: "max.nuding" Date: Wed, 15 Nov 2023 13:27:58 +0100 Subject: [PATCH 002/356] Fix tests to include translationservice dependency --- .../form/builder/form-builder.service.spec.ts | 6 +++- .../builder/parsers/date-field-parser.spec.ts | 10 ++++-- .../parsers/disabled-field-parser.spec.ts | 8 +++-- .../parsers/dropdown-field-parser.spec.ts | 8 +++-- .../builder/parsers/list-field-parser.spec.ts | 10 +++--- .../parsers/lookup-field-parser.spec.ts | 8 +++-- .../parsers/lookup-name-field-parser.spec.ts | 8 +++-- .../builder/parsers/name-field-parser.spec.ts | 10 +++--- .../parsers/onebox-field-parser.spec.ts | 12 ++++--- .../relation-group-field-parser.spec.ts | 10 +++--- .../form/builder/parsers/row-parser.spec.ts | 35 ++++++++++++------- .../parsers/series-field-parser.spec.ts | 10 +++--- .../builder/parsers/tag-field-parser.spec.ts | 8 +++-- .../parsers/textarea-field-parser.spec.ts | 8 +++-- 14 files changed, 96 insertions(+), 55 deletions(-) diff --git a/src/app/shared/form/builder/form-builder.service.spec.ts b/src/app/shared/form/builder/form-builder.service.spec.ts index fd3588e73ef..94c88aae174 100644 --- a/src/app/shared/form/builder/form-builder.service.spec.ts +++ b/src/app/shared/form/builder/form-builder.service.spec.ts @@ -51,6 +51,8 @@ import { FormRowModel } from '../../../core/config/models/config-submission-form import {ConfigurationDataService} from '../../../core/data/configuration-data.service'; import {createSuccessfulRemoteDataObject$} from '../../remote-data.utils'; import {ConfigurationProperty} from '../../../core/shared/configuration-property.model'; +import { getMockTranslateService } from '../../mocks/translate.service.mock'; +import { TranslateService } from '@ngx-translate/core'; describe('FormBuilderService test suite', () => { @@ -81,6 +83,7 @@ describe('FormBuilderService test suite', () => { beforeEach(() => { configSpy = createConfigSuccessSpy(typeFieldTestValue); + let translateService = getMockTranslateService(); TestBed.configureTestingModule({ imports: [ReactiveFormsModule], providers: [ @@ -88,7 +91,8 @@ describe('FormBuilderService test suite', () => { { provide: DynamicFormValidationService, useValue: {} }, { provide: NG_VALIDATORS, useValue: testValidator, multi: true }, { provide: NG_ASYNC_VALIDATORS, useValue: testAsyncValidator, multi: true }, - { provide: ConfigurationDataService, useValue: configSpy } + { provide: ConfigurationDataService, useValue: configSpy }, + { provide: TranslateService, useValue: translateService }, ] }); diff --git a/src/app/shared/form/builder/parsers/date-field-parser.spec.ts b/src/app/shared/form/builder/parsers/date-field-parser.spec.ts index 9ab43709ad0..891bb1d9d46 100644 --- a/src/app/shared/form/builder/parsers/date-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/date-field-parser.spec.ts @@ -3,10 +3,14 @@ import { DateFieldParser } from './date-field-parser'; import { DynamicDsDatePickerModel } from '../ds-dynamic-form-ui/models/date-picker/date-picker.model'; import { FormFieldMetadataValueObject } from '../models/form-field-metadata-value.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; + + describe('DateFieldParser test suite', () => { let field: FormFieldModel; let initFormValues: any = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -37,13 +41,13 @@ describe('DateFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new DateFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new DateFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof DateFieldParser).toBe(true); }); it('should return a DynamicDsDatePickerModel object when repeatable option is false', () => { - const parser = new DateFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new DateFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -56,7 +60,7 @@ describe('DateFieldParser test suite', () => { }; const expectedValue = '1983-11-18'; - const parser = new DateFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new DateFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); diff --git a/src/app/shared/form/builder/parsers/disabled-field-parser.spec.ts b/src/app/shared/form/builder/parsers/disabled-field-parser.spec.ts index d69f0e48e90..2168d7e2bf3 100644 --- a/src/app/shared/form/builder/parsers/disabled-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/disabled-field-parser.spec.ts @@ -2,10 +2,12 @@ import { FormFieldModel } from '../models/form-field.model'; import { ParserOptions } from './parser-options'; import { DisabledFieldParser } from './disabled-field-parser'; import { DynamicDisabledModel } from '../ds-dynamic-form-ui/models/disabled/dynamic-disabled.model'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('DisabledFieldParser test suite', () => { let field: FormFieldModel; let initFormValues: any = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -35,13 +37,13 @@ describe('DisabledFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new DisabledFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new DisabledFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof DisabledFieldParser).toBe(true); }); it('should return a DynamicDisabledModel object when repeatable option is false', () => { - const parser = new DisabledFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new DisabledFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -56,7 +58,7 @@ describe('DisabledFieldParser test suite', () => { }; const expectedValue = 'test description'; - const parser = new DisabledFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new DisabledFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); expect(fieldModel.value.value).toEqual(expectedValue); diff --git a/src/app/shared/form/builder/parsers/dropdown-field-parser.spec.ts b/src/app/shared/form/builder/parsers/dropdown-field-parser.spec.ts index 3dca7558b34..08a93f76d36 100644 --- a/src/app/shared/form/builder/parsers/dropdown-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/dropdown-field-parser.spec.ts @@ -2,9 +2,11 @@ import { FormFieldModel } from '../models/form-field.model'; import { DropdownFieldParser } from './dropdown-field-parser'; import { DynamicScrollableDropdownModel } from '../ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('DropdownFieldParser test suite', () => { let field: FormFieldModel; + let translateService = getMockTranslateService(); const submissionId = '1234'; const initFormValues = {}; @@ -37,13 +39,13 @@ describe('DropdownFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new DropdownFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new DropdownFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof DropdownFieldParser).toBe(true); }); it('should return a DynamicScrollableDropdownModel object when repeatable option is false', () => { - const parser = new DropdownFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new DropdownFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -52,7 +54,7 @@ describe('DropdownFieldParser test suite', () => { it('should throw when authority is not passed', () => { field.selectableMetadata[0].controlledVocabulary = null; - const parser = new DropdownFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new DropdownFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(() => parser.parse()) .toThrow(); diff --git a/src/app/shared/form/builder/parsers/list-field-parser.spec.ts b/src/app/shared/form/builder/parsers/list-field-parser.spec.ts index 30d1913a519..ba9f9291cf0 100644 --- a/src/app/shared/form/builder/parsers/list-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/list-field-parser.spec.ts @@ -4,10 +4,12 @@ import { ListFieldParser } from './list-field-parser'; import { DynamicListCheckboxGroupModel } from '../ds-dynamic-form-ui/models/list/dynamic-list-checkbox-group.model'; import { DynamicListRadioGroupModel } from '../ds-dynamic-form-ui/models/list/dynamic-list-radio-group.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('ListFieldParser test suite', () => { let field: FormFieldModel; let initFormValues = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -39,13 +41,13 @@ describe('ListFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new ListFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new ListFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof ListFieldParser).toBe(true); }); it('should return a DynamicListCheckboxGroupModel object when repeatable option is true', () => { - const parser = new ListFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new ListFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -54,7 +56,7 @@ describe('ListFieldParser test suite', () => { it('should return a DynamicListRadioGroupModel object when repeatable option is false', () => { field.repeatable = false; - const parser = new ListFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new ListFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -67,7 +69,7 @@ describe('ListFieldParser test suite', () => { }; const expectedValue = [new FormFieldMetadataValueObject('test type')]; - const parser = new ListFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new ListFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); diff --git a/src/app/shared/form/builder/parsers/lookup-field-parser.spec.ts b/src/app/shared/form/builder/parsers/lookup-field-parser.spec.ts index 24efcf34622..a932dc637c2 100644 --- a/src/app/shared/form/builder/parsers/lookup-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/lookup-field-parser.spec.ts @@ -3,10 +3,12 @@ import { FormFieldMetadataValueObject } from '../models/form-field-metadata-valu import { LookupFieldParser } from './lookup-field-parser'; import { DynamicLookupModel } from '../ds-dynamic-form-ui/models/lookup/dynamic-lookup.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('LookupFieldParser test suite', () => { let field: FormFieldModel; let initFormValues = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -38,13 +40,13 @@ describe('LookupFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new LookupFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new LookupFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof LookupFieldParser).toBe(true); }); it('should return a DynamicLookupModel object when repeatable option is false', () => { - const parser = new LookupFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new LookupFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -57,7 +59,7 @@ describe('LookupFieldParser test suite', () => { }; const expectedValue = new FormFieldMetadataValueObject('test journal'); - const parser = new LookupFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new LookupFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); diff --git a/src/app/shared/form/builder/parsers/lookup-name-field-parser.spec.ts b/src/app/shared/form/builder/parsers/lookup-name-field-parser.spec.ts index d0281681ef6..6220a6e74ce 100644 --- a/src/app/shared/form/builder/parsers/lookup-name-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/lookup-name-field-parser.spec.ts @@ -3,10 +3,12 @@ import { FormFieldMetadataValueObject } from '../models/form-field-metadata-valu import { LookupNameFieldParser } from './lookup-name-field-parser'; import { DynamicLookupNameModel } from '../ds-dynamic-form-ui/models/lookup/dynamic-lookup-name.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('LookupNameFieldParser test suite', () => { let field: FormFieldModel; let initFormValues = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -38,13 +40,13 @@ describe('LookupNameFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new LookupNameFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new LookupNameFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof LookupNameFieldParser).toBe(true); }); it('should return a DynamicLookupNameModel object when repeatable option is false', () => { - const parser = new LookupNameFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new LookupNameFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -57,7 +59,7 @@ describe('LookupNameFieldParser test suite', () => { }; const expectedValue = new FormFieldMetadataValueObject('test author'); - const parser = new LookupNameFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new LookupNameFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); diff --git a/src/app/shared/form/builder/parsers/name-field-parser.spec.ts b/src/app/shared/form/builder/parsers/name-field-parser.spec.ts index 6b520142cc4..e124181b247 100644 --- a/src/app/shared/form/builder/parsers/name-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/name-field-parser.spec.ts @@ -3,12 +3,14 @@ import { NameFieldParser } from './name-field-parser'; import { DynamicConcatModel } from '../ds-dynamic-form-ui/models/ds-dynamic-concat.model'; import { FormFieldMetadataValueObject } from '../models/form-field-metadata-value.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('NameFieldParser test suite', () => { let field1: FormFieldModel; let field2: FormFieldModel; let field3: FormFieldModel; let initFormValues: any = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -71,13 +73,13 @@ describe('NameFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new NameFieldParser(submissionId, field1, initFormValues, parserOptions); + const parser = new NameFieldParser(submissionId, field1, initFormValues, parserOptions, translateService); expect(parser instanceof NameFieldParser).toBe(true); }); it('should return a DynamicConcatModel object when repeatable option is false', () => { - const parser = new NameFieldParser(submissionId, field2, initFormValues, parserOptions); + const parser = new NameFieldParser(submissionId, field2, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -85,7 +87,7 @@ describe('NameFieldParser test suite', () => { }); it('should return a DynamicConcatModel object with the correct separator', () => { - const parser = new NameFieldParser(submissionId, field2, initFormValues, parserOptions); + const parser = new NameFieldParser(submissionId, field2, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -98,7 +100,7 @@ describe('NameFieldParser test suite', () => { }; const expectedValue = new FormFieldMetadataValueObject('test, name', undefined, undefined, 'test'); - const parser = new NameFieldParser(submissionId, field1, initFormValues, parserOptions); + const parser = new NameFieldParser(submissionId, field1, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); diff --git a/src/app/shared/form/builder/parsers/onebox-field-parser.spec.ts b/src/app/shared/form/builder/parsers/onebox-field-parser.spec.ts index a4c71d1f426..6b7ac65a587 100644 --- a/src/app/shared/form/builder/parsers/onebox-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/onebox-field-parser.spec.ts @@ -5,11 +5,13 @@ import { DynamicOneboxModel } from '../ds-dynamic-form-ui/models/onebox/dynamic- import { DsDynamicInputModel } from '../ds-dynamic-form-ui/models/ds-dynamic-input.model'; import { ParserOptions } from './parser-options'; import { FieldParser } from './field-parser'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('OneboxFieldParser test suite', () => { let field1: FormFieldModel; let field2: FormFieldModel; let field3: FormFieldModel; + let translateService = getMockTranslateService(); const submissionId = '1234'; const initFormValues = {}; @@ -73,13 +75,13 @@ describe('OneboxFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new OneboxFieldParser(submissionId, field1, initFormValues, parserOptions); + const parser = new OneboxFieldParser(submissionId, field1, initFormValues, parserOptions, translateService); expect(parser instanceof OneboxFieldParser).toBe(true); }); it('should return a DynamicQualdropModel object when selectableMetadata is multiple', () => { - const parser = new OneboxFieldParser(submissionId, field2, initFormValues, parserOptions); + const parser = new OneboxFieldParser(submissionId, field2, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -87,7 +89,7 @@ describe('OneboxFieldParser test suite', () => { }); it('should return a DsDynamicInputModel object when selectableMetadata is not multiple', () => { - const parser = new OneboxFieldParser(submissionId, field3, initFormValues, parserOptions); + const parser = new OneboxFieldParser(submissionId, field3, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -95,7 +97,7 @@ describe('OneboxFieldParser test suite', () => { }); it('should return a DynamicOneboxModel object when selectableMetadata has authority', () => { - const parser = new OneboxFieldParser(submissionId, field1, initFormValues, parserOptions); + const parser = new OneboxFieldParser(submissionId, field1, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -124,7 +126,7 @@ describe('OneboxFieldParser test suite', () => { languageCodes: [] } as FormFieldModel; - parser = new OneboxFieldParser(submissionId, regexField, initFormValues, parserOptions); + parser = new OneboxFieldParser(submissionId, regexField, initFormValues, parserOptions, translateService); fieldModel = parser.parse(); }); diff --git a/src/app/shared/form/builder/parsers/relation-group-field-parser.spec.ts b/src/app/shared/form/builder/parsers/relation-group-field-parser.spec.ts index 7d48ad2d002..8ae0ccfedf3 100644 --- a/src/app/shared/form/builder/parsers/relation-group-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/relation-group-field-parser.spec.ts @@ -3,10 +3,12 @@ import { RelationGroupFieldParser } from './relation-group-field-parser'; import { DynamicRelationGroupModel } from '../ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.model'; import { FormFieldMetadataValueObject } from '../models/form-field-metadata-value.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('RelationGroupFieldParser test suite', () => { let field: FormFieldModel; let initFormValues = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -73,13 +75,13 @@ describe('RelationGroupFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new RelationGroupFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new RelationGroupFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof RelationGroupFieldParser).toBe(true); }); it('should return a DynamicRelationGroupModel object', () => { - const parser = new RelationGroupFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new RelationGroupFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -88,7 +90,7 @@ describe('RelationGroupFieldParser test suite', () => { it('should throw when rows configuration is empty', () => { field.rows = null; - const parser = new RelationGroupFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new RelationGroupFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(() => parser.parse()) .toThrow(); @@ -99,7 +101,7 @@ describe('RelationGroupFieldParser test suite', () => { author: [new FormFieldMetadataValueObject('test author')], affiliation: [new FormFieldMetadataValueObject('test affiliation')] }; - const parser = new RelationGroupFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new RelationGroupFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); const expectedValue = [{ diff --git a/src/app/shared/form/builder/parsers/row-parser.spec.ts b/src/app/shared/form/builder/parsers/row-parser.spec.ts index 1f9bde8a7fb..fca16b28e35 100644 --- a/src/app/shared/form/builder/parsers/row-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/row-parser.spec.ts @@ -3,6 +3,10 @@ import { RowParser } from './row-parser'; import { DynamicRowGroupModel } from '../ds-dynamic-form-ui/models/ds-dynamic-row-group-model'; import { DynamicRowArrayModel } from '../ds-dynamic-form-ui/models/ds-dynamic-row-array-model'; import { FormRowModel } from '../../../../core/config/models/config-submission-form.model'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; +import { TestBed } from '@angular/core/testing'; +import { TranslateService } from '@ngx-translate/core'; +import { Injector } from '@angular/core'; describe('RowParser test suite', () => { @@ -16,6 +20,7 @@ describe('RowParser test suite', () => { let row8: FormRowModel; let row9: FormRowModel; let row10: FormRowModel; + let injector: Injector; const submissionId = '1234'; const scopeUUID = 'testScopeUUID'; @@ -25,6 +30,12 @@ describe('RowParser test suite', () => { const typeField = 'dc_type'; beforeEach(() => { + let translateService = getMockTranslateService(); + injector = Injector.create({ + providers: [ + { provide: TranslateService, useValue: translateService }, + ], + }); row1 = { fields: [ { @@ -330,14 +341,14 @@ describe('RowParser test suite', () => { }); it('should init parser properly', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); expect(parser instanceof RowParser).toBe(true); }); describe('parse', () => { it('should return a DynamicRowGroupModel object', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row1, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -345,7 +356,7 @@ describe('RowParser test suite', () => { }); it('should return a row with three fields', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row1, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -353,7 +364,7 @@ describe('RowParser test suite', () => { }); it('should return a DynamicRowArrayModel object', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row2, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -361,7 +372,7 @@ describe('RowParser test suite', () => { }); it('should return a row that contains only scoped fields', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row3, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -369,7 +380,7 @@ describe('RowParser test suite', () => { }); it('should be able to parse a dropdown combo field', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row4, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -377,7 +388,7 @@ describe('RowParser test suite', () => { }); it('should be able to parse a lookup-name field', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row5, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -385,7 +396,7 @@ describe('RowParser test suite', () => { }); it('should be able to parse a list field', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row6, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -393,7 +404,7 @@ describe('RowParser test suite', () => { }); it('should be able to parse a date field', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row7, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -401,7 +412,7 @@ describe('RowParser test suite', () => { }); it('should be able to parse a tag field', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row8, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -409,7 +420,7 @@ describe('RowParser test suite', () => { }); it('should be able to parse a textarea field', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row9, scopeUUID, initFormValues, submissionScope, readOnly, typeField); @@ -417,7 +428,7 @@ describe('RowParser test suite', () => { }); it('should be able to parse a group field', () => { - const parser = new RowParser(undefined); + const parser = new RowParser(injector); const rowModel = parser.parse(submissionId, row10, scopeUUID, initFormValues, submissionScope, readOnly, typeField); diff --git a/src/app/shared/form/builder/parsers/series-field-parser.spec.ts b/src/app/shared/form/builder/parsers/series-field-parser.spec.ts index 0761cfe60e2..0ce50081e40 100644 --- a/src/app/shared/form/builder/parsers/series-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/series-field-parser.spec.ts @@ -3,10 +3,12 @@ import { DynamicConcatModel } from '../ds-dynamic-form-ui/models/ds-dynamic-conc import { SeriesFieldParser } from './series-field-parser'; import { FormFieldMetadataValueObject } from '../models/form-field-metadata-value.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('SeriesFieldParser test suite', () => { let field: FormFieldModel; let initFormValues: any = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -34,13 +36,13 @@ describe('SeriesFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new SeriesFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new SeriesFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof SeriesFieldParser).toBe(true); }); it('should return a DynamicConcatModel object when repeatable option is false', () => { - const parser = new SeriesFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new SeriesFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -48,7 +50,7 @@ describe('SeriesFieldParser test suite', () => { }); it('should return a DynamicConcatModel object with the correct separator', () => { - const parser = new SeriesFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new SeriesFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -61,7 +63,7 @@ describe('SeriesFieldParser test suite', () => { }; const expectedValue = new FormFieldMetadataValueObject('test; series', undefined, undefined, 'test'); - const parser = new SeriesFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new SeriesFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); diff --git a/src/app/shared/form/builder/parsers/tag-field-parser.spec.ts b/src/app/shared/form/builder/parsers/tag-field-parser.spec.ts index 115829f8d38..ae10dbd3861 100644 --- a/src/app/shared/form/builder/parsers/tag-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/tag-field-parser.spec.ts @@ -3,10 +3,12 @@ import { FormFieldMetadataValueObject } from '../models/form-field-metadata-valu import { TagFieldParser } from './tag-field-parser'; import { DynamicTagModel } from '../ds-dynamic-form-ui/models/tag/dynamic-tag.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('TagFieldParser test suite', () => { let field: FormFieldModel; let initFormValues: any = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -38,13 +40,13 @@ describe('TagFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new TagFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new TagFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof TagFieldParser).toBe(true); }); it('should return a DynamicTagModel object when repeatable option is false', () => { - const parser = new TagFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new TagFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -59,7 +61,7 @@ describe('TagFieldParser test suite', () => { ], }; - const parser = new TagFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new TagFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); diff --git a/src/app/shared/form/builder/parsers/textarea-field-parser.spec.ts b/src/app/shared/form/builder/parsers/textarea-field-parser.spec.ts index 855e464f21d..259f8a60e14 100644 --- a/src/app/shared/form/builder/parsers/textarea-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/textarea-field-parser.spec.ts @@ -3,10 +3,12 @@ import { FormFieldMetadataValueObject } from '../models/form-field-metadata-valu import { TextareaFieldParser } from './textarea-field-parser'; import { DsDynamicTextAreaModel } from '../ds-dynamic-form-ui/models/ds-dynamic-textarea.model'; import { ParserOptions } from './parser-options'; +import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; describe('TextareaFieldParser test suite', () => { let field: FormFieldModel; let initFormValues: any = {}; + let translateService = getMockTranslateService(); const submissionId = '1234'; const parserOptions: ParserOptions = { @@ -36,13 +38,13 @@ describe('TextareaFieldParser test suite', () => { }); it('should init parser properly', () => { - const parser = new TextareaFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new TextareaFieldParser(submissionId, field, initFormValues, parserOptions, translateService); expect(parser instanceof TextareaFieldParser).toBe(true); }); it('should return a DsDynamicTextAreaModel object when repeatable option is false', () => { - const parser = new TextareaFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new TextareaFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); @@ -57,7 +59,7 @@ describe('TextareaFieldParser test suite', () => { }; const expectedValue = 'test description'; - const parser = new TextareaFieldParser(submissionId, field, initFormValues, parserOptions); + const parser = new TextareaFieldParser(submissionId, field, initFormValues, parserOptions, translateService); const fieldModel = parser.parse(); From ab23613b799f488a0845aeb309a371eabc3035c6 Mon Sep 17 00:00:00 2001 From: "max.nuding" Date: Wed, 15 Nov 2023 13:42:53 +0100 Subject: [PATCH 003/356] Remove unnecessary import --- src/app/shared/form/builder/parsers/row-parser.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/shared/form/builder/parsers/row-parser.spec.ts b/src/app/shared/form/builder/parsers/row-parser.spec.ts index fca16b28e35..f414715f5be 100644 --- a/src/app/shared/form/builder/parsers/row-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/row-parser.spec.ts @@ -4,7 +4,6 @@ import { DynamicRowGroupModel } from '../ds-dynamic-form-ui/models/ds-dynamic-ro import { DynamicRowArrayModel } from '../ds-dynamic-form-ui/models/ds-dynamic-row-array-model'; import { FormRowModel } from '../../../../core/config/models/config-submission-form.model'; import { getMockTranslateService } from 'src/app/shared/mocks/translate.service.mock'; -import { TestBed } from '@angular/core/testing'; import { TranslateService } from '@ngx-translate/core'; import { Injector } from '@angular/core'; From 5a839c2906ef7709409731092ff6d8715cb18649 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Wed, 15 Nov 2023 14:37:06 -0600 Subject: [PATCH 004/356] Update version tag for development of next release --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1ddb27078f2..852e5e57f3d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dspace-angular", - "version": "7.6.1", + "version": "7.6.2-next", "scripts": { "ng": "ng", "config:watch": "nodemon", From 02eb618c5f8dcb7295301e294901dadcb53cd1fe Mon Sep 17 00:00:00 2001 From: Thomas Misilo Date: Thu, 16 Nov 2023 11:36:06 -0600 Subject: [PATCH 005/356] Setup the Docker GH Action for Matrix Building This change enables building of the amd64 and arm64 images simultaneously. Once both images finish, the manifest is sent to Docker Hub, allowing for a single image that has both the amd64/arm64 images. --- .github/workflows/docker.yml | 173 +++++++++++++++++++++++++++++++---- 1 file changed, 153 insertions(+), 20 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 0c36d5af987..18bdef78be6 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -17,6 +17,7 @@ permissions: env: + REGISTRY_IMAGE: dspace/dspace-angular # Define tags to use for Docker images based on Git tags/branches (for docker/metadata-action) # For a new commit on default branch (main), use the literal tag 'latest' on Docker image. # For a new commit on other branches, use the branch name as the tag for Docker image. @@ -30,21 +31,30 @@ env: # We manage the 'latest' tag ourselves to the 'main' branch (see settings above) TAGS_FLAVOR: | latest=false - # Architectures / Platforms for which we will build Docker images - # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. - # If this is NOT a PR (e.g. a tag or merge commit), also build for ARM64. - PLATFORMS: linux/amd64${{ github.event_name != 'pull_request' && ', linux/arm64' || '' }} - jobs: - ############################################### - # Build/Push the 'dspace/dspace-angular' image - ############################################### + ############################################################# + # Build/Push the '${{ env.REGISTRY_IMAGE }}' image + ############################################################# dspace-angular: # Ensure this job never runs on forked repos. It's only executed for 'dspace/dspace-angular' if: github.repository == 'dspace/dspace-angular' - runs-on: ubuntu-latest + strategy: + matrix: + isPr: + - ${{ github.event_name == 'pull_request' }} + # Architectures / Platforms for which we will build Docker images + # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. + # If this is NOT a PR (e.g. a tag or merge commit), also build for ARM64. + arch: ['linux/amd64', 'linux/arm64'] + os: [ubuntu-latest] + exclude: + - isPr: true + os: ubuntu-latest + arch: linux/arm64 + + runs-on: ${{ matrix.os }} steps: # https://github.com/actions/checkout - name: Checkout codebase @@ -61,7 +71,7 @@ jobs: # https://github.com/docker/login-action - name: Login to DockerHub # Only login if not a PR, as PRs only trigger a Docker build and not a push - if: github.event_name != 'pull_request' + if: ${{ ! matrix.isPr }} uses: docker/login-action@v2 with: username: ${{ secrets.DOCKER_USERNAME }} @@ -73,7 +83,7 @@ jobs: id: meta_build uses: docker/metadata-action@v4 with: - images: dspace/dspace-angular + images: ${{ env.REGISTRY_IMAGE }} tags: ${{ env.IMAGE_TAGS }} flavor: ${{ env.TAGS_FLAVOR }} @@ -84,22 +94,89 @@ jobs: with: context: . file: ./Dockerfile - platforms: ${{ env.PLATFORMS }} + platforms: ${{ matrix.arch }} # For pull requests, we run the Docker build (to ensure no PR changes break the build), # but we ONLY do an image push to DockerHub if it's NOT a PR - push: ${{ github.event_name != 'pull_request' }} + push: ${{ ! matrix.isPr }} # Use tags / labels provided by 'docker/metadata-action' above tags: ${{ steps.meta_build.outputs.tags }} labels: ${{ steps.meta_build.outputs.labels }} + - name: Export digest + if: ${{ ! matrix.isPr }} + run: | + mkdir -p /tmp/digests + digest="${{ steps.docker_build.outputs.digest }}" + touch "/tmp/digests/${digest#sha256:}" + + - name: Upload digest + if: ${{ ! matrix.isPr }} + uses: actions/upload-artifact@v3 + with: + name: digests + path: /tmp/digests/* + if-no-files-found: error + retention-days: 1 + + merge: + if: ${{ github.event_name != 'pull_request' }} + runs-on: ubuntu-latest + needs: + - dspace-angular + steps: + - name: Download digests + uses: actions/download-artifact@v3 + with: + name: digests + path: /tmp/digests + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + - name: Docker meta + id: meta + uses: docker/metadata-action@v5 + with: + images: ${{ env.REGISTRY_IMAGE }} + tags: ${{ env.IMAGE_TAGS }} + flavor: ${{ env.TAGS_FLAVOR }} + + - name: Login to Docker Hub + uses: docker/login-action@v3 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_ACCESS_TOKEN }} + + - name: Create manifest list and push + working-directory: /tmp/digests + run: | + docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ + $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + + - name: Inspect image + run: | + docker buildx imagetools inspect ${{ env.REGISTRY_IMAGE }}:${{ steps.meta.outputs.version }} + ############################################################# - # Build/Push the 'dspace/dspace-angular' image ('-dist' tag) + # Build/Push the '${{ env.REGISTRY_IMAGE }}' image ('-dist' tag) ############################################################# dspace-angular-dist: # Ensure this job never runs on forked repos. It's only executed for 'dspace/dspace-angular' if: github.repository == 'dspace/dspace-angular' - runs-on: ubuntu-latest + strategy: + matrix: + isPr: + - ${{ github.event_name == 'pull_request' }} + # Architectures / Platforms for which we will build Docker images + # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. + # If this is NOT a PR (e.g. a tag or merge commit), also build for ARM64. + arch: ['linux/amd64', 'linux/arm64'] + os: [ubuntu-latest] + exclude: + - isPr: true + os: ubuntu-latest + arch: linux/arm64 + + runs-on: ${{ matrix.os }} steps: # https://github.com/actions/checkout - name: Checkout codebase @@ -116,7 +193,7 @@ jobs: # https://github.com/docker/login-action - name: Login to DockerHub # Only login if not a PR, as PRs only trigger a Docker build and not a push - if: github.event_name != 'pull_request' + if: ${{ ! matrix.isPr }} uses: docker/login-action@v2 with: username: ${{ secrets.DOCKER_USERNAME }} @@ -128,10 +205,10 @@ jobs: id: meta_build_dist uses: docker/metadata-action@v4 with: - images: dspace/dspace-angular + images: ${{ env.REGISTRY_IMAGE }} tags: ${{ env.IMAGE_TAGS }} # As this is a "dist" image, its tags are all suffixed with "-dist". Otherwise, it uses the same - # tagging logic as the primary 'dspace/dspace-angular' image above. + # tagging logic as the primary '${{ env.REGISTRY_IMAGE }}' image above. flavor: ${{ env.TAGS_FLAVOR }} suffix=-dist @@ -141,10 +218,66 @@ jobs: with: context: . file: ./Dockerfile.dist - platforms: ${{ env.PLATFORMS }} + platforms: ${{ matrix.arch }} # For pull requests, we run the Docker build (to ensure no PR changes break the build), # but we ONLY do an image push to DockerHub if it's NOT a PR - push: ${{ github.event_name != 'pull_request' }} + push: ${{ ! matrix.isPr }} # Use tags / labels provided by 'docker/metadata-action' above tags: ${{ steps.meta_build_dist.outputs.tags }} labels: ${{ steps.meta_build_dist.outputs.labels }} + + - name: Export digest + if: ${{ ! matrix.isPr }} + run: | + mkdir -p /tmp/digests/dist + digest="${{ steps.docker_build_dist.outputs.digest }}" + touch "/tmp/digests/dist/${digest#sha256:}" + + - name: Upload digest + if: ${{ ! matrix.isPr }} + uses: actions/upload-artifact@v3 + with: + name: digests + path: /tmp/digests/dist/* + if-no-files-found: error + retention-days: 1 + + merge-dist: + if: ${{ github.event_name != 'pull_request' }} + runs-on: ubuntu-latest + needs: + - dspace-angular-dist + steps: + - name: Download digests + uses: actions/download-artifact@v3 + with: + name: digests + path: /tmp/digests + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + - name: Docker meta + id: meta_dist + uses: docker/metadata-action@v5 + with: + images: ${{ env.REGISTRY_IMAGE }} + tags: ${{ env.IMAGE_TAGS }} + # As this is a "dist" image, its tags are all suffixed with "-dist". Otherwise, it uses the same + # tagging logic as the primary '${{ env.REGISTRY_IMAGE }}' image above. + flavor: ${{ env.TAGS_FLAVOR }} + suffix=-dist + + - name: Login to Docker Hub + uses: docker/login-action@v3 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_ACCESS_TOKEN }} + + - name: Create manifest list and push + working-directory: /tmp/digests/dist + run: | + docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ + $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + + - name: Inspect image + run: | + docker buildx imagetools inspect ${{ env.REGISTRY_IMAGE }}:${{ steps.meta_dist.outputs.version }} From 526da8cddf45e09bee4df4c73a9c389659a2c6cf Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 17 Nov 2023 09:31:09 -0600 Subject: [PATCH 006/356] Fix bug in Docker manifest. Each build must use a separate artifact to store digests. Other minor cleanup & comments added. --- .github/workflows/docker.yml | 81 ++++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 18bdef78be6..23919d573c7 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -42,13 +42,13 @@ jobs: strategy: matrix: - isPr: - - ${{ github.event_name == 'pull_request' }} # Architectures / Platforms for which we will build Docker images - # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. - # If this is NOT a PR (e.g. a tag or merge commit), also build for ARM64. arch: ['linux/amd64', 'linux/arm64'] os: [ubuntu-latest] + isPr: + - ${{ github.event_name == 'pull_request' }} + # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. + # The below exclude therefore ensures we do NOT build ARM64 for PRs. exclude: - isPr: true os: ubuntu-latest @@ -58,21 +58,21 @@ jobs: steps: # https://github.com/actions/checkout - name: Checkout codebase - uses: actions/checkout@v3 + uses: actions/checkout@v4 # https://github.com/docker/setup-buildx-action - name: Setup Docker Buildx - uses: docker/setup-buildx-action@v2 + uses: docker/setup-buildx-action@v3 # https://github.com/docker/setup-qemu-action - name: Set up QEMU emulation to build for multiple architectures - uses: docker/setup-qemu-action@v2 + uses: docker/setup-qemu-action@v3 # https://github.com/docker/login-action - name: Login to DockerHub # Only login if not a PR, as PRs only trigger a Docker build and not a push if: ${{ ! matrix.isPr }} - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_ACCESS_TOKEN }} @@ -81,7 +81,7 @@ jobs: # Get Metadata for docker_build step below - name: Sync metadata (tags, labels) from GitHub to Docker for 'dspace-angular' image id: meta_build - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v5 with: images: ${{ env.REGISTRY_IMAGE }} tags: ${{ env.IMAGE_TAGS }} @@ -90,7 +90,7 @@ jobs: # https://github.com/docker/build-push-action - name: Build and push 'dspace-angular' image id: docker_build - uses: docker/build-push-action@v4 + uses: docker/build-push-action@v5 with: context: . file: ./Dockerfile @@ -102,6 +102,7 @@ jobs: tags: ${{ steps.meta_build.outputs.tags }} labels: ${{ steps.meta_build.outputs.labels }} + # Export the digest of Docker build locally (for non PRs only) - name: Export digest if: ${{ ! matrix.isPr }} run: | @@ -109,6 +110,7 @@ jobs: digest="${{ steps.docker_build.outputs.digest }}" touch "/tmp/digests/${digest#sha256:}" + # Upload digest to an artifact, so that it can be used in manifest below - name: Upload digest if: ${{ ! matrix.isPr }} uses: actions/upload-artifact@v3 @@ -118,7 +120,12 @@ jobs: if-no-files-found: error retention-days: 1 - merge: + # Merge digests into a manifest. + # This runs after all Docker builds complete above, and it tells hub.docker.com + # that these builds should be all included in the manifest for this tag. + # (e.g. AMD64 and ARM64 should be listed as options under the same tagged Docker image) + # Borrowed from https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners + dspace-angular_manifest: if: ${{ github.event_name != 'pull_request' }} runs-on: ubuntu-latest needs: @@ -129,9 +136,11 @@ jobs: with: name: digests path: /tmp/digests + - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 - - name: Docker meta + + - name: Add Docker metadata for image id: meta uses: docker/metadata-action@v5 with: @@ -145,7 +154,7 @@ jobs: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_ACCESS_TOKEN }} - - name: Create manifest list and push + - name: Create manifest list from digests and push working-directory: /tmp/digests run: | docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ @@ -164,13 +173,13 @@ jobs: strategy: matrix: - isPr: - - ${{ github.event_name == 'pull_request' }} # Architectures / Platforms for which we will build Docker images - # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. - # If this is NOT a PR (e.g. a tag or merge commit), also build for ARM64. arch: ['linux/amd64', 'linux/arm64'] os: [ubuntu-latest] + isPr: + - ${{ github.event_name == 'pull_request' }} + # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. + # The below exclude therefore ensures we do NOT build ARM64 for PRs. exclude: - isPr: true os: ubuntu-latest @@ -180,21 +189,21 @@ jobs: steps: # https://github.com/actions/checkout - name: Checkout codebase - uses: actions/checkout@v3 + uses: actions/checkout@v4 # https://github.com/docker/setup-buildx-action - name: Setup Docker Buildx - uses: docker/setup-buildx-action@v2 + uses: docker/setup-buildx-action@v3 # https://github.com/docker/setup-qemu-action - name: Set up QEMU emulation to build for multiple architectures - uses: docker/setup-qemu-action@v2 + uses: docker/setup-qemu-action@v3 # https://github.com/docker/login-action - name: Login to DockerHub # Only login if not a PR, as PRs only trigger a Docker build and not a push if: ${{ ! matrix.isPr }} - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_ACCESS_TOKEN }} @@ -203,7 +212,7 @@ jobs: # Get Metadata for docker_build_dist step below - name: Sync metadata (tags, labels) from GitHub to Docker for 'dspace-angular-dist' image id: meta_build_dist - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v5 with: images: ${{ env.REGISTRY_IMAGE }} tags: ${{ env.IMAGE_TAGS }} @@ -214,7 +223,7 @@ jobs: - name: Build and push 'dspace-angular-dist' image id: docker_build_dist - uses: docker/build-push-action@v4 + uses: docker/build-push-action@v5 with: context: . file: ./Dockerfile.dist @@ -226,6 +235,7 @@ jobs: tags: ${{ steps.meta_build_dist.outputs.tags }} labels: ${{ steps.meta_build_dist.outputs.labels }} + # Export the digest of Docker build locally (for non PRs only) - name: Export digest if: ${{ ! matrix.isPr }} run: | @@ -233,29 +243,38 @@ jobs: digest="${{ steps.docker_build_dist.outputs.digest }}" touch "/tmp/digests/dist/${digest#sha256:}" + # Upload Digest to an artifact, so that it can be used in manifest below - name: Upload digest if: ${{ ! matrix.isPr }} uses: actions/upload-artifact@v3 with: - name: digests - path: /tmp/digests/dist/* + # NOTE: It's important that this artifact has a unique name so that two + # image builds don't upload digests to the same artifact. + name: digests-dist + path: /tmp/digests/* if-no-files-found: error retention-days: 1 - merge-dist: + # Merge *-dist digests into a manifest. + # This runs after all Docker builds complete above, and it tells hub.docker.com + # that these builds should be all included in the manifest for this tag. + # (e.g. AMD64 and ARM64 should be listed as options under the same tagged Docker image) + dspace-angular-dist_manifest: if: ${{ github.event_name != 'pull_request' }} runs-on: ubuntu-latest needs: - dspace-angular-dist steps: - - name: Download digests + - name: Download digests for -dist builds uses: actions/download-artifact@v3 with: - name: digests + name: digests-dist path: /tmp/digests + - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 - - name: Docker meta + + - name: Add Docker metadata for image id: meta_dist uses: docker/metadata-action@v5 with: @@ -272,8 +291,8 @@ jobs: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_ACCESS_TOKEN }} - - name: Create manifest list and push - working-directory: /tmp/digests/dist + - name: Create manifest list from digests and push + working-directory: /tmp/digests run: | docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) From 63e792990f46620f510013fea569a76abb1ba653 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 17 Nov 2023 11:56:48 -0600 Subject: [PATCH 007/356] Fix directory structure for -dist digests --- .github/workflows/docker.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 23919d573c7..04112f7b70b 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -239,9 +239,9 @@ jobs: - name: Export digest if: ${{ ! matrix.isPr }} run: | - mkdir -p /tmp/digests/dist + mkdir -p /tmp/digests digest="${{ steps.docker_build_dist.outputs.digest }}" - touch "/tmp/digests/dist/${digest#sha256:}" + touch "/tmp/digests/${digest#sha256:}" # Upload Digest to an artifact, so that it can be used in manifest below - name: Upload digest From 5ab87ec6c31d736b8eae0891aad92c20228d301d Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 17 Nov 2023 11:05:46 -0600 Subject: [PATCH 008/356] Update GH actions to latest versions. Fix bug in codecov to ensure it retries on error. --- .github/workflows/build.yml | 16 ++++++++++------ .github/workflows/codescan.yml | 2 +- .github/workflows/port_merged_pull_request.yml | 4 ++-- .github/workflows/pull_request_opened.yml | 2 +- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 219074780e3..e2680420a21 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -43,11 +43,11 @@ jobs: steps: # https://github.com/actions/checkout - name: Checkout codebase - uses: actions/checkout@v3 + uses: actions/checkout@v4 # https://github.com/actions/setup-node - name: Install Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} @@ -118,7 +118,7 @@ jobs: # https://github.com/cypress-io/github-action # (NOTE: to run these e2e tests locally, just use 'ng e2e') - name: Run e2e tests (integration tests) - uses: cypress-io/github-action@v5 + uses: cypress-io/github-action@v6 with: # Run tests in Chrome, headless mode (default) browser: chrome @@ -191,7 +191,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Download artifacts from previous 'tests' job - name: Download coverage artifacts @@ -203,10 +203,14 @@ jobs: # Retry action: https://github.com/marketplace/actions/retry-action # Codecov action: https://github.com/codecov/codecov-action - name: Upload coverage to Codecov.io - uses: Wandalen/wretry.action@v1.0.36 + uses: Wandalen/wretry.action@v1.3.0 with: action: codecov/codecov-action@v3 - # Try upload 5 times max + # Ensure codecov-action throws an error when it fails to upload + # This allows us to auto-restart the action if an error is thrown + with: | + fail_ci_if_error: true + # Try re-running action 5 times max attempt_limit: 5 # Run again in 30 seconds attempt_delay: 30000 diff --git a/.github/workflows/codescan.yml b/.github/workflows/codescan.yml index 8b415296c71..d96e786cc37 100644 --- a/.github/workflows/codescan.yml +++ b/.github/workflows/codescan.yml @@ -35,7 +35,7 @@ jobs: steps: # https://github.com/actions/checkout - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Initializes the CodeQL tools for scanning. # https://github.com/github/codeql-action diff --git a/.github/workflows/port_merged_pull_request.yml b/.github/workflows/port_merged_pull_request.yml index 109835d14d3..857f22755e4 100644 --- a/.github/workflows/port_merged_pull_request.yml +++ b/.github/workflows/port_merged_pull_request.yml @@ -23,11 +23,11 @@ jobs: if: github.event.pull_request.merged steps: # Checkout code - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 # Port PR to other branch (ONLY if labeled with "port to") # See https://github.com/korthout/backport-action - name: Create backport pull requests - uses: korthout/backport-action@v1 + uses: korthout/backport-action@v2 with: # Trigger based on a "port to [branch]" label on PR # (This label must specify the branch name to port to) diff --git a/.github/workflows/pull_request_opened.yml b/.github/workflows/pull_request_opened.yml index 9b61af72d18..f16e81c9fd2 100644 --- a/.github/workflows/pull_request_opened.yml +++ b/.github/workflows/pull_request_opened.yml @@ -21,4 +21,4 @@ jobs: # Assign the PR to whomever created it. This is useful for visualizing assignments on project boards # See https://github.com/toshimaru/auto-author-assign - name: Assign PR to creator - uses: toshimaru/auto-author-assign@v1.6.2 + uses: toshimaru/auto-author-assign@v2.0.1 From 62ccd18345e5be5171bc33479d7aa65e7dfc81f5 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 21 Nov 2023 12:15:42 -0600 Subject: [PATCH 009/356] Trigger redeploy of demo/sandbox from GitHub Actions after DockerHub image updated. --- .github/workflows/docker.yml | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 04112f7b70b..c482d8f29a2 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -23,8 +23,8 @@ env: # For a new commit on other branches, use the branch name as the tag for Docker image. # For a new tag, copy that tag name as the tag for Docker image. IMAGE_TAGS: | - type=raw,value=latest,enable=${{ endsWith(github.ref, github.event.repository.default_branch) }} - type=ref,event=branch,enable=${{ !endsWith(github.ref, github.event.repository.default_branch) }} + type=raw,value=latest,enable=${{ github.ref_name == github.event.repository.default_branch }} + type=ref,event=branch,enable=${{ github.ref_name != github.event.repository.default_branch }} type=ref,event=tag # Define default tag "flavor" for docker/metadata-action per # https://github.com/docker/metadata-action#flavor-input @@ -300,3 +300,25 @@ jobs: - name: Inspect image run: | docker buildx imagetools inspect ${{ env.REGISTRY_IMAGE }}:${{ steps.meta_dist.outputs.version }} + + # Deploy latest -dist image to Demo or Sandbox site, based on the branch updated + dspace-angular-dist_deploy: + if: ${{ github.event_name != 'pull_request' }} + runs-on: ubuntu-latest + needs: + # Requires manifest to be fully updated on DockerHub + - dspace-angular-dist_manifest + steps: + - name: Redeploy sandbox.dspace.org (based on main branch) + if: ${{ github.ref_name == github.event.repository.default_branch }} + run: | + curl -X POST -d '{}' $REDEPLOY_SANDBOX_URL + env: + REDEPLOY_SANDBOX_URL: ${{ secrets.REDEPLOY_SANDBOX_URL }} + + - name: Redeploy demo.dspace.org (based on maintenace branch) + if: ${{ github.ref_name == 'dspace-7_x' }} + run: | + curl -X POST -d '{}' $REDEPLOY_DEMO_URL + env: + REDEPLOY_DEMO_URL: ${{ secrets.REDEPLOY_DEMO_URL }} From bd78acd5594492a4e87e09e3a68d5570368290be Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Wed, 22 Nov 2023 10:51:28 -0600 Subject: [PATCH 010/356] Redeploy demo/sandbox more quickly by only waiting for AMD64 image --- .github/workflows/docker.yml | 40 ++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index c482d8f29a2..a581b63e7b0 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -255,6 +255,24 @@ jobs: if-no-files-found: error retention-days: 1 + # If the 'linux/amd64' -dist image was just updated for the 'main' branch, + # Then redeploy https://sandbox.dspace.org using that updated image. + - name: Redeploy sandbox.dspace.org (based on main branch) + if: ${{ ! matrix.isPr && matrix.arch == 'linux/amd64' && github.ref_name == github.event.repository.default_branch }} + run: | + curl -X POST $REDEPLOY_SANDBOX_URL + env: + REDEPLOY_SANDBOX_URL: ${{ secrets.REDEPLOY_SANDBOX_URL }} + + # If the 'linux/amd64' -dist image was just updated for the maintenance branch, + # Then redeploy https://demo.dspace.org using that updated image. + - name: Redeploy demo.dspace.org (based on maintenace branch) + if: ${{ ! matrix.isPr && matrix.arch == 'linux/amd64' && github.ref_name == 'dspace-7_x' }} + run: | + curl -X POST $REDEPLOY_DEMO_URL + env: + REDEPLOY_DEMO_URL: ${{ secrets.REDEPLOY_DEMO_URL }} + # Merge *-dist digests into a manifest. # This runs after all Docker builds complete above, and it tells hub.docker.com # that these builds should be all included in the manifest for this tag. @@ -300,25 +318,3 @@ jobs: - name: Inspect image run: | docker buildx imagetools inspect ${{ env.REGISTRY_IMAGE }}:${{ steps.meta_dist.outputs.version }} - - # Deploy latest -dist image to Demo or Sandbox site, based on the branch updated - dspace-angular-dist_deploy: - if: ${{ github.event_name != 'pull_request' }} - runs-on: ubuntu-latest - needs: - # Requires manifest to be fully updated on DockerHub - - dspace-angular-dist_manifest - steps: - - name: Redeploy sandbox.dspace.org (based on main branch) - if: ${{ github.ref_name == github.event.repository.default_branch }} - run: | - curl -X POST -d '{}' $REDEPLOY_SANDBOX_URL - env: - REDEPLOY_SANDBOX_URL: ${{ secrets.REDEPLOY_SANDBOX_URL }} - - - name: Redeploy demo.dspace.org (based on maintenace branch) - if: ${{ github.ref_name == 'dspace-7_x' }} - run: | - curl -X POST -d '{}' $REDEPLOY_DEMO_URL - env: - REDEPLOY_DEMO_URL: ${{ secrets.REDEPLOY_DEMO_URL }} From c3f424dae4eaabcbf42d31af5cdd2ffd5f65278c Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Sat, 18 Nov 2023 15:24:56 +0100 Subject: [PATCH 011/356] 108587: Deselect all fields on component destruction (cherry picked from commit ecca8286b5e8c4d90d9d04e9ea71238dd5046f8e) --- .../metadata-schema/metadata-schema.component.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.ts b/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.ts index d0827e6e4d5..4dec129eadd 100644 --- a/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.ts +++ b/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.ts @@ -1,6 +1,6 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, OnInit, OnDestroy } from '@angular/core'; import { RegistryService } from '../../../core/registry/registry.service'; -import { ActivatedRoute, Router } from '@angular/router'; +import { ActivatedRoute } from '@angular/router'; import { BehaviorSubject, combineLatest as observableCombineLatest, @@ -32,7 +32,7 @@ import { PaginationService } from '../../../core/pagination/pagination.service'; * A component used for managing all existing metadata fields within the current metadata schema. * The admin can create, edit or delete metadata fields here. */ -export class MetadataSchemaComponent implements OnInit { +export class MetadataSchemaComponent implements OnInit, OnDestroy { /** * The metadata schema */ @@ -60,7 +60,6 @@ export class MetadataSchemaComponent implements OnInit { constructor(private registryService: RegistryService, private route: ActivatedRoute, private notificationsService: NotificationsService, - private router: Router, private paginationService: PaginationService, private translateService: TranslateService) { @@ -86,7 +85,7 @@ export class MetadataSchemaComponent implements OnInit { */ private updateFields() { this.metadataFields$ = this.paginationService.getCurrentPagination(this.config.id, this.config).pipe( - switchMap((currentPagination) => combineLatest(this.metadataSchema$, this.needsUpdate$, observableOf(currentPagination))), + switchMap((currentPagination) => combineLatest([this.metadataSchema$, this.needsUpdate$, observableOf(currentPagination)])), switchMap(([schema, update, currentPagination]: [MetadataSchema, boolean, PaginationComponentOptions]) => { if (update) { this.needsUpdate$.next(false); @@ -193,10 +192,10 @@ export class MetadataSchemaComponent implements OnInit { showNotification(success: boolean, amount: number) { const prefix = 'admin.registries.schema.notification'; const suffix = success ? 'success' : 'failure'; - const messages = observableCombineLatest( + const messages = observableCombineLatest([ this.translateService.get(success ? `${prefix}.${suffix}` : `${prefix}.${suffix}`), this.translateService.get(`${prefix}.field.deleted.${suffix}`, { amount: amount }) - ); + ]); messages.subscribe(([head, content]) => { if (success) { this.notificationsService.success(head, content); @@ -207,6 +206,7 @@ export class MetadataSchemaComponent implements OnInit { } ngOnDestroy(): void { this.paginationService.clearPagination(this.config.id); + this.registryService.deselectAllMetadataField(); } } From b83a8421a3552efdb2eb92c1a4435f8472d75fe4 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 28 Nov 2023 16:04:50 -0600 Subject: [PATCH 012/356] Refactor to simply use the reusable-docker-build.yml from DSpace/DSpace. --- .github/workflows/docker.yml | 305 ++++------------------------------- 1 file changed, 30 insertions(+), 275 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index a581b63e7b0..ece1f880c0a 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -3,6 +3,9 @@ name: Docker images # Run this Build for all pushes to 'main' or maintenance branches, or tagged releases. # Also run for PRs to ensure PR doesn't break Docker build process +# NOTE: uses "reusable-docker-build.yml" in DSpace/DSpace to actually build each of the Docker images +# https://github.com/DSpace/DSpace/blob/main/.github/workflows/reusable-docker-build.yml +# on: push: branches: @@ -17,7 +20,6 @@ permissions: env: - REGISTRY_IMAGE: dspace/dspace-angular # Define tags to use for Docker images based on Git tags/branches (for docker/metadata-action) # For a new commit on default branch (main), use the literal tag 'latest' on Docker image. # For a new commit on other branches, use the branch name as the tag for Docker image. @@ -34,287 +36,40 @@ env: jobs: ############################################################# - # Build/Push the '${{ env.REGISTRY_IMAGE }}' image + # Build/Push the 'dspace/dspace-angular' image ############################################################# dspace-angular: # Ensure this job never runs on forked repos. It's only executed for 'dspace/dspace-angular' if: github.repository == 'dspace/dspace-angular' - - strategy: - matrix: - # Architectures / Platforms for which we will build Docker images - arch: ['linux/amd64', 'linux/arm64'] - os: [ubuntu-latest] - isPr: - - ${{ github.event_name == 'pull_request' }} - # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. - # The below exclude therefore ensures we do NOT build ARM64 for PRs. - exclude: - - isPr: true - os: ubuntu-latest - arch: linux/arm64 - - runs-on: ${{ matrix.os }} - steps: - # https://github.com/actions/checkout - - name: Checkout codebase - uses: actions/checkout@v4 - - # https://github.com/docker/setup-buildx-action - - name: Setup Docker Buildx - uses: docker/setup-buildx-action@v3 - - # https://github.com/docker/setup-qemu-action - - name: Set up QEMU emulation to build for multiple architectures - uses: docker/setup-qemu-action@v3 - - # https://github.com/docker/login-action - - name: Login to DockerHub - # Only login if not a PR, as PRs only trigger a Docker build and not a push - if: ${{ ! matrix.isPr }} - uses: docker/login-action@v3 - with: - username: ${{ secrets.DOCKER_USERNAME }} - password: ${{ secrets.DOCKER_ACCESS_TOKEN }} - - # https://github.com/docker/metadata-action - # Get Metadata for docker_build step below - - name: Sync metadata (tags, labels) from GitHub to Docker for 'dspace-angular' image - id: meta_build - uses: docker/metadata-action@v5 - with: - images: ${{ env.REGISTRY_IMAGE }} - tags: ${{ env.IMAGE_TAGS }} - flavor: ${{ env.TAGS_FLAVOR }} - - # https://github.com/docker/build-push-action - - name: Build and push 'dspace-angular' image - id: docker_build - uses: docker/build-push-action@v5 - with: - context: . - file: ./Dockerfile - platforms: ${{ matrix.arch }} - # For pull requests, we run the Docker build (to ensure no PR changes break the build), - # but we ONLY do an image push to DockerHub if it's NOT a PR - push: ${{ ! matrix.isPr }} - # Use tags / labels provided by 'docker/metadata-action' above - tags: ${{ steps.meta_build.outputs.tags }} - labels: ${{ steps.meta_build.outputs.labels }} - - # Export the digest of Docker build locally (for non PRs only) - - name: Export digest - if: ${{ ! matrix.isPr }} - run: | - mkdir -p /tmp/digests - digest="${{ steps.docker_build.outputs.digest }}" - touch "/tmp/digests/${digest#sha256:}" - - # Upload digest to an artifact, so that it can be used in manifest below - - name: Upload digest - if: ${{ ! matrix.isPr }} - uses: actions/upload-artifact@v3 - with: - name: digests - path: /tmp/digests/* - if-no-files-found: error - retention-days: 1 - - # Merge digests into a manifest. - # This runs after all Docker builds complete above, and it tells hub.docker.com - # that these builds should be all included in the manifest for this tag. - # (e.g. AMD64 and ARM64 should be listed as options under the same tagged Docker image) - # Borrowed from https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners - dspace-angular_manifest: - if: ${{ github.event_name != 'pull_request' }} - runs-on: ubuntu-latest - needs: - - dspace-angular - steps: - - name: Download digests - uses: actions/download-artifact@v3 - with: - name: digests - path: /tmp/digests - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 - - - name: Add Docker metadata for image - id: meta - uses: docker/metadata-action@v5 - with: - images: ${{ env.REGISTRY_IMAGE }} - tags: ${{ env.IMAGE_TAGS }} - flavor: ${{ env.TAGS_FLAVOR }} - - - name: Login to Docker Hub - uses: docker/login-action@v3 - with: - username: ${{ secrets.DOCKER_USERNAME }} - password: ${{ secrets.DOCKER_ACCESS_TOKEN }} - - - name: Create manifest list from digests and push - working-directory: /tmp/digests - run: | - docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) - - - name: Inspect image - run: | - docker buildx imagetools inspect ${{ env.REGISTRY_IMAGE }}:${{ steps.meta.outputs.version }} + # Use the reusable-docker-build.yml script from DSpace/DSpace repo to build our Docker image + uses: DSpace/DSpace/.github/workflows/reusable-docker-build.yml@main + with: + build_id: dspace-angular + image_name: dspace/dspace-angular + dockerfile_path: ./Dockerfile + secrets: + DOCKER_USERNAME: ${{ secrets.DOCKER_USERNAME }} + DOCKER_ACCESS_TOKEN: ${{ secrets.DOCKER_ACCESS_TOKEN }} ############################################################# - # Build/Push the '${{ env.REGISTRY_IMAGE }}' image ('-dist' tag) + # Build/Push the 'dspace/dspace-angular' image ('-dist' tag) ############################################################# dspace-angular-dist: # Ensure this job never runs on forked repos. It's only executed for 'dspace/dspace-angular' if: github.repository == 'dspace/dspace-angular' - - strategy: - matrix: - # Architectures / Platforms for which we will build Docker images - arch: ['linux/amd64', 'linux/arm64'] - os: [ubuntu-latest] - isPr: - - ${{ github.event_name == 'pull_request' }} - # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. - # The below exclude therefore ensures we do NOT build ARM64 for PRs. - exclude: - - isPr: true - os: ubuntu-latest - arch: linux/arm64 - - runs-on: ${{ matrix.os }} - steps: - # https://github.com/actions/checkout - - name: Checkout codebase - uses: actions/checkout@v4 - - # https://github.com/docker/setup-buildx-action - - name: Setup Docker Buildx - uses: docker/setup-buildx-action@v3 - - # https://github.com/docker/setup-qemu-action - - name: Set up QEMU emulation to build for multiple architectures - uses: docker/setup-qemu-action@v3 - - # https://github.com/docker/login-action - - name: Login to DockerHub - # Only login if not a PR, as PRs only trigger a Docker build and not a push - if: ${{ ! matrix.isPr }} - uses: docker/login-action@v3 - with: - username: ${{ secrets.DOCKER_USERNAME }} - password: ${{ secrets.DOCKER_ACCESS_TOKEN }} - - # https://github.com/docker/metadata-action - # Get Metadata for docker_build_dist step below - - name: Sync metadata (tags, labels) from GitHub to Docker for 'dspace-angular-dist' image - id: meta_build_dist - uses: docker/metadata-action@v5 - with: - images: ${{ env.REGISTRY_IMAGE }} - tags: ${{ env.IMAGE_TAGS }} - # As this is a "dist" image, its tags are all suffixed with "-dist". Otherwise, it uses the same - # tagging logic as the primary '${{ env.REGISTRY_IMAGE }}' image above. - flavor: ${{ env.TAGS_FLAVOR }} - suffix=-dist - - - name: Build and push 'dspace-angular-dist' image - id: docker_build_dist - uses: docker/build-push-action@v5 - with: - context: . - file: ./Dockerfile.dist - platforms: ${{ matrix.arch }} - # For pull requests, we run the Docker build (to ensure no PR changes break the build), - # but we ONLY do an image push to DockerHub if it's NOT a PR - push: ${{ ! matrix.isPr }} - # Use tags / labels provided by 'docker/metadata-action' above - tags: ${{ steps.meta_build_dist.outputs.tags }} - labels: ${{ steps.meta_build_dist.outputs.labels }} - - # Export the digest of Docker build locally (for non PRs only) - - name: Export digest - if: ${{ ! matrix.isPr }} - run: | - mkdir -p /tmp/digests - digest="${{ steps.docker_build_dist.outputs.digest }}" - touch "/tmp/digests/${digest#sha256:}" - - # Upload Digest to an artifact, so that it can be used in manifest below - - name: Upload digest - if: ${{ ! matrix.isPr }} - uses: actions/upload-artifact@v3 - with: - # NOTE: It's important that this artifact has a unique name so that two - # image builds don't upload digests to the same artifact. - name: digests-dist - path: /tmp/digests/* - if-no-files-found: error - retention-days: 1 - - # If the 'linux/amd64' -dist image was just updated for the 'main' branch, - # Then redeploy https://sandbox.dspace.org using that updated image. - - name: Redeploy sandbox.dspace.org (based on main branch) - if: ${{ ! matrix.isPr && matrix.arch == 'linux/amd64' && github.ref_name == github.event.repository.default_branch }} - run: | - curl -X POST $REDEPLOY_SANDBOX_URL - env: - REDEPLOY_SANDBOX_URL: ${{ secrets.REDEPLOY_SANDBOX_URL }} - - # If the 'linux/amd64' -dist image was just updated for the maintenance branch, - # Then redeploy https://demo.dspace.org using that updated image. - - name: Redeploy demo.dspace.org (based on maintenace branch) - if: ${{ ! matrix.isPr && matrix.arch == 'linux/amd64' && github.ref_name == 'dspace-7_x' }} - run: | - curl -X POST $REDEPLOY_DEMO_URL - env: - REDEPLOY_DEMO_URL: ${{ secrets.REDEPLOY_DEMO_URL }} - - # Merge *-dist digests into a manifest. - # This runs after all Docker builds complete above, and it tells hub.docker.com - # that these builds should be all included in the manifest for this tag. - # (e.g. AMD64 and ARM64 should be listed as options under the same tagged Docker image) - dspace-angular-dist_manifest: - if: ${{ github.event_name != 'pull_request' }} - runs-on: ubuntu-latest - needs: - - dspace-angular-dist - steps: - - name: Download digests for -dist builds - uses: actions/download-artifact@v3 - with: - name: digests-dist - path: /tmp/digests - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 - - - name: Add Docker metadata for image - id: meta_dist - uses: docker/metadata-action@v5 - with: - images: ${{ env.REGISTRY_IMAGE }} - tags: ${{ env.IMAGE_TAGS }} - # As this is a "dist" image, its tags are all suffixed with "-dist". Otherwise, it uses the same - # tagging logic as the primary '${{ env.REGISTRY_IMAGE }}' image above. - flavor: ${{ env.TAGS_FLAVOR }} - suffix=-dist - - - name: Login to Docker Hub - uses: docker/login-action@v3 - with: - username: ${{ secrets.DOCKER_USERNAME }} - password: ${{ secrets.DOCKER_ACCESS_TOKEN }} - - - name: Create manifest list from digests and push - working-directory: /tmp/digests - run: | - docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) - - - name: Inspect image - run: | - docker buildx imagetools inspect ${{ env.REGISTRY_IMAGE }}:${{ steps.meta_dist.outputs.version }} + # Use the reusable-docker-build.yml script from DSpace/DSpace repo to build our Docker image + uses: DSpace/DSpace/.github/workflows/reusable-docker-build.yml@main + with: + build_id: dspace-angular-dist + image_name: dspace/dspace-angular + dockerfile_path: ./Dockerfile.dist + # As this is a "dist" image, its tags are all suffixed with "-dist". Otherwise, it uses the same + # tagging logic as the primary 'dspace/dspace-angular' image above. + tags_flavor: suffix=-dist + secrets: + DOCKER_USERNAME: ${{ secrets.DOCKER_USERNAME }} + DOCKER_ACCESS_TOKEN: ${{ secrets.DOCKER_ACCESS_TOKEN }} + # Enable redeploy of sandbox & demo if the branch for this image matches the deployment branch of + # these sites as specified in reusable-docker-build.xml + REDEPLOY_SANDBOX_URL: ${{ secrets.REDEPLOY_SANDBOX_URL }} + REDEPLOY_DEMO_URL: ${{ secrets.REDEPLOY_DEMO_URL }} \ No newline at end of file From 65fff9361cc42e9a8d453f256cc2ad5ef04ce69d Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 28 Nov 2023 16:13:56 -0600 Subject: [PATCH 013/356] Use the script from the dspace-7_x branch --- .github/workflows/docker.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index ece1f880c0a..5223ab100bc 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -4,7 +4,7 @@ name: Docker images # Run this Build for all pushes to 'main' or maintenance branches, or tagged releases. # Also run for PRs to ensure PR doesn't break Docker build process # NOTE: uses "reusable-docker-build.yml" in DSpace/DSpace to actually build each of the Docker images -# https://github.com/DSpace/DSpace/blob/main/.github/workflows/reusable-docker-build.yml +# https://github.com/DSpace/DSpace/blob/dspace-7_x/.github/workflows/reusable-docker-build.yml # on: push: @@ -42,7 +42,7 @@ jobs: # Ensure this job never runs on forked repos. It's only executed for 'dspace/dspace-angular' if: github.repository == 'dspace/dspace-angular' # Use the reusable-docker-build.yml script from DSpace/DSpace repo to build our Docker image - uses: DSpace/DSpace/.github/workflows/reusable-docker-build.yml@main + uses: DSpace/DSpace/.github/workflows/reusable-docker-build.yml@dspace-7_x with: build_id: dspace-angular image_name: dspace/dspace-angular @@ -58,7 +58,7 @@ jobs: # Ensure this job never runs on forked repos. It's only executed for 'dspace/dspace-angular' if: github.repository == 'dspace/dspace-angular' # Use the reusable-docker-build.yml script from DSpace/DSpace repo to build our Docker image - uses: DSpace/DSpace/.github/workflows/reusable-docker-build.yml@main + uses: DSpace/DSpace/.github/workflows/reusable-docker-build.yml@dspace-7_x with: build_id: dspace-angular-dist image_name: dspace/dspace-angular From 92d25dd2a8a2003d53f466761217b8f84ee642b7 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 28 Nov 2023 16:50:55 -0600 Subject: [PATCH 014/356] Remove unused env variables --- .github/workflows/docker.yml | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 5223ab100bc..3a3183f8cd6 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -18,22 +18,6 @@ on: permissions: contents: read # to fetch code (actions/checkout) - -env: - # Define tags to use for Docker images based on Git tags/branches (for docker/metadata-action) - # For a new commit on default branch (main), use the literal tag 'latest' on Docker image. - # For a new commit on other branches, use the branch name as the tag for Docker image. - # For a new tag, copy that tag name as the tag for Docker image. - IMAGE_TAGS: | - type=raw,value=latest,enable=${{ github.ref_name == github.event.repository.default_branch }} - type=ref,event=branch,enable=${{ github.ref_name != github.event.repository.default_branch }} - type=ref,event=tag - # Define default tag "flavor" for docker/metadata-action per - # https://github.com/docker/metadata-action#flavor-input - # We manage the 'latest' tag ourselves to the 'main' branch (see settings above) - TAGS_FLAVOR: | - latest=false - jobs: ############################################################# # Build/Push the 'dspace/dspace-angular' image From 38752d9d71b2441b83bbcb616347df66b7fb5e75 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 29 Nov 2023 14:25:36 +0100 Subject: [PATCH 015/356] ensure HALEndpointService doesn't use stale requests --- 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 044609ef427..f65a7deca7c 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 65ca2b0c498..79c34c36590 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 8b6316a6ce2..98ab6a16ea0 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 9c69a77d435a8b19de86ceb4f0a66a2a8e036c99 Mon Sep 17 00:00:00 2001 From: Michael Spalti Date: Wed, 29 Nov 2023 15:42:30 -0800 Subject: [PATCH 016/356] Fix for thumbnail images in items. Revert changes in html template Revert changes in html template Revert changes in html template (cherry picked from commit 88c39e8b26d4418c370185467e2fee006c3cd811) --- src/app/item-page/media-viewer/media-viewer.component.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/item-page/media-viewer/media-viewer.component.ts b/src/app/item-page/media-viewer/media-viewer.component.ts index 242e50646e3..5a17c4f01b4 100644 --- a/src/app/item-page/media-viewer/media-viewer.component.ts +++ b/src/app/item-page/media-viewer/media-viewer.component.ts @@ -1,4 +1,4 @@ -import { Component, Input, OnDestroy, OnInit } from '@angular/core'; +import { ChangeDetectorRef, Component, Input, OnDestroy, OnInit } from '@angular/core'; import { BehaviorSubject, Observable } from 'rxjs'; import { filter, take } from 'rxjs/operators'; import { BitstreamDataService } from '../../core/data/bitstream-data.service'; @@ -42,6 +42,7 @@ export class MediaViewerComponent implements OnDestroy, OnInit { constructor( protected bitstreamDataService: BitstreamDataService, + protected changeDetectorRef: ChangeDetectorRef ) { } @@ -85,6 +86,7 @@ export class MediaViewerComponent implements OnDestroy, OnInit { })); } this.isLoading = false; + this.changeDetectorRef.detectChanges(); })); } })); From 3b568f7d32fc10fd003caf07a7af588817b24155 Mon Sep 17 00:00:00 2001 From: Michael Spalti Date: Mon, 4 Dec 2023 11:37:14 -0800 Subject: [PATCH 017/356] Updated bitstream read description in en.json5 (cherry picked from commit 2592f87356d1caa4f7d396f8e7f9188393e5efb7) --- src/assets/i18n/en.json5 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 6dee5d54e66..c39106ff1bd 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1258,7 +1258,7 @@ "comcol-role.edit.bitstream_read.name": "Default bitstream read access", - "comcol-role.edit.bitstream_read.description": "Community administrators can create sub-communities or collections, and manage or assign management for those sub-communities or collections. In addition, they decide who can submit items to any sub-collections, edit item metadata (after submission), and add (map) existing items from other collections (subject to authorization).", + "comcol-role.edit.bitstream_read.description": "E-People and Groups that can read new bitstreams submitted to this collection. Changes to this role are not retroactive. Existing bitstreams in the system will still be viewable by those who had read access at the time of their addition.", "comcol-role.edit.bitstream_read.anonymous-group": "Default read for incoming bitstreams is currently set to Anonymous.", From 9f95eb452ccc6a30047d8acd2441d33e73786981 Mon Sep 17 00:00:00 2001 From: "max.nuding" Date: Wed, 6 Dec 2023 09:57:43 +0100 Subject: [PATCH 018/356] enable type-bind for checkbox inputs during submission --- .../models/list/dynamic-list-radio-group.model.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/list/dynamic-list-radio-group.model.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/list/dynamic-list-radio-group.model.ts index 0a32498173e..2bcf8b5f139 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/list/dynamic-list-radio-group.model.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/list/dynamic-list-radio-group.model.ts @@ -1,5 +1,6 @@ import { DynamicFormControlLayout, + DynamicFormControlRelation, DynamicRadioGroupModel, DynamicRadioGroupModelConfig, serializable @@ -15,12 +16,14 @@ export interface DynamicListModelConfig extends DynamicRadioGroupModelConfig { @serializable() vocabularyOptions: VocabularyOptions; @serializable() repeatable: boolean; + @serializable() typeBindRelations: DynamicFormControlRelation[]; @serializable() groupLength: number; @serializable() required: boolean; @serializable() hint: string; @@ -35,6 +38,7 @@ export class DynamicListRadioGroupModel extends DynamicRadioGroupModel { this.required = config.required; this.hint = config.hint; this.value = config.value; + this.typeBindRelations = config.typeBindRelations ? config.typeBindRelations : []; } get hasAuthority(): boolean { From c8ac260b783d6bd9426325f7e2893e4b4f8d6f0d Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Fri, 1 Dec 2023 16:19:12 +0100 Subject: [PATCH 019/356] also skip loading hal requests --- 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 98ab6a16ea0..5cdd7ccfad7 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 790e7171992f2cefc6999306703e52586204986b Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 6 Dec 2023 11:14:41 +0100 Subject: [PATCH 020/356] add ResponsePendingStale state --- .../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 | 222 +++++++++++++++--- src/app/core/data/request.reducer.ts | 55 +++-- src/app/core/data/request.service.ts | 2 +- .../core/shared/hal-endpoint.service.spec.ts | 180 +++++++++++--- src/app/core/shared/hal-endpoint.service.ts | 25 +- 9 files changed, 658 insertions(+), 139 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 098f075c101..75662a691fa 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 edd6d9e2a42..c7cd5b0a705 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 00000000000..7daa655566a --- /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 a813b6e7436..3aeace39d29 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 05f074a96a7..86b9c4cd5dc 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 an entry with state: Error`, () => { - it(`should set the state to ErrorStale, in response to a STALE action`, () => { - const state = testErrorState; + 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 action = new RequestStaleAction(id1); - const newState = requestReducer(state, action); + const ssStartState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.SuccessStale + }) + }); - expect(newState[id1].state).toEqual(RequestEntryState.ErrorStale); - expect(newState[id1].lastUpdated).toBe(action.lastUpdated); + 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); + }); + }); + }); }); diff --git a/src/app/core/data/request.reducer.ts b/src/app/core/data/request.reducer.ts index 9bf17faf8d7..9cf4fee0e2c 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 27176390d7e..c908c696db3 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 56e890b3189..b81d0806dfd 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); - - const expected = cold('(b|)', { b: endpointMap.test.href }); - expect(result).toBeObservable(expected); + 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 = '(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('----')); - - const result = service.isEnabledOnRestApi(linkPath); - const expected = cold('b---', { b: undefined }); - expect(result).toBeObservable(expected); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getRootEndpointMap').and + .returnValue(cold('----')); + + 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 = '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); + }); + }); - const result = service.isEnabledOnRestApi('unknown'); - const expected = cold('b-c-', { b: undefined, c: false }); - expect(result).toBeObservable(expected); + 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 5cdd7ccfad7..07754616c73 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 d32303bf47413cc2a98faf25ab42410a73657938 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Sat, 25 Nov 2023 14:21:57 +0100 Subject: [PATCH 021/356] Added missing aria labels to edit bitstream tab (cherry picked from commit e229be44d6450f9c50ed2d53f4b5d9cb7939fcd1) --- .../item-bitstreams/item-bitstreams.component.html | 7 +++++++ .../item-edit-bitstream-bundle.component.html | 1 + 2 files changed, 8 insertions(+) diff --git a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html index 4cb9577fcb5..bd4c9334ad3 100644 --- a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html +++ b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html @@ -1,21 +1,25 @@
diff --git a/src/app/admin/admin-sidebar/admin-sidebar.component.spec.ts b/src/app/admin/admin-sidebar/admin-sidebar.component.spec.ts index 88efd2a711e..9522be29ce3 100644 --- a/src/app/admin/admin-sidebar/admin-sidebar.component.spec.ts +++ b/src/app/admin/admin-sidebar/admin-sidebar.component.spec.ts @@ -143,7 +143,7 @@ describe('AdminSidebarComponent', () => { describe('when the collapse link is clicked', () => { beforeEach(() => { spyOn(menuService, 'toggleMenu'); - const sidebarToggler = fixture.debugElement.query(By.css('#sidebar-collapse-toggle > a')); + const sidebarToggler = fixture.debugElement.query(By.css('#sidebar-collapse-toggle > button')); sidebarToggler.triggerEventHandler('click', { preventDefault: () => {/**/ } diff --git a/src/app/footer/footer.component.html b/src/app/footer/footer.component.html index 13d84e6e2e1..d4c0cd1a377 100644 --- a/src/app/footer/footer.component.html +++ b/src/app/footer/footer.component.html @@ -62,10 +62,11 @@
Footer Content
{{ 'footer.link.lyrasis' | translate}}

-