Fix tests pt. 1

src/app/admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.spec.ts

@@ -79,17 +79,14 @@ describe('ExpandableAdminSidebarSectionComponent', () => {
   describe('when there are no subsections', () => {
     beforeEach(waitForAsync(() => {
       TestBed.configureTestingModule({
-        imports: [NoopAnimationsModule, TranslateModule.forRoot()],
+        imports: [NoopAnimationsModule, TranslateModule.forRoot(), ExpandableAdminSidebarSectionComponent, TestComponent],
-        declarations: [ExpandableAdminSidebarSectionComponent, TestComponent],
         providers: [
           { provide: 'sectionDataProvider', useValue: { icon: iconString, model: {} } },
           { provide: MenuService, useValue: menuService },
           { provide: CSSVariableService, useClass: CSSVariableServiceStub },
           { provide: Router, useValue: new RouterStub() },
         ],
-      }).overrideComponent(ExpandableAdminSidebarSectionComponent, {
+      }).compileComponents();
-      })
-        .compileComponents();
     }));
 
     beforeEach(() => {

src/app/shared/dso-page/dso-edit-menu/dso-edit-expandable-menu-section/dso-edit-menu-expandable-section.component.spec.ts

@@ -74,8 +74,7 @@ describe('DsoEditMenuExpandableSectionComponent', () => {
   describe('when there are no subsections', () => {
     beforeEach(waitForAsync(() => {
       TestBed.configureTestingModule({
-        imports: [TranslateModule.forRoot()],
+        imports: [TranslateModule.forRoot(), DsoEditMenuExpandableSectionComponent, TestComponent],
-        declarations: [DsoEditMenuExpandableSectionComponent, TestComponent],
         providers: [
           { provide: 'sectionDataProvider', useValue: dummySection },
           { provide: MenuService, useValue: menuService },

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

@@ -23,6 +23,7 @@ import { AbstractMenuSectionComponent } from './abstract-menu-section.component'
 @Component({
   selector: 'ds-some-menu-section',
   template: '',
+  standalone: true,
 })
 class SomeMenuSectionComponent extends AbstractMenuSectionComponent {
   constructor(
@@ -47,8 +48,7 @@ describe('MenuSectionComponent', () => {
       active: false,
     } as any;
     TestBed.configureTestingModule({
-      imports: [TranslateModule.forRoot(), NoopAnimationsModule, SomeMenuSectionComponent],
+      imports: [TranslateModule.forRoot(), NoopAnimationsModule, SomeMenuSectionComponent, AbstractMenuSectionComponent],
-      declarations: [AbstractMenuSectionComponent],
       providers: [
         { provide: Injector, useValue: {} },
         { provide: MenuService, useClass: MenuServiceStub },

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

@@ -54,6 +54,7 @@ const mockMenuID = 'mock-menuID' as MenuID;
   // eslint-disable-next-line @angular-eslint/component-selector
   selector: '',
   template: '',
+  standalone: true,
 })
 @rendersSectionForMenu(mockMenuID, true)
 class TestExpandableMenuComponent {
@@ -63,6 +64,7 @@ class TestExpandableMenuComponent {
   // eslint-disable-next-line @angular-eslint/component-selector
   selector: '',
   template: '',
+  standalone: true,
 })
 @rendersSectionForMenu(mockMenuID, false)
 class TestMenuComponent {
@@ -85,8 +87,6 @@ describe('MenuComponent', () => {
     visible: true,
   };
 
-  const mockMenuID = 'mock-menuID' as MenuID;
-
   const mockStatisticSection = { 'id': 'statistics_site', 'active': true, 'visible': true, 'index': 2, 'type': 'statistics', 'model': { 'type': 1, 'text': 'menu.section.statistics', 'link': 'statistics' } };
 
   let authorizationService: AuthorizationDataService;
@@ -144,7 +144,7 @@ describe('MenuComponent', () => {
     });
 
     TestBed.configureTestingModule({
-      imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule, MenuComponent, StoreModule.forRoot(authReducer, storeModuleConfig)],
+      imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule, MenuComponent, StoreModule.forRoot(authReducer, storeModuleConfig), TestExpandableMenuComponent, TestMenuComponent],
       providers: [
         Injector,
         { provide: ThemeService, useValue: getMockThemeService() },
@@ -152,8 +152,6 @@ describe('MenuComponent', () => {
         provideMockStore({ initialState }),
         { provide: AuthorizationDataService, useValue: authorizationService },
         { provide: ActivatedRoute, useValue: routeStub },
-        TestExpandableMenuComponent,
-        TestMenuComponent,
       ],
       schemas: [NO_ERRORS_SCHEMA],
     }).overrideComponent(MenuComponent, {
@@ -272,35 +270,4 @@ describe('MenuComponent', () => {
       expect(menuService.collapseMenuPreview).toHaveBeenCalledWith(comp.menuID);
     }));
   });
-
-  describe('when unauthorized statistics', () => {
-
-    beforeEach(() => {
-      (authorizationService as any).isAuthorized.and.returnValue(observableOf(false));
-      fixture.detectChanges();
-    });
-
-    it('should return observable of empty object', done => {
-      comp.getAuthorizedStatistics(mockStatisticSection).subscribe((res) => {
-        expect(res).toEqual({});
-        done();
-      });
-    });
-  });
-
-  describe('get authorized statistics', () => {
-
-    beforeEach(() => {
-      (authorizationService as any).isAuthorized.and.returnValue(observableOf(true));
-      fixture.detectChanges();
-    });
-
-    it('should return observable of statistics section menu', done => {
-      comp.getAuthorizedStatistics(mockStatisticSection).subscribe((res) => {
-        expect(res).toEqual(mockStatisticSection);
-        done();
-      });
-    });
-  });
-
 });

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

@@ -9,29 +9,26 @@ import { ActivatedRoute } from '@angular/router';
 import {
   BehaviorSubject,
   Observable,
-  of as observableOf,
   Subscription,
 } from 'rxjs';
 import {
   distinctUntilChanged,
   map,
-  mergeMap,
   switchMap,
 } from 'rxjs/operators';
 
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
-import { FeatureID } from '../../core/data/feature-authorization/feature-id';
 import { GenericConstructor } from '../../core/shared/generic-constructor';
 import {
   hasValue,
   isNotEmptyOperator,
 } from '../empty.util';
 import { ThemeService } from '../theme-support/theme.service';
-import { MenuService } from './menu.service';
 import { MenuID } from './menu-id.model';
 import { getComponentForMenu } from './menu-section.decorator';
 import { MenuSection } from './menu-section.model';
 import { AbstractMenuSectionComponent } from './menu-section/abstract-menu-section.component';
+import { MenuService } from './menu.service';
 
 /**
  * A basic implementation of a MenuComponent
@@ -93,8 +90,12 @@ export class MenuComponent implements OnInit, OnDestroy {
 
   private activatedRouteLastChild: ActivatedRoute;
 
-  constructor(protected menuService: MenuService, protected injector: Injector, public authorizationService: AuthorizationDataService,
+  constructor(
-              public route: ActivatedRoute, protected themeService: ThemeService,
+    protected menuService: MenuService,
+    protected injector: Injector,
+    public authorizationService: AuthorizationDataService,
+    public route: ActivatedRoute,
+    protected themeService: ThemeService,
   ) {
   }
 
@@ -113,12 +114,6 @@ export class MenuComponent implements OnInit, OnDestroy {
         // if you return an array from a switchMap it will emit each element as a separate event.
         // So this switchMap is equivalent to a subscribe with a forEach inside
         switchMap((sections: MenuSection[]) => sections),
-        mergeMap((section: MenuSection) => {
-          if (section.id.includes('statistics')) {
-            return this.getAuthorizedStatistics(section);
-          }
-          return observableOf(section);
-        }),
         isNotEmptyOperator(),
         switchMap((section: MenuSection) => this.getSectionComponent(section).pipe(
           map((component: GenericConstructor<AbstractMenuSectionComponent>) => ({ section, component })),
@@ -146,32 +141,6 @@ export class MenuComponent implements OnInit, OnDestroy {
     }
   }
 
-  /**
-   *  Get section of statistics after checking authorization
-   */
-  getAuthorizedStatistics(section) {
-    return this.activatedRouteLastChild.data.pipe(
-      switchMap((data) => {
-        return this.authorizationService.isAuthorized(FeatureID.CanViewUsageStatistics, this.getObjectUrl(data)).pipe(
-          map((canViewUsageStatistics: boolean) => {
-            if (!canViewUsageStatistics) {
-              return {};
-            } else {
-              return section;
-            }
-          }));
-      }),
-    );
-  }
-
-  /**
-   *  Get statistics route dso data
-   */
-  getObjectUrl(data) {
-    const object = data.site ? data.site : data.dso?.payload;
-    return object?._links?.self?.href;
-  }
-
   /**
    *  Collapse this menu when it's currently expanded, expand it when its currently collapsed
    * @param {Event} event The user event that triggered this method

src/app/shared/menu/providers/browse.menu.spec.ts

@@ -51,7 +51,7 @@ describe('BrowseMenuProvider', () => {
 
 
   let provider: BrowseMenuProvider;
-  let browseServiceStub = new BrowseServiceStub();
+  let browseServiceStub = BrowseServiceStub;
 
   beforeEach(() => {
     spyOn(browseServiceStub, 'getBrowseDefinitions').and.returnValue(

src/app/shared/menu/providers/statistics.menu.spec.ts

@@ -1,5 +1,8 @@
+import { inject } from '@angular/core';
 import { TestBed } from '@angular/core/testing';
 import { TranslateModule } from '@ngx-translate/core';
+import { of as observableOf } from 'rxjs';
+import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service';
 
 import { Item } from '../../../core/shared/item.model';
 import { ITEM } from '../../../core/shared/item.resource-type';
@@ -34,6 +37,18 @@ describe('StatisticsMenuProvider', () => {
     },
   ];
 
+  const expectedSectionsForItemInvisible: PartialMenuSection[] = [
+    {
+      visible: false,
+      model: {
+        type: MenuItemType.LINK,
+        text: 'menu.section.statistics',
+        link: `statistics/items/test-item-uuid`,
+      },
+      icon: 'chart-line',
+    },
+  ];
+
   let provider: StatisticsMenuProvider;
 
   const item: Item = Object.assign(new Item(), {
@@ -57,13 +72,18 @@ describe('StatisticsMenuProvider', () => {
       }],
     },
   });
+  let authorizationService: AuthorizationDataService;
 
   beforeEach(() => {
+    authorizationService = jasmine.createSpyObj('authorizationService', {
+      isAuthorized: observableOf(true),
+    });
 
     TestBed.configureTestingModule({
       imports: [TranslateModule.forRoot()],
       providers: [
         StatisticsMenuProvider,
+        { provide: AuthorizationDataService, useValue: authorizationService },
       ],
     });
     provider = TestBed.inject(StatisticsMenuProvider);
@@ -86,6 +106,13 @@ describe('StatisticsMenuProvider', () => {
         done();
       });
     });
+    it('should not return anything if not authorized to view statistics', (done) => {
+      (TestBed.inject(AuthorizationDataService) as any).isAuthorized.and.returnValue(observableOf(false));
+      provider.getSectionsForContext(item).subscribe((sections) => {
+        expect(sections).toEqual(expectedSectionsForItemInvisible);
+        done();
+      });
+    });
   });
 
   describe('getRouteContext', () => {

src/app/shared/menu/providers/statistics.menu.ts

@@ -12,11 +12,15 @@ import {
   RouterStateSnapshot,
 } from '@angular/router';
 import {
+  combineLatest,
+  map,
   Observable,
   of,
 } from 'rxjs';
 
 import { getDSORoute } from '../../../app-routing-paths';
+import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service';
+import { FeatureID } from '../../../core/data/feature-authorization/feature-id';
 import { RemoteData } from '../../../core/data/remote-data';
 import { DSpaceObject } from '../../../core/shared/dspace-object.model';
 import {
@@ -34,6 +38,11 @@ import { AbstractRouteContextMenuProvider } from './helper-providers/route-conte
  */
 @Injectable()
 export class StatisticsMenuProvider extends AbstractRouteContextMenuProvider<DSpaceObject> {
+  constructor(
+    protected authorizationService: AuthorizationDataService,
+  ) {
+    super();
+  }
 
   public getRouteContext(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<DSpaceObject> {
     let dsoRD: RemoteData<DSpaceObject> = route.data.dso;
@@ -51,7 +60,10 @@ export class StatisticsMenuProvider extends AbstractRouteContextMenuProvider<DSp
   }
 
   public getSectionsForContext(dso: DSpaceObject): Observable<PartialMenuSection[]> {
-
+    return combineLatest([
+      this.authorizationService.isAuthorized(FeatureID.CanViewUsageStatistics, dso?._links.self.href),
+    ]).pipe(
+      map(([authorized]) => {
         let link = `statistics`;
 
         let dsoRoute;
@@ -62,9 +74,9 @@ export class StatisticsMenuProvider extends AbstractRouteContextMenuProvider<DSp
           }
         }
 
-    return of([
+        return [
           {
-        visible: true,
+            visible: authorized,
             model: {
               type: MenuItemType.LINK,
               text: 'menu.section.statistics',
@@ -72,7 +84,8 @@ export class StatisticsMenuProvider extends AbstractRouteContextMenuProvider<DSp
             },
             icon: 'chart-line',
           },
-    ] as PartialMenuSection[]);
+        ];
+      }),
+    );
   }
-
 }