Server: Fixed sidebar menu selection

packages/server/src/middleware/routeHandler.ts

@@ -10,7 +10,7 @@ export default async function(ctx: AppContext) {
 	const requestStartTime = Date.now();
 
 	try {
-		const responseObject = await execRequest(ctx.joplin.routes, ctx);
+		const { response: responseObject, path } = await execRequest(ctx.joplin.routes, ctx);
 
 		if (responseObject instanceof Response) {
 			ctx.response = responseObject.response;
@@ -20,7 +20,7 @@ export default async function(ctx: AppContext) {
 			const view = responseObject as View;
 			ctx.response.status = view?.content?.error ? view?.content?.error?.httpCode || 500 : 200;
 			ctx.response.body = await ctx.joplin.services.mustache.renderView(view, {
-				currentUrl: ctx.URL,
+				currentPath: path,
 				notifications: ctx.joplin.notifications || [],
 				hasNotifications: !!ctx.joplin.notifications && !!ctx.joplin.notifications.length,
 				owner: ctx.joplin.owner,

packages/server/src/services/MustacheService.ts

@@ -5,14 +5,13 @@ import config from '../config';
 import { filename } from '@joplin/lib/path-utils';
 import { NotificationView } from '../utils/types';
 import { User } from '../services/database/types';
-import { makeUrl, UrlType } from '../utils/routeUtils';
+import { makeUrl, SubPath, urlMatchesSchema, UrlType } from '../utils/routeUtils';
 import MarkdownIt = require('markdown-it');
 import { headerAnchor } from '@joplin/renderer';
 import { _ } from '@joplin/lib/locale';
-import { adminDashboardUrl, adminEmailsUrl, adminTasksUrl, adminUserDeletionsUrl, adminUsersUrl, changesUrl, homeUrl, itemsUrl, stripOffQueryParameters } from '../utils/urlUtils';
-import { URL } from 'url';
+import { adminDashboardUrl, adminEmailsUrl, adminTasksUrl, adminUserDeletionsUrl, adminUsersUrl, changesUrl, homeUrl, itemsUrl } from '../utils/urlUtils';
 
-type MenuItemSelectedCondition = (selectedUrl: URL)=> boolean;
+type MenuItemSelectedCondition = (selectedUrl: SubPath)=> boolean;
 
 export interface MenuItem {
 	title: string;
@@ -64,7 +63,7 @@ interface GlobalParams {
 	isAdminPage?: boolean;
 	adminMenu?: MenuItem[];
 	navbarMenu?: MenuItem[];
-	currentUrl?: URL;
+	currentPath?: SubPath;
 }
 
 export function isView(o: any): boolean {
@@ -112,25 +111,23 @@ export default class MustacheService {
 		return `${config().layoutDir}/${name}.mustache`;
 	}
 
-	private setSelectedMenu(selectedUrl: URL, menuItems: MenuItem[]) {
-		if (!selectedUrl) return;
+	private setSelectedMenu(selectedPath: SubPath, menuItems: MenuItem[]) {
+		if (!selectedPath) return;
 		if (!menuItems) return;
 
-		const url = stripOffQueryParameters(selectedUrl.href);
-
 		for (const menuItem of menuItems) {
 			if (menuItem.url) {
 				if (menuItem.selectedCondition) {
-					menuItem.selected = menuItem.selectedCondition(selectedUrl);
+					menuItem.selected = menuItem.selectedCondition(selectedPath);
 				} else {
-					menuItem.selected = url === menuItem.url;
+					menuItem.selected = urlMatchesSchema(menuItem.url, selectedPath.schema);
 				}
 			}
-			this.setSelectedMenu(selectedUrl, menuItem.children);
+			this.setSelectedMenu(selectedPath, menuItem.children);
 		}
 	}
 
-	private makeAdminMenu(selectedUrl: URL): MenuItem[] {
+	private makeAdminMenu(selectedPath: SubPath): MenuItem[] {
 		const output: MenuItem[] = [
 			{
 				title: _('General'),
@@ -159,12 +156,12 @@ export default class MustacheService {
 			},
 		];
 
-		this.setSelectedMenu(selectedUrl, output);
+		this.setSelectedMenu(selectedPath, output);
 
 		return output;
 	}
 
-	private makeNavbar(selectedUrl: URL, isAdmin: boolean): MenuItem[] {
+	private makeNavbar(selectedPath: SubPath, isAdmin: boolean): MenuItem[] {
 		let output: MenuItem[] = [
 			{
 				title: _('Home'),
@@ -186,14 +183,14 @@ export default class MustacheService {
 					title: _('Admin'),
 					url: adminDashboardUrl(),
 					icon: 'fas fa-hammer',
-					selectedCondition: (selectedUrl: URL) => {
-						return selectedUrl.pathname.startsWith('/admin/') || selectedUrl.pathname === '/admin';
+					selectedCondition: (selectedPath: SubPath) => {
+						return selectedPath.schema.startsWith('admin/') || selectedPath.schema === 'admin';
 					},
 				},
 			]);
 		}
 
-		this.setSelectedMenu(selectedUrl, output);
+		this.setSelectedMenu(selectedPath, output);
 
 		return output;
 	}
@@ -295,8 +292,8 @@ export default class MustacheService {
 		globalParams = {
 			...this.defaultLayoutOptions,
 			...globalParams,
-			adminMenu: globalParams ? this.makeAdminMenu(globalParams.currentUrl) : null,
-			navbarMenu: this.makeNavbar(globalParams?.currentUrl, globalParams?.owner ? !!globalParams.owner.is_admin : false),
+			adminMenu: globalParams ? this.makeAdminMenu(globalParams.currentPath) : null,
+			navbarMenu: this.makeNavbar(globalParams?.currentPath, globalParams?.owner ? !!globalParams.owner.is_admin : false),
 			userDisplayName: this.userDisplayName(globalParams ? globalParams.owner : null),
 			isAdminPage,
 			s: {

packages/server/src/utils/routeUtils.test.ts

@@ -1,4 +1,4 @@
-import { findMatchingRoute, isValidOrigin, parseSubPath, splitItemPath } from './routeUtils';
+import { findMatchingRoute, isValidOrigin, parseSubPath, splitItemPath, urlMatchesSchema } from './routeUtils';
 import { ItemAddressingType } from '../services/database/types';
 import { RouteType } from './types';
 import { expectThrow } from './testing/testUtils';
@@ -99,7 +99,7 @@ describe('routeUtils', function() {
 	});
 
 	it('should check the request origin for API URLs', async function() {
-		const testCases: any[] = [
+		const testCases: [string, string, boolean][] = [
 			[
 				'https://example.com', // Request origin
 				'https://example.com', // Config base URL
@@ -141,7 +141,7 @@ describe('routeUtils', function() {
 	});
 
 	it('should check the request origin for User Content URLs', async function() {
-		const testCases: any[] = [
+		const testCases: [string, string, boolean][] = [
 			[
 				'https://usercontent.local', // Request origin
 				'https://usercontent.local', // Config base URL
@@ -170,4 +170,40 @@ describe('routeUtils', function() {
 		}
 	});
 
+	it('should check if a URL matches a schema', async function() {
+		const testCases: [string, string, boolean][] = [
+			[
+				'https://test.com/items/123/children',
+				'items/:id/children',
+				true,
+			],
+			[
+				'https://test.com/items/123',
+				'items/:id',
+				true,
+			],
+			[
+				'https://test.com/items',
+				'items',
+				true,
+			],
+			[
+				'https://test.com/items/123/children',
+				'items/:id',
+				false,
+			],
+			[
+				'',
+				'items/:id',
+				false,
+			],
+		];
+
+		for (const testCase of testCases) {
+			const [url, schema, expected] = testCase;
+			const actual = urlMatchesSchema(url, schema);
+			expect(actual).toBe(expected);
+		}
+	});
+
 });

packages/server/src/utils/routeUtils.ts

@@ -6,6 +6,7 @@ import { AppContext, HttpMethod, RouteType } from './types';
 import { URL } from 'url';
 import { csrfCheck } from './csrf';
 import { contextSessionId } from './requestUtils';
+import { stripOffQueryParameters } from './urlUtils';
 
 const { ltrimSlashes, rtrimSlashes } = require('@joplin/lib/path-utils');
 
@@ -112,6 +113,12 @@ export function isPathBasedAddressing(fileId: string): boolean {
 	return fileId.indexOf(':') >= 0;
 }
 
+export const urlMatchesSchema = (url: string, schema: string): boolean => {
+	url = stripOffQueryParameters(url);
+	const regex = new RegExp(`${schema.replace(/:id/, '[a-zA-Z0-9]+')}$`);
+	return !!url.match(regex);
+};
+
 // Allows parsing the two types of paths supported by the API:
 //
 // root:/Documents/MyFile.md:/content
@@ -189,7 +196,12 @@ function disabledAccountCheck(route: MatchedRoute, user: User) {
 	if (route.subPath.schema.startsWith('api/')) throw new ErrorForbidden(`This account is disabled. Please login to ${config().baseUrl} for more information.`);
 }
 
-export async function execRequest(routes: Routers, ctx: AppContext) {
+interface ExecRequestResult {
+	response: any;
+	path: SubPath;
+}
+
+export async function execRequest(routes: Routers, ctx: AppContext): Promise<ExecRequestResult> {
 	const match = findMatchingRoute(ctx.path, routes);
 	if (!match) throw new ErrorNotFound();
 
@@ -215,7 +227,10 @@ export async function execRequest(routes: Routers, ctx: AppContext) {
 	await csrfCheck(ctx, isPublicRoute);
 	disabledAccountCheck(match, ctx.joplin.owner);
 
-	return endPoint.handler(match.subPath, ctx);
+	return {
+		response: await endPoint.handler(match.subPath, ctx),
+		path: match.subPath,
+	};
 }
 
 // In a path such as "/api/files/SOME_ID/content" we want to find: