Fix issue with onClick models in dso edit menus

src/app/app.metareducers.ts

@@ -1,4 +1,5 @@
 import { StoreActionTypes } from './store.actions';
+import { initialMenusState } from './shared/menu/initial-menus-state';
 
 // fallback ngrx debugger
 let actionCounter = 0;
@@ -18,7 +19,14 @@ export function universalMetaReducer(reducer) {
   return (state, action) => {
     switch (action.type) {
       case StoreActionTypes.REHYDRATE:
-        state = Object.assign({}, state, action.payload);
+        state = Object.assign({}, state, action.payload, {
+          /**
+           * Reset menus after the store is rehydrated, in order to force them to be recreated client side.
+           * The reason is that menu options stored on the server may contain methods that don't survive the
+           * (de)serialization to/from JSON
+           */
+          menus: initialMenusState
+        });
         break;
       case StoreActionTypes.REPLAY:
       default:

src/app/core/core.effects.ts

@@ -7,7 +7,6 @@ import { ServerSyncBufferEffects } from './cache/server-sync-buffer.effects';
 import { ObjectUpdatesEffects } from './data/object-updates/object-updates.effects';
 import { RouteEffects } from './services/route.effects';
 import { RouterEffects } from './router/router.effects';
-import { MenuEffects } from '../shared/menu/menu.effects';
 
 export const coreEffects = [
   RequestEffects,
@@ -19,5 +18,4 @@ export const coreEffects = [
   ObjectUpdatesEffects,
   RouteEffects,
   RouterEffects,
-  MenuEffects
 ];

src/app/init.service.spec.ts

@@ -124,6 +124,7 @@ describe('InitService', () => {
     let transferStateSpy;
     let metadataServiceSpy;
     let breadcrumbsServiceSpy;
+    let menuServiceSpy;
 
     const BLOCKING = {
       t: {  core: { auth: { blocking: true } } },
@@ -150,6 +151,9 @@ describe('InitService', () => {
       metadataServiceSpy = jasmine.createSpyObj('metadataService', [
         'listenForRouteChange',
       ]);
+      menuServiceSpy = jasmine.createSpyObj('menuServiceSpy', [
+        'listenForRouteChanges',
+      ]);
 
 
       TestBed.resetTestingModule();
@@ -175,7 +179,7 @@ describe('InitService', () => {
           { provide: AuthService, useValue: new AuthServiceMock() },
           { provide: Router, useValue: new RouterMock() },
           { provide: ActivatedRoute, useValue: new MockActivatedRoute() },
-          { provide: MenuService, useValue: new MenuServiceStub() },
+          { provide: MenuService, useValue: menuServiceSpy },
           { provide: ThemeService, useValue: getMockThemeService() },
           provideMockStore({ initialState }),
           AppComponent,
@@ -190,6 +194,7 @@ describe('InitService', () => {
         service.initRouteListeners();
         expect(metadataServiceSpy.listenForRouteChange).toHaveBeenCalledTimes(1);
         expect(breadcrumbsServiceSpy.listenForRouteChanges).toHaveBeenCalledTimes(1);
+        expect(breadcrumbsServiceSpy.listenForRouteChanges).toHaveBeenCalledTimes(1);
       }));
     });
 

src/app/init.service.ts

@@ -23,6 +23,7 @@ import { ThemeService } from './shared/theme-support/theme.service';
 import { isAuthenticationBlocking } from './core/auth/selectors';
 import { distinctUntilChanged, find } from 'rxjs/operators';
 import { Observable } from 'rxjs';
+import { MenuService } from './shared/menu/menu.service';
 
 /**
  * Performs the initialization of the app.
@@ -51,6 +52,8 @@ export abstract class InitService {
     protected metadata: MetadataService,
     protected breadcrumbsService: BreadcrumbsService,
     protected themeService: ThemeService,
+    protected menuService: MenuService,
+
   ) {
   }
 
@@ -184,6 +187,7 @@ export abstract class InitService {
     this.metadata.listenForRouteChange();
     this.breadcrumbsService.listenForRouteChanges();
     this.themeService.listenForRouteChanges();
+    this.menuService.listenForRouteChanges();
   }
 
   /**

src/app/shared/menu/menu.effects.spec.ts

@@ -1,144 +0,0 @@
-import { LinkMenuItemModel } from './menu-item/models/link.model';
-import { TestBed } from '@angular/core/testing';
-import { MenuService } from './menu.service';
-import { ActivatedRoute } from '@angular/router';
-import { Observable, of as observableOf } from 'rxjs';
-import { provideMockActions } from '@ngrx/effects/testing';
-import { cold, hot } from 'jasmine-marbles';
-import { ROUTER_NAVIGATED } from '@ngrx/router-store';
-import { MenuEffects } from './menu.effects';
-import { MenuSection } from './menu-section.model';
-import { MenuID } from './menu-id.model';
-import { MenuItemType } from './menu-item-type.model';
-
-describe('MenuEffects', () => {
-  let menuEffects: MenuEffects;
-  let routeDataMenuSection: MenuSection;
-  let routeDataMenuSectionResolved: MenuSection;
-  let routeDataMenuChildSection: MenuSection;
-  let toBeRemovedMenuSection: MenuSection;
-  let alreadyPresentMenuSection: MenuSection;
-  let route;
-  let menuService;
-  let actions: Observable<any>;
-
-  function init() {
-    routeDataMenuSection = {
-      id: 'mockSection_:idparam',
-      active: false,
-      visible: true,
-      model: {
-        type: MenuItemType.LINK,
-        text: 'menu.section.mockSection',
-        link: 'path/:linkparam'
-      } as LinkMenuItemModel
-    };
-    routeDataMenuSectionResolved = {
-      id: 'mockSection_id_param_resolved',
-      active: false,
-      visible: true,
-      model: {
-        type: MenuItemType.LINK,
-        text: 'menu.section.mockSection',
-        link: 'path/link_param_resolved'
-      } as LinkMenuItemModel
-    };
-    routeDataMenuChildSection = {
-      id: 'mockChildSection',
-      parentID: 'mockSection',
-      active: false,
-      visible: true,
-      model: {
-        type: MenuItemType.LINK,
-        text: 'menu.section.mockChildSection',
-        link: ''
-      } as LinkMenuItemModel
-    };
-    toBeRemovedMenuSection = {
-      id: 'toBeRemovedSection',
-      active: false,
-      visible: true,
-      model: {
-        type: MenuItemType.LINK,
-        text: 'menu.section.toBeRemovedSection',
-        link: ''
-      } as LinkMenuItemModel
-    };
-    alreadyPresentMenuSection = {
-      id: 'alreadyPresentSection',
-      active: false,
-      visible: true,
-      model: {
-        type: MenuItemType.LINK,
-        text: 'menu.section.alreadyPresentSection',
-        link: ''
-      } as LinkMenuItemModel
-    };
-    route = {
-      root: {
-        snapshot: {
-          data: {
-            menu: {
-              [MenuID.PUBLIC]: [routeDataMenuSection, alreadyPresentMenuSection]
-            }
-          },
-          params: {
-            idparam: 'id_param_resolved',
-            linkparam: 'link_param_resolved',
-          }
-        },
-        firstChild: {
-          snapshot: {
-            data: {
-              menu: {
-                [MenuID.PUBLIC]: routeDataMenuChildSection
-              }
-            }
-          }
-        }
-      }
-    };
-
-    menuService = jasmine.createSpyObj('menuService', {
-      getNonPersistentMenuSections: observableOf([toBeRemovedMenuSection, alreadyPresentMenuSection]),
-      addSection: {},
-      removeSection: {}
-    });
-  }
-
-  beforeEach(() => {
-    init();
-    TestBed.configureTestingModule({
-      providers: [
-        MenuEffects,
-        { provide: MenuService, useValue: menuService },
-        { provide: ActivatedRoute, useValue: route },
-        provideMockActions(() => actions)
-      ]
-    });
-
-    menuEffects = TestBed.inject(MenuEffects);
-  });
-
-  describe('buildRouteMenuSections$', () => {
-    it('should add and remove menu sections depending on the current route', () => {
-      actions = hot('--a-', {
-        a: {
-          type: ROUTER_NAVIGATED
-        }
-      });
-
-      const expected = cold('--b-', {
-        b: {
-          type: ROUTER_NAVIGATED
-        }
-      });
-
-      expect(menuEffects.buildRouteMenuSections$).toBeObservable(expected);
-      expect(menuService.addSection).toHaveBeenCalledWith(MenuID.PUBLIC, routeDataMenuSectionResolved);
-      expect(menuService.addSection).toHaveBeenCalledWith(MenuID.PUBLIC, routeDataMenuChildSection);
-      expect(menuService.addSection).not.toHaveBeenCalledWith(MenuID.PUBLIC, alreadyPresentMenuSection);
-      expect(menuService.removeSection).toHaveBeenCalledWith(MenuID.PUBLIC, toBeRemovedMenuSection.id);
-    });
-  });
-});

src/app/shared/menu/menu.effects.ts

@@ -1,122 +0,0 @@
-import { ActivatedRoute } from '@angular/router';
-import { hasNoValue, hasValue } from '../empty.util';
-import { Actions, createEffect, ofType } from '@ngrx/effects';
-import { MenuService } from './menu.service';
-import { Observable } from 'rxjs';
-import { Action } from '@ngrx/store';
-import { ROUTER_NAVIGATED } from '@ngrx/router-store';
-import { Injectable } from '@angular/core';
-import { map, take, tap } from 'rxjs/operators';
-import { MenuSection } from './menu-section.model';
-import { MenuID } from './menu-id.model';
-
-/**
- * Effects modifying the state of menus
- */
-@Injectable()
-export class MenuEffects {
-
-  /**
-   * On route change, build menu sections for every menu type depending on the current route data
-   */
-  public buildRouteMenuSections$: Observable<Action> = createEffect(() => this.actions$
-    .pipe(
-      ofType(ROUTER_NAVIGATED),
-      tap(() => {
-        Object.values(MenuID).forEach((menuID) => {
-          this.buildRouteMenuSections(menuID);
-        });
-      })
-    ), { dispatch: false });
-
-  constructor(private actions$: Actions,
-    private menuService: MenuService,
-    private route: ActivatedRoute) {
-  }
-
-  /**
-   * Build menu sections depending on the current route
-   * - Adds sections found in the current route data that aren't active yet
-   * - Removes sections that are active, but not present in the current route data
-   * @param menuID  The menu to add/remove sections to/from
-   */
-  buildRouteMenuSections(menuID: MenuID) {
-    this.menuService.getNonPersistentMenuSections(menuID).pipe(
-      map((sections) => sections.map((section) => section.id)),
-      take(1)
-    ).subscribe((shouldNotPersistIDs: string[]) => {
-      const resolvedSections = this.resolveRouteMenuSections(this.route.root, menuID);
-      resolvedSections.forEach((section) => {
-        const index = shouldNotPersistIDs.indexOf(section.id);
-        if (index > -1) {
-          shouldNotPersistIDs.splice(index, 1);
-        } else {
-          this.menuService.addSection(menuID, section);
-        }
-      });
-      shouldNotPersistIDs.forEach((id) => {
-        this.menuService.removeSection(menuID, id);
-      });
-    });
-  }
-
-  /**
-   * Resolve menu sections defined in the current route data (including parent routes)
-   * @param route   The route to resolve data for
-   * @param menuID  The menu to resolve data for
-   */
-  resolveRouteMenuSections(route: ActivatedRoute, menuID: MenuID): MenuSection[] {
-    const data = route.snapshot.data;
-    const params = route.snapshot.params;
-    const last: boolean = hasNoValue(route.firstChild);
-
-    if (hasValue(data) && hasValue(data.menu) && hasValue(data.menu[menuID])) {
-      let menuSections: MenuSection[] | MenuSection = data.menu[menuID];
-      menuSections = this.resolveSubstitutions(menuSections, params);
-
-      if (!Array.isArray(menuSections)) {
-        menuSections = [menuSections];
-      }
-
-      if (!last) {
-        return [...menuSections, ...this.resolveRouteMenuSections(route.firstChild, menuID)];
-      } else {
-        return [...menuSections];
-      }
-    }
-
-    return !last ? this.resolveRouteMenuSections(route.firstChild, menuID) : [];
-  }
-
-  private resolveSubstitutions(object, params) {
-
-    let resolved;
-    if (typeof object === 'string') {
-      resolved = object;
-      let match: RegExpMatchArray;
-      do {
-        match = resolved.match(/:(\w+)/);
-        if (match) {
-          const substitute = params[match[1]];
-          if (hasValue(substitute)) {
-            resolved = resolved.replace(match[0], `${substitute}`);
-          }
-        }
-      } while (match);
-    } else if (Array.isArray(object)) {
-      resolved = [];
-      object.forEach((entry, index) => {
-        resolved[index] = this.resolveSubstitutions(object[index], params);
-      });
-    } else if (typeof object === 'object') {
-      resolved = {};
-      Object.keys(object).forEach((key) => {
-        resolved[key] = this.resolveSubstitutions(object[key], params);
-      });
-    } else {
-      resolved = object;
-    }
-    return resolved;
-  }
-
-}

src/app/shared/menu/menu.service.spec.ts

@@ -24,6 +24,9 @@ import { menusReducer } from './menu.reducer';
 import { storeModuleConfig } from '../../app.reducer';
 import { MenuSection } from './menu-section.model';
 import { MenuID } from './menu-id.model';
+import { MenuItemType } from './menu-item-type.model';
+import { LinkMenuItemModel } from './menu-item/models/link.model';
+import { NavigationEnd } from '@angular/router';
 
 describe('MenuService', () => {
   let service: MenuService;
@@ -35,6 +38,14 @@ describe('MenuService', () => {
   let subSection4;
   let topSections;
   let initialState;
+  let routeDataMenuSection: MenuSection;
+  let routeDataMenuSectionResolved: MenuSection;
+  let routeDataMenuChildSection: MenuSection;
+  let toBeRemovedMenuSection: MenuSection;
+  let alreadyPresentMenuSection: MenuSection;
+  let route;
+  let router;
+
 
   function init() {
 
@@ -85,6 +96,85 @@ describe('MenuService', () => {
       }
     };
 
+    routeDataMenuSection = {
+      id: 'mockSection_:idparam',
+      active: false,
+      visible: true,
+      model: {
+        type: MenuItemType.LINK,
+        text: 'menu.section.mockSection',
+        link: 'path/:linkparam'
+      } as LinkMenuItemModel
+    };
+    routeDataMenuSectionResolved = {
+      id: 'mockSection_id_param_resolved',
+      active: false,
+      visible: true,
+      model: {
+        type: MenuItemType.LINK,
+        text: 'menu.section.mockSection',
+        link: 'path/link_param_resolved'
+      } as LinkMenuItemModel
+    };
+    routeDataMenuChildSection = {
+      id: 'mockChildSection',
+      parentID: 'mockSection',
+      active: false,
+      visible: true,
+      model: {
+        type: MenuItemType.LINK,
+        text: 'menu.section.mockChildSection',
+        link: ''
+      } as LinkMenuItemModel
+    };
+    toBeRemovedMenuSection = {
+      id: 'toBeRemovedSection',
+      active: false,
+      visible: true,
+      model: {
+        type: MenuItemType.LINK,
+        text: 'menu.section.toBeRemovedSection',
+        link: ''
+      } as LinkMenuItemModel
+    };
+    alreadyPresentMenuSection = {
+      id: 'alreadyPresentSection',
+      active: false,
+      visible: true,
+      model: {
+        type: MenuItemType.LINK,
+        text: 'menu.section.alreadyPresentSection',
+        link: ''
+      } as LinkMenuItemModel
+    };
+    route = {
+      root: {
+        snapshot: {
+          data: {
+            menu: {
+              [MenuID.PUBLIC]: [routeDataMenuSection, alreadyPresentMenuSection]
+            }
+          },
+          params: {
+            idparam: 'id_param_resolved',
+            linkparam: 'link_param_resolved',
+          }
+        },
+        firstChild: {
+          snapshot: {
+            data: {
+              menu: {
+                [MenuID.PUBLIC]: routeDataMenuChildSection
+              }
+            }
+          }
+        }
+      }
+    };
+
+    router = {
+      events: observableOf(new NavigationEnd(1, 'test-url', 'test-url'))
+    };
   }
 
   beforeEach(waitForAsync(() => {
@@ -102,7 +192,7 @@ describe('MenuService', () => {
 
   beforeEach(() => {
     store = TestBed.inject(Store);
-    service = new MenuService(store);
+    service = new MenuService(store, route, router);
     spyOn(store, 'dispatch');
   });
 
@@ -371,4 +461,32 @@ describe('MenuService', () => {
       expect(store.dispatch).toHaveBeenCalledWith(new DeactivateMenuSectionAction(MenuID.ADMIN, 'fakeID'));
     });
   });
+
+  describe('buildRouteMenuSections', () => {
+    it('should add and remove menu sections depending on the current route', () => {
+      spyOn(service, 'addSection');
+      spyOn(service, 'removeSection');
+
+      spyOn(service, 'getNonPersistentMenuSections').and.returnValue(observableOf([toBeRemovedMenuSection, alreadyPresentMenuSection]));
+
+      service.buildRouteMenuSections(MenuID.PUBLIC);
+
+      expect(service.addSection).toHaveBeenCalledWith(MenuID.PUBLIC, routeDataMenuSectionResolved);
+      expect(service.addSection).toHaveBeenCalledWith(MenuID.PUBLIC, routeDataMenuChildSection);
+      expect(service.addSection).not.toHaveBeenCalledWith(MenuID.PUBLIC, alreadyPresentMenuSection);
+      expect(service.removeSection).toHaveBeenCalledWith(MenuID.PUBLIC, toBeRemovedMenuSection.id);
+    });
+  });
+
+  describe('listenForRouteChanges', () => {
+    it('should build the menu sections on NavigationEnd event', () => {
+      spyOn(service, 'buildRouteMenuSections');
+
+      service.listenForRouteChanges();
+
+      expect(service.buildRouteMenuSections).toHaveBeenCalledWith(MenuID.ADMIN);
+      expect(service.buildRouteMenuSections).toHaveBeenCalledWith(MenuID.PUBLIC);
+    });
+  });
+
 });

src/app/shared/menu/menu.service.ts

@@ -2,7 +2,7 @@ import { Injectable } from '@angular/core';
 import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store';
 import { AppState, keySelector } from '../../app.reducer';
 import { combineLatest as observableCombineLatest, Observable } from 'rxjs';
-import { map, switchMap } from 'rxjs/operators';
+import { filter, map, switchMap, take } from 'rxjs/operators';
 import {
   ActivateMenuSectionAction,
   AddMenuSectionAction,
@@ -22,6 +22,7 @@ import { MenuState } from './menu-state.model';
 import { MenuSections } from './menu-sections.model';
 import { MenuSection } from './menu-section.model';
 import { MenuID } from './menu-id.model';
+import { ActivatedRoute, NavigationEnd, Router } from '@angular/router';
 
 export function menuKeySelector<T>(key: string, selector): MemoizedSelector<MenuState, T> {
   return createSelector(selector, (state) => {
@@ -54,7 +55,11 @@ const getSubSectionsFromSectionSelector = (id: string): MemoizedSelector<MenuSta
 @Injectable()
 export class MenuService {
 
-  constructor(private store: Store<AppState>) {
+  constructor(
+    protected store: Store<AppState>,
+    protected route: ActivatedRoute,
+    protected router: Router,
+  ) {
   }
 
   /**
@@ -295,4 +300,100 @@ export class MenuService {
     return this.getMenuSection(menuID, id).pipe(map((section) => section.visible));
   }
 
+  listenForRouteChanges(): void {
+    this.router.events.pipe(
+      filter(event => event instanceof NavigationEnd),
+    ).subscribe(() => {
+      Object.values(MenuID).forEach((menuID) => {
+        this.buildRouteMenuSections(menuID);
+      });
+    });
+  }
+
+  /**
+   * Build menu sections depending on the current route
+   * - Adds sections found in the current route data that aren't active yet
+   * - Removes sections that are active, but not present in the current route data
+   * @param menuID  The menu to add/remove sections to/from
+   */
+  buildRouteMenuSections(menuID: MenuID) {
+    this.getNonPersistentMenuSections(menuID).pipe(
+      map((sections) => sections.map((section) => section.id)),
+      take(1)
+    ).subscribe((shouldNotPersistIDs: string[]) => {
+      const resolvedSections = this.resolveRouteMenuSections(this.route.root, menuID);
+      resolvedSections.forEach((section) => {
+        const index = shouldNotPersistIDs.indexOf(section.id);
+        if (index > -1) {
+          shouldNotPersistIDs.splice(index, 1);
+        } else {
+          this.addSection(menuID, section);
+        }
+      });
+      shouldNotPersistIDs.forEach((id) => {
+        this.removeSection(menuID, id);
+      });
+    });
+  }
+
+  /**
+   * Resolve menu sections defined in the current route data (including parent routes)
+   * @param route   The route to resolve data for
+   * @param menuID  The menu to resolve data for
+   */
+  resolveRouteMenuSections(route: ActivatedRoute, menuID: MenuID): MenuSection[] {
+    const data = route.snapshot.data;
+    const params = route.snapshot.params;
+    const last: boolean = hasNoValue(route.firstChild);
+
+    if (hasValue(data) && hasValue(data.menu) && hasValue(data.menu[menuID])) {
+      let menuSections: MenuSection[] | MenuSection = data.menu[menuID];
+      menuSections = this.resolveSubstitutions(menuSections, params);
+
+      if (!Array.isArray(menuSections)) {
+        menuSections = [menuSections];
+      }
+
+      if (!last) {
+        return [...menuSections, ...this.resolveRouteMenuSections(route.firstChild, menuID)];
+      } else {
+        return [...menuSections];
+      }
+    }
+
+    return !last ? this.resolveRouteMenuSections(route.firstChild, menuID) : [];
+  }
+
+  protected resolveSubstitutions(object, params) {
+
+    let resolved;
+    if (typeof object === 'string') {
+      resolved = object;
+      let match: RegExpMatchArray;
+      do {
+        match = resolved.match(/:(\w+)/);
+        if (match) {
+          const substitute = params[match[1]];
+          if (hasValue(substitute)) {
+            resolved = resolved.replace(match[0], `${substitute}`);
+          }
+        }
+      } while (match);
+    } else if (Array.isArray(object)) {
+      resolved = [];
+      object.forEach((entry, index) => {
+        resolved[index] = this.resolveSubstitutions(object[index], params);
+      });
+    } else if (typeof object === 'object') {
+      resolved = {};
+      Object.keys(object).forEach((key) => {
+        resolved[key] = this.resolveSubstitutions(object[key], params);
+      });
+    } else {
+      resolved = object;
+    }
+    return resolved;
+  }
+
+
 }

src/modules/app/browser-init.service.ts

@@ -29,6 +29,7 @@ import { coreSelector } from '../../app/core/core.selectors';
 import { find, map } from 'rxjs/operators';
 import { isNotEmpty } from '../../app/shared/empty.util';
 import { logStartupMessage } from '../../../startup-message';
+import { MenuService } from '../../app/shared/menu/menu.service';
 
 /**
  * Performs client-side initialization.
@@ -49,6 +50,7 @@ export class BrowserInitService extends InitService {
     protected klaroService: KlaroService,
     protected authService: AuthService,
     protected themeService: ThemeService,
+    protected menuService: MenuService,
   ) {
     super(
       store,
@@ -60,6 +62,7 @@ export class BrowserInitService extends InitService {
       metadata,
       breadcrumbsService,
       themeService,
+      menuService,
     );
   }
 

src/modules/app/server-init.service.ts

@@ -21,6 +21,7 @@ import { BreadcrumbsService } from '../../app/breadcrumbs/breadcrumbs.service';
 import { CSSVariableService } from '../../app/shared/sass-helper/sass-helper.service';
 import { ThemeService } from '../../app/shared/theme-support/theme.service';
 import { take } from 'rxjs/operators';
+import { MenuService } from '../../app/shared/menu/menu.service';
 
 /**
  * Performs server-side initialization.
@@ -39,6 +40,7 @@ export class ServerInitService extends InitService {
     protected breadcrumbsService: BreadcrumbsService,
     protected cssService: CSSVariableService,
     protected themeService: ThemeService,
+    protected menuService: MenuService,
   ) {
     super(
       store,
@@ -50,6 +52,7 @@ export class ServerInitService extends InitService {
       metadata,
       breadcrumbsService,
       themeService,
+      menuService,
     );
   }