diff --git a/src/app/admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.spec.ts b/src/app/admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.spec.ts index 0ec0307427..fb227926b6 100644 --- a/src/app/admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.spec.ts +++ b/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()], - declarations: [ExpandableAdminSidebarSectionComponent, TestComponent], + imports: [NoopAnimationsModule, TranslateModule.forRoot(), 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(() => { diff --git a/src/app/shared/dso-page/dso-edit-menu/dso-edit-expandable-menu-section/dso-edit-menu-expandable-section.component.spec.ts b/src/app/shared/dso-page/dso-edit-menu/dso-edit-expandable-menu-section/dso-edit-menu-expandable-section.component.spec.ts index 35eecb60ab..fa11538526 100644 --- a/src/app/shared/dso-page/dso-edit-menu/dso-edit-expandable-menu-section/dso-edit-menu-expandable-section.component.spec.ts +++ b/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()], - declarations: [DsoEditMenuExpandableSectionComponent, TestComponent], + imports: [TranslateModule.forRoot(), DsoEditMenuExpandableSectionComponent, TestComponent], providers: [ { provide: 'sectionDataProvider', useValue: dummySection }, { provide: MenuService, useValue: menuService }, diff --git a/src/app/shared/menu/menu-section/menu-section.component.spec.ts b/src/app/shared/menu/menu-section/menu-section.component.spec.ts index 952a1c77ba..1cfaa1dd99 100644 --- a/src/app/shared/menu/menu-section/menu-section.component.spec.ts +++ b/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], - declarations: [AbstractMenuSectionComponent], + imports: [TranslateModule.forRoot(), NoopAnimationsModule, SomeMenuSectionComponent, AbstractMenuSectionComponent], providers: [ { provide: Injector, useValue: {} }, { provide: MenuService, useClass: MenuServiceStub }, diff --git a/src/app/shared/menu/menu.component.spec.ts b/src/app/shared/menu/menu.component.spec.ts index e128fac3ab..57ad797b37 100644 --- a/src/app/shared/menu/menu.component.spec.ts +++ b/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(); - }); - }); - }); - }); diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index 97f9d95480..4db4f724e1 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/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, - public route: ActivatedRoute, protected themeService: ThemeService, + constructor( + 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) => ({ 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 diff --git a/src/app/shared/menu/providers/browse.menu.spec.ts b/src/app/shared/menu/providers/browse.menu.spec.ts index 663c4825ab..ce395b05ee 100644 --- a/src/app/shared/menu/providers/browse.menu.spec.ts +++ b/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( diff --git a/src/app/shared/menu/providers/statistics.menu.spec.ts b/src/app/shared/menu/providers/statistics.menu.spec.ts index ee1c0c47ad..09748d4360 100644 --- a/src/app/shared/menu/providers/statistics.menu.spec.ts +++ b/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', () => { diff --git a/src/app/shared/menu/providers/statistics.menu.ts b/src/app/shared/menu/providers/statistics.menu.ts index 37f17cd260..f2ac73b708 100644 --- a/src/app/shared/menu/providers/statistics.menu.ts +++ b/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 { + constructor( + protected authorizationService: AuthorizationDataService, + ) { + super(); + } public getRouteContext(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { let dsoRD: RemoteData = route.data.dso; @@ -51,28 +60,32 @@ export class StatisticsMenuProvider extends AbstractRouteContextMenuProvider { + return combineLatest([ + this.authorizationService.isAuthorized(FeatureID.CanViewUsageStatistics, dso?._links.self.href), + ]).pipe( + map(([authorized]) => { + let link = `statistics`; - let link = `statistics`; + let dsoRoute; + if (hasValue(dso)) { + dsoRoute = getDSORoute(dso); + if (hasValue(dsoRoute)) { + link = `statistics${dsoRoute}`; + } + } - let dsoRoute; - if (hasValue(dso)) { - dsoRoute = getDSORoute(dso); - if (hasValue(dsoRoute)) { - link = `statistics${dsoRoute}`; - } - } - - return of([ - { - visible: true, - model: { - type: MenuItemType.LINK, - text: 'menu.section.statistics', - link, - }, - icon: 'chart-line', - }, - ] as PartialMenuSection[]); + return [ + { + visible: authorized, + model: { + type: MenuItemType.LINK, + text: 'menu.section.statistics', + link, + }, + icon: 'chart-line', + }, + ]; + }), + ); } - }