From d7a5f42687664ea0325f4b53ec56127bfe837948 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henry=20T=C3=A4schner?= <129834483+HenryT-CG@users.noreply.github.com> Date: Sun, 22 Dec 2024 12:13:45 +0100 Subject: [PATCH] Fix: save on copy (#316) * fix: save a new parameter on copy action * fix: simplify code --- .../announcement-detail.component.html | 16 +-- .../announcement-detail.component.spec.ts | 121 +++++++++++------- .../announcement-detail.component.ts | 13 +- src/assets/i18n/de.json | 5 +- src/assets/i18n/en.json | 5 +- 5 files changed, 85 insertions(+), 75 deletions(-) diff --git a/src/app/announcement/announcement-detail/announcement-detail.component.html b/src/app/announcement/announcement-detail/announcement-detail.component.html index bf94468..f4b5c22 100644 --- a/src/app/announcement/announcement-detail/announcement-detail.component.html +++ b/src/app/announcement/announcement-detail/announcement-detail.component.html @@ -1,9 +1,7 @@
{ providers: [ provideHttpClient(), provideHttpClientTesting(), + { provide: UserService, useValue: mockUserService }, { provide: PortalMessageService, useValue: msgServiceSpy }, - { provide: AnnouncementInternalAPIService, useValue: apiServiceSpy }, - { provide: UserService, useValue: mockUserService } + { provide: AnnouncementInternalAPIService, useValue: apiServiceSpy } ] }).compileComponents() msgServiceSpy.success.calls.reset() msgServiceSpy.error.calls.reset() msgServiceSpy.info.calls.reset() msgServiceSpy.warning.calls.reset() + // to spy data: reset apiServiceSpy.getAnnouncementById.calls.reset() apiServiceSpy.createAnnouncement.calls.reset() apiServiceSpy.updateAnnouncementById.calls.reset() @@ -100,17 +101,19 @@ describe('AnnouncementDetailComponent', () => { component.formGroup.reset() }) - it('should create', () => { - expect(component).toBeTruthy() - }) - - it('should create but not initialize if dialog is not open', () => { - expect(component).toBeTruthy() - component.displayDialog = false - component.ngOnChanges() + describe('construction', () => { + it('should create', () => { + expect(component).toBeTruthy() + }) }) describe('ngOnChange - init form', () => { + it('should create but not initialize if dialog is not open', () => { + expect(component).toBeTruthy() + component.displayDialog = false + component.ngOnChanges() + }) + describe('VIEW', () => { it('should reject initializing if dialog is not open', () => { apiServiceSpy.getAnnouncementById.and.returnValue(of(announcement)) @@ -141,10 +144,9 @@ describe('AnnouncementDetailComponent', () => { component.ngOnChanges() expect(apiServiceSpy.getAnnouncementById).toHaveBeenCalled() + expect(component.loading).toBeFalse() expect(component.formGroup.disabled).toBeTrue() expect(component.formGroup.controls['title'].value).toBe(announcement.title) - expect(component.loading).toBeFalse() - //expect(component.productOptions).toEqual(allProductsSI) }) it('should prepare viewing an announcement - failed: missing id', () => { @@ -185,6 +187,7 @@ describe('AnnouncementDetailComponent', () => { component.ngOnChanges() expect(apiServiceSpy.getAnnouncementById).toHaveBeenCalled() + expect(component.loading).toBeFalse() expect(component.formGroup.enabled).toBeTrue() expect(component.formGroup.controls['title'].value).toEqual(announcement.title) expect(component.formGroup.controls['content'].value).toEqual(announcement.content) @@ -239,6 +242,8 @@ describe('AnnouncementDetailComponent', () => { component.ngOnChanges() expect(component.formGroup.reset).toHaveBeenCalled() + expect(component.formGroup.enabled).toBeTrue() + expect(component.formGroup.controls['title'].value).toBe(null) // check default values expect(component.formGroup.controls['priority'].value).toEqual(AnnouncementPriorityType.Normal) expect(component.formGroup.controls['status'].value).toEqual(AnnouncementStatus.Inactive) @@ -247,18 +252,20 @@ describe('AnnouncementDetailComponent', () => { }) describe('COPY', () => { - it('should prepare copying an announcement - start with data from other announcement', () => { + it('should prepare copying an announcement - use data from other announcement', () => { component.changeMode = 'COPY' component.announcement = announcement component.ngOnChanges() expect(apiServiceSpy.getAnnouncementById).not.toHaveBeenCalled() + expect(component.formGroup.enabled).toBeTrue() + expect(component.formGroup.controls['title'].value).toBe(announcement.title) }) it('should prepare copying an announcement - without date values', () => { component.changeMode = 'COPY' - component.announcement = { ...announcement, id: undefined, startDate: undefined, endDate: undefined } + component.announcement = { ...announcement, startDate: undefined, endDate: undefined } component.ngOnChanges() @@ -269,7 +276,7 @@ describe('AnnouncementDetailComponent', () => { }) }) - describe('saving', () => { + describe('onSave - creating and updating a parameter', () => { describe('CREATE', () => { it('should create an announcement', () => { apiServiceSpy.createAnnouncement.and.returnValue(of({})) @@ -293,13 +300,26 @@ describe('AnnouncementDetailComponent', () => { component.onSave() expect(component.formGroup.valid).toBeTrue() - expect(console.error).toHaveBeenCalledWith('createAnnouncement', errorResponse) expect(msgServiceSpy.error).toHaveBeenCalledWith({ summaryKey: 'ACTIONS.CREATE.MESSAGE.NOK' }) + expect(console.error).toHaveBeenCalledWith('createAnnouncement', errorResponse) }) }) + describe('COPY', () => { + it('should create a parameter based on another', () => { + apiServiceSpy.createAnnouncement.and.returnValue(of({})) + component.changeMode = 'COPY' + spyOn(component.hideDialogAndChanged, 'emit') + component.formGroup = formGroup + + component.onSave() + + expect(msgServiceSpy.success).toHaveBeenCalledWith({ summaryKey: 'ACTIONS.CREATE.MESSAGE.OK' }) + expect(component.hideDialogAndChanged.emit).toHaveBeenCalledWith(true) + }) + }) describe('EDIT', () => { - it('should update an announcement', () => { + it('should update an announcement - successful', () => { apiServiceSpy.updateAnnouncementById.and.returnValue(of({})) component.changeMode = 'EDIT' component.announcement = announcement @@ -323,12 +343,44 @@ describe('AnnouncementDetailComponent', () => { component.onSave() - expect(console.error).toHaveBeenCalledWith('updateAnnouncementById', errorResponse) expect(msgServiceSpy.error).toHaveBeenCalledWith({ summaryKey: 'ACTIONS.EDIT.MESSAGE.NOK' }) + expect(console.error).toHaveBeenCalledWith('updateAnnouncementById', errorResponse) + }) + }) + }) + + /* + * UI ACTIONS + */ + describe('Extra UI actions', () => { + describe('Closing dialog', () => { + it('should close the dialog if user triggers hiding', () => { + spyOn(component.hideDialogAndChanged, 'emit') + component.onDialogHide() + + expect(component.hideDialogAndChanged.emit).toHaveBeenCalledWith(false) }) }) }) + /** + * Language tests + */ + it('should set a German date format', () => { + expect(component.dateFormat).toEqual('dd.mm.yy') + }) + + it('should set default date format', () => { + mockUserService.lang$.getValue.and.returnValue('en') + fixture = TestBed.createComponent(AnnouncementDetailComponent) + component = fixture.componentInstance + fixture.detectChanges() + expect(component.dateFormat).toEqual('mm/dd/yy') + }) + + /** + * Extras + */ describe('form invalid', () => { it('should display warning when trying to save an invalid announcement', () => { component.formGroup = formGroup @@ -387,33 +439,4 @@ describe('AnnouncementDetailComponent', () => { expect(dateFormGroup.errors).toEqual(null) }) }) - - /* - * UI ACTIONS - */ - describe('Extra UI actions', () => { - describe('Closing dialog', () => { - it('should close the dialog if user triggers hiding', () => { - spyOn(component.hideDialogAndChanged, 'emit') - component.onDialogHide() - - expect(component.hideDialogAndChanged.emit).toHaveBeenCalledWith(false) - }) - }) - }) - - /** - * Language tests - */ - it('should set a German date format', () => { - expect(component.dateFormat).toEqual('dd.mm.yy') - }) - - it('should set default date format', () => { - mockUserService.lang$.getValue.and.returnValue('en') - fixture = TestBed.createComponent(AnnouncementDetailComponent) - component = fixture.componentInstance - fixture.detectChanges() - expect(component.dateFormat).toEqual('mm/dd/yy') - }) }) diff --git a/src/app/announcement/announcement-detail/announcement-detail.component.ts b/src/app/announcement/announcement-detail/announcement-detail.component.ts index 32aef5c..dd1a453 100644 --- a/src/app/announcement/announcement-detail/announcement-detail.component.ts +++ b/src/app/announcement/announcement-detail/announcement-detail.component.ts @@ -6,8 +6,6 @@ import { SelectItem } from 'primeng/api' import { PortalMessageService, UserService } from '@onecx/portal-integration-angular' import { - CreateAnnouncementRequest, - UpdateAnnouncementRequest, Announcement, AnnouncementPriorityType, AnnouncementStatus, @@ -82,6 +80,7 @@ export class AnnouncementDetailComponent implements OnChanges { public ngOnChanges() { if (!this.displayDialog) return + this.exceptionKey = undefined // matching mode and given data? if ('CREATE' === this.changeMode && this.announcement) return if (['EDIT', 'VIEW'].includes(this.changeMode)) @@ -159,7 +158,7 @@ export class AnnouncementDetailComponent implements OnChanges { this.announcementApi .updateAnnouncementById({ id: this.announcement?.id, - updateAnnouncementRequest: this.submitFormValues() as UpdateAnnouncementRequest + updateAnnouncementRequest: this.formGroup.value }) .subscribe({ next: () => { @@ -172,10 +171,10 @@ export class AnnouncementDetailComponent implements OnChanges { } }) } - if (this.changeMode === 'CREATE') { + if (['COPY', 'CREATE'].includes(this.changeMode)) { this.announcementApi .createAnnouncement({ - createAnnouncementRequest: this.submitFormValues() as CreateAnnouncementRequest + createAnnouncementRequest: this.formGroup.value }) .subscribe({ next: () => { @@ -190,10 +189,6 @@ export class AnnouncementDetailComponent implements OnChanges { } } - private submitFormValues(): Announcement { - return { ...this.formGroup.value } as Announcement - } - /**************************************************************************** * SERVER responses & internal */ diff --git a/src/assets/i18n/de.json b/src/assets/i18n/de.json index bd394b6..722a482 100644 --- a/src/assets/i18n/de.json +++ b/src/assets/i18n/de.json @@ -78,8 +78,6 @@ }, "CANCEL": "Abbrechen", "CHOOSE": "Auswählen", - "CLOSE": "Schließen", - "CONFIRM": "Bestätigen", "LOADING": "...wird geladen", "RELOAD": "Neuladen", "SAVE": "Speichern", @@ -89,9 +87,8 @@ "CANCEL": "Die Aktion abbrechen", "CANCEL_AND_CLOSE": "Abbrechen und Dialog schließen", "CANCEL_WITHOUT_SAVE": "Die Aktion ohne Speichern abbrechen", - "CLOSE": "Dialog schließen", "SAVE": "Die Änderungen speichern", - "SAVE_AS": "Speichern als neues {{TYPE}}" + "SAVE_AS": "Speichern als neuen Parameter" } }, "INTERNAL": { diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index 97ef825..1a0851d 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -78,8 +78,6 @@ }, "CANCEL": "Cancel", "CHOOSE": "Choose", - "CLOSE": "Close", - "CONFIRM": "Confirm", "LOADING": "...is loading", "RELOAD": "Reload", "SAVE": "Save", @@ -89,9 +87,8 @@ "CANCEL": "Cancel action", "CANCEL_AND_CLOSE": "Cancel and close dialog", "CANCEL_WITHOUT_SAVE": "Cancel action without saving", - "CLOSE": "Close dialog", "SAVE": "Save changes", - "SAVE_AS": "Save as new {{TYPE}}" + "SAVE_AS": "Save as new Announcement" } }, "INTERNAL": {