chore: fix admin routes should respect plan data (#9828)

https://linear.app/unleash/issue/2-2852/sidebar-bug-with-enterprisepro-only-route-constraints

Fixes an issue where admin routes didn't respect plan data if their flag
was enabled.

First noticed here:
https://github.com/Unleash/unleash/pull/8469#discussion_r1804361222

Issue was that only `adminRoutes` respected plan data. `mainNavRoutes`
and `primaryRoutes` did not follow the same filtering logic.

We can probably clean this up even further in the future, but didn't
want to extend the PR too much.

Also adds tests to validate the intended behavior.

frontend/src/component/admin/filterAdminRoutes.test.ts

@@ -1,9 +1,9 @@
-import { filterAdminRoutes } from './filterAdminRoutes';
+import { filterRoutesByPlanData } from './filterRoutesByPlanData';
 
-describe('filterAdminRoutes - open souce routes', () => {
+describe('filterRoutesByPlanData - open souce routes', () => {
     test('open source - should show menu item if mode paid plan mode is not defined', () => {
         expect(
-            filterAdminRoutes(
+            filterRoutesByPlanData(
                 {},
                 {
                     pro: false,
@@ -21,12 +21,14 @@ describe('filterAdminRoutes - open souce routes', () => {
             billing: false,
         };
 
-        expect(filterAdminRoutes({ mode: ['pro'] }, state)).toBe(false);
-        expect(filterAdminRoutes({ mode: ['enterprise'] }, state)).toBe(false);
-        expect(filterAdminRoutes({ mode: ['pro', 'enterprise'] }, state)).toBe(
+        expect(filterRoutesByPlanData({ mode: ['pro'] }, state)).toBe(false);
+        expect(filterRoutesByPlanData({ mode: ['enterprise'] }, state)).toBe(
             false,
         );
-        expect(filterAdminRoutes({ billing: true }, state)).toBe(false);
+        expect(
+            filterRoutesByPlanData({ mode: ['pro', 'enterprise'] }, state),
+        ).toBe(false);
+        expect(filterRoutesByPlanData({ billing: true }, state)).toBe(false);
     });
 
     test('pro - should show menu item for pro customers', () => {
@@ -36,12 +38,14 @@ describe('filterAdminRoutes - open souce routes', () => {
             billing: false,
         };
 
-        expect(filterAdminRoutes({ mode: ['pro'] }, state)).toBe(true);
-        expect(filterAdminRoutes({ mode: ['pro', 'enterprise'] }, state)).toBe(
+        expect(filterRoutesByPlanData({ mode: ['pro'] }, state)).toBe(true);
+        expect(
+            filterRoutesByPlanData({ mode: ['pro', 'enterprise'] }, state),
+        ).toBe(true);
+        // This is to show enterprise badge in pro mode
+        expect(filterRoutesByPlanData({ mode: ['enterprise'] }, state)).toBe(
             true,
         );
-        // This is to show enterprise badge in pro mode
-        expect(filterAdminRoutes({ mode: ['enterprise'] }, state)).toBe(true);
     });
 
     test('enterprise - should show menu item if mode enterprise is defined or mode is undefined', () => {
@@ -51,16 +55,18 @@ describe('filterAdminRoutes - open souce routes', () => {
             billing: false,
         };
 
-        expect(filterAdminRoutes({ mode: ['enterprise'] }, state)).toBe(true);
-        expect(filterAdminRoutes({ mode: ['pro', 'enterprise'] }, state)).toBe(
+        expect(filterRoutesByPlanData({ mode: ['enterprise'] }, state)).toBe(
             true,
         );
-        expect(filterAdminRoutes({ mode: ['pro'] }, state)).toBe(false);
+        expect(
+            filterRoutesByPlanData({ mode: ['pro', 'enterprise'] }, state),
+        ).toBe(true);
+        expect(filterRoutesByPlanData({ mode: ['pro'] }, state)).toBe(false);
     });
 
     test('billing - should show menu item if billing is defined', () => {
         expect(
-            filterAdminRoutes(
+            filterRoutesByPlanData(
                 { mode: ['pro'], billing: true },
                 {
                     pro: true,
@@ -70,7 +76,7 @@ describe('filterAdminRoutes - open souce routes', () => {
             ),
         ).toBe(true);
         expect(
-            filterAdminRoutes(
+            filterRoutesByPlanData(
                 { mode: ['enterprise'], billing: true },
                 {
                     pro: false,
@@ -80,7 +86,7 @@ describe('filterAdminRoutes - open souce routes', () => {
             ),
         ).toBe(true);
         expect(
-            filterAdminRoutes(
+            filterRoutesByPlanData(
                 { mode: ['pro', 'enterprise'], billing: true },
                 {
                     pro: true,
@@ -90,7 +96,7 @@ describe('filterAdminRoutes - open souce routes', () => {
             ),
         ).toBe(true);
         expect(
-            filterAdminRoutes(
+            filterRoutesByPlanData(
                 { mode: ['pro'], billing: true },
                 {
                     pro: false,

frontend/src/component/admin/filterRoutesByPlanData.ts

@@ -1,12 +1,14 @@
 import type { INavigationMenuItem } from 'interfaces/route';
 
-export const filterAdminRoutes = (
+export type PlanData = {
+    enterprise: boolean;
+    pro: boolean;
+    billing: boolean;
+};
+
+export const filterRoutesByPlanData = (
     menu: INavigationMenuItem['menu'],
-    {
-        pro,
-        enterprise,
-        billing,
-    }: { pro?: boolean; enterprise?: boolean; billing?: boolean },
+    { pro, enterprise, billing }: PlanData,
 ): boolean => {
     const mode = menu?.mode;
     if (menu?.billing && !billing) return false;

frontend/src/component/admin/useAdminRoutes.ts

@@ -2,7 +2,7 @@ import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
 import { adminRoutes as oldAdminRoutes } from './oldAdminRoutes';
 import { adminRoutes } from './adminRoutes';
 import { useInstanceStatus } from 'hooks/api/getters/useInstanceStatus/useInstanceStatus';
-import { filterAdminRoutes } from './filterAdminRoutes';
+import { filterRoutesByPlanData } from './filterRoutesByPlanData';
 import { filterByConfig, mapRouteLink } from 'component/common/util';
 import { useUiFlag } from 'hooks/useUiFlag';
 
@@ -25,7 +25,7 @@ export const useAdminRoutes = () => {
     return routes
         .filter(filterByConfig(uiConfig))
         .filter((route) =>
-            filterAdminRoutes(route?.menu, {
+            filterRoutesByPlanData(route?.menu, {
                 enterprise: isEnterprise(),
                 pro: isPro(),
                 billing: isBilling,

frontend/src/component/layout/MainLayout/AdminMenu/AdminNavigationItems.tsx

@@ -16,7 +16,7 @@ import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
 import { useInstanceStatus } from 'hooks/api/getters/useInstanceStatus/useInstanceStatus';
 import { Link, useLocation } from 'react-router-dom';
 import { filterByConfig } from 'component/common/util';
-import { filterAdminRoutes } from 'component/admin/filterAdminRoutes';
+import { filterRoutesByPlanData } from 'component/admin/filterRoutesByPlanData';
 import { adminGroups, adminRoutes } from 'component/admin/adminRoutes';
 import { useEffect, useState, type ReactNode } from 'react';
 import type { INavigationMenuItem } from 'interfaces/route';
@@ -147,7 +147,7 @@ export const AdminNavigationItems = ({
     const routes = adminRoutes
         .filter(filterByConfig(uiConfig))
         .filter((route) =>
-            filterAdminRoutes(route?.menu, {
+            filterRoutesByPlanData(route?.menu, {
                 enterprise: isEnterprise(),
                 pro: isPro(),
                 billing: isBilling,

frontend/src/component/layout/MainLayout/NavigationSidebar/useRoutes.test.ts

@@ -0,0 +1,170 @@
+import { renderHook } from '@testing-library/react';
+import { useRoutes } from './useRoutes';
+import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
+import { useInstanceStatus } from 'hooks/api/getters/useInstanceStatus/useInstanceStatus';
+import { type Mock, vi } from 'vitest';
+
+vi.mock('hooks/api/getters/useUiConfig/useUiConfig');
+vi.mock('hooks/api/getters/useInstanceStatus/useInstanceStatus');
+vi.mock('component/menu/routes', () => ({
+    getNavRoutes: () => [
+        { path: '/features', title: 'Features', menu: { main: true } },
+        {
+            path: '/enterprise',
+            title: 'Enterprise',
+            menu: { main: true, mode: ['enterprise'] },
+        },
+        { path: '/pro', title: 'Pro', menu: { main: true, mode: ['pro'] } },
+        {
+            path: '/billing',
+            title: 'Billing',
+            menu: { main: true, billing: true },
+        },
+        {
+            path: '/flagged-enterprise',
+            title: 'Flagged Enterprise',
+            menu: { main: true, mode: ['enterprise'] },
+            flag: 'someFeatureFlag',
+        },
+    ],
+    getPrimaryRoutes: () => [
+        { path: '/overview', title: 'Overview', menu: { primary: true } },
+        {
+            path: '/admin',
+            title: 'Admin',
+            menu: { primary: true, mode: ['enterprise'] },
+        },
+    ],
+}));
+
+describe('useRoutes', () => {
+    beforeEach(() => {
+        vi.resetAllMocks();
+    });
+
+    test('filters routes based on enterprise access', () => {
+        (useUiConfig as Mock).mockReturnValue({
+            uiConfig: { flags: {} },
+            isEnterprise: () => true,
+            isPro: () => false,
+        });
+        (useInstanceStatus as Mock).mockReturnValue({
+            isBilling: false,
+        });
+
+        const { result } = renderHook(() => useRoutes());
+
+        expect(result.current.routes.mainNavRoutes).toHaveLength(2);
+        expect(result.current.routes.mainNavRoutes[0].path).toBe('/features');
+        expect(result.current.routes.mainNavRoutes[1].path).toBe('/enterprise');
+    });
+
+    test('filters routes based on pro access (still shows enterprise routes with badge)', () => {
+        (useUiConfig as Mock).mockReturnValue({
+            uiConfig: { flags: {} },
+            isEnterprise: () => false,
+            isPro: () => true,
+        });
+        (useInstanceStatus as Mock).mockReturnValue({
+            isBilling: false,
+        });
+
+        const { result } = renderHook(() => useRoutes());
+
+        expect(result.current.routes.mainNavRoutes).toHaveLength(3);
+        expect(result.current.routes.mainNavRoutes[0].path).toBe('/features');
+        expect(result.current.routes.mainNavRoutes[1].path).toBe('/enterprise');
+        expect(result.current.routes.mainNavRoutes[2].path).toBe('/pro');
+    });
+
+    test('filters routes based on billing access', () => {
+        (useUiConfig as Mock).mockReturnValue({
+            uiConfig: { flags: {} },
+            isEnterprise: () => false,
+            isPro: () => false,
+        });
+        (useInstanceStatus as Mock).mockReturnValue({
+            isBilling: true,
+        });
+
+        const { result } = renderHook(() => useRoutes());
+
+        expect(result.current.routes.mainNavRoutes).toHaveLength(2);
+        expect(result.current.routes.mainNavRoutes[0].path).toBe('/features');
+        expect(result.current.routes.mainNavRoutes[1].path).toBe('/billing');
+    });
+
+    test('filters primary routes based on enterprise access', () => {
+        (useUiConfig as Mock).mockReturnValue({
+            uiConfig: { flags: {} },
+            isEnterprise: () => true,
+            isPro: () => false,
+        });
+        (useInstanceStatus as Mock).mockReturnValue({
+            isBilling: false,
+        });
+
+        const { result } = renderHook(() => useRoutes());
+
+        expect(result.current.routes.primaryRoutes).toHaveLength(2);
+        expect(result.current.routes.primaryRoutes[0].path).toBe('/overview');
+        expect(result.current.routes.primaryRoutes[1].path).toBe('/admin');
+    });
+
+    test('filters primary routes without enterprise access', () => {
+        (useUiConfig as Mock).mockReturnValue({
+            uiConfig: { flags: {} },
+            isEnterprise: () => false,
+            isPro: () => false,
+        });
+        (useInstanceStatus as Mock).mockReturnValue({
+            isBilling: false,
+        });
+
+        const { result } = renderHook(() => useRoutes());
+
+        expect(result.current.routes.primaryRoutes).toHaveLength(1);
+        expect(result.current.routes.primaryRoutes[0].path).toBe('/overview');
+    });
+
+    test('does not show enterprise routes if not enterprise, even if feature flag is enabled', () => {
+        (useUiConfig as Mock).mockReturnValue({
+            uiConfig: { flags: { someFeatureFlag: true } },
+            isEnterprise: () => false,
+            isPro: () => false,
+        });
+        (useInstanceStatus as Mock).mockReturnValue({
+            isBilling: false,
+        });
+
+        const { result } = renderHook(() => useRoutes());
+
+        expect(result.current.routes.mainNavRoutes).toHaveLength(1);
+        expect(result.current.routes.mainNavRoutes[0].path).toBe('/features');
+        expect(
+            result.current.routes.mainNavRoutes.find(
+                (r) => r.path === '/flagged-enterprise',
+            ),
+        ).toBeUndefined();
+    });
+
+    test('shows enterprise routes with enabled feature flags when enterprise', () => {
+        (useUiConfig as Mock).mockReturnValue({
+            uiConfig: { flags: { someFeatureFlag: true } },
+            isEnterprise: () => true,
+            isPro: () => false,
+        });
+        (useInstanceStatus as Mock).mockReturnValue({
+            isBilling: false,
+        });
+
+        const { result } = renderHook(() => useRoutes());
+
+        expect(result.current.routes.mainNavRoutes).toHaveLength(3);
+        expect(result.current.routes.mainNavRoutes[0].path).toBe('/features');
+        expect(result.current.routes.mainNavRoutes[1].path).toBe('/enterprise');
+        expect(result.current.routes.mainNavRoutes[2].path).toBe(
+            '/flagged-enterprise',
+        );
+    });
+});

frontend/src/component/layout/MainLayout/NavigationSidebar/useRoutes.ts

@@ -2,21 +2,48 @@ import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
 import { getNavRoutes, getPrimaryRoutes } from 'component/menu/routes';
 import { useAdminRoutes } from 'component/admin/useAdminRoutes';
 import { filterByConfig, mapRouteLink } from 'component/common/util';
+import {
+    filterRoutesByPlanData,
+    type PlanData,
+} from 'component/admin/filterRoutesByPlanData';
+import { useInstanceStatus } from 'hooks/api/getters/useInstanceStatus/useInstanceStatus';
+import type { INavigationMenuItem } from 'interfaces/route';
+import type { IUiConfig } from 'interfaces/uiConfig';
+
+const filterRoutes = (
+    routes: INavigationMenuItem[],
+    uiConfig: IUiConfig,
+    { enterprise, pro, billing }: PlanData,
+) => {
+    return routes
+        .filter(filterByConfig(uiConfig))
+        .filter((route) =>
+            filterRoutesByPlanData(route?.menu, {
+                enterprise,
+                pro,
+                billing,
+            }),
+        )
+        .map(mapRouteLink);
+};
 
 export const useRoutes = () => {
-    const { uiConfig } = useUiConfig();
+    const { uiConfig, isPro, isEnterprise } = useUiConfig();
+    const { isBilling } = useInstanceStatus();
     const routes = getNavRoutes();
     const adminRoutes = useAdminRoutes();
     const primaryRoutes = getPrimaryRoutes();
 
+    const planData: PlanData = {
+        enterprise: isEnterprise(),
+        pro: isPro(),
+        billing: isBilling,
+    };
+
     const filteredMainRoutes = {
-        mainNavRoutes: routes
-            .filter(filterByConfig(uiConfig))
-            .map(mapRouteLink),
+        mainNavRoutes: filterRoutes(routes, uiConfig, planData),
         adminRoutes,
-        primaryRoutes: primaryRoutes
-            .filter(filterByConfig(uiConfig))
-            .map(mapRouteLink),
+        primaryRoutes: filterRoutes(primaryRoutes, uiConfig, planData),
     };
 
     return { routes: filteredMainRoutes };