From 3e8d180f1b92adc2c69a4f74d864d165150611ea Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Mon, 18 Sep 2023 16:43:32 +0200 Subject: [PATCH] 106974: Angular SSR menu issues --- src/app/core/core.effects.ts | 2 ++ src/app/shared/menu/menu.actions.ts | 11 ++++++- src/app/shared/menu/menu.effects.spec.ts | 41 ++++++++++++++++++++++++ src/app/shared/menu/menu.effects.ts | 23 +++++++++++++ src/app/shared/menu/menu.reducer.spec.ts | 13 +++++++- src/app/shared/menu/menu.reducer.ts | 19 ++++++----- src/app/shared/menu/menu.service.spec.ts | 27 ++++++++++++++-- src/app/shared/menu/menu.service.ts | 3 +- 8 files changed, 126 insertions(+), 13 deletions(-) create mode 100644 src/app/shared/menu/menu.effects.spec.ts create mode 100644 src/app/shared/menu/menu.effects.ts diff --git a/src/app/core/core.effects.ts b/src/app/core/core.effects.ts index b569df290d5..1724e88743b 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, @@ -18,4 +19,5 @@ export const coreEffects = [ ObjectUpdatesEffects, RouteEffects, RouterEffects, + MenuEffects, ]; diff --git a/src/app/shared/menu/menu.actions.ts b/src/app/shared/menu/menu.actions.ts index f02fd73292e..268c149c5ba 100644 --- a/src/app/shared/menu/menu.actions.ts +++ b/src/app/shared/menu/menu.actions.ts @@ -18,6 +18,7 @@ export const MenuActionTypes = { EXPAND_MENU: type('dspace/menu/EXPAND_MENU'), SHOW_MENU: type('dspace/menu/SHOW_MENU'), HIDE_MENU: type('dspace/menu/HIDE_MENU'), + REINIT_MENUS: type('dspace/menu/REINIT_MENUS'), COLLAPSE_MENU_PREVIEW: type('dspace/menu/COLLAPSE_MENU_PREVIEW'), EXPAND_MENU_PREVIEW: type('dspace/menu/EXPAND_MENU_PREVIEW'), ADD_SECTION: type('dspace/menu-section/ADD_SECTION'), @@ -115,6 +116,13 @@ export class ExpandMenuPreviewAction implements Action { } } +/** + * Action used to re-initialise the menus + */ +export class ReinitMenuAction implements Action { + type = MenuActionTypes.REINIT_MENUS; +} + // MENU SECTION ACTIONS /** * Action used to perform state changes for a section of a certain menu @@ -224,4 +232,5 @@ export type MenuAction = | DeactivateMenuSectionAction | ToggleActiveMenuSectionAction | CollapseMenuPreviewAction - | ExpandMenuPreviewAction; + | ExpandMenuPreviewAction + | ReinitMenuAction; 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 00000000000..5da53286747 --- /dev/null +++ b/src/app/shared/menu/menu.effects.spec.ts @@ -0,0 +1,41 @@ +import { Observable } from 'rxjs'; +import { TestBed, waitForAsync } from '@angular/core/testing'; +import { provideMockActions } from '@ngrx/effects/testing'; +import { cold, hot } from 'jasmine-marbles'; +import { MenuEffects } from './menu.effects'; +import { ReinitMenuAction } from './menu.actions'; +import { StoreAction, StoreActionTypes } from '../../store.actions'; + +describe('MenuEffects', () => { + let menuEffects: MenuEffects; + let actions: Observable; + beforeEach(waitForAsync(() => { + TestBed.configureTestingModule({ + providers: [ + MenuEffects, + provideMockActions(() => actions), + ], + }); + })); + + beforeEach(() => { + menuEffects = TestBed.inject(MenuEffects); + }); + + describe('reinitDSOMenus', () => { + describe('When a REHYDRATE action is triggered', () => { + let action; + beforeEach(() => { + action = new StoreAction(StoreActionTypes.REHYDRATE, null); + }); + it('should return a ReinitMenuAction', () => { + actions = hot('--a-', {a: action}); + const expected = cold('--b-', {b: new ReinitMenuAction}); + + expect(menuEffects.reinitDSOMenus).toBeObservable(expected); + }); + }); + }); + + +}); diff --git a/src/app/shared/menu/menu.effects.ts b/src/app/shared/menu/menu.effects.ts new file mode 100644 index 00000000000..2787f80b78a --- /dev/null +++ b/src/app/shared/menu/menu.effects.ts @@ -0,0 +1,23 @@ +import { map } from 'rxjs/operators'; +import { Injectable } from '@angular/core'; +import { Actions, createEffect, ofType } from '@ngrx/effects'; + +import { StoreActionTypes } from '../../store.actions'; +import { ReinitMenuAction } from './menu.actions'; + +@Injectable() +export class MenuEffects { + + /** + * When the store is rehydrated in the browser, re-initialise the menus to ensure + * the menus with functions are loaded correctly. + */ + reinitDSOMenus = createEffect(() => this.actions$ + .pipe(ofType(StoreActionTypes.REHYDRATE), + map(() => new ReinitMenuAction()) + )); + + constructor(private actions$: Actions) { + } + +} diff --git a/src/app/shared/menu/menu.reducer.spec.ts b/src/app/shared/menu/menu.reducer.spec.ts index 2865e887fca..8f540c016d5 100644 --- a/src/app/shared/menu/menu.reducer.spec.ts +++ b/src/app/shared/menu/menu.reducer.spec.ts @@ -9,7 +9,7 @@ import { ExpandMenuAction, ExpandMenuPreviewAction, HideMenuAction, - HideMenuSectionAction, + HideMenuSectionAction, ReinitMenuAction, RemoveMenuSectionAction, ShowMenuAction, ShowMenuSectionAction, @@ -317,6 +317,17 @@ describe('menusReducer', () => { // is mutated, and any uncaught exception will cause the test to fail }); + it('should reset the menu state to the initial state when performing the REINIT_MENUS action without affecting the previous state', () => { + dummyState[MenuID.ADMIN].visible = true; + const state = dummyState; + deepFreeze([state]); + + const action = new ReinitMenuAction(); + const menusState = menusReducer(state, action); + expect(menusState).toEqual(initialMenusState); + + }); + it('should set add a new section for the correct menu in response to the ADD_SECTION action', () => { const state = dummyState; const action = new AddMenuSectionAction(menuID, visibleSection1); diff --git a/src/app/shared/menu/menu.reducer.ts b/src/app/shared/menu/menu.reducer.ts index c7ea017cc24..4c9ee31403a 100644 --- a/src/app/shared/menu/menu.reducer.ts +++ b/src/app/shared/menu/menu.reducer.ts @@ -26,36 +26,39 @@ import { MenuID } from './menu-id.model'; * @returns {MenusState} The new, reducer MenusState */ export function menusReducer(state: MenusState = initialMenusState, action: MenuAction): MenusState { - const menuState: MenuState = state[action.menuID]; switch (action.type) { case MenuActionTypes.COLLAPSE_MENU: { - const newMenuState = Object.assign({}, menuState, { collapsed: true }); + const newMenuState = Object.assign({}, state[action.menuID], { collapsed: true }); return Object.assign({}, state, { [action.menuID]: newMenuState }); } case MenuActionTypes.EXPAND_MENU: { - const newMenuState = Object.assign({}, menuState, { collapsed: false }); + const newMenuState = Object.assign({}, state[action.menuID], { collapsed: false }); return Object.assign({}, state, { [action.menuID]: newMenuState }); } case MenuActionTypes.COLLAPSE_MENU_PREVIEW: { - const newMenuState = Object.assign({}, menuState, { previewCollapsed: true }); + const newMenuState = Object.assign({}, state[action.menuID], { previewCollapsed: true }); return Object.assign({}, state, { [action.menuID]: newMenuState }); } case MenuActionTypes.EXPAND_MENU_PREVIEW: { - const newMenuState = Object.assign({}, menuState, { previewCollapsed: false }); + const newMenuState = Object.assign({}, state[action.menuID], { previewCollapsed: false }); return Object.assign({}, state, { [action.menuID]: newMenuState }); } case MenuActionTypes.TOGGLE_MENU: { + const menuState = state[action.menuID]; const newMenuState = Object.assign({}, menuState, { collapsed: !menuState.collapsed }); return Object.assign({}, state, { [action.menuID]: newMenuState }); } case MenuActionTypes.SHOW_MENU: { - const newMenuState = Object.assign({}, menuState, { visible: true }); + const newMenuState = Object.assign({}, state[action.menuID], { visible: true }); return Object.assign({}, state, { [action.menuID]: newMenuState }); } case MenuActionTypes.HIDE_MENU: { - const newMenuState = Object.assign({}, menuState, { visible: false }); + const newMenuState = Object.assign({}, state[action.menuID], { visible: false }); return Object.assign({}, state, { [action.menuID]: newMenuState }); } + case MenuActionTypes.REINIT_MENUS: { + return Object.assign({}, initialMenusState); + } case MenuActionTypes.ADD_SECTION: { return addSection(state, action as AddMenuSectionAction); } @@ -230,7 +233,7 @@ function toggleActiveSection(state: MenusState, action: ToggleActiveMenuSectionA * @param {MenuSection} section The section that will be added or replaced in the state * @returns {MenusState} The new reduced state */ -function putSectionState(state: MenusState, action: MenuAction, section: MenuSection): MenusState { +function putSectionState(state: MenusState, action: MenuSectionAction, section: MenuSection): MenusState { const menuState: MenuState = state[action.menuID]; const newSections = Object.assign({}, menuState.sections, { [section.id]: section diff --git a/src/app/shared/menu/menu.service.spec.ts b/src/app/shared/menu/menu.service.spec.ts index 0d8d669a0aa..6af7c54d58c 100644 --- a/src/app/shared/menu/menu.service.spec.ts +++ b/src/app/shared/menu/menu.service.spec.ts @@ -41,6 +41,7 @@ describe('MenuService', () => { let routeDataMenuSection: MenuSection; let routeDataMenuSectionResolved: MenuSection; let routeDataMenuChildSection: MenuSection; + let routeDataMenuOverwrittenChildSection: MenuSection; let toBeRemovedMenuSection: MenuSection; let alreadyPresentMenuSection: MenuSection; let route; @@ -127,6 +128,17 @@ describe('MenuService', () => { link: '' } as LinkMenuItemModel }; + routeDataMenuOverwrittenChildSection = { + id: 'mockChildSection', + parentID: 'mockSection', + active: false, + visible: true, + model: { + type: MenuItemType.LINK, + text: 'menu.section.mockChildOverwrittenSection', + link: '' + } as LinkMenuItemModel + }; toBeRemovedMenuSection = { id: 'toBeRemovedSection', active: false, @@ -167,7 +179,17 @@ describe('MenuService', () => { [MenuID.PUBLIC]: routeDataMenuChildSection } } + }, + firstChild: { + snapshot: { + data: { + menu: { + [MenuID.PUBLIC]: routeDataMenuOverwrittenChildSection + } + } + } } + } } }; @@ -541,7 +563,7 @@ describe('MenuService', () => { }); describe('buildRouteMenuSections', () => { - it('should add and remove menu sections depending on the current route', () => { + it('should add and remove menu sections depending on the current route and overwrite menu sections when they have the same ID with the child route version', () => { spyOn(service, 'addSection'); spyOn(service, 'removeSection'); @@ -550,7 +572,8 @@ describe('MenuService', () => { 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, routeDataMenuChildSection); + expect(service.addSection).toHaveBeenCalledWith(MenuID.PUBLIC, routeDataMenuOverwrittenChildSection); expect(service.addSection).not.toHaveBeenCalledWith(MenuID.PUBLIC, alreadyPresentMenuSection); expect(service.removeSection).toHaveBeenCalledWith(MenuID.PUBLIC, toBeRemovedMenuSection.id); }); diff --git a/src/app/shared/menu/menu.service.ts b/src/app/shared/menu/menu.service.ts index fac3b3fba70..0ec7d67f252 100644 --- a/src/app/shared/menu/menu.service.ts +++ b/src/app/shared/menu/menu.service.ts @@ -399,7 +399,8 @@ export class MenuService { } if (!last) { - return [...menuSections, ...this.resolveRouteMenuSections(route.firstChild, menuID)]; + const childMenuSections = this.resolveRouteMenuSections(route.firstChild, menuID); + return [...menuSections.filter(menu => !(childMenuSections).map(childMenu => childMenu.id).includes(menu.id)), ...childMenuSections]; } else { return [...menuSections]; }