From 17c29af3b7b2ee313d0204abd6d70f2b5b63a2d7 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 04:45:23 +0100 Subject: [PATCH 01/17] Convert attachmentToBeCreated to signal --- .../lecture-attachments.component.html | 24 ++++++------ .../lecture/lecture-attachments.component.ts | 34 ++++++++--------- .../lecture-attachments.component.spec.ts | 38 +++++++++---------- 3 files changed, 48 insertions(+), 48 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.html b/src/main/webapp/app/lecture/lecture-attachments.component.html index 6192f270d64e..a46a6b96efee 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.html +++ b/src/main/webapp/app/lecture/lecture-attachments.component.html @@ -53,7 +53,7 @@

@for (attachment of attachments; track trackId($index, attachment)) { - + {{ attachment.id }} @@ -91,7 +91,7 @@

} From f9df11431e7c7eb593eb02d0fb8f6ab2d6cf1f75 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 04:59:53 +0100 Subject: [PATCH 06/17] Merge table columns --- .../app/lecture/lecture-attachments.component.html | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.html b/src/main/webapp/app/lecture/lecture-attachments.component.html index 98297ed4c8a3..23e180f64f09 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.html +++ b/src/main/webapp/app/lecture/lecture-attachments.component.html @@ -44,7 +44,6 @@

- @@ -57,21 +56,21 @@

{{ attachment.id }} - {{ attachment.name }} - {{ attachment.attachmentType }} @if (!isDownloadingAttachmentLink) { {{ attachment.name }} - } - @if (isDownloadingAttachmentLink === attachment.link) { + } @else if (isDownloadingAttachmentLink === attachment.link) { {{ 'artemisApp.courseOverview.lectureDetails.isDownloading' | artemisTranslate }} + } @else { + {{ attachment.name }} } + {{ attachment.attachmentType }} {{ attachment.releaseDate | artemisDate }} {{ attachment.uploadDate | artemisDate }} From 254be49ba59236ef61d6b8655759ab414466f909 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:00:52 +0100 Subject: [PATCH 07/17] Use form --- .../webapp/app/lecture/lecture-attachments.component.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.html b/src/main/webapp/app/lecture/lecture-attachments.component.html index 23e180f64f09..5d5412aab945 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.html +++ b/src/main/webapp/app/lecture/lecture-attachments.component.html @@ -125,7 +125,7 @@

@if (attachmentToBeUpdatedOrCreated()) { -
+
@if (!attachmentToBeUpdatedOrCreated().id) { @@ -203,7 +203,7 @@

-
+ } @if (!attachmentToBeUpdatedOrCreated()) {
From a641ab317b9f9d9db073f7285e8f32433d7c1328 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:02:13 +0100 Subject: [PATCH 08/17] Remove outdated validation method --- src/main/webapp/app/lecture/lecture-attachments.component.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.ts b/src/main/webapp/app/lecture/lecture-attachments.component.ts index 4e52615b4067..200f722e03af 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.ts +++ b/src/main/webapp/app/lecture/lecture-attachments.component.ts @@ -119,10 +119,6 @@ export class LectureAttachmentsComponent implements OnDestroy { return attachmentLink.endsWith('.pdf') ?? false; } - get isSubmitPossible(): boolean { - return !!(this.attachmentToBeUpdatedOrCreated()?.name && (this.attachmentFile() || this.attachmentToBeUpdatedOrCreated()?.link)); - } - addAttachment(): void { const newAttachment = new Attachment(); newAttachment.lecture = this.lecture(); From 8165b51bf21323a48528a288668cc831e1396c12 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:05:07 +0100 Subject: [PATCH 09/17] Fix tests --- .../lecture-attachments.component.spec.ts | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/test/javascript/spec/component/lecture/lecture-attachments.component.spec.ts b/src/test/javascript/spec/component/lecture/lecture-attachments.component.spec.ts index e2d30fea9efa..ba36201693f5 100644 --- a/src/test/javascript/spec/component/lecture/lecture-attachments.component.spec.ts +++ b/src/test/javascript/spec/component/lecture/lecture-attachments.component.spec.ts @@ -9,18 +9,19 @@ import { LectureAttachmentsComponent } from 'app/lecture/lecture-attachments.com import { AttachmentService } from 'app/lecture/attachment.service'; import { FileService } from 'app/shared/http/file.service'; import { HtmlForMarkdownPipe } from 'app/shared/pipes/html-for-markdown.pipe'; -import { MockComponent, MockDirective, MockPipe, MockProvider } from 'ng-mocks'; +import { MockDirective, MockModule, MockPipe, MockProvider } from 'ng-mocks'; import { MockFileService } from '../../helpers/mocks/service/mock-file.service'; import { ArtemisDatePipe } from 'app/shared/pipes/artemis-date.pipe'; import { ArtemisTranslatePipe } from 'app/shared/pipes/artemis-translate.pipe'; import { FormDateTimePickerComponent } from 'app/shared/date-time-picker/date-time-picker.component'; import { DeleteButtonDirective } from 'app/shared/delete-dialog/delete-button.directive'; -import { NgModel } from '@angular/forms'; +import { FormsModule, ReactiveFormsModule } from '@angular/forms'; import { of, take, throwError } from 'rxjs'; import { HttpResponse } from '@angular/common/http'; import { NgbTooltip } from '@ng-bootstrap/ng-bootstrap'; import { LectureService } from 'app/lecture/lecture.service'; import { RouterModule } from '@angular/router'; +import { OwlDateTimeModule, OwlNativeDateTimeModule } from '@danielmoncada/angular-datetime-picker'; describe('LectureAttachmentsComponent', () => { let comp: LectureAttachmentsComponent; @@ -85,12 +86,19 @@ describe('LectureAttachmentsComponent', () => { beforeEach(() => { return TestBed.configureTestingModule({ - imports: [ArtemisTestModule, MockDirective(NgbTooltip), RouterModule], + imports: [ + ArtemisTestModule, + MockDirective(NgbTooltip), + RouterModule, + ReactiveFormsModule, + FormsModule, + MockModule(OwlDateTimeModule), + MockModule(OwlNativeDateTimeModule), + ], declarations: [ LectureAttachmentsComponent, - MockComponent(FormDateTimePickerComponent), + FormDateTimePickerComponent, MockDirective(DeleteButtonDirective), - MockDirective(NgModel), MockPipe(ArtemisTranslatePipe), MockPipe(HtmlForMarkdownPipe), MockPipe(ArtemisDatePipe), @@ -148,7 +156,10 @@ describe('LectureAttachmentsComponent', () => { const uploadAttachmentButton = fixture.debugElement.query(By.css('#upload-attachment')); expect(uploadAttachmentButton).not.toBeNull(); expect(comp.attachmentToBeUpdatedOrCreated()).not.toBeNull(); - comp.attachmentToBeUpdatedOrCreated()!.name = 'Test File Name'; + comp.form.patchValue({ + attachmentName: 'Test File Name', + releaseDate: dayjs(), + }); fixture.detectChanges(); expect(uploadAttachmentButton.nativeElement.disabled).toBeFalse(); uploadAttachmentButton.nativeElement.click(); @@ -171,7 +182,10 @@ describe('LectureAttachmentsComponent', () => { const uploadAttachmentButton = fixture.debugElement.query(By.css('#upload-attachment')); expect(uploadAttachmentButton).not.toBeNull(); expect(comp.attachmentToBeUpdatedOrCreated()).not.toBeNull(); - comp.attachmentToBeUpdatedOrCreated()!.name = 'Test File Name'; + comp.form.patchValue({ + attachmentName: 'Test File Name', + releaseDate: dayjs(), + }); fixture.detectChanges(); expect(uploadAttachmentButton.nativeElement.disabled).toBeFalse(); uploadAttachmentButton.nativeElement.click(); From 0281f22bdc96cced4aff7fbc7a3b2a228a8bc638 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:07:05 +0100 Subject: [PATCH 10/17] Make sure to use values from form --- src/main/webapp/app/lecture/lecture-attachments.component.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.ts b/src/main/webapp/app/lecture/lecture-attachments.component.ts index 200f722e03af..f4b2b1f01892 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.ts +++ b/src/main/webapp/app/lecture/lecture-attachments.component.ts @@ -137,6 +137,8 @@ export class LectureAttachmentsComponent implements OnDestroy { } this.attachmentToBeUpdatedOrCreated()!.version!++; this.attachmentToBeUpdatedOrCreated()!.uploadDate = dayjs(); + this.attachmentToBeUpdatedOrCreated()!.name = this.form.value.attachmentName ?? undefined; + this.attachmentToBeUpdatedOrCreated()!.releaseDate = this.form.value.releaseDate ?? undefined; if (!this.attachmentFile() && !this.attachmentToBeUpdatedOrCreated()!.id) { return; From d58aab80ae82c339f4e6a9a2764d7e9ce52e19cb Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:09:36 +0100 Subject: [PATCH 11/17] Use notification from form and fix test --- .../webapp/app/lecture/lecture-attachments.component.ts | 1 + .../component/lecture/lecture-attachments.component.spec.ts | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.ts b/src/main/webapp/app/lecture/lecture-attachments.component.ts index f4b2b1f01892..d809541c5b3b 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.ts +++ b/src/main/webapp/app/lecture/lecture-attachments.component.ts @@ -139,6 +139,7 @@ export class LectureAttachmentsComponent implements OnDestroy { this.attachmentToBeUpdatedOrCreated()!.uploadDate = dayjs(); this.attachmentToBeUpdatedOrCreated()!.name = this.form.value.attachmentName ?? undefined; this.attachmentToBeUpdatedOrCreated()!.releaseDate = this.form.value.releaseDate ?? undefined; + this.notificationText = this.form.value.notificationText ?? undefined; if (!this.attachmentFile() && !this.attachmentToBeUpdatedOrCreated()!.id) { return; diff --git a/src/test/javascript/spec/component/lecture/lecture-attachments.component.spec.ts b/src/test/javascript/spec/component/lecture/lecture-attachments.component.spec.ts index ba36201693f5..0ec3ea836ec9 100644 --- a/src/test/javascript/spec/component/lecture/lecture-attachments.component.spec.ts +++ b/src/test/javascript/spec/component/lecture/lecture-attachments.component.spec.ts @@ -251,12 +251,14 @@ describe('LectureAttachmentsComponent', () => { comp.attachmentFile.set(file); } comp.attachmentToBeUpdatedOrCreated.set(attachment); - comp.notificationText = notification; comp.attachmentBackup = backup; comp.attachments = [attachment]; // Do change - attachment.name = 'New Name'; + comp.form.patchValue({ + attachmentName: 'New Name', + notificationText: notification, + }); const attachmentServiceUpdateStub = jest.spyOn(attachmentService, 'update').mockReturnValue(throwError(() => new Error(errorMessage))); comp.saveAttachment(); From cb7f5b48f7ae8cbad74a1a8afd7419d8ff2879a0 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:13:28 +0100 Subject: [PATCH 12/17] Refactor resetting form --- .../lecture/lecture-attachments.component.ts | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.ts b/src/main/webapp/app/lecture/lecture-attachments.component.ts index d809541c5b3b..f15fb4b0fa47 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.ts +++ b/src/main/webapp/app/lecture/lecture-attachments.component.ts @@ -152,9 +152,7 @@ export class LectureAttachmentsComponent implements OnDestroy { } this.attachmentService.update(this.attachmentToBeUpdatedOrCreated()!.id!, this.attachmentToBeUpdatedOrCreated()!, this.attachmentFile(), requestOptions).subscribe({ next: (attachmentRes: HttpResponse) => { - this.attachmentFile.set(undefined); - this.attachmentToBeUpdatedOrCreated.set(undefined); - this.attachmentBackup = undefined; + this.resetAttachmentFormVariables(); this.notificationText = undefined; this.attachments = this.attachments.map((el) => { return el.id === attachmentRes.body!.id ? attachmentRes.body! : el; @@ -169,16 +167,30 @@ export class LectureAttachmentsComponent implements OnDestroy { this.lectureService.findWithDetails(this.lecture().id!).subscribe((lectureResponse: HttpResponse) => { this.lecture.set(lectureResponse.body!); }); - this.attachmentFile.set(undefined); - this.attachmentToBeUpdatedOrCreated.set(undefined); - this.attachmentBackup = undefined; this.loadAttachments(); + this.resetAttachmentFormVariables(); }, error: (error: HttpErrorResponse) => this.handleFailedUpload(error), }); } } + private clearFormValues(): void { + this.form.reset({ + attachmentName: undefined, + attachmentFileName: undefined, + releaseDate: undefined, + notificationText: undefined, + }); + } + + private resetAttachmentFormVariables() { + this.attachmentFile.set(undefined); + this.attachmentToBeUpdatedOrCreated.set(undefined); + this.attachmentBackup = undefined; + this.clearFormValues(); + } + private handleFailedUpload(error: HttpErrorResponse): void { this.errorMessage = error.message; this.erroredFile = this.attachmentFile(); From db84c69a133be12efbe4cf93e289796ad7daa24e Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:17:01 +0100 Subject: [PATCH 13/17] Reset form values and refactor setLectureAttachment --- .../lecture/lecture-attachments.component.ts | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.ts b/src/main/webapp/app/lecture/lecture-attachments.component.ts index f15fb4b0fa47..5974bcb3631c 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.ts +++ b/src/main/webapp/app/lecture/lecture-attachments.component.ts @@ -199,7 +199,24 @@ export class LectureAttachmentsComponent implements OnDestroy { this.resetAttachment(); } + private setFormValues(formValues: LectureAttachmentFormData): void { + this.form.patchValue(formValues); + } + editAttachment(attachment: Attachment): void { + if (this.fileInput) { + this.fileInput.nativeElement.value = ''; + } + + // attachmentFileName can only be set to an empty string due to security reasons in current angular version (18) + this.setFormValues({ + attachmentName: attachment?.name, + releaseDate: dayjs(attachment?.releaseDate), + notificationText: this.notificationText, + }); + + this.attachmentToBeUpdatedOrCreated.set(attachment); + this.attachmentToBeUpdatedOrCreated.set(attachment); this.attachmentBackup = Object.assign({}, attachment, {}); } @@ -220,6 +237,7 @@ export class LectureAttachmentsComponent implements OnDestroy { } this.attachmentToBeUpdatedOrCreated.set(undefined); this.erroredFile = undefined; + this.resetAttachmentFormVariables(); } resetAttachment(): void { @@ -255,12 +273,19 @@ export class LectureAttachmentsComponent implements OnDestroy { if (!input.files?.length) { return; } - const attachmentFile = input.files[0]; - this.attachmentFile.set(attachmentFile); - this.attachmentToBeUpdatedOrCreated()!.link = attachmentFile.name; - // automatically set the name in case it is not yet specified - if (this.attachmentToBeUpdatedOrCreated()!.name == undefined || this.attachmentToBeUpdatedOrCreated()!.name == '') { - this.attachmentToBeUpdatedOrCreated()!.name = this.attachmentFile()!.name.replace(/\.[^/.]+$/, ''); + const file = input.files[0]; + this.attachmentFile.set(file); + this.attachmentToBeUpdatedOrCreated()!.link = file.name; + + if (!this.attachmentToBeUpdatedOrCreated()!.name) { + const derivedFileName = this.determineAttachmentNameBasedOnFileName(file.name); + this.attachmentToBeUpdatedOrCreated()!.name = derivedFileName; + this.form.patchValue({ attachmentName: derivedFileName }); } } + + private determineAttachmentNameBasedOnFileName(fileName: string): string { + const FILE_EXTENSION_REGEX = /\.[^/.]+$/; + return fileName.replace(FILE_EXTENSION_REGEX, ''); + } } From dd2ce647465b903bbf284d9a7350cf921a7571a1 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:36:46 +0100 Subject: [PATCH 14/17] Remove duplicated setter --- src/main/webapp/app/lecture/lecture-attachments.component.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.ts b/src/main/webapp/app/lecture/lecture-attachments.component.ts index 5974bcb3631c..9580b2c708a6 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.ts +++ b/src/main/webapp/app/lecture/lecture-attachments.component.ts @@ -215,8 +215,6 @@ export class LectureAttachmentsComponent implements OnDestroy { notificationText: this.notificationText, }); - this.attachmentToBeUpdatedOrCreated.set(attachment); - this.attachmentToBeUpdatedOrCreated.set(attachment); this.attachmentBackup = Object.assign({}, attachment, {}); } From ed4f33e412f6c4c222f81cc21d8e81a2ecd57f36 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:41:43 +0100 Subject: [PATCH 15/17] Add form field for attachmentFileName --- src/main/webapp/app/lecture/lecture-attachments.component.html | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.html b/src/main/webapp/app/lecture/lecture-attachments.component.html index 5d5412aab945..f82ecbded895 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.html +++ b/src/main/webapp/app/lecture/lecture-attachments.component.html @@ -165,6 +165,7 @@

(change)="setLectureAttachment($event)" [class.ng-invalid]="!isFileSelectionValid()" [class.invalid-file-input-margin]="!isFileSelectionValid()" + formControlName="attachmentFileName" aria-describedby="fileHelp" />
From 486e15288f4c253094a11236470430c1ccec83c0 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 28 Nov 2024 05:43:33 +0100 Subject: [PATCH 16/17] Fix client build issues --- .../app/lecture/lecture-attachments.component.html | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/webapp/app/lecture/lecture-attachments.component.html b/src/main/webapp/app/lecture/lecture-attachments.component.html index f82ecbded895..82050a7c3efe 100644 --- a/src/main/webapp/app/lecture/lecture-attachments.component.html +++ b/src/main/webapp/app/lecture/lecture-attachments.component.html @@ -128,7 +128,7 @@

- @if (!attachmentToBeUpdatedOrCreated().id) { + @if (!attachmentToBeUpdatedOrCreated()!.id) {

} @else {

@@ -143,7 +143,7 @@

- @if (attachmentToBeUpdatedOrCreated().id) { + @if (attachmentToBeUpdatedOrCreated()!.id) {
- } - @if (!attachmentToBeUpdatedOrCreated()) { + } @else {