Merge pull request DSpace#2610 from alexandrevryghem/menu-section-imp…

…rovements_contribute-7.6

Fixed menu not updating when a new sub section is added after rendering has already completed

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

@@ -1,12 +1,12 @@
+// eslint-disable-next-line max-classes-per-file
 import { ComponentFixture, fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 import { TranslateModule } from '@ngx-translate/core';
-import { ChangeDetectionStrategy, Injector, NO_ERRORS_SCHEMA } from '@angular/core';
+import { ChangeDetectionStrategy, Injector, NO_ERRORS_SCHEMA, Component } from '@angular/core';
 import { MenuService } from './menu.service';
 import { MenuComponent } from './menu.component';
-import { MenuServiceStub } from '../testing/menu-service.stub';
-import { of as observableOf } from 'rxjs';
-import { Router, ActivatedRoute } from '@angular/router';
+import { of as observableOf, BehaviorSubject } from 'rxjs';
+import { ActivatedRoute } from '@angular/router';
 import { RouterTestingModule } from '@angular/router/testing';
 import { MenuSection } from './menu-section.model';
 import { MenuID } from './menu-id.model';
@@ -15,14 +15,39 @@ import { AuthorizationDataService } from '../../core/data/feature-authorization/
 import { createSuccessfulRemoteDataObject } from '../remote-data.utils';
 import { ThemeService } from '../theme-support/theme.service';
 import { getMockThemeService } from '../mocks/theme-service.mock';
+import { MenuItemType } from './menu-item-type.model';
+import { LinkMenuItemModel } from './menu-item/models/link.model';
+import { provideMockStore, MockStore } from '@ngrx/store/testing';
+import { StoreModule, Store } from '@ngrx/store';
+import { authReducer } from '../../core/auth/auth.reducer';
+import { storeModuleConfig, AppState } from '../../app.reducer';
+import { rendersSectionForMenu } from './menu-section.decorator';
+
+const mockMenuID = 'mock-menuID' as MenuID;
+
+@Component({
+  // eslint-disable-next-line @angular-eslint/component-selector
+  selector: '',
+  template: '',
+})
+@rendersSectionForMenu(mockMenuID, true)
+class TestExpandableMenuComponent {
+}
+
+@Component({
+  // eslint-disable-next-line @angular-eslint/component-selector
+  selector: '',
+  template: '',
+})
+@rendersSectionForMenu(mockMenuID, false)
+class TestMenuComponent {
+}
 
 describe('MenuComponent', () => {
   let comp: MenuComponent;
   let fixture: ComponentFixture<MenuComponent>;
   let menuService: MenuService;
-  let router: any;
-
-  const mockMenuID = 'mock-menuID' as MenuID;
+  let store: MockStore;
 
   const mockStatisticSection = { 'id': 'statistics_site', 'active': true, 'visible': true, 'index': 2, 'type': 'statistics', 'model': { 'type': 1, 'text': 'menu.section.statistics', 'link': 'statistics' } };
 
@@ -48,21 +73,55 @@ describe('MenuComponent', () => {
     children: []
   };
 
+  const initialState = {
+    menus: {
+      [mockMenuID]: {
+        collapsed: true,
+        id: mockMenuID,
+        previewCollapsed: true,
+        sectionToSubsectionIndex: {
+          section1: [],
+        },
+        sections: {
+          section1: {
+            id: 'section1',
+            active: false,
+            visible: true,
+            model: {
+              type: MenuItemType.LINK,
+              text: 'test',
+              link: '/test',
+            } as LinkMenuItemModel,
+          },
+        },
+        visible: true,
+      },
+    },
+  };
+
   beforeEach(waitForAsync(() => {
 
     authorizationService = jasmine.createSpyObj('authorizationService', {
       isAuthorized: observableOf(false)
     });
 
-    TestBed.configureTestingModule({
-      imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule],
+    void TestBed.configureTestingModule({
+      imports: [
+        TranslateModule.forRoot(),
+        NoopAnimationsModule,
+        RouterTestingModule,
+        StoreModule.forRoot(authReducer, storeModuleConfig),
+      ],
       declarations: [MenuComponent],
       providers: [
         Injector,
         { provide: ThemeService, useValue: getMockThemeService() },
-        { provide: MenuService, useClass: MenuServiceStub },
+        MenuService,
+        provideMockStore({ initialState }),
         { provide: AuthorizationDataService, useValue: authorizationService },
         { provide: ActivatedRoute, useValue: routeStub },
+        TestExpandableMenuComponent,
+        TestMenuComponent,
       ],
       schemas: [NO_ERRORS_SCHEMA]
     }).overrideComponent(MenuComponent, {
@@ -74,13 +133,62 @@ describe('MenuComponent', () => {
     fixture = TestBed.createComponent(MenuComponent);
     comp = fixture.componentInstance; // SearchPageComponent test instance
     comp.menuID = mockMenuID;
-    menuService = (comp as any).menuService;
-    router = TestBed.inject(Router);
+    menuService = TestBed.inject(MenuService);
+    store = TestBed.inject(Store) as MockStore<AppState>;
     spyOn(comp as any, 'getSectionDataInjector').and.returnValue(MenuSection);
-    spyOn(comp as any, 'getSectionComponent').and.returnValue(observableOf({}));
     fixture.detectChanges();
   });
 
+  describe('ngOnInit', () => {
+    it('should trigger the section observable again when a new sub section has been added', () => {
+      spyOn(comp.sectionMap$, 'next').and.callThrough();
+      const hasSubSections = new BehaviorSubject(false);
+      spyOn(menuService, 'hasSubSections').and.returnValue(hasSubSections.asObservable());
+      spyOn(store, 'dispatch').and.callThrough();
+
+      store.setState({
+        menus: {
+          [mockMenuID]: {
+            collapsed: true,
+            id: mockMenuID,
+            previewCollapsed: true,
+            sectionToSubsectionIndex: {
+              section1: ['test'],
+            },
+            sections: {
+              section1: {
+                id: 'section1',
+                active: false,
+                visible: true,
+                model: {
+                  type: MenuItemType.LINK,
+                  text: 'test',
+                  link: '/test',
+                } as LinkMenuItemModel,
+              },
+              test: {
+                id: 'test',
+                parentID: 'section1',
+                active: false,
+                visible: true,
+                model: {
+                  type: MenuItemType.LINK,
+                  text: 'test',
+                  link: '/test',
+                } as LinkMenuItemModel,
+              }
+            },
+            visible: true,
+          },
+        },
+      });
+      expect(menuService.hasSubSections).toHaveBeenCalled();
+      hasSubSections.next(true);
+
+      expect(comp.sectionMap$.next).toHaveBeenCalled();
+    });
+  });
+
   describe('toggle', () => {
     beforeEach(() => {
       spyOn(menuService, 'toggleMenu');

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

@@ -6,7 +6,6 @@ import { GenericConstructor } from '../../core/shared/generic-constructor';
 import { hasValue, isNotEmptyOperator } from '../empty.util';
 import { MenuSectionComponent } from './menu-section/menu-section.component';
 import { getComponentForMenu } from './menu-section.decorator';
-import { compareArraysUsingIds } from '../../item-page/simple/item-types/shared/item-relationships-utils';
 import { MenuSection } from './menu-section.model';
 import { MenuID } from './menu-id.model';
 import { ActivatedRoute } from '@angular/router';
@@ -86,7 +85,7 @@ export class MenuComponent implements OnInit, OnDestroy {
     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(distinctUntilChanged(compareArraysUsingIds()));
+    this.sections = this.menuService.getMenuTopSections(this.menuID);
 
     this.subs.push(
       this.sections.pipe(
@@ -103,7 +102,7 @@ export class MenuComponent implements OnInit, OnDestroy {
         switchMap((section: MenuSection) => this.getSectionComponent(section).pipe(
           map((component: GenericConstructor<MenuSectionComponent>) => ({ section, component }))
         )),
-        distinctUntilChanged((x, y) => x.section.id === y.section.id)
+        distinctUntilChanged((x, y) => x.section.id === y.section.id && x.component.prototype === y.component.prototype),
       ).subscribe(({ section, component }) => {
         const nextMap = this.sectionMap$.getValue();
         nextMap.set(section.id, {

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 { filter, map, switchMap, take } from 'rxjs/operators';
+import { distinctUntilChanged, filter, map, switchMap, take } from 'rxjs/operators';
 import {
   ActivateMenuSectionAction,
   AddMenuSectionAction,
@@ -23,6 +23,7 @@ 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';
+import { compareArraysUsingIds } from '../../item-page/simple/item-types/shared/item-relationships-utils';
 
 export function menuKeySelector<T>(key: string, selector): MemoizedSelector<MenuState, T> {
   return createSelector(selector, (state) => {
@@ -81,8 +82,10 @@ export class MenuService {
     return this.store.pipe(
       select(menuByIDSelector(menuID)),
       select(menuSectionStateSelector),
-      map((sections: MenuSections) => {
-          return Object.values(sections)
+      map((sections: MenuSections) => Object.values(sections)),
+      distinctUntilChanged(compareArraysUsingIds()),
+      map((sections: MenuSection[]) => {
+          return sections
             .filter((section: MenuSection) => hasNoValue(section.parentID))
             .filter((section: MenuSection) => !mustBeVisible || section.visible);
         }