Merge pull request #700 from atmire/Fix-context-sensitive-menus

Fix context sensitive menus

src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts

@@ -13,6 +13,8 @@ import { AuthService } from '../../core/auth/auth.service';
 import { of as observableOf } from 'rxjs';
 import { By } from '@angular/platform-browser';
 import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
+import { RouterTestingModule } from '@angular/router/testing';
+import { ActivatedRoute } from '@angular/router';
 
 describe('AdminSidebarComponent', () => {
   let comp: AdminSidebarComponent;
@@ -21,13 +23,14 @@ describe('AdminSidebarComponent', () => {
 
   beforeEach(async(() => {
     TestBed.configureTestingModule({
-      imports: [TranslateModule.forRoot(), NoopAnimationsModule],
+      imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule],
       declarations: [AdminSidebarComponent],
       providers: [
         { provide: Injector, useValue: {} },
         { provide: MenuService, useValue: menuService },
         { provide: CSSVariableService, useClass: CSSVariableServiceStub },
         { provide: AuthService, useClass: AuthServiceStub },
+        { provide: ActivatedRoute, useValue: {} },
         {
           provide: NgbModal, useValue: {
             open: () => {/*comment*/}

src/app/+admin/admin-sidebar/admin-sidebar.component.ts

@@ -93,7 +93,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
   /**
    * Initialize all menu sections and items for this menu
    */
-  private createMenu() {
+  createMenu() {
     const menuList = [
       /* News */
       {
@@ -472,7 +472,9 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
         index: 10
       },
     ];
-    menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, menuSection));
+    menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, Object.assign(menuSection, {
+      shouldPersistOnRouteChange: true
+    })));
 
   }
 

src/app/core/core.effects.ts

@@ -7,6 +7,7 @@ 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,
@@ -17,5 +18,6 @@ export const coreEffects = [
   ServerSyncBufferEffects,
   ObjectUpdatesEffects,
   RouteEffects,
-  RouterEffects
+  RouterEffects,
+  MenuEffects
 ];

src/app/navbar/navbar.component.spec.ts

@@ -11,6 +11,8 @@ import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 import { Injector, NO_ERRORS_SCHEMA } from '@angular/core';
 import { MenuService } from '../shared/menu/menu.service';
 import { MenuServiceStub } from '../shared/testing/menu-service.stub';
+import { ActivatedRoute } from '@angular/router';
+import { RouterTestingModule } from '@angular/router/testing';
 
 let comp: NavbarComponent;
 let fixture: ComponentFixture<NavbarComponent>;
@@ -24,12 +26,14 @@ describe('NavbarComponent', () => {
       imports: [
         TranslateModule.forRoot(),
         NoopAnimationsModule,
-        ReactiveFormsModule],
+        ReactiveFormsModule,
+        RouterTestingModule],
       declarations: [NavbarComponent],
       providers: [
         { provide: Injector, useValue: {} },
         { provide: MenuService, useValue: menuService },
         { provide: HostWindowService, useValue: new HostWindowServiceStub(800) },
+        { provide: ActivatedRoute, useValue: {} }
       ],
       schemas: [NO_ERRORS_SCHEMA]
     })

src/app/navbar/navbar.component.ts

@@ -17,7 +17,7 @@ import { environment } from '../../environments/environment';
   templateUrl: './navbar.component.html',
   animations: [slideMobileNav]
 })
-export class NavbarComponent extends MenuComponent implements OnInit {
+export class NavbarComponent extends MenuComponent {
   /**
    * The menu ID of the Navbar is PUBLIC
    * @type {MenuID.PUBLIC}
@@ -93,7 +93,9 @@ export class NavbarComponent extends MenuComponent implements OnInit {
         } as LinkMenuItemModel
       });
     });
-    menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, menuSection));
+    menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, Object.assign(menuSection, {
+      shouldPersistOnRouteChange: true
+    })));
 
   }
 

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

@@ -7,19 +7,25 @@ import { MenuComponent } from './menu.component';
 import { MenuServiceStub } from '../testing/menu-service.stub';
 import { of as observableOf } from 'rxjs';
 import { MenuSection } from './menu.reducer';
+import { Router } from '@angular/router';
+import { RouterTestingModule } from '@angular/router/testing';
+import { MenuID } from './initial-menus-state';
 
 describe('MenuComponent', () => {
   let comp: MenuComponent;
   let fixture: ComponentFixture<MenuComponent>;
   let menuService: MenuService;
+  let router: any;
+
+  const mockMenuID = 'mock-menuID' as MenuID;
 
   beforeEach(async(() => {
     TestBed.configureTestingModule({
-      imports: [TranslateModule.forRoot(), NoopAnimationsModule],
+      imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule],
       declarations: [MenuComponent],
       providers: [
         { provide: Injector, useValue: {} },
-        { provide: MenuService, useClass: MenuServiceStub },
+        { provide: MenuService, useClass: MenuServiceStub }
       ],
       schemas: [NO_ERRORS_SCHEMA]
     }).overrideComponent(MenuComponent, {
@@ -30,7 +36,9 @@ describe('MenuComponent', () => {
   beforeEach(() => {
     fixture = TestBed.createComponent(MenuComponent);
     comp = fixture.componentInstance; // SearchPageComponent test instance
+    comp.menuID = mockMenuID;
     menuService = (comp as any).menuService;
+    router = TestBed.get(Router);
     spyOn(comp as any, 'getSectionDataInjector').and.returnValue(MenuSection);
     spyOn(comp as any, 'getSectionComponent').and.returnValue(observableOf({}));
     fixture.detectChanges();

src/app/shared/menu/menu.component.ts

@@ -1,13 +1,14 @@
-import { ChangeDetectionStrategy, Component, Injector, OnInit } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Injector, OnDestroy, OnInit } from '@angular/core';
 import { Observable } from 'rxjs/internal/Observable';
-import { MenuService } from '../../shared/menu/menu.service';
-import { MenuID } from '../../shared/menu/initial-menus-state';
-import { MenuSection } from '../../shared/menu/menu.reducer';
-import { first, map } from 'rxjs/operators';
+import { MenuService } from './menu.service';
+import { MenuID } from './initial-menus-state';
+import { MenuSection } from './menu.reducer';
+import { distinctUntilChanged, first, map } from 'rxjs/operators';
 import { GenericConstructor } from '../../core/shared/generic-constructor';
 import { hasValue } from '../empty.util';
 import { MenuSectionComponent } from './menu-section/menu-section.component';
 import { getComponentForMenu } from './menu-section.decorator';
+import { Subscription } from 'rxjs/internal/Subscription';
 
 /**
  * A basic implementation of a MenuComponent
@@ -16,7 +17,7 @@ import { getComponentForMenu } from './menu-section.decorator';
   selector: 'ds-menu',
   template: ''
 })
-export class MenuComponent implements OnInit {
+export class MenuComponent implements OnInit, OnDestroy {
   /**
    * The ID of the Menu (See MenuID)
    */
@@ -62,6 +63,12 @@ export class MenuComponent implements OnInit {
    */
   private previewTimer;
 
+  /**
+   * Array to track all subscriptions and unsubscribe them onDestroy
+   * @type {Array}
+   */
+  subs: Subscription[] = [];
+
   constructor(protected menuService: MenuService, protected injector: Injector) {
   }
 
@@ -72,13 +79,13 @@ export class MenuComponent implements OnInit {
     this.menuCollapsed = this.menuService.isMenuCollapsed(this.menuID);
     this.menuPreviewCollapsed = this.menuService.isMenuPreviewCollapsed(this.menuID);
     this.menuVisible = this.menuService.isMenuVisible(this.menuID);
-    this.sections = this.menuService.getMenuTopSections(this.menuID).pipe(first());
-    this.sections.subscribe((sections: MenuSection[]) => {
+    this.sections = this.menuService.getMenuTopSections(this.menuID).pipe(distinctUntilChanged((x, y) => JSON.stringify(x) === JSON.stringify(y)));
+    this.subs.push(this.sections.subscribe((sections: MenuSection[]) => {
       sections.forEach((section: MenuSection) => {
         this.sectionInjectors.set(section.id, this.getSectionDataInjector(section));
         this.getSectionComponent(section).pipe(first()).subscribe((constr) => this.sectionComponents.set(section.id, constr));
       });
-    });
+    }));
   }
 
   /**
@@ -164,4 +171,13 @@ export class MenuComponent implements OnInit {
       parent: this.injector
     });
   }
+
+  /**
+   * Unsubscribe from open subscriptions
+   */
+  ngOnDestroy(): void {
+    this.subs
+      .filter((subscription) => hasValue(subscription))
+      .forEach((subscription) => subscription.unsubscribe());
+  }
 }

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

@@ -0,0 +1,129 @@
+import { MenuID, MenuItemType } from './initial-menus-state';
+import { MenuSection } from './menu.reducer';
+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 } from 'rxjs/internal/Observable';
+import { provideMockActions } from '@ngrx/effects/testing';
+import { of as observableOf } from 'rxjs';
+import { cold, hot } from 'jasmine-marbles';
+import { ROUTER_NAVIGATED } from '@ngrx/router-store';
+import { MenuEffects } from './menu.effects';
+
+describe('MenuEffects', () => {
+  let menuEffects: MenuEffects;
+  let routeDataMenuSection: MenuSection;
+  let routeDataMenuChildSection: MenuSection;
+  let toBeRemovedMenuSection: MenuSection;
+  let alreadyPresentMenuSection: MenuSection;
+  let route;
+  let menuService;
+  let actions: Observable<any>;
+
+  function init() {
+    routeDataMenuSection = {
+      id: 'mockSection',
+      active: false,
+      visible: true,
+      model: {
+        type: MenuItemType.LINK,
+        text: 'menu.section.mockSection',
+        link: ''
+      } 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]
+            }
+          }
+        },
+        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.get(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, routeDataMenuSection);
+      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

@@ -0,0 +1,84 @@
+import { ActivatedRoute } from '@angular/router';
+import { MenuSection } from './menu.reducer';
+import { hasNoValue, hasValue } from '../empty.util';
+import { Actions, Effect, ofType } from '@ngrx/effects';
+import { MenuService } from './menu.service';
+import { Observable } from 'rxjs/internal/Observable';
+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 { MenuID } from './initial-menus-state';
+
+/**
+ * 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
+   */
+  @Effect({ dispatch: false })
+  public buildRouteMenuSections$: Observable<Action> = this.actions$
+    .pipe(
+      ofType(ROUTER_NAVIGATED),
+      tap(() => {
+        Object.values(MenuID).forEach((menuID) => {
+          this.buildRouteMenuSections(menuID);
+        });
+      })
+    );
+
+  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 last: boolean = hasNoValue(route.firstChild);
+
+    if (hasValue(data) && hasValue(data.menu) && hasValue(data.menu[menuID])) {
+      if (!last) {
+        return [...data.menu[menuID], ...this.resolveRouteMenuSections(route.firstChild, menuID)]
+      } else {
+        return [...data.menu[menuID]];
+      }
+    }
+
+    return !last ? this.resolveRouteMenuSections(route.firstChild, menuID) : [];
+  }
+
+}

src/app/shared/menu/menu.reducer.ts

@@ -58,6 +58,7 @@ export class MenuSection {
   model: MenuItemModel;
   index?: number;
   icon?: string;
+  shouldPersistOnRouteChange? = false;
 }
 
 /**
@@ -176,7 +177,9 @@ function removeSection(state: MenusState, action: RemoveMenuSectionAction) {
   const menuState: MenuState = state[action.menuID];
   const id = action.id;
   const newState = removeFromIndex(state, menuState.sections[action.id], action.menuID);
-  const newMenuState = Object.assign({}, newState[action.menuID]);
+  const newMenuState = Object.assign({}, newState[action.menuID], {
+    sections: Object.assign({}, newState[action.menuID].sections)
+  });
   delete newMenuState.sections[id];
   return Object.assign({}, newState, { [action.menuID]: newMenuState });
 }

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

@@ -19,7 +19,7 @@ import {
   ToggleActiveMenuSectionAction,
   ToggleMenuAction,
 } from './menu.actions';
-import { hasNoValue, hasValue, isNotEmpty } from '../empty.util';
+import { hasNoValue, hasValue, hasValueOperator, isNotEmpty } from '../empty.util';
 
 export function menuKeySelector<T>(key: string, selector): MemoizedSelector<MenuState, T> {
   return createSelector(selector, (state) => {
@@ -98,7 +98,7 @@ export class MenuService {
       switchMap((ids: string[]) =>
         observableCombineLatest(ids.map((id: string) => this.getMenuSection(menuID, id)))
       ),
-      map((sections: MenuSection[]) => sections.filter((section: MenuSection) => !mustBeVisible || section.visible))
+      map((sections: MenuSection[]) => sections.filter((section: MenuSection) => hasValue(section) && (!mustBeVisible || section.visible)))
     );
   }
 
@@ -129,6 +129,16 @@ export class MenuService {
     );
   }
 
+  /**
+   * Retrieve menu sections that shouldn't persist on route change
+   * @param menuID  The ID of the menu the sections reside in
+   */
+  getNonPersistentMenuSections(menuID: MenuID): Observable<MenuSection[]> {
+    return this.getMenu(menuID).pipe(
+      map((state: MenuState) => Object.values(state.sections).filter((section: MenuSection) => !section.shouldPersistOnRouteChange))
+    );
+  }
+
   /**
    * Add a new section to the store
    * @param {MenuID} menuID The menu to which the new section is to be added
@@ -270,7 +280,7 @@ export class MenuService {
    * @returns {Observable<boolean>} Emits true when the given section is currently active, false when the given section is currently inactive
    */
   isSectionActive(menuID: MenuID, id: string): Observable<boolean> {
-    return this.getMenuSection(menuID, id).pipe(map((section) => section.active));
+    return this.getMenuSection(menuID, id).pipe(hasValueOperator(), map((section) => section.active));
   }
 
   /**

src/app/shared/testing/menu-service.stub.ts

@@ -59,6 +59,9 @@ export class MenuServiceStub {
   removeSection(): void { /***/
   };
 
+  resetSections(): void { /***/
+  };
+
   isMenuVisible(id: MenuID): Observable<boolean> {
     return observableOf(true)
   };