diff --git a/artifacts/api-server/src/routes/apps.ts b/artifacts/api-server/src/routes/apps.ts index 50ba7b1e..61bd6d5d 100644 --- a/artifacts/api-server/src/routes/apps.ts +++ b/artifacts/api-server/src/routes/apps.ts @@ -1,6 +1,6 @@ import { Router, type IRouter } from "express"; import { eq, and, asc, inArray, sql } from "drizzle-orm"; -import { db } from "@workspace/db"; +import { db, isBuiltinAppSlug } from "@workspace/db"; import { appsTable, appPermissionsTable, @@ -381,6 +381,25 @@ router.patch("/apps/:id", requireAdmin, async (req, res): Promise => { .from(appsTable) .where(eq(appsTable.id, params.data.id)); + // Task #517: lock the route field for built-in apps. Their paths are + // hardcoded in artifacts/tx-os/src/App.tsx — letting an admin retype + // the path silently breaks the launcher entry (the user reported this + // after editing the Executive Meetings app). We allow admins to keep + // editing every other field (icon, image, color, name, etc.) but a + // route change targeting a built-in slug is rejected up-front. + if ( + previous && + isBuiltinAppSlug(previous.slug) && + parsed.data.route !== undefined && + parsed.data.route !== previous.route + ) { + res.status(400).json({ + error: "The path of a built-in app cannot be changed.", + code: "builtin_route_locked", + }); + return; + } + const [app] = await db .update(appsTable) .set(parsed.data) diff --git a/artifacts/api-server/tests/apps-builtin-route-lock.test.mjs b/artifacts/api-server/tests/apps-builtin-route-lock.test.mjs new file mode 100644 index 00000000..a75f1976 --- /dev/null +++ b/artifacts/api-server/tests/apps-builtin-route-lock.test.mjs @@ -0,0 +1,199 @@ +// Task #517: PATCH /apps/:id must reject route changes whose target row's +// slug is in BUILTIN_APP_SLUGS — those routes are hardcoded in the SPA +// (artifacts/tx-os/src/App.tsx) and silently break the launcher when an +// admin edits them. Other fields (icon, image, color, name, openMode, +// externalUrl, sortOrder) must still be editable, including for built-in +// apps. Apps whose slug is not built-in keep accepting route changes. +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import pg from "pg"; + +const API_BASE = process.env.TEST_API_BASE ?? "http://localhost:8080"; +const DATABASE_URL = process.env.DATABASE_URL; +if (!DATABASE_URL) { + throw new Error("DATABASE_URL must be set to run these tests"); +} + +const TEST_PASSWORD = "TestPass123!"; +const TEST_PASSWORD_HASH = + "$2b$10$Bs636ukPMyz01nKrsi.5m.JlDXSN22AVCvn8cgPWWDbo5yJRQX2vu"; + +const pool = new pg.Pool({ connectionString: DATABASE_URL }); + +let adminId; +let adminUsername; +let adminCookie; +const createdAppIds = []; + +async function loginAndGetCookie(username, password) { + const res = await fetch(`${API_BASE}/api/auth/login`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ username, password }), + }); + assert.equal(res.status, 200, `login expected 200, got ${res.status}`); + const setCookie = res.headers.get("set-cookie"); + return setCookie + .split(",") + .map((c) => c.split(";")[0].trim()) + .find((c) => c.startsWith("connect.sid=")); +} + +async function createAppRow({ slug, route }) { + const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`; + const realSlug = slug ?? `customapp_${stamp}`; + const realRoute = route ?? `/custom-${stamp}`; + const { rows } = await pool.query( + `INSERT INTO apps (slug, name_ar, name_en, route, is_active, sort_order) + VALUES ($1, $2, $3, $4, TRUE, 0) RETURNING id`, + [realSlug, realSlug, realSlug, realRoute], + ); + createdAppIds.push(rows[0].id); + return { id: rows[0].id, slug: realSlug, route: realRoute }; +} + +before(async () => { + const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`; + adminUsername = `admin_lock_${stamp}`; + const { rows } = await pool.query( + `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active) + VALUES ($1, $2, $3, $1, 'en', TRUE) RETURNING id`, + [adminUsername, `${adminUsername}@example.com`, TEST_PASSWORD_HASH], + ); + adminId = rows[0].id; + await pool.query( + `INSERT INTO user_roles (user_id, role_id) SELECT $1, id FROM roles WHERE name = 'admin'`, + [adminId], + ); + adminCookie = await loginAndGetCookie(adminUsername, TEST_PASSWORD); +}); + +after(async () => { + if (createdAppIds.length > 0) { + await pool.query(`DELETE FROM apps WHERE id = ANY($1::int[])`, [createdAppIds]); + } + if (adminId) { + await pool.query(`DELETE FROM user_roles WHERE user_id = $1`, [adminId]); + await pool.query(`DELETE FROM users WHERE id = $1`, [adminId]); + } + await pool.end(); +}); + +test("PATCH /apps/:id rejects route changes for built-in slugs with code=builtin_route_locked", async () => { + // Pick a built-in slug. We INSERT a row with that exact slug — the live + // DB seeds them too, but using a freshly inserted row keeps the test + // self-contained and avoids touching the seeded /admin/etc apps. + // First clean any stray prior test row for this slug; the schema has a + // UNIQUE constraint on slug. + const slug = "executive-meetings"; + const original = await pool.query(`SELECT id, route FROM apps WHERE slug = $1`, [slug]); + let appId; + let originalRoute; + if (original.rowCount > 0) { + appId = original.rows[0].id; + originalRoute = original.rows[0].route; + } else { + const created = await createAppRow({ slug, route: "/executive-meetings" }); + appId = created.id; + originalRoute = created.route; + } + + const res = await fetch(`${API_BASE}/api/apps/${appId}`, { + method: "PATCH", + headers: { + "Content-Type": "application/json", + Cookie: adminCookie, + }, + body: JSON.stringify({ route: "/something-else" }), + }); + assert.equal(res.status, 400, `expected 400, got ${res.status}`); + const body = await res.json(); + assert.equal(body.code, "builtin_route_locked"); + + // Confirm the row was NOT mutated. + const after = await pool.query(`SELECT route FROM apps WHERE id = $1`, [appId]); + assert.equal(after.rows[0].route, originalRoute); +}); + +test("PATCH /apps/:id allows non-route updates on built-in apps", async () => { + const slug = "notes"; + const original = await pool.query(`SELECT id, route, icon_name FROM apps WHERE slug = $1`, [slug]); + let appId; + let originalRoute; + let originalIcon; + if (original.rowCount > 0) { + appId = original.rows[0].id; + originalRoute = original.rows[0].route; + originalIcon = original.rows[0].icon_name; + } else { + const created = await createAppRow({ slug, route: "/notes" }); + appId = created.id; + originalRoute = created.route; + originalIcon = "Grid"; + } + + // Change icon + openMode but keep route untouched (omit it). + const res = await fetch(`${API_BASE}/api/apps/${appId}`, { + method: "PATCH", + headers: { + "Content-Type": "application/json", + Cookie: adminCookie, + }, + body: JSON.stringify({ iconName: "Sparkles", openMode: "internal" }), + }); + assert.equal(res.status, 200, `expected 200, got ${res.status}`); + + const after = await pool.query( + `SELECT route, icon_name, open_mode FROM apps WHERE id = $1`, + [appId], + ); + assert.equal(after.rows[0].route, originalRoute, "route must not change"); + assert.equal(after.rows[0].icon_name, "Sparkles"); + assert.equal(after.rows[0].open_mode, "internal"); + + // Restore icon so the test is idempotent for re-runs. + await pool.query(`UPDATE apps SET icon_name = $1 WHERE id = $2`, [originalIcon, appId]); +}); + +test("PATCH /apps/:id allows route changes on non-built-in apps", async () => { + const created = await createAppRow({}); + const newRoute = `${created.route}_v2`; + const res = await fetch(`${API_BASE}/api/apps/${created.id}`, { + method: "PATCH", + headers: { + "Content-Type": "application/json", + Cookie: adminCookie, + }, + body: JSON.stringify({ route: newRoute }), + }); + assert.equal(res.status, 200, `expected 200, got ${res.status}`); + const after = await pool.query(`SELECT route FROM apps WHERE id = $1`, [created.id]); + assert.equal(after.rows[0].route, newRoute); +}); + +test("PATCH /apps/:id with same route on built-in slug is a no-op success", async () => { + // Sending route equal to current value should NOT be rejected, since + // it isn't actually changing the path. + const slug = "executive-meetings"; + const original = await pool.query(`SELECT id, route FROM apps WHERE slug = $1`, [slug]); + let appId; + let route; + if (original.rowCount > 0) { + appId = original.rows[0].id; + route = original.rows[0].route; + } else { + const created = await createAppRow({ slug, route: "/executive-meetings" }); + appId = created.id; + route = created.route; + } + + const res = await fetch(`${API_BASE}/api/apps/${appId}`, { + method: "PATCH", + headers: { + "Content-Type": "application/json", + Cookie: adminCookie, + }, + body: JSON.stringify({ route }), + }); + assert.equal(res.status, 200, `expected 200, got ${res.status}`); +}); diff --git a/artifacts/tx-os/src/App.tsx b/artifacts/tx-os/src/App.tsx index 70c23881..53c3681a 100644 --- a/artifacts/tx-os/src/App.tsx +++ b/artifacts/tx-os/src/App.tsx @@ -22,6 +22,7 @@ import NotesPage from "@/pages/notes"; import MyOrdersPage from "@/pages/my-orders"; import OrdersIncomingPage from "@/pages/orders-incoming"; import ExecutiveMeetingsPage from "@/pages/executive-meetings"; +import EmbeddedAppPage from "@/pages/embedded-app"; const queryClient = new QueryClient({ defaultOptions: { @@ -58,6 +59,7 @@ function Router() { + diff --git a/artifacts/tx-os/src/locales/ar.json b/artifacts/tx-os/src/locales/ar.json index 93f5dd74..24fb93a0 100644 --- a/artifacts/tx-os/src/locales/ar.json +++ b/artifacts/tx-os/src/locales/ar.json @@ -298,6 +298,15 @@ "appRoute": "المسار", "appIcon": "الأيقونة", "appColor": "اللون", + "appImage": "صورة", + "appExternalUrl": "رابط خارجي", + "appOpenMode": { + "label": "طريقة الفتح", + "internal": "صفحة داخلية (افتراضي)", + "external_tab": "فتح الرابط في تبويب جديد", + "external_iframe": "فتح الرابط داخل التطبيق" + }, + "builtinPathLocked": "لا يمكن تغيير مسار التطبيقات المدمجة.", "appSortOrder": "الترتيب", "appPermissions": { "title": "الصلاحيات المطلوبة", @@ -904,6 +913,14 @@ "next": "التالي", "empty": "(فارغ)" }, + "embeddedFrame": { + "title": "تطبيق مدمج", + "back": "العودة للرئيسية", + "openInNewTab": "فتح في تبويب جديد", + "loadError": "تعذّر تحميل التطبيق.", + "notFound": "التطبيق غير موجود.", + "notEmbeddable": "هذا التطبيق غير مهيّأ للعرض المدمج." + }, "notes": { "title": "الملاحظات", "searchPlaceholder": "ابحث في الملاحظات", diff --git a/artifacts/tx-os/src/locales/en.json b/artifacts/tx-os/src/locales/en.json index cada88f1..4e6ed271 100644 --- a/artifacts/tx-os/src/locales/en.json +++ b/artifacts/tx-os/src/locales/en.json @@ -298,6 +298,15 @@ "appRoute": "Route", "appIcon": "Icon", "appColor": "Color", + "appImage": "Image", + "appExternalUrl": "External URL", + "appOpenMode": { + "label": "How to open", + "internal": "Built-in page (default)", + "external_tab": "Open external link in a new tab", + "external_iframe": "Open external link inside the app" + }, + "builtinPathLocked": "The path of a built-in app cannot be changed.", "appSortOrder": "Sort Order", "appPermissions": { "title": "Required permissions", @@ -816,6 +825,14 @@ "next": "Next", "empty": "(empty)" }, + "embeddedFrame": { + "title": "Embedded app", + "back": "Back to home", + "openInNewTab": "Open in a new tab", + "loadError": "Could not load the app.", + "notFound": "App not found.", + "notEmbeddable": "This app is not configured to be embedded." + }, "notes": { "title": "Notes", "searchPlaceholder": "Search notes", diff --git a/artifacts/tx-os/src/pages/admin.tsx b/artifacts/tx-os/src/pages/admin.tsx index 4aa99983..943e2cea 100644 --- a/artifacts/tx-os/src/pages/admin.tsx +++ b/artifacts/tx-os/src/pages/admin.tsx @@ -130,7 +130,7 @@ import { useQueryClient, useQuery } from "@tanstack/react-query"; import { useAuth } from "@/contexts/AuthContext"; import { useUpload, type UploadResponse } from "@workspace/object-storage-web"; import { resolveServiceImageUrl } from "@/lib/image-url"; -import { ArrowRight, ArrowLeft, Settings, Plus, Pencil, Trash2, Shield, Upload, Loader2, LayoutDashboard, Grid2X2, Coffee, Users as UsersIcon, Menu as MenuIcon, TrendingUp, TrendingDown, UserPlus, Activity, KeyRound, Copy, ChevronDown, ScrollText, Download, X, ImageIcon, Power, HelpCircle, Palette, Link2, Hash, type LucideIcon } from "lucide-react"; +import { ArrowRight, ArrowLeft, Settings, Plus, Pencil, Trash2, Shield, Upload, Loader2, LayoutDashboard, Grid2X2, Coffee, Users as UsersIcon, Menu as MenuIcon, TrendingUp, TrendingDown, UserPlus, Activity, KeyRound, Copy, ChevronDown, ScrollText, Download, X, ImageIcon, Power, HelpCircle, Palette, Link2, Hash, Lock, ExternalLink, type LucideIcon } from "lucide-react"; import * as LucideIcons from "lucide-react"; function resolveAppIcon(name: string): LucideIcon | null { @@ -215,12 +215,20 @@ function LeaderboardAvatar({ ); } +type AppOpenMode = "internal" | "external_tab" | "external_iframe"; + type AppForm = { nameAr: string; nameEn: string; slug: string; iconName: string; + // Optional uploaded image — when set, the launcher renders this instead + // of the Lucide icon. Stored as the object-storage path returned by the + // upload presigner (e.g. "/objects/"). Empty string = none. + imageUrl: string; route: string; + externalUrl: string; + openMode: AppOpenMode; color: string; sortOrder: number; // Only used when creating a new app — admins can pre-set the required @@ -887,7 +895,10 @@ export default function AdminPage() { nameEn: app.nameEn ?? "", slug: app.slug ?? "", iconName: app.iconName ?? "Grid2X2", + imageUrl: app.imageUrl ?? "", route: app.route ?? "/", + externalUrl: app.externalUrl ?? "", + openMode: ((app.openMode as AppOpenMode) ?? "internal"), color: app.color ?? "#6366f1", sortOrder: app.sortOrder ?? 0, permissionIds: [], @@ -1032,7 +1043,7 @@ export default function AdminPage() { ); }; - const emptyAppForm: AppForm = { nameAr: "", nameEn: "", slug: "", iconName: "Grid2X2", route: "/", color: "#6366f1", sortOrder: 0, permissionIds: [] }; + const emptyAppForm: AppForm = { nameAr: "", nameEn: "", slug: "", iconName: "Grid2X2", imageUrl: "", route: "/", externalUrl: "", openMode: "internal", color: "#6366f1", sortOrder: 0, permissionIds: [] }; const emptyServiceForm: ServiceForm = { nameAr: "", nameEn: "", descriptionAr: "", descriptionEn: "", imageUrl: "", isAvailable: true }; // #196: Switch to the audit-log section pre-filtered by the given @@ -1061,32 +1072,67 @@ export default function AdminPage() { const handleSaveApp = () => { if (!editingApp) return; + // Normalize the optional URL/image fields. The OpenAPI spec types + // them as ["string","null"], so an empty string from the form would + // be rejected by zod. We send null whenever the admin left them + // blank (which is the meaningful value for "no image"/"no URL"). + const normalize = (form: AppForm) => ({ + ...form, + imageUrl: form.imageUrl?.trim() ? form.imageUrl.trim() : null, + externalUrl: form.externalUrl?.trim() ? form.externalUrl.trim() : null, + }); + // Surface server-side errors that the user can act on. The + // built-in route lock returns { code: "builtin_route_locked" }. + const onError = async (err: unknown) => { + let message: string | undefined; + if (err && typeof err === "object" && "response" in err) { + const r = (err as { response?: Response }).response; + if (r) { + try { + const body = await r.clone().json(); + if (body?.code === "builtin_route_locked") { + message = t("admin.builtinPathLocked"); + } else if (typeof body?.error === "string") { + message = body.error; + } + } catch { + /* fall through */ + } + } + } + toast({ + title: t("admin.uploadFailed"), + description: message, + variant: "destructive", + }); + }; if (editingApp.id) { // Edit path: permissionIds aren't part of the update payload — the // existing AppPermissionsEditor manages them with its own impact // preview + audit, so strip the field before sending. - const { permissionIds: _ignored, ...updateData } = editingApp.form; + const { permissionIds: _ignored, ...rest } = normalize(editingApp.form); void _ignored; updateApp.mutate( - { id: editingApp.id, data: updateData }, + { id: editingApp.id, data: rest }, { onSuccess: () => { queryClient.invalidateQueries({ queryKey: ADMIN_APPS_KEY }); setEditingApp(null); toast({ title: t("common.success") }); }, + onError, }, ); } else { - const { permissionIds, ...rest } = editingApp.form; + const { permissionIds, ...rest } = normalize(editingApp.form); createApp.mutate( { // Only include permissionIds if the admin actually picked some // so the request body stays minimal and the server's "no // restriction" path is unchanged. data: - permissionIds.length > 0 - ? { ...rest, permissionIds } + (editingApp.form.permissionIds.length > 0) + ? { ...rest, permissionIds: editingApp.form.permissionIds } : rest, }, { @@ -1095,8 +1141,10 @@ export default function AdminPage() { setEditingApp(null); toast({ title: t("common.success") }); }, + onError, }, ); + void permissionIds; } }; @@ -1190,6 +1238,26 @@ export default function AdminPage() { const previewColor = /^#[0-9a-fA-F]{6}$/.test(editingApp.form.color) ? editingApp.form.color : "#6366f1"; + // Mirror of BUILTIN_APP_SLUGS in @workspace/db (see + // lib/db/src/built-in-apps.ts). Their `route` is hardcoded in + // App.tsx, so the API rejects route changes for these slugs. + // Duplicated here because @workspace/db pulls in `pg` which + // can't run in the browser; kept tiny and pinned next to the + // server constant in code review. + const BUILTIN_APP_SLUGS_FE = [ + "services", + "notifications", + "admin", + "notes", + "my-orders", + "executive-meetings", + ] as const; + const isBuiltin = + editingApp.id !== undefined && + (BUILTIN_APP_SLUGS_FE as readonly string[]).includes( + editingApp.form.slug, + ); + const isExternal = editingApp.form.openMode !== "internal"; return (
{t("admin.appRoute")} + {isBuiltin && ( +
+ {/* App image (optional, replaces the Lucide icon on the launcher) */} +
+ + + setEditingApp({ + ...editingApp, + form: { ...editingApp.form, imageUrl: url }, + }) + } + /> +

+ {lang === "ar" + ? "صورة اختيارية تظهر مكان الأيقونة في الشاشة الرئيسية" + : "Optional image shown on the launcher tile in place of the icon"} +

+
+ + {/* Open mode + external URL */} +
+ + + {isExternal && ( +
+ + + setEditingApp({ + ...editingApp, + form: { ...editingApp.form, externalUrl: e.target.value }, + }) + } + className="bg-slate-50/70 border-slate-200 rounded-lg h-10 text-sm focus-visible:ring-2 focus-visible:ring-indigo-500/30 focus-visible:border-indigo-400" + dir="ltr" + placeholder="https://example.com" + type="url" + /> +
+ )} +
+ {/* Visuals row — icon + color with live previews */}
{/* Icon picker */} @@ -1752,7 +1900,10 @@ export default function AdminPage() { nameEn: app.nameEn ?? "", slug: app.slug ?? "", iconName: app.iconName ?? "Grid2X2", + imageUrl: app.imageUrl ?? "", route: app.route ?? "/", + externalUrl: app.externalUrl ?? "", + openMode: ((app.openMode as AppOpenMode) ?? "internal"), color: app.color ?? "#6366f1", sortOrder: app.sortOrder ?? 0, permissionIds: [], diff --git a/artifacts/tx-os/src/pages/embedded-app.tsx b/artifacts/tx-os/src/pages/embedded-app.tsx new file mode 100644 index 00000000..e290e2c8 --- /dev/null +++ b/artifacts/tx-os/src/pages/embedded-app.tsx @@ -0,0 +1,102 @@ +import { useTranslation } from "react-i18next"; +import { useLocation, useRoute } from "wouter"; +import { + useGetApp, + getGetAppQueryKey, +} from "@workspace/api-client-react"; +import { ArrowLeft, ArrowRight, ExternalLink } from "lucide-react"; + +export default function EmbeddedAppPage() { + const { t, i18n } = useTranslation(); + const lang = i18n.language; + const isRtl = lang === "ar"; + const [, setLocation] = useLocation(); + const [, params] = useRoute<{ id: string }>("/embedded/:id"); + const idStr = params?.id ?? ""; + const id = Number(idStr); + const validId = Number.isFinite(id) && id > 0; + + const { data: app, isLoading, error } = useGetApp(id, { + query: { + queryKey: getGetAppQueryKey(id), + enabled: validId, + }, + }); + + const goBack = () => setLocation("/"); + const Back = isRtl ? ArrowRight : ArrowLeft; + + if (!validId) { + return ( +
+
+

{t("embeddedFrame.notFound")}

+ +
+
+ ); + } + + const name = app ? (lang === "ar" ? app.nameAr : app.nameEn) : ""; + const url = app?.externalUrl ?? ""; + // openMode must be external_iframe AND externalUrl must be set; otherwise + // there's nothing to render — fall back to a friendly notice. + const canEmbed = !!app && app.openMode === "external_iframe" && !!url; + + return ( +
+
+ +

+ {isLoading ? "…" : (name || t("embeddedFrame.title"))} +

+ {url && ( + + + + )} +
+
+ {error ? ( +
+ {t("embeddedFrame.loadError")} +
+ ) : !canEmbed ? ( +
+ {isLoading ? t("common.loading") : t("embeddedFrame.notEmbeddable")} +
+ ) : ( +