Merge pull request #1600 from atmire/w2p-90155_fix-menu-issues

Fix for menu issues

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

@@ -237,7 +237,24 @@ describe('AdminSidebarComponent', () => {
         expect(menuService.addSection).toHaveBeenCalledWith(comp.menuID, jasmine.objectContaining({
           parentID: 'access_control', visible: false,
         }));
+      });
 
+      // We check that the menu section has not been called with visible set to true
+      // The reason why we don't check if it has been called with visible set to false
+      // Is because the function does not get called unless a user is authorised
+      it('should not show the import section', () => {
+        expect(menuService.addSection).not.toHaveBeenCalledWith(comp.menuID, jasmine.objectContaining({
+          id: 'import', visible: true,
+        }));
+      });
+
+      // We check that the menu section has not been called with visible set to true
+      // The reason why we don't check if it has been called with visible set to false
+      // Is because the function does not get called unless a user is authorised
+      it('should not show the export section', () => {
+        expect(menuService.addSection).not.toHaveBeenCalledWith(comp.menuID, jasmine.objectContaining({
+          id: 'export', visible: true,
+        }));
       });
     });
 
@@ -268,6 +285,15 @@ describe('AdminSidebarComponent', () => {
         expect(menuService.addSection).toHaveBeenCalledWith(comp.menuID, jasmine.objectContaining({
           id: 'workflow', visible: true,
         }));
+        expect(menuService.addSection).toHaveBeenCalledWith(comp.menuID, jasmine.objectContaining({
+          id: 'workflow', visible: true,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(comp.menuID, jasmine.objectContaining({
+          id: 'import', visible: true,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(comp.menuID, jasmine.objectContaining({
+          id: 'export', visible: true,
+        }));
       });
     });
 

src/app/admin/admin-sidebar/admin-sidebar.component.ts

@@ -1,9 +1,13 @@
 import { Component, HostListener, Injector, OnInit } from '@angular/core';
 import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
 import { combineLatest, combineLatest as observableCombineLatest, Observable, BehaviorSubject } from 'rxjs';
-import { debounceTime, first, map, take, distinctUntilChanged, withLatestFrom } from 'rxjs/operators';
+import { debounceTime, first, map, take, filter, distinctUntilChanged, withLatestFrom } from 'rxjs/operators';
 import { AuthService } from '../../core/auth/auth.service';
-import { ScriptDataService } from '../../core/data/processes/script-data.service';
+import {
+  ScriptDataService,
+  METADATA_IMPORT_SCRIPT_NAME,
+  METADATA_EXPORT_SCRIPT_NAME
+} from '../../core/data/processes/script-data.service';
 import { slideHorizontal, slideSidebar } from '../../shared/animations/slide';
 import { CreateCollectionParentSelectorComponent } from '../../shared/dso-selector/modal-wrappers/create-collection-parent-selector/create-collection-parent-selector.component';
 import { CreateCommunityParentSelectorComponent } from '../../shared/dso-selector/modal-wrappers/create-community-parent-selector/create-community-parent-selector.component';
@@ -321,19 +325,6 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
    */
   createExportMenuSections() {
     const menuList = [
-      /* Export */
-      {
-        id: 'export',
-        active: false,
-        visible: true,
-        model: {
-          type: MenuItemType.TEXT,
-          text: 'menu.section.export'
-        } as TextMenuItemModel,
-        icon: 'file-export',
-        index: 3,
-        shouldPersistOnRouteChange: true
-      },
       // TODO: enable this menu item once the feature has been implemented
       // {
       //   id: 'export_community',
@@ -378,12 +369,26 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
 
     observableCombineLatest(
       this.authorizationService.isAuthorized(FeatureID.AdministratorOf),
-      // this.scriptDataService.scriptWithNameExistsAndCanExecute(METADATA_EXPORT_SCRIPT_NAME)
+      this.scriptDataService.scriptWithNameExistsAndCanExecute(METADATA_EXPORT_SCRIPT_NAME)
     ).pipe(
-      // TODO uncomment when #635 (https://github.com/DSpace/dspace-angular/issues/635) is fixed; otherwise even in production mode, the metadata export button is only available after a refresh (and not in dev mode)
-      // filter(([authorized, metadataExportScriptExists]: boolean[]) => authorized && metadataExportScriptExists),
+      filter(([authorized, metadataExportScriptExists]: boolean[]) => authorized && metadataExportScriptExists),
       take(1)
     ).subscribe(() => {
+      // Hides the export menu for unauthorised people
+      // If in the future more sub-menus are added,
+      // it should be reviewed if they need to be in this subscribe
+      this.menuService.addSection(this.menuID, {
+          id: 'export',
+          active: false,
+          visible: true,
+          model: {
+            type: MenuItemType.TEXT,
+            text: 'menu.section.export'
+          } as TextMenuItemModel,
+          icon: 'file-export',
+          index: 3,
+          shouldPersistOnRouteChange: true
+        });
       this.menuService.addSection(this.menuID, {
         id: 'export_metadata',
         parentID: 'export',
@@ -407,18 +412,6 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
    */
   createImportMenuSections() {
     const menuList = [
-      /* Import */
-      {
-        id: 'import',
-        active: false,
-        visible: true,
-        model: {
-          type: MenuItemType.TEXT,
-          text: 'menu.section.import'
-        } as TextMenuItemModel,
-        icon: 'file-import',
-        index: 2
-      },
       // TODO: enable this menu item once the feature has been implemented
       // {
       //   id: 'import_batch',
@@ -438,12 +431,25 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
 
     observableCombineLatest(
       this.authorizationService.isAuthorized(FeatureID.AdministratorOf),
-      // this.scriptDataService.scriptWithNameExistsAndCanExecute(METADATA_IMPORT_SCRIPT_NAME)
+      this.scriptDataService.scriptWithNameExistsAndCanExecute(METADATA_IMPORT_SCRIPT_NAME)
     ).pipe(
-      // TODO uncomment when #635 (https://github.com/DSpace/dspace-angular/issues/635) is fixed
-      // filter(([authorized, metadataImportScriptExists]: boolean[]) => authorized && metadataImportScriptExists),
+      filter(([authorized, metadataImportScriptExists]: boolean[]) => authorized && metadataImportScriptExists),
       take(1)
     ).subscribe(() => {
+      // Hides the import menu for unauthorised people
+      // If in the future more sub-menus are added,
+      // it should be reviewed if they need to be in this subscribe
+      this.menuService.addSection(this.menuID, {
+          id: 'import',
+          active: false,
+          visible: true,
+          model: {
+            type: MenuItemType.TEXT,
+            text: 'menu.section.import'
+          } as TextMenuItemModel,
+          icon: 'file-import',
+          index: 2
+        });
       this.menuService.addSection(this.menuID, {
         id: 'import_metadata',
         parentID: 'import',

src/app/core/data/processes/script-data.service.ts

@@ -19,6 +19,8 @@ import { Observable } from 'rxjs';
 import { dataService } from '../../cache/builders/build-decorators';
 import { SCRIPT } from '../../../process-page/scripts/script.resource-type';
 import { Process } from '../../../process-page/processes/process.model';
+import { hasValue } from '../../../shared/empty.util';
+import { getFirstCompletedRemoteData } from '../../shared/operators';
 
 export const METADATA_IMPORT_SCRIPT_NAME = 'metadata-import';
 export const METADATA_EXPORT_SCRIPT_NAME = 'metadata-export';
@@ -62,4 +64,16 @@ export class ScriptDataService extends DataService<Script> {
     });
     return form;
   }
+
+  /**
+   * Check whether a script with given name exist; user needs to be allowed to execute script for this to to not throw a 401 Unauthorized
+   * @param scriptName    script we want to check exists (and we can execute)
+   */
+  public scriptWithNameExistsAndCanExecute(scriptName: string): Observable<boolean> {
+    return this.findById(scriptName).pipe(
+      getFirstCompletedRemoteData(),
+      map((rd: RemoteData<Script>) => {
+        return hasValue(rd.payload);
+      }));
+  }
 }

src/app/shared/menu/menu-item/link-menu-item.component.html

@@ -1,8 +1,10 @@
+
 <a class="nav-item nav-link"
    [ngClass]="{ 'disabled': !hasLink }"
    [attr.aria-disabled]="!hasLink"
    [title]="item.text | translate"
    [routerLink]="getRouterLink()"
+   [queryParams]="item.queryParams"
    (click)="$event.stopPropagation()"
    (keyup.space)="navigate($event)"
    (keyup.enter)="navigate($event)"

src/app/shared/menu/menu-item/link-menu-item.component.spec.ts

@@ -5,6 +5,7 @@ import { By } from '@angular/platform-browser';
 import { LinkMenuItemComponent } from './link-menu-item.component';
 import { RouterLinkDirectiveStub } from '../../testing/router-link-directive.stub';
 import { environment } from '../../../../environments/environment';
+import { QueryParamsDirectiveStub } from '../../testing/query-params-directive.stub';
 import { RouterStub } from '../../testing/router.stub';
 import { Router } from '@angular/router';
 
@@ -14,19 +15,21 @@ describe('LinkMenuItemComponent', () => {
   let debugElement: DebugElement;
   let text;
   let link;
+  let queryParams;
 
   function init() {
     text = 'HELLO';
     link = 'http://google.com';
+    queryParams = {params: true};
   }
 
   beforeEach(waitForAsync(() => {
     init();
     TestBed.configureTestingModule({
       imports: [TranslateModule.forRoot()],
-      declarations: [LinkMenuItemComponent, RouterLinkDirectiveStub],
+      declarations: [LinkMenuItemComponent, RouterLinkDirectiveStub, QueryParamsDirectiveStub],
       providers: [
-        { provide: 'itemModelProvider', useValue: { text: text, link: link } },
+        { provide: 'itemModelProvider', useValue: { text: text, link: link, queryParams: queryParams } },
         { provide: Router, useValue: RouterStub },
       ],
       schemas: [NO_ERRORS_SCHEMA]
@@ -57,4 +60,12 @@ describe('LinkMenuItemComponent', () => {
     expect(routerLinkQuery.length).toBe(1);
     expect(routerLinkQuery[0].routerLink).toBe(environment.ui.nameSpace + link);
   });
+
+  it('should have the right queryParams attribute', () => {
+    const queryDes = fixture.debugElement.queryAll(By.directive(QueryParamsDirectiveStub));
+    const routerParamsQuery = queryDes.map((de) => de.injector.get(QueryParamsDirectiveStub));
+
+    expect(routerParamsQuery.length).toBe(1);
+    expect(routerParamsQuery[0].queryParams).toBe(queryParams);
+  });
 });

src/app/shared/menu/menu-item/models/link.model.ts

@@ -1,5 +1,6 @@
 import { MenuItemModel } from './menu-item.model';
 import { MenuItemType } from '../../initial-menus-state';
+import { Params } from '@angular/router';
 
 /**
  * Model representing an Link Menu Section
@@ -8,4 +9,5 @@ export class LinkMenuItemModel implements MenuItemModel {
   type = MenuItemType.LINK;
   text: string;
   link: string;
+  queryParams?: Params | null;
 }