diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts index 581d3341da..e3e8d08753 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts +++ b/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*/} diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index 97b791c067..eedebae80b 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/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 + }))); } diff --git a/src/app/core/core.effects.ts b/src/app/core/core.effects.ts index 9f4242cc9a..cd14a527e8 100644 --- a/src/app/core/core.effects.ts +++ b/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 ]; diff --git a/src/app/navbar/navbar.component.spec.ts b/src/app/navbar/navbar.component.spec.ts index 206afda003..907ddc8b87 100644 --- a/src/app/navbar/navbar.component.spec.ts +++ b/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; @@ -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] }) diff --git a/src/app/navbar/navbar.component.ts b/src/app/navbar/navbar.component.ts index 6055aac263..ae9000352a 100644 --- a/src/app/navbar/navbar.component.ts +++ b/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 + }))); } diff --git a/src/app/shared/menu/menu.component.spec.ts b/src/app/shared/menu/menu.component.spec.ts index 625c96e795..2b83ec4414 100644 --- a/src/app/shared/menu/menu.component.spec.ts +++ b/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; 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(); diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index 1e81fa2dda..28e906c456 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/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()); + } } diff --git a/src/app/shared/menu/menu.effects.spec.ts b/src/app/shared/menu/menu.effects.spec.ts new file mode 100644 index 0000000000..11b468eded --- /dev/null +++ b/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; + + 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); + }); + }); +}); diff --git a/src/app/shared/menu/menu.effects.ts b/src/app/shared/menu/menu.effects.ts new file mode 100644 index 0000000000..be314cfd49 --- /dev/null +++ b/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 = 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) : []; + } + +} diff --git a/src/app/shared/menu/menu.reducer.ts b/src/app/shared/menu/menu.reducer.ts index 078343a990..0ef328aa15 100644 --- a/src/app/shared/menu/menu.reducer.ts +++ b/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 }); } diff --git a/src/app/shared/menu/menu.service.ts b/src/app/shared/menu/menu.service.ts index 95611daa21..fd3ba1ed5c 100644 --- a/src/app/shared/menu/menu.service.ts +++ b/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(key: string, selector): MemoizedSelector { 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 { + 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} Emits true when the given section is currently active, false when the given section is currently inactive */ isSectionActive(menuID: MenuID, id: string): Observable { - return this.getMenuSection(menuID, id).pipe(map((section) => section.active)); + return this.getMenuSection(menuID, id).pipe(hasValueOperator(), map((section) => section.active)); } /** diff --git a/src/app/shared/testing/menu-service.stub.ts b/src/app/shared/testing/menu-service.stub.ts index de71e3483d..0246aa39d1 100644 --- a/src/app/shared/testing/menu-service.stub.ts +++ b/src/app/shared/testing/menu-service.stub.ts @@ -59,6 +59,9 @@ export class MenuServiceStub { removeSection(): void { /***/ }; + resetSections(): void { /***/ + }; + isMenuVisible(id: MenuID): Observable { return observableOf(true) };