Skip to content

Commit

Permalink
Add feature flag to config and filter routes by flags.
Browse files Browse the repository at this point in the history
  • Loading branch information
@srpiatt
srpiatt committed May 17, 2024
1 parent b30e283 commit ff65159
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 175 deletions.
31 changes: 2 additions & 29 deletions src/lib/components/Navigation.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,7 @@
import { page } from '$app/stores';
import { goto } from '$app/navigation';
import logo from '$lib/assets/app-logo.png';
import { user, logout } from '$lib/stores/User';
import { PicsurePrivileges } from '$lib/models/Privilege';
import { routes } from '$lib/configuration';
let allowedRoutes = routes.filter((route) => !route.privilege);
function getUsersRoutes() {
if (!$user || !$user.privileges) {
allowedRoutes = routes.filter((route) => !route.privilege);
return allowedRoutes;
}
if ($user && $user.privileges && $user.privileges.includes(PicsurePrivileges.SUPER)) {
allowedRoutes = routes;
return allowedRoutes;
}
Object.values(PicsurePrivileges).forEach((privilege) => {
if ($user?.privileges?.includes(privilege)) {
allowedRoutes = [
...allowedRoutes,
...routes.filter(
(route) => route.privilege === privilege && !allowedRoutes.includes(route),
),
];
}
});
return allowedRoutes;
}
import { user, userRoutes, logout } from '$lib/stores/User';
function setDropdown(path: string) {
dropdownPath = path;
Expand All @@ -55,7 +29,6 @@
}
$: dropdownPath = '';
$: accessableRoutes = $user && getUsersRoutes();
</script>

<AppBar padding="py-0 pl-2 pr-5" background="bg-surface-50">
Expand All @@ -66,7 +39,7 @@
</svelte:fragment>
<nav id="page-navigation">
<ul>
{#each accessableRoutes as route}
{#each $userRoutes as route}
{#if route.children && route.children.length > 0}
<li
class={`has-dropdown ${dropdownPath === route.path ? 'open' : ''}`}
Expand Down
12 changes: 10 additions & 2 deletions src/lib/configuration.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { PicsurePrivileges } from './models/Privilege';
import type { Route } from './models/Route';
import type { Indexable } from './types';

export const branding = {
applicationName: 'PIC‑SURE',
Expand Down Expand Up @@ -144,18 +145,25 @@ export const routes: Route[] = [
],
},
{ path: '/admin/users', text: 'User Management', privilege: PicsurePrivileges.ADMIN },
{ path: '/admin/requests', text: 'Data Requests', privilege: PicsurePrivileges.DATA_ADMIN },
{
path: '/admin/requests',
text: 'Data Requests',
feature: 'dataRequests',
privilege: PicsurePrivileges.DATA_ADMIN,
},
{ path: '/explorer', text: 'Explorer' },
{ path: '/api', text: 'API', privilege: PicsurePrivileges.QUERY },
{ path: '/dataset', text: 'Dataset Management', privilege: PicsurePrivileges.QUERY },
{ path: '/help', text: 'Help' },
];

export const features = {
export const features: Indexable = {
explorer: {
allowExport: true,
exportsEnableExport: true,
},
// Env Feature Flags
dataRequests: true,
};

export const resources = {
Expand Down
1 change: 1 addition & 0 deletions src/lib/models/Route.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ export type Route = {
path: string;
text: string;
privilege?: PicsurePrivileges;
feature?: string;
children?: Route[];
};
38 changes: 36 additions & 2 deletions src/lib/stores/User.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,44 @@
import { get, writable, type Writable } from 'svelte/store';
import { get, writable, derived, type Writable, type Readable } from 'svelte/store';
import { browser } from '$app/environment';

import * as api from '$lib/api';
import type { User } from '../models/User';
import type { Route } from '$lib/models/Route';
import type { User } from '$lib/models/User';
import { PicsurePrivileges } from '$lib/models/Privilege';
import { routes, features } from '$lib/configuration';

export const user: Writable<User> = writable({});
export const userRoutes: Readable<Route[]> = derived(user, ($user) => {
const userPrivileges: string[] = $user?.privileges || [];

if (userPrivileges.length === 0) {
// Public routes for non-logged in user
return routes.filter((route) => !route.privilege);
}

function featureRoutes(routeList: Route[]): Route[] {
return routeList
.filter((route) => (route.feature ? !!features[route.feature] : true))
.map((route: Route) =>
route.children ? { ...route, children: featureRoutes(route.children) } : route,
);
}
const featured = featureRoutes(routes);

if (userPrivileges.includes(PicsurePrivileges.SUPER)) {
// All routes in feature for super users
return featured;
}

function allowedRoutes(routeList: Route[]): Route[] {
return routeList
.filter((route) => (route.privilege ? userPrivileges.includes(route.privilege) : true))
.map((route: Route) =>
route.children ? { ...route, children: allowedRoutes(route.children) } : route,
);
}
return allowedRoutes(featured);
});

export async function getUser(force?: boolean) {
if (force || !get(user)?.privileges || !get(user)?.token) {
Expand Down
196 changes: 72 additions & 124 deletions tests/lib/component/navigation/test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,83 +1,49 @@
import { expect } from '@playwright/test';
import { getUserTest, test, unauthedTest } from '../../../custom-context';
import { picsureUser, picsureAdmin, topAdmin, mockLoginResponse } from '../../../mock-data';
import { picsureUser, mockLoginResponse } from '../../../mock-data';
import { routes } from '../../../../src/lib/configuration';
import { PicsurePrivileges } from '../../../../src/lib/models/Privilege';
import type { Route } from '../../../../src/lib/models/Route';

// TODO: This should probably be moved to a component test, not an e2e/integration test.

const unprotectedRoutes = routes.filter((route) => route.privilege === undefined);
const protectedRoutes = [
...unprotectedRoutes,
...routes.filter((route) => route.privilege === PicsurePrivileges.QUERY),
];
const adminRoutes = [
...protectedRoutes,
...routes.filter((route) => route.privilege === PicsurePrivileges.ADMIN),
];
const superRoutes = [
...adminRoutes,
...routes.filter(
(route) => route.privilege === PicsurePrivileges.SUPER && route.path !== '/admin',
),
];

const userTest = getUserTest(picsureUser);
const adminTest = getUserTest(picsureAdmin);
const topAdminTest = getUserTest(topAdmin);

unauthedTest.describe('Unauthorized Navigation', () => {
unprotectedRoutes.forEach((route, _index, routes) => {
unauthedTest(
`Path ${route.path} navigation bar has correct active element before login`,
async ({ page }) => {
// Given
await page.goto(route.path);

// Then
// Check that this element is active
const navItem = page.locator(`#nav-link${route.path.replaceAll('/', '-')}`);
await expect(navItem).toHaveAttribute('aria-current', 'page');

// Check that other routes elements aren't active
const inactive = routes
.filter((altRoute) => altRoute.path !== route.path)
.map((altRoute) => {
const navItem = page.locator(`#nav-link${altRoute.path.replaceAll('/', '-')}`);
return expect(navItem).not.toHaveAttribute('aria-current');
});
await Promise.all(inactive);
},
);
});
const testCases = {
generalUser: [PicsurePrivileges.QUERY],
adminUser: [PicsurePrivileges.QUERY, PicsurePrivileges.ADMIN],
superUser: [PicsurePrivileges.QUERY, PicsurePrivileges.SUPER],
dataUser: [PicsurePrivileges.QUERY, PicsurePrivileges.DATA_ADMIN],
};

unauthedTest.describe('Public Routes Navigation', () => {
routes
.filter((route) => route.privilege === undefined)
.forEach((route, _index, routes) => {
unauthedTest(
`Path ${route.path} navigation bar has correct active element before login`,
async ({ page }) => {
// Given
await page.goto(route.path);

// Then
// Check that this element is active
const navItem = page.locator(`#nav-link${route.path.replaceAll('/', '-')}`);
await expect(navItem).toHaveAttribute('aria-current', 'page');

// Check that other routes elements aren't active
const inactive = routes
.filter((altRoute) => altRoute.path !== route.path)
.map((altRoute) => {
const navItem = page.locator(`#nav-link${altRoute.path.replaceAll('/', '-')}`);
return expect(navItem).not.toHaveAttribute('aria-current');
});
await Promise.all(inactive);
},
);
});
});

userTest.describe('Authorized Navigation', () => {
protectedRoutes.forEach((route, _index, routes) => {
userTest(
`Users: Path ${route.path} navigation bar has correct active element after login`,
async ({ page }) => {
// Given
await page.goto(mockLoginResponse);
await page.waitForURL('/');
const navItem = page.locator(`#nav-link${route.path.replaceAll('/', '-')}`);
await navItem.click();

// Then
// Check that this element is active
await expect(navItem).toHaveAttribute('aria-current', 'page');

// Check that other routes elements aren't active
const inactive = routes
.filter((altRoute) => altRoute.path !== route.path)
.map((altRoute) => {
const navItem = page.locator(`#nav-link${altRoute.path.replaceAll('/', '-')}`);
return expect(navItem).not.toHaveAttribute('aria-current');
});
await Promise.all(inactive);
},
);
});
const userTest = getUserTest({ ...picsureUser, privileges: testCases.generalUser });
userTest.describe('Logged in users', () => {
userTest('Session avatar should reflect correct user initial after login', async ({ page }) => {
// Given
await page.goto(mockLoginResponse);
Expand All @@ -103,62 +69,44 @@ userTest.describe('Authorized Navigation', () => {
});
});

adminTest.describe('Admin Navigation', () => {
adminRoutes.forEach((route, _index, routes) => {
adminTest(
`Admins: Path ${route.path} navigation bar has correct active element after login`,
async ({ page }) => {
// Given
await page.goto(mockLoginResponse);
await page.waitForURL('/');
const navItem = page.locator(`#nav-link${route.path.replaceAll('/', '-')}`);
await navItem.click();

// Then
// Check that this element is active
await expect(navItem).toHaveAttribute('aria-current', 'page');

// Check that other routes elements aren't active
const inactive = routes
.filter((altRoute) => altRoute.path !== route.path)
.map((altRoute) => {
const navItem = page.locator(`#nav-link${altRoute.path.replaceAll('/', '-')}`);
return expect(navItem).not.toHaveAttribute('aria-current');
});
await Promise.all(inactive);
},
);
});
});

topAdminTest.describe('Super Admin Navigation', () => {
superRoutes.forEach((route, _index, routes) => {
topAdminTest(
`Top Admins: Path ${route.path} navigation bar has correct active element after login`,
async ({ page }) => {
// Given
await page.goto(mockLoginResponse);
await page.waitForURL('/');
const navItem = page.locator(`#nav-link${route.path.replaceAll('/', '-')}`);
await navItem.click();

// Then
// Check that this element is active
await expect(navItem).toHaveAttribute('aria-current', 'page');

// Check that other routes elements aren't active
const inactive = routes
.filter((altRoute) => altRoute.path !== route.path)
.map((altRoute) => {
const navItem = page.locator(`#nav-link${altRoute.path.replaceAll('/', '-')}`);
return expect(navItem).not.toHaveAttribute('aria-current');
});
await Promise.all(inactive);
},
);
Object.entries(testCases).forEach(([testCase, privileges]) => {
const privTest = getUserTest({ ...picsureUser, privileges });
const testRoutes = routes.filter((route: Route) =>
route.privilege ? privileges.includes(route.privilege) : true,
);

privTest.describe(`${testCase} Navigation`, () => {
testRoutes
.filter((route) => !route.children)
.forEach((route, _index, routes) => {
privTest(
`Path ${route.path} navigation bar has correct active element after login`,
async ({ page }) => {
// Given
await page.goto(mockLoginResponse);
await page.waitForURL('/');
const navItem = page.locator(`#nav-link${route.path.replaceAll('/', '-')}`);
await navItem.click();

// Then
// Check that this element is active
await expect(navItem).toHaveAttribute('aria-current', 'page');

// Check that other routes elements aren't active
const inactive = routes
.filter((altRoute) => altRoute.path !== route.path)
.map((altRoute) => {
const navItem = page.locator(`#nav-link${altRoute.path.replaceAll('/', '-')}`);
return expect(navItem).not.toHaveAttribute('aria-current');
});
await Promise.all(inactive);
},
);
});
});
});

const topAdminTest = getUserTest({ ...picsureUser, privileges: testCases.superUser });
topAdminTest.describe('Keyboard navigation', () => {
topAdminTest('Pressing Enter on a dropdown item opens dropdown', async ({ page }) => {
// Given
Expand Down
18 changes: 0 additions & 18 deletions tests/mock-data.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,6 @@ export const picsureUser: User = {
acceptedTOS: true,
};

export const picsureAdmin: User = {
uuid: '1234',
email: 'admin@pic-sure.org',
privileges: [PicsurePrivileges.QUERY, PicsurePrivileges.ADMIN],
// expired token
token: mockToken,
acceptedTOS: true,
};

export const topAdmin: User = {
uuid: '1234',
email: 'admin@pic-sure.org',
privileges: [PicsurePrivileges.QUERY, PicsurePrivileges.SUPER],
// expired token
token: mockToken,
acceptedTOS: true,
};

export const searchResultPath = `*/**/picsure/search/${resources.hpds}`;
export const searchResults = {
results: {
Expand Down

0 comments on commit ff65159

Please sign in to comment.