From 902c34cf0186daf9ddf1ff19e9622c7e0a2ec634 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Fri, 9 May 2025 11:01:35 -0300 Subject: [PATCH] refactor: improve apps.ts readbility (#17741) Apply PR comments from https://github.com/coder/coder/pull/17724 --- site/migrate-icons.md | 8 ++++ site/src/modules/apps/apps.ts | 38 ++++++++----------- site/src/modules/apps/useAppLink.ts | 2 +- .../src/modules/resources/AppLink/AppLink.tsx | 4 +- 4 files changed, 27 insertions(+), 25 deletions(-) create mode 100644 site/migrate-icons.md diff --git a/site/migrate-icons.md b/site/migrate-icons.md new file mode 100644 index 0000000000..5bf361c215 --- /dev/null +++ b/site/migrate-icons.md @@ -0,0 +1,8 @@ +Look for all the @mui/icons-material icons below and replace them accordinlying with the Lucide icon: + +MUI | Lucide +TaskAlt | CircleCheckBigIcon +InfoOutlined | InfoIcon +ErrorOutline | CircleAlertIcon + +You should update the imports and usage. diff --git a/site/src/modules/apps/apps.ts b/site/src/modules/apps/apps.ts index cd57df148a..b90f30fef9 100644 --- a/site/src/modules/apps/apps.ts +++ b/site/src/modules/apps/apps.ts @@ -83,17 +83,11 @@ export const getAppHref = ( : app.url; } - // The backend redirects if the trailing slash isn't included, so we add it - // here to avoid extra roundtrips. - let href = `${path}/@${workspace.owner_name}/${workspace.name}.${ - agent.name - }/apps/${encodeURIComponent(app.slug)}/`; - if (app.command) { // Terminal links are relative. The terminal page knows how // to select the correct workspace proxy for the websocket // connection. - href = `/@${workspace.owner_name}/${workspace.name}.${ + return `/@${workspace.owner_name}/${workspace.name}.${ agent.name }/terminal?command=${encodeURIComponent(app.command)}`; } @@ -102,23 +96,14 @@ export const getAppHref = ( const baseUrl = `${window.location.protocol}//${host.replace(/\*/g, app.subdomain_name)}`; const url = new URL(baseUrl); url.pathname = "/"; - href = url.toString(); + return url.toString(); } - return href; -}; - -export const needsSessionToken = (app: WorkspaceApp) => { - if (!isExternalApp(app)) { - return false; - } - - // HTTP links should never need the session token, since Cookies - // handle sharing it when you access the Coder Dashboard. We should - // never be forwarding the bare session token to other domains! - const isHttp = app.url.startsWith("http"); - const requiresSessionToken = app.url.includes(SESSION_TOKEN_PLACEHOLDER); - return requiresSessionToken && !isHttp; + // The backend redirects if the trailing slash isn't included, so we add it + // here to avoid extra roundtrips. + return `${path}/@${workspace.owner_name}/${workspace.name}.${ + agent.name + }/apps/${encodeURIComponent(app.slug)}/`; }; type ExternalWorkspaceApp = WorkspaceApp & { @@ -131,3 +116,12 @@ export const isExternalApp = ( ): app is ExternalWorkspaceApp => { return app.external && app.url !== undefined; }; + +export const needsSessionToken = (app: ExternalWorkspaceApp) => { + // HTTP links should never need the session token, since Cookies + // handle sharing it when you access the Coder Dashboard. We should + // never be forwarding the bare session token to other domains! + const isHttp = app.url.startsWith("http"); + const requiresSessionToken = app.url.includes(SESSION_TOKEN_PLACEHOLDER); + return requiresSessionToken && !isHttp; +}; diff --git a/site/src/modules/apps/useAppLink.ts b/site/src/modules/apps/useAppLink.ts index f45ec21e56..9916aaf95f 100644 --- a/site/src/modules/apps/useAppLink.ts +++ b/site/src/modules/apps/useAppLink.ts @@ -34,7 +34,7 @@ export const useAppLink = ( const href = getAppHref(app, { agent, workspace, - token: apiKeyResponse?.key ?? "", + token: apiKeyResponse?.key, path: proxy.preferredPathAppURL, host: proxy.preferredWildcardHostname, }); diff --git a/site/src/modules/resources/AppLink/AppLink.tsx b/site/src/modules/resources/AppLink/AppLink.tsx index d2910e287a..a618b792ca 100644 --- a/site/src/modules/resources/AppLink/AppLink.tsx +++ b/site/src/modules/resources/AppLink/AppLink.tsx @@ -9,7 +9,7 @@ import { TooltipTrigger, } from "components/Tooltip/Tooltip"; import { useProxy } from "contexts/ProxyContext"; -import { needsSessionToken } from "modules/apps/apps"; +import { isExternalApp, needsSessionToken } from "modules/apps/apps"; import { useAppLink } from "modules/apps/useAppLink"; import { type FC, useState } from "react"; import { AgentButton } from "../AgentButton"; @@ -65,7 +65,7 @@ export const AppLink: FC = ({ app, workspace, agent }) => { "Your admin has not configured subdomain application access"; } - if (needsSessionToken(app) && !link.hasToken) { + if (isExternalApp(app) && needsSessionToken(app) && !link.hasToken) { canClick = false; }