From 3fa86a389f8e0b7d5cd77729061c610c119fdf8c Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Sat, 2 Nov 2024 23:17:48 +0100 Subject: [PATCH 1/6] Use signals in attachment-unit-form and fix code smells --- .../lecture/lecture-attachments.component.ts | 31 +++---- .../attachment-unit-form.component.html | 6 +- .../attachment-unit-form.component.ts | 92 +++++++------------ .../create-exercise-unit.component.ts | 30 ++---- .../constants/file-extensions.constants.ts | 4 + 5 files changed, 64 insertions(+), 99 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.ts b/src/main/webapp/app/lecture/lecture-attachments.component.ts index 5b12fa24b510..cc62c5ed9726 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.ts +++ b/src/main/webapp/app/lecture/lecture-attachments.component.ts @@ -1,15 +1,14 @@ import { Component, ElementRef, Input, OnDestroy, OnInit, ViewChild } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; -import { HttpClient, HttpErrorResponse, HttpResponse } from '@angular/common/http'; +import { HttpErrorResponse, HttpResponse } from '@angular/common/http'; import { Lecture } from 'app/entities/lecture.model'; -import { FileUploaderService } from 'app/shared/http/file-uploader.service'; import dayjs from 'dayjs/esm'; import { Subject } from 'rxjs'; import { FileService } from 'app/shared/http/file.service'; import { Attachment, AttachmentType } from 'app/entities/attachment.model'; import { AttachmentService } from 'app/lecture/attachment.service'; import { faEye, faPaperclip, faPencilAlt, faQuestionCircle, faSpinner, faTimes, faTrash } from '@fortawesome/free-solid-svg-icons'; -import { UPLOAD_FILE_EXTENSIONS } from 'app/shared/constants/file-extensions.constants'; +import { ACCEPTED_FILE_EXTENSIONS_FILE_BROWSER, ALLOWED_FILE_EXTENSIONS_HUMAN_READABLE } from 'app/shared/constants/file-extensions.constants'; import { LectureService } from 'app/lecture/lecture.service'; @Component({ @@ -18,6 +17,16 @@ import { LectureService } from 'app/lecture/lecture.service'; styleUrls: ['./lecture-attachments.component.scss'], }) export class LectureAttachmentsComponent implements OnInit, OnDestroy { + protected readonly faSpinner = faSpinner; + protected readonly faTimes = faTimes; + protected readonly faTrash = faTrash; + protected readonly faPencilAlt = faPencilAlt; + protected readonly faPaperclip = faPaperclip; + protected readonly faQuestionCircle = faQuestionCircle; + protected readonly faEye = faEye; + protected readonly allowedFileExtensions = ALLOWED_FILE_EXTENSIONS_HUMAN_READABLE; + protected readonly acceptedFileExtensionsFileBrowser = ACCEPTED_FILE_EXTENSIONS_FILE_BROWSER; + @ViewChild('fileInput', { static: false }) fileInput: ElementRef; @Input() lectureId: number | undefined; @Input() showHeader = true; @@ -33,29 +42,13 @@ export class LectureAttachmentsComponent implements OnInit, OnDestroy { errorMessage?: string; viewButtonAvailable: Record = {}; - // A human-readable list of allowed file extensions - readonly allowedFileExtensions = UPLOAD_FILE_EXTENSIONS.join(', '); - // The list of file extensions for the "accept" attribute of the file input field - readonly acceptedFileExtensionsFileBrowser = UPLOAD_FILE_EXTENSIONS.map((ext) => '.' + ext).join(','); - private dialogErrorSource = new Subject(); dialogError$ = this.dialogErrorSource.asObservable(); - // Icons - faSpinner = faSpinner; - faTimes = faTimes; - faTrash = faTrash; - faPencilAlt = faPencilAlt; - faPaperclip = faPaperclip; - faQuestionCircle = faQuestionCircle; - faEye = faEye; - constructor( protected activatedRoute: ActivatedRoute, private attachmentService: AttachmentService, private lectureService: LectureService, - private httpClient: HttpClient, - private fileUploaderService: FileUploaderService, private fileService: FileService, ) {} diff --git a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.html b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.html index c7d9568fb7c2..4de9ac96dcfa 100644 --- a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.html +++ b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.html @@ -60,13 +60,13 @@ /> - @if (isFileTooBig) { + @if (isFileTooBig()) {
{{ 'artemisApp.attachmentUnit.createAttachmentUnit.fileTooBig' | artemisTranslate }} {{ 'artemisApp.attachmentUnit.createAttachmentUnit.fileLimitation' | artemisTranslate }}
} - @if (!fileName && fileInputTouched) { + @if (!fileName() && fileInputTouched) {
} @@ -105,7 +105,7 @@
- @if (hasCancelButton) { diff --git a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts index 766379c5847b..e48e513cdb2f 100644 --- a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts +++ b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts @@ -1,11 +1,11 @@ -import { Component, ElementRef, EventEmitter, Input, OnChanges, OnInit, Output, ViewChild } from '@angular/core'; +import { Component, ElementRef, EventEmitter, Input, OnChanges, Output, ViewChild, computed, inject, signal } from '@angular/core'; import dayjs from 'dayjs/esm'; import { FormBuilder, FormGroup, Validators } from '@angular/forms'; -import { TranslateService } from '@ngx-translate/core'; import { faQuestionCircle, faTimes } from '@fortawesome/free-solid-svg-icons'; -import { UPLOAD_FILE_EXTENSIONS } from 'app/shared/constants/file-extensions.constants'; -import { CompetencyLectureUnitLink } from 'app/entities/competency.model'; +import { ACCEPTED_FILE_EXTENSIONS_FILE_BROWSER, ALLOWED_FILE_EXTENSIONS_HUMAN_READABLE } from 'app/shared/constants/file-extensions.constants'; +import { Competency, CompetencyLectureUnitLink } from 'app/entities/competency.model'; import { MAX_FILE_SIZE } from 'app/shared/constants/input.constants'; +import { toSignal } from '@angular/core/rxjs-interop'; export interface AttachmentUnitFormData { formProperties: FormProperties; @@ -32,75 +32,57 @@ export interface FileProperties { selector: 'jhi-attachment-unit-form', templateUrl: './attachment-unit-form.component.html', }) -export class AttachmentUnitFormComponent implements OnInit, OnChanges { - @Input() - formData: AttachmentUnitFormData; - @Input() - isEditMode = false; +export class AttachmentUnitFormComponent implements OnChanges { + protected readonly faQuestionCircle = faQuestionCircle; + protected readonly faTimes = faTimes; + protected readonly allowedFileExtensions = ALLOWED_FILE_EXTENSIONS_HUMAN_READABLE; + protected readonly acceptedFileExtensionsFileBrowser = ACCEPTED_FILE_EXTENSIONS_FILE_BROWSER; - // A human-readable list of allowed file extensions - readonly allowedFileExtensions = UPLOAD_FILE_EXTENSIONS.join(', '); - // The list of file extensions for the "accept" attribute of the file input field - readonly acceptedFileExtensionsFileBrowser = UPLOAD_FILE_EXTENSIONS.map((ext) => '.' + ext).join(','); + @Input() formData: AttachmentUnitFormData; + @Input() isEditMode = false; - faQuestionCircle = faQuestionCircle; + @Output() formSubmitted: EventEmitter = new EventEmitter(); - @Output() - formSubmitted: EventEmitter = new EventEmitter(); - form: FormGroup; - - @Input() - hasCancelButton: boolean; - @Output() - onCancel: EventEmitter = new EventEmitter(); - - faTimes = faTimes; + @Input() hasCancelButton: boolean; + @Output() onCancel: EventEmitter = new EventEmitter(); // have to handle the file input as a special case at is not part of the reactive form @ViewChild('fileInput', { static: false }) fileInput: ElementRef; file: File; - fileName?: string; fileInputTouched = false; - isFileTooBig: boolean; - constructor( - private translateService: TranslateService, - private fb: FormBuilder, - ) {} + fileName = signal(undefined); + isFileTooBig = signal(false); + + private readonly formBuilder = inject(FormBuilder); + form: FormGroup = this.formBuilder.group({ + name: [undefined as string | undefined, [Validators.required, Validators.maxLength(255)]], + description: [undefined as string | undefined, [Validators.maxLength(1000)]], + releaseDate: [undefined as dayjs.Dayjs | undefined], + version: [{ value: 1, disabled: true }], + updateNotificationText: [undefined as string | undefined, [Validators.maxLength(1000)]], + competencies: [undefined as Competency[] | undefined], + }); + private readonly statusChanges = toSignal(this.form.statusChanges ?? 'INVALID'); + + isFormValid = computed(() => { + return (this.statusChanges() === 'VALID' || this.fileName()) && !this.isFileTooBig(); + }); ngOnChanges(): void { - this.initializeForm(); if (this.isEditMode && this.formData) { this.setFormValues(this.formData); } } - ngOnInit(): void { - this.initializeForm(); - } - - private initializeForm() { - if (this.form) { - return; - } - this.form = this.fb.group({ - name: [undefined as string | undefined, [Validators.required, Validators.maxLength(255)]], - description: [undefined as string | undefined, [Validators.maxLength(1000)]], - releaseDate: [undefined as dayjs.Dayjs | undefined], - version: [{ value: 1, disabled: true }], - updateNotificationText: [undefined as string | undefined, [Validators.maxLength(1000)]], - competencyLinks: [undefined as CompetencyLectureUnitLink[] | undefined], - }); - } - onFileChange(event: Event): void { const input = event.target as HTMLInputElement; if (!input.files?.length) { return; } this.file = input.files[0]; - this.fileName = this.file.name; + this.fileName.set(this.file.name); // automatically set the name in case it is not yet specified if (this.form && (this.nameControl?.value == undefined || this.nameControl?.value == '')) { this.form.patchValue({ @@ -108,7 +90,7 @@ export class AttachmentUnitFormComponent implements OnInit, OnChanges { name: this.file.name.replace(/\.[^/.]+$/, ''), }); } - this.isFileTooBig = this.file.size > MAX_FILE_SIZE; + this.isFileTooBig.set(this.file.size > MAX_FILE_SIZE); } get nameControl() { @@ -131,16 +113,12 @@ export class AttachmentUnitFormComponent implements OnInit, OnChanges { return this.form.get('version'); } - get isSubmitPossible() { - return !(this.form.invalid || !this.fileName) && !this.isFileTooBig; - } - submitForm() { const formValue = this.form.value; const formProperties: FormProperties = { ...formValue }; const fileProperties: FileProperties = { file: this.file, - fileName: this.fileName, + fileName: this.fileName(), }; this.formSubmitted.emit({ @@ -157,7 +135,7 @@ export class AttachmentUnitFormComponent implements OnInit, OnChanges { this.file = formData?.fileProperties?.file; } if (formData?.fileProperties?.fileName) { - this.fileName = formData?.fileProperties?.fileName; + this.fileName.set(formData?.fileProperties?.fileName); } } diff --git a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/create-exercise-unit/create-exercise-unit.component.ts b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/create-exercise-unit/create-exercise-unit.component.ts index 2c3ee4dee97d..78fdcf085a83 100644 --- a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/create-exercise-unit/create-exercise-unit.component.ts +++ b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/create-exercise-unit/create-exercise-unit.component.ts @@ -18,25 +18,18 @@ import { faSort, faTimes } from '@fortawesome/free-solid-svg-icons'; styleUrls: ['./create-exercise-unit.component.scss'], }) export class CreateExerciseUnitComponent implements OnInit { - @Input() - hasCancelButton: boolean; - @Input() - hasCreateExerciseButton: boolean; - @Input() - shouldNavigateOnSubmit = true; - @Input() - lectureId: number | undefined; - @Input() - courseId: number | undefined; - @Input() - currentWizardStep: number; + protected readonly faTimes = faTimes; + protected readonly faSort = faSort; - @Output() - onCancel: EventEmitter = new EventEmitter(); - @Output() - onExerciseUnitCreated: EventEmitter = new EventEmitter(); + @Input() hasCancelButton: boolean; + @Input() hasCreateExerciseButton: boolean; + @Input() shouldNavigateOnSubmit = true; + @Input() lectureId: number | undefined; + @Input() courseId: number | undefined; + @Input() currentWizardStep: number; - faTimes = faTimes; + @Output() onCancel: EventEmitter = new EventEmitter(); + @Output() onExerciseUnitCreated: EventEmitter = new EventEmitter(); predicate = 'type'; reverse = false; @@ -45,9 +38,6 @@ export class CreateExerciseUnitComponent implements OnInit { exercisesAvailableForUnitCreation: Exercise[] = []; exercisesToCreateUnitFor: Exercise[] = []; - // Icons - faSort = faSort; - constructor( private activatedRoute: ActivatedRoute, private router: Router, diff --git a/src/main/webapp/app/shared/constants/file-extensions.constants.ts b/src/main/webapp/app/shared/constants/file-extensions.constants.ts index 3d1797cb308d..5995f5506568 100644 --- a/src/main/webapp/app/shared/constants/file-extensions.constants.ts +++ b/src/main/webapp/app/shared/constants/file-extensions.constants.ts @@ -45,6 +45,10 @@ export const UPLOAD_FILE_EXTENSIONS = [ 'odf', ]; +export const ALLOWED_FILE_EXTENSIONS_HUMAN_READABLE = UPLOAD_FILE_EXTENSIONS.join(', '); +// The list of file extensions for the "accept" attribute of the file input field +export const ACCEPTED_FILE_EXTENSIONS_FILE_BROWSER = UPLOAD_FILE_EXTENSIONS.map((ext) => '.' + ext).join(','); + /** * The list of file extensions that are readable in a file editor. * Extensions are case-sensitive. From 2f811a5ad7efad5108d7146d2777bef53b5ccb38 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Sat, 2 Nov 2024 23:39:23 +0100 Subject: [PATCH 2/6] Fix client tests --- .../attachment-unit-form.component.ts | 4 ++-- .../attachment-unit-form.component.spec.ts | 12 +++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts index e48e513cdb2f..2d41ce99db23 100644 --- a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts +++ b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts @@ -3,7 +3,7 @@ import dayjs from 'dayjs/esm'; import { FormBuilder, FormGroup, Validators } from '@angular/forms'; import { faQuestionCircle, faTimes } from '@fortawesome/free-solid-svg-icons'; import { ACCEPTED_FILE_EXTENSIONS_FILE_BROWSER, ALLOWED_FILE_EXTENSIONS_HUMAN_READABLE } from 'app/shared/constants/file-extensions.constants'; -import { Competency, CompetencyLectureUnitLink } from 'app/entities/competency.model'; +import { CompetencyLectureUnitLink } from 'app/entities/competency.model'; import { MAX_FILE_SIZE } from 'app/shared/constants/input.constants'; import { toSignal } from '@angular/core/rxjs-interop'; @@ -62,7 +62,7 @@ export class AttachmentUnitFormComponent implements OnChanges { releaseDate: [undefined as dayjs.Dayjs | undefined], version: [{ value: 1, disabled: true }], updateNotificationText: [undefined as string | undefined, [Validators.maxLength(1000)]], - competencies: [undefined as Competency[] | undefined], + competencyLinks: [undefined as CompetencyLectureUnitLink[] | undefined], }); private readonly statusChanges = toSignal(this.form.statusChanges ?? 'INVALID'); diff --git a/src/test/javascript/spec/component/lecture-unit/attachment-unit/attachment-unit-form.component.spec.ts b/src/test/javascript/spec/component/lecture-unit/attachment-unit/attachment-unit-form.component.spec.ts index 4037d5764924..fa218d84c467 100644 --- a/src/test/javascript/spec/component/lecture-unit/attachment-unit/attachment-unit-form.component.spec.ts +++ b/src/test/javascript/spec/component/lecture-unit/attachment-unit/attachment-unit-form.component.spec.ts @@ -1,12 +1,11 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { FormsModule, ReactiveFormsModule } from '@angular/forms'; import { FaIconComponent } from '@fortawesome/angular-fontawesome'; -import { TranslateService } from '@ngx-translate/core'; import { AttachmentUnitFormComponent, AttachmentUnitFormData } from 'app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component'; import { FormDateTimePickerComponent } from 'app/shared/date-time-picker/date-time-picker.component'; import { ArtemisTranslatePipe } from 'app/shared/pipes/artemis-translate.pipe'; import dayjs from 'dayjs/esm'; -import { MockComponent, MockDirective, MockPipe, MockProviders } from 'ng-mocks'; +import { MockComponent, MockDirective, MockPipe } from 'ng-mocks'; import { NgbTooltip } from '@ng-bootstrap/ng-bootstrap'; import { CompetencySelectionComponent } from 'app/shared/competency-selection/competency-selection.component'; import { MAX_FILE_SIZE } from 'app/shared/constants/input.constants'; @@ -25,7 +24,6 @@ describe('AttachmentUnitFormComponent', () => { MockComponent(FaIconComponent), MockComponent(CompetencySelectionComponent), ], - providers: [MockProviders(TranslateService)], schemas: [], }) .compileComponents() @@ -71,7 +69,7 @@ describe('AttachmentUnitFormComponent', () => { expect(attachmentUnitFormComponent.descriptionControl?.value).toEqual(formData.formProperties.description); expect(attachmentUnitFormComponent.versionControl?.value).toEqual(formData.formProperties.version); expect(attachmentUnitFormComponent.updateNotificationTextControl?.value).toEqual(formData.formProperties.updateNotificationText); - expect(attachmentUnitFormComponent.fileName).toEqual(formData.fileProperties.fileName); + expect(attachmentUnitFormComponent.fileName()).toEqual(formData.fileProperties.fileName); expect(attachmentUnitFormComponent.file).toEqual(formData.fileProperties.file); }); it('should submit valid form', () => { @@ -90,7 +88,7 @@ describe('AttachmentUnitFormComponent', () => { const fakeFile = new File([''], 'Test-File.pdf', { type: 'application/pdf' }); attachmentUnitFormComponent.file = fakeFile; const exampleFileName = 'lorem Ipsum'; - attachmentUnitFormComponent.fileName = exampleFileName; + attachmentUnitFormComponent.fileName.set(exampleFileName); attachmentUnitFormComponentFixture.detectChanges(); expect(attachmentUnitFormComponent.form.valid).toBeTrue(); @@ -132,7 +130,7 @@ describe('AttachmentUnitFormComponent', () => { attachmentUnitFormComponent.updateNotificationTextControl!.setValue(exampleUpdateNotificationText); const fakeFile = new File([''], 'Test-File.pdf', { type: 'application/pdf' }); attachmentUnitFormComponent.file = fakeFile; - attachmentUnitFormComponent.fileName = 'lorem Ipsum'; + attachmentUnitFormComponent.fileName.set('lorem Ipsum'); expect(attachmentUnitFormComponent.form.invalid).toBeTrue(); const submitFormSpy = jest.spyOn(attachmentUnitFormComponent, 'submitForm'); @@ -166,7 +164,7 @@ describe('AttachmentUnitFormComponent', () => { attachmentUnitFormComponentFixture.detectChanges(); const submitButton = attachmentUnitFormComponentFixture.debugElement.nativeElement.querySelector('#submitButton'); - expect(attachmentUnitFormComponent.isFileTooBig).toBeTrue(); + expect(attachmentUnitFormComponent.isFileTooBig()).toBeTrue(); expect(submitButton.disabled).toBeTrue(); }); }); From 277f0ab4d0b8f33c33adcb07251a9aa912697c66 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Mon, 4 Nov 2024 01:20:59 +0100 Subject: [PATCH 3/6] Refactor all inputs and outputs to signals --- .../attachment-unit-form.component.html | 4 ++-- .../attachment-unit-form.component.ts | 16 ++++++++-------- .../attachment-unit-form.component.spec.ts | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.html b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.html index 4de9ac96dcfa..7c8434dc9023 100644 --- a/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.html +++ b/src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.html @@ -81,7 +81,7 @@ formControlName="competencyLinks" />
-
+