Merge pull request #1999 from atmire/w2p-97183_fix-user-authorization-issues-with-admin-sidebar

Fix user authorization issues with admin sidebar

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

@@ -100,6 +100,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
         }
       }
     });
+    this.menuVisible = this.menuService.isMenuVisibleWithVisibleSections(this.menuID);
   }
 
   @HostListener('focusin')

src/app/core/data/feature-authorization/feature-id.ts

@@ -29,5 +29,7 @@ export enum FeatureID {
   CanViewUsageStatistics = 'canViewUsageStatistics',
   CanSendFeedback = 'canSendFeedback',
   CanClaimItem = 'canClaimItem',
-  CanSynchronizeWithORCID = 'canSynchronizeWithORCID'
+  CanSynchronizeWithORCID = 'canSynchronizeWithORCID',
+  CanSubmit = 'canSubmit',
+  CanEditItem = 'canEditItem',
 }

src/app/menu.resolver.spec.ts

@@ -142,29 +142,7 @@ describe('MenuResolver', () => {
   });
 
   describe('createAdminMenu$', () => {
-    it('should retrieve the menu by ID return an Observable that emits true as soon as it is created', () => {
+    const dontShowAdminSections = () => {
-      (menuService as any).getMenu.and.returnValue(cold('--u--m', {
-        u: undefined,
-        m: MENU_STATE,
-      }));
-
-      expect(resolver.createAdminMenu$()).toBeObservable(cold('-----(t|)', BOOLEAN));
-      expect(menuService.getMenu).toHaveBeenCalledOnceWith(MenuID.ADMIN);
-    });
-
-    describe('for regular user', () => {
-      beforeEach(() => {
-        authorizationService.isAuthorized = createSpy('isAuthorized').and.callFake(() => {
-          return observableOf(false);
-        });
-      });
-
-      beforeEach((done) => {
-        resolver.createAdminMenu$().subscribe((_) => {
-          done();
-        });
-      });
-
       it('should not show site admin section', () => {
         expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
           id: 'admin_search', visible: false,
@@ -183,19 +161,6 @@ describe('MenuResolver', () => {
         }));
       });
 
-      it('should not show edit_community', () => {
-        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
-          id: 'edit_community', visible: false,
-        }));
-
-      });
-
-      it('should not show edit_collection', () => {
-        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
-          id: 'edit_collection', visible: false,
-        }));
-      });
-
       it('should not show access control section', () => {
         expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
           id: 'access_control', visible: false,
@@ -222,6 +187,122 @@ describe('MenuResolver', () => {
           id: 'export', visible: true,
         }));
       });
+    };
+
+    const dontShowNewSection = () => {
+      it('should not show the "New" section', () => {
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'new_community', visible: false,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'new_collection', visible: false,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'new_item', visible: false,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'new', visible: false,
+        }));
+      });
+    };
+
+    const dontShowEditSection = () => {
+      it('should not show the "Edit" section', () => {
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'edit_community', visible: false,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'edit_collection', visible: false,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'edit_item', visible: false,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'edit', visible: false,
+        }));
+      });
+    };
+
+    it('should retrieve the menu by ID return an Observable that emits true as soon as it is created', () => {
+      (menuService as any).getMenu.and.returnValue(cold('--u--m', {
+        u: undefined,
+        m: MENU_STATE,
+      }));
+
+      expect(resolver.createAdminMenu$()).toBeObservable(cold('-----(t|)', BOOLEAN));
+      expect(menuService.getMenu).toHaveBeenCalledOnceWith(MenuID.ADMIN);
+    });
+
+    describe('for regular user', () => {
+      beforeEach(() => {
+        authorizationService.isAuthorized = createSpy('isAuthorized').and.callFake((featureID) => {
+          return observableOf(false);
+        });
+      });
+
+      beforeEach((done) => {
+        resolver.createAdminMenu$().subscribe((_) => {
+          done();
+        });
+      });
+
+      dontShowAdminSections();
+      dontShowNewSection();
+      dontShowEditSection();
+    });
+
+    describe('regular user who can submit', () => {
+      beforeEach(() => {
+        authorizationService.isAuthorized = createSpy('isAuthorized')
+          .and.callFake((featureID: FeatureID) => {
+            return observableOf(featureID === FeatureID.CanSubmit);
+          });
+      });
+
+      beforeEach((done) => {
+        resolver.createAdminMenu$().subscribe((_) => {
+          done();
+        });
+      });
+
+      it('should show "New Item" section', () => {
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'new_item', visible: true,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'new', visible: true,
+        }));
+      });
+
+      dontShowAdminSections();
+      dontShowEditSection();
+    });
+
+    describe('regular user who can edit items', () => {
+      beforeEach(() => {
+        authorizationService.isAuthorized = createSpy('isAuthorized')
+          .and.callFake((featureID: FeatureID) => {
+            return observableOf(featureID === FeatureID.CanEditItem);
+          });
+      });
+
+      beforeEach((done) => {
+        resolver.createAdminMenu$().subscribe((_) => {
+          done();
+        });
+      });
+
+      it('should show "Edit Item" section', () => {
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'edit_item', visible: true,
+        }));
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'edit', visible: true,
+        }));
+      });
+
+      dontShowAdminSections();
+      dontShowNewSection();
     });
 
     describe('for site admin', () => {
@@ -237,6 +318,12 @@ describe('MenuResolver', () => {
         });
       });
 
+      it('should show new_process', () => {
+        expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
+          id: 'new_process', visible: true,
+        }));
+      });
+
       it('should contain site admin section', () => {
         expect(menuService.addSection).toHaveBeenCalledWith(MenuID.ADMIN, jasmine.objectContaining({
           id: 'admin_search', visible: true,

src/app/menu.resolver.ts

@@ -167,21 +167,11 @@ export class MenuResolver implements Resolve<boolean> {
     combineLatest([
       this.authorizationService.isAuthorized(FeatureID.IsCollectionAdmin),
       this.authorizationService.isAuthorized(FeatureID.IsCommunityAdmin),
-      this.authorizationService.isAuthorized(FeatureID.AdministratorOf)
+      this.authorizationService.isAuthorized(FeatureID.AdministratorOf),
-    ]).subscribe(([isCollectionAdmin, isCommunityAdmin, isSiteAdmin]) => {
+      this.authorizationService.isAuthorized(FeatureID.CanSubmit),
-      const menuList = [
+      this.authorizationService.isAuthorized(FeatureID.CanEditItem),
-        /* News */
+    ]).subscribe(([isCollectionAdmin, isCommunityAdmin, isSiteAdmin, canSubmit, canEditItem]) => {
-        {
+      const newSubMenuList = [
-          id: 'new',
-          active: false,
-          visible: true,
-          model: {
-            type: MenuItemType.TEXT,
-            text: 'menu.section.new'
-          } as TextMenuItemModel,
-          icon: 'plus',
-          index: 0
-        },
         {
           id: 'new_community',
           parentID: 'new',
@@ -212,7 +202,7 @@ export class MenuResolver implements Resolve<boolean> {
           id: 'new_item',
           parentID: 'new',
           active: false,
-          visible: true,
+          visible: canSubmit,
           model: {
             type: MenuItemType.ONCLICK,
             text: 'menu.section.new_item',
@@ -225,38 +215,16 @@ export class MenuResolver implements Resolve<boolean> {
           id: 'new_process',
           parentID: 'new',
           active: false,
-          visible: isCollectionAdmin,
+          visible: isSiteAdmin,
           model: {
             type: MenuItemType.LINK,
             text: 'menu.section.new_process',
             link: '/processes/new'
           } as LinkMenuItemModel,
         },
-        // TODO: enable this menu item once the feature has been implemented
+      ];
-        // {
+      const editSubMenuList = [
-        //   id: 'new_item_version',
-        //   parentID: 'new',
-        //   active: false,
-        //   visible: true,
-        //   model: {
-        //     type: MenuItemType.LINK,
-        //     text: 'menu.section.new_item_version',
-        //     link: ''
-        //   } as LinkMenuItemModel,
-        // },
-
         /* Edit */
-        {
-          id: 'edit',
-          active: false,
-          visible: true,
-          model: {
-            type: MenuItemType.TEXT,
-            text: 'menu.section.edit'
-          } as TextMenuItemModel,
-          icon: 'pencil-alt',
-          index: 1
-        },
         {
           id: 'edit_community',
           parentID: 'edit',
@@ -287,7 +255,7 @@ export class MenuResolver implements Resolve<boolean> {
           id: 'edit_item',
           parentID: 'edit',
           active: false,
-          visible: true,
+          visible: canEditItem,
           model: {
             type: MenuItemType.ONCLICK,
             text: 'menu.section.edit_item',
@@ -296,6 +264,47 @@ export class MenuResolver implements Resolve<boolean> {
             }
           } as OnClickMenuItemModel,
         },
+      ];
+      const newSubMenu = {
+        id: 'new',
+        active: false,
+        visible: newSubMenuList.some(subMenu => subMenu.visible),
+        model: {
+          type: MenuItemType.TEXT,
+          text: 'menu.section.new'
+        } as TextMenuItemModel,
+        icon: 'plus',
+        index: 0
+      };
+      const editSubMenu = {
+        id: 'edit',
+        active: false,
+        visible: editSubMenuList.some(subMenu => subMenu.visible),
+        model: {
+          type: MenuItemType.TEXT,
+          text: 'menu.section.edit'
+        } as TextMenuItemModel,
+        icon: 'pencil-alt',
+        index: 1
+      };
+
+      const menuList = [
+        ...newSubMenuList,
+        newSubMenu,
+        ...editSubMenuList,
+        editSubMenu,
+        // TODO: enable this menu item once the feature has been implemented
+        // {
+        //   id: 'new_item_version',
+        //   parentID: 'new',
+        //   active: false,
+        //   visible: true,
+        //   model: {
+        //     type: MenuItemType.LINK,
+        //     text: 'menu.section.new_item_version',
+        //     link: ''
+        //   } as LinkMenuItemModel,
+        // },
 
         /* Statistics */
         // TODO: enable this menu item once the feature has been implemented

src/app/root/root.component.ts

@@ -61,7 +61,7 @@ export class RootComponent implements OnInit {
   }
 
   ngOnInit() {
-    this.sidebarVisible = this.menuService.isMenuVisible(MenuID.ADMIN);
+    this.sidebarVisible = this.menuService.isMenuVisibleWithVisibleSections(MenuID.ADMIN);
 
     this.collapsedSidebarWidth = this.cssService.getVariable('--ds-collapsed-sidebar-width');
     this.totalSidebarWidth = this.cssService.getVariable('--ds-total-sidebar-width');

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

@@ -243,6 +243,84 @@ describe('MenuService', () => {
     });
   });
 
+  describe('isMenuVisibleWithVisibleSections', () => {
+    it('should return false when the menu is empty', () => {
+      const testMenu = {
+        id: MenuID.ADMIN,
+        collapsed: false,
+        visible: true,
+        sections: {},
+        previewCollapsed: false,
+        sectionToSubsectionIndex: {}
+      } as any;
+      spyOn(service, 'getMenu').and.returnValue(observableOf(testMenu));
+
+      const result = service.isMenuVisibleWithVisibleSections(MenuID.ADMIN);
+      const expected = cold('(b|)', {
+        b: false
+      });
+
+      expect(result).toBeObservable(expected);
+    });
+    it('should return false when no top-level sections are visible', () => {
+      const noTopLevelVisibleSections = {
+        section: {id: 's1', visible: false},
+        section_2: {id: 's2', visible: false},
+        section_3: {id: 's3', visible: false},
+        section_4: {id: 's1_1', visible: true, parentID: 's1'},
+        section_5: {id: 's2_1', visible: true, parentID: 's2'},
+      };
+      const testMenu = {
+        id: MenuID.ADMIN,
+        collapsed: false,
+        visible: true,
+        sections: noTopLevelVisibleSections,
+        previewCollapsed: false,
+        sectionToSubsectionIndex: {
+          'section': ['section_4'],
+          'section_2': ['section_5'],
+        }
+      } as any;
+      spyOn(service, 'getMenu').and.returnValue(observableOf(testMenu));
+
+      const result = service.isMenuVisibleWithVisibleSections(MenuID.ADMIN);
+      const expected = cold('(b|)', {
+        b: false
+      });
+
+      expect(result).toBeObservable(expected);
+    });
+
+    it('should return true when any top-level section is visible', () => {
+      const noTopLevelVisibleSections = {
+        section: {id: 's1', visible: false},
+        section_2: {id: 's2', visible: true},
+        section_3: {id: 's3', visible: false},
+        section_4: {id: 's1_1', visible: true, parentID: 's1'},
+        section_5: {id: 's2_1', visible: true, parentID: 's2'},
+      };
+      const testMenu = {
+        id: MenuID.ADMIN,
+        collapsed: false,
+        visible: true,
+        sections: noTopLevelVisibleSections,
+        previewCollapsed: false,
+        sectionToSubsectionIndex: {
+          'section': ['section_4'],
+          'section_2': ['section_5'],
+        }
+      } as any;
+      spyOn(service, 'getMenu').and.returnValue(observableOf(testMenu));
+
+      const result = service.isMenuVisibleWithVisibleSections(MenuID.ADMIN);
+      const expected = cold('(b|)', {
+        b: true
+      });
+
+      expect(result).toBeObservable(expected);
+    });
+  });
+
   describe('isMenuVisible', () => {
     beforeEach(() => {
       spyOn(service, 'getMenu').and.returnValue(observableOf(fakeMenu));

src/app/shared/menu/menu.service.ts

@@ -181,6 +181,18 @@ export class MenuService {
     );
   }
 
+  /**
+   * Check if a given menu is visible and has visible top-level (!) sections
+   * @param {MenuID} menuID The ID of the menu that is to be checked
+   * @returns {Observable<boolean>} Emits true if the given menu is
+   *   visible and has visible sections, emits false when it's hidden
+   */
+  isMenuVisibleWithVisibleSections(menuID: MenuID): Observable<boolean> {
+    return observableCombineLatest([this.isMenuVisible(menuID), this.menuHasVisibleSections(menuID)]).pipe(
+      map(([menuVisible, visibleSections]) => menuVisible && visibleSections)
+    );
+  }
+
   /**
    * Check if a given menu is visible
    * @param {MenuID} menuID The ID of the menu that is to be checked
@@ -192,6 +204,20 @@ export class MenuService {
     );
   }
 
+  /**
+   * Check if a menu has at least one top-level (!) section that is visible.
+   * @param {MenuID} menuID The ID of the menu that is to be checked
+   * @returns {Observable<boolean>} Emits true if the given menu has visible sections, emits false otherwise
+   */
+  menuHasVisibleSections(menuID: MenuID): Observable<boolean> {
+    return this.getMenu(menuID).pipe(
+      map((state: MenuState) => hasValue(state)
+        ? Object.values(state.sections)
+          .some(section => section.visible && section.parentID === undefined)
+        : undefined)
+    );
+  }
+
   /**
    * Expands a given menu
    * @param {MenuID} menuID The ID of the menu

src/app/shared/testing/menu-service.stub.ts

@@ -66,6 +66,10 @@ export class MenuServiceStub {
     return observableOf(true);
   }
 
+  isMenuVisibleWithVisibleSections(id: MenuID): Observable<boolean> {
+    return observableOf(true);
+  }
+
   isMenuCollapsed(id: MenuID): Observable<boolean> {
     return observableOf(false);
   }