Show error box in detail component if call fails, loading box while l…

…oading (#289) * fix: show error box in detail component if call fails * fix: show loading box in detail * fix: use observable in theme detail and fix tests * fix: missing display name on import * fix: detail tests * fix: integrate extra ft in original ft in theme designer --------- Co-authored-by: Christian Badura <[email protected]> Co-authored-by: Henry Täschner <[email protected]>
onecx · Nov 4, 2024 · 5fb2484 · 5fb2484
1 parent 8d70343
commit 5fb2484
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 116 deletions.
diff --git a/src/app/theme/theme-designer/theme-designer.component.ts b/src/app/theme/theme-designer/theme-designer.component.ts
@@ -225,18 +225,18 @@ export class ThemeDesignerComponent implements OnInit {
   private loadThemeTemplates(): void {
     this.themeApi.getThemes({}).subscribe((data) => {
       if (data.stream !== undefined) {
-        this.themeTemplates = [...data.stream.map(this.themeDropdownMappingFn).sort(dropDownSortItemsByLabel)]
+        this.themeTemplates = [
+          ...data.stream
+            .map((theme) => ({
+              label: theme.name,
+              value: theme.id
+            }))
+            .sort(dropDownSortItemsByLabel)
+        ]
       }
     })
   }
 
-  private themeDropdownMappingFn = (theme: Theme) => {
-    return {
-      label: theme.name,
-      value: theme.id
-    }
-  }
-
   public onThemeTemplateDropdownChange(): void {
     const themeName = dropDownGetLabelByValue(this.themeTemplates, this.themeTemplateSelectedId)
     this.confirmTemplateTheme(themeName)

diff --git a/src/app/theme/theme-detail/theme-detail.component.html b/src/app/theme/theme-detail/theme-detail.component.html
@@ -1,5 +1,6 @@
 <ocx-portal-page permission="THEME#VIEW" helpArticleId="PAGE_THEME_DETAIL">
   <ocx-page-header
+    *ngIf="theme$ | async as theme"
     [loading]="loading"
     [header]="theme?.displayName ?? ''"
     [subheader]="'THEME.DETAIL.SUBHEADER' | translate"
@@ -9,7 +10,23 @@
   >
   </ocx-page-header>
 
-  <ocx-page-content>
+  <div *ngIf="exceptionKey" id="th_detail_error" class="card px-3 align-items-center">
+    <p-message
+      id="th_detail_error_message"
+      severity="error"
+      styleClass="p-2"
+      [text]="exceptionKey | translate"
+    ></p-message>
+  </div>
+  <div *ngIf="loading" id="ws_detail_loading" class="card px-3 align-items-center">
+    <p-message
+      id="ws_detail_loading_message"
+      severity="info"
+      styleClass="p-2"
+      [text]="'ACTIONS.LOADING' | translate"
+    ></p-message>
+  </div>
+  <ocx-page-content *ngIf="!exceptionKey && (theme$ | async) as theme">
     <p-tabView (onChange)="onTabChange($event)">
       <p-tabPanel
         id="th_detail_panel_props"

diff --git a/src/app/theme/theme-detail/theme-detail.component.spec.ts b/src/app/theme/theme-detail/theme-detail.component.spec.ts
@@ -15,6 +15,11 @@ import { RefType, Theme, ThemesAPIService } from 'src/app/shared/generated'
 import { ThemeDetailComponent } from './theme-detail.component'
 import { bffImageUrl, getCurrentDateTime } from 'src/app/shared/utils'
 
+const theme: Theme = {
+  name: 'themeName',
+  id: 'theme-id'
+}
+
 describe('ThemeDetailComponent', () => {
   let component: ThemeDetailComponent
   let fixture: ComponentFixture<ThemeDetailComponent>
@@ -97,14 +102,17 @@ describe('ThemeDetailComponent', () => {
     component = fixture.componentInstance
     fixture.detectChanges()
 
-    themesApiSpy.getThemeByName.calls.reset()
+    themesApiSpy.getThemeByName.and.returnValue(of({ resource: theme } as any))
     component.loading = true
 
     await component.ngOnInit()
-    expect(component.themeName).toBe(name)
-    expect(component.dateFormat).toBe('medium')
-    expect(themesApiSpy.getThemeByName).toHaveBeenCalledOnceWith({ name: name })
-    expect(component.loading).toBe(false)
+
+    component.theme$?.subscribe((data) => {
+      expect(data).toBe(theme)
+      expect(component.themeName).toBe(name)
+      expect(component.dateFormat).toBe('medium')
+      expect(themesApiSpy.getThemeByName).toHaveBeenCalled()
+    })
   })
 
   it('should set showOperatorMessage to false', async () => {
@@ -130,71 +138,22 @@ describe('ThemeDetailComponent', () => {
   })
 
   it('should load theme and action translations on successful call', async () => {
-    const themeResponse = {
-      resource: { name: 'themeName', displayName: 'Theme' },
-      workspaces: [{ name: 'workspace', description: 'workspaceDesc' }]
-    }
-    themesApiSpy.getThemeByName.and.returnValue(of(themeResponse) as any)
-
-    const translateService = TestBed.inject(TranslateService)
-    const actionsTranslations = {
-      'ACTIONS.NAVIGATION.BACK': 'actionNavigationClose',
-      'ACTIONS.NAVIGATION.BACK.TOOLTIP': 'actionNavigationCloseTooltip',
-      'ACTIONS.EDIT.LABEL': 'actionEditLabel',
-      'ACTIONS.EDIT.TOOLTIP': 'actionEditTooltip',
-      'ACTIONS.EXPORT.LABEL': 'actionExportLabel',
-      'ACTIONS.EXPORT.TOOLTIP': 'actionExportTooltip',
-      'ACTIONS.DELETE.LABEL': 'actionDeleteLabel',
-      'ACTIONS.DELETE.TOOLTIP': 'actionDeleteTooltip',
-      'ACTIONS.DELETE.THEME_MESSAGE': '{{ITEM}} actionDeleteThemeMessage'
-    }
-    const generalTranslations = {
-      'DETAIL.CREATION_DATE': 'detailCreationDate',
-      'DETAIL.TOOLTIPS.CREATION_DATE': 'detailTooltipsCreationDate',
-      'DETAIL.MODIFICATION_DATE': 'detailModificationDate',
-      'DETAIL.TOOLTIPS.MODIFICATION_DATE': 'detailTooltipsModificationDate',
-      'THEME.WORKSPACES': 'themeWorkspaces',
-      'THEME.TOOLTIPS.WORKSPACES': 'themeTooltipsWorkspaces'
-    }
-    spyOn(translateService, 'get').and.returnValues(of(actionsTranslations), of(generalTranslations))
-
-    await component.ngOnInit()
-
-    expect(component.theme).toEqual(themeResponse['resource'])
+    spyOn(component, 'onClose')
+    spyOn(component, 'onExportTheme')
+    component.themeDeleteVisible = false
 
     let actions: any = []
     component.actions$!.subscribe((act) => (actions = act))
 
-    expect(actions.length).toBe(4)
-    const closeAction = actions.filter(
-      (a: { label: string; title: string }) =>
-        a.label === 'actionNavigationClose' && a.title === 'actionNavigationCloseTooltip'
-    )[0]
-    spyOn(component, 'onClose')
-    closeAction.actionCallback()
-    expect(component.onClose).toHaveBeenCalledTimes(1)
+    actions[0].actionCallback()
+    actions[1].actionCallback()
+    actions[2].actionCallback()
+    actions[3].actionCallback()
 
-    const editAction = actions.filter(
-      (a: { label: string; title: string }) => a.label === 'actionEditLabel' && a.title === 'actionEditTooltip'
-    )[0]
-    const router = TestBed.inject(Router)
-    spyOn(router, 'navigate')
-    editAction.actionCallback()
-    expect(router.navigate).toHaveBeenCalledOnceWith(['./edit'], jasmine.any(Object))
-
-    const exportAction = actions.filter(
-      (a: { label: string; title: string }) => a.label === 'actionExportLabel' && a.title === 'actionExportTooltip'
-    )[0]
-    spyOn(component, 'onExportTheme')
-    exportAction.actionCallback()
-    expect(component.onExportTheme).toHaveBeenCalledTimes(1)
-
-    const deleteAction = actions.filter(
-      (a: { label: string; title: string }) => a.label === 'actionDeleteLabel' && a.title === 'actionDeleteTooltip'
-    )[0]
-    expect(component.themeDeleteVisible).toBe(false)
-    deleteAction.actionCallback()
-    expect(component.themeDeleteVisible).toBe(true)
+    expect(actions.length).toBe(4)
+    expect(component.onClose).toHaveBeenCalled()
+    expect(component.onExportTheme).toHaveBeenCalled()
+    expect(component.themeDeleteVisible).toBeTrue()
   })
 
   it('should load prepare object details on successfull call', async () => {
@@ -234,44 +193,40 @@ describe('ThemeDetailComponent', () => {
     await component.ngOnInit()
   })
 
-  it('should display not found error and close page on theme fetch failure', () => {
-    spyOn(component, 'onClose')
+  it('should display not found error', () => {
     themesApiSpy.getThemeByName.and.returnValue(
       throwError(
         () =>
           new HttpErrorResponse({
-            error: 'err: was not found'
+            status: 404
           })
       )
     )
+    component.exceptionKey = ''
 
     component.ngOnInit()
 
-    expect(msgServiceSpy.error).toHaveBeenCalledOnceWith({
-      summaryKey: 'THEME.LOAD_ERROR',
-      detailKey: 'THEME.NOT_FOUND'
+    component.theme$?.subscribe(() => {
+      expect(component.exceptionKey).toBe('THEME.NOT_FOUND')
     })
-    expect(component.onClose).toHaveBeenCalledTimes(1)
   })
 
-  it('should display catched error and close page on theme fetch failure', () => {
-    spyOn(component, 'onClose')
+  it('should display load error', () => {
     themesApiSpy.getThemeByName.and.returnValue(
       throwError(
         () =>
           new HttpErrorResponse({
-            error: 'does not contain checked string'
+            status: 400
           })
       )
     )
+    component.exceptionKey = ''
 
     component.ngOnInit()
 
-    expect(msgServiceSpy.error).toHaveBeenCalledOnceWith({
-      summaryKey: 'THEME.LOAD_ERROR',
-      detailKey: 'does not contain checked string'
+    component.theme$?.subscribe(() => {
+      expect(component.exceptionKey).toBe('THEME.LOAD_ERROR')
     })
-    expect(component.onClose).toHaveBeenCalledTimes(1)
   })
 
   it('should navigate back on close', () => {
@@ -293,7 +248,9 @@ describe('ThemeDetailComponent', () => {
 
     await component.ngOnInit()
 
-    expect(component.headerImageUrl).toBe('logo123.png')
+    component.theme$?.subscribe(() => {
+      expect(component.headerImageUrl).toBe('logo123.png')
+    })
   })
 
   it('should set header image url without prefix when theme logo has http/https', async () => {
@@ -307,7 +264,10 @@ describe('ThemeDetailComponent', () => {
     }
     themesApiSpy.getThemeByName.and.returnValue(of(themeResponse) as any)
     await component.ngOnInit()
-    expect(component.headerImageUrl).toBe(url)
+
+    component.theme$?.subscribe(() => {
+      expect(component.headerImageUrl).toBe(url)
+    })
   })
 
   it('should hide dialog, inform and navigate on successfull deletion', () => {

diff --git a/src/app/theme/theme-detail/theme-detail.component.ts b/src/app/theme/theme-detail/theme-detail.component.ts
@@ -2,7 +2,7 @@ import { Component, OnInit } from '@angular/core'
 import { Location } from '@angular/common'
 import { ActivatedRoute, Router } from '@angular/router'
 import { TranslateService } from '@ngx-translate/core'
-import { Observable, finalize, map } from 'rxjs'
+import { Observable, catchError, finalize, map, of } from 'rxjs'
 import FileSaver from 'file-saver'
 
 import { Action, PortalMessageService, UserService } from '@onecx/portal-integration-angular'
@@ -22,6 +22,7 @@ import {
 })
 export class ThemeDetailComponent implements OnInit {
   public theme: Theme | undefined
+  public theme$: Observable<Theme> | undefined
   public themeName!: string
   public themeDeleteVisible = false
   public showOperatorMessage = true // display initially only
@@ -31,6 +32,7 @@ export class ThemeDetailComponent implements OnInit {
   // page header
   public actions$: Observable<Action[]> | undefined
   public headerImageUrl?: string
+  public exceptionKey = ''
 
   constructor(
     private readonly user: UserService,
@@ -47,27 +49,24 @@ export class ThemeDetailComponent implements OnInit {
   }
 
   ngOnInit(): void {
-    this.themeApi
-      .getThemeByName({ name: this.themeName })
-      .pipe(
-        finalize(() => {
-          this.loading = false
-        })
-      )
-      .subscribe({
-        next: (data) => {
-          this.theme = data.resource
-          this.preparePage()
-          this.headerImageUrl = this.getImageUrl(this.theme, RefType.Logo)
-        },
-        error: (err) => {
-          this.msgService.error({
-            summaryKey: 'THEME.LOAD_ERROR',
-            detailKey: err.error.indexOf('was not found') > 1 ? 'THEME.NOT_FOUND' : err.error
-          })
-          this.onClose()
-        }
+    this.loading = true
+    this.theme$ = this.themeApi.getThemeByName({ name: this.themeName }).pipe(
+      map((data) => {
+        this.preparePage()
+        if (data.resource) this.theme = data.resource
+        this.headerImageUrl = this.getImageUrl(this.theme, RefType.Logo)
+        return data.resource
+      }),
+      catchError((err) => {
+        if (err.status === 404) this.exceptionKey = 'THEME.NOT_FOUND'
+        else this.exceptionKey = 'THEME.LOAD_ERROR'
+        console.error('getThemeByName():', err)
+        return of({} as Theme)
+      }),
+      finalize(() => {
+        this.loading = false
       })
+    )
   }
 
   private preparePage() {

diff --git a/src/app/theme/theme-import/theme-import.component.html b/src/app/theme/theme-import/theme-import.component.html
@@ -19,8 +19,7 @@
     (onRemove)="onImportThemeClear()"
     (onSelect)="onImportThemeSelect($event)"
     (uploadHandler)="onThemeUpload()"
-    [showUploadButton]="true"
-    [showUploadButton]="!themeImportError"
+    [showUploadButton]="displayName && !themeImportError"
     [uploadLabel]="'ACTIONS.UPLOAD' | translate"
     [cancelLabel]="'ACTIONS.CANCEL' | translate"
     [chooseLabel]="'ACTIONS.CHOOSE' | translate"

diff --git a/src/app/theme/theme-import/theme-import.component.ts b/src/app/theme/theme-import/theme-import.component.ts
@@ -20,7 +20,7 @@ export class ThemeImportComponent implements OnInit {
 
   public themes!: Theme[]
   public themeName = ''
-  public displayName = ''
+  public displayName: string | null = ''
   public themeNameExists = false
   public displayNameExists = false
   public themeImportError = false
@@ -53,7 +53,7 @@ export class ThemeImportComponent implements OnInit {
           if (themeSnapshot.themes) {
             const key: string[] = Object.keys(themeSnapshot.themes)
             this.themeName = key[0]
-            this.displayName = themeSnapshot.themes[key[0]].displayName ?? ''
+            this.displayName = themeSnapshot.themes[key[0]].displayName ?? null
             this.properties = themeSnapshot.themes[key[0]].properties
           }
           this.checkThemeExistence()
@@ -82,7 +82,7 @@ export class ThemeImportComponent implements OnInit {
   public onThemeUpload(): void {
     if (!this.themeSnapshot?.themes) return
     const key: string[] = Object.keys(this.themeSnapshot?.themes)
-    this.themeSnapshot.themes[key[0]].displayName = this.displayName
+    this.themeSnapshot.themes[key[0]].displayName = this.displayName ?? undefined
     if (key[0] !== this.themeName) {
       // save the theme properties to be reassigned on new key
       const themeProps = Object.getOwnPropertyDescriptor(this.themeSnapshot.themes, key[0])

diff --git a/src/assets/i18n/de.json b/src/assets/i18n/de.json
@@ -63,6 +63,7 @@
     },
     "CANCEL": "Abbrechen",
     "CHOOSE": "Auswählen",
+    "LOADING": "...wird geladen",
     "SAVE": "Speichern",
     "SAVE_AS": "Speichern als ...",
     "UPLOAD": "Hochladen",

diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json
@@ -63,6 +63,7 @@
     },
     "CANCEL": "Cancel",
     "CHOOSE": "Choose",
+    "LOADING": "...is loading",
     "SAVE": "Save",
     "SAVE_AS": "Save as ...",
     "UPLOAD": "Upload",