Skip to content

fix(frontend): clickable namespaces in deployments table#5104

Merged
jog1t merged 1 commit into
mainfrom
05-26-fix_frontend_clickable_namespaces_in_deployments_table
May 26, 2026
Merged

fix(frontend): clickable namespaces in deployments table#5104
jog1t merged 1 commit into
mainfrom
05-26-fix_frontend_clickable_namespaces_in_deployments_table

Conversation

@jog1t
Copy link
Copy Markdown
Contributor

@jog1t jog1t commented May 26, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Copy Markdown
Contributor Author

jog1t commented May 26, 2026

Merge activity

  • May 26, 10:23 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 26, 10:29 PM UTC: Graphite rebased this pull request as part of a merge.
  • May 26, 10:29 PM UTC: @jog1t merged this pull request with Graphite.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 26, 2026

PR Review: fix(frontend): clickable namespaces in deployments table

Summary: This PR wraps the namespace display name in DeploymentNamespace with a TanStack Router <Link> pointing to /orgs/$organization/projects/$project/ns/$namespace. Two imports are added (Link, useParams), and useParams({ strict: false }) extracts the organization and project path params needed to build the link.


Issues

Unsafe type assertion (primary concern)

const { organization, project } = useParams({ strict: false }) as {
  organization: string;
  project: string;
};

useParams({ strict: false }) returns Partial<AllParams>, meaning organization and project can be undefined. The cast suppresses TypeScript's check entirely. If DeploymentNamespace is ever rendered outside the expected route hierarchy, the link will silently navigate to /orgs/undefined/projects/undefined/ns/... rather than failing loudly.

Two safer approaches:

  • Pass as props. The parent TagRow renders inside a route that already has organization and project available (e.g. via Route.useParams()). Pass them down explicitly — this is type-safe and matches how other components in the codebase handle route-context-dependent data.
  • Add a null guard. At minimum, check that both params are defined before rendering the <Link> and fall back to plain text otherwise.

Pattern inconsistency

The surrounding deployments.tsx uses Route.useParams() for type-safe param access. Using useParams({ strict: false }) as ... in a child component is inconsistent with that pattern and moves type safety in the wrong direction.


What looks good

  • The target route path (/orgs/$organization/projects/$project/ns/$namespace) matches the actual route file at frontend/src/routes/_context/orgs.$organization/projects.$project/ns.$namespace.tsx.
  • Both current call sites (deployments.tsx and actors-grid.tsx) live within the ns.$namespace route hierarchy, so params will be present at runtime for existing usages.
  • No CLAUDE.md violations: no Rust changes, no vi.mock/jest.mock, no route loader pattern issues, no em dashes or style violations.

Recommendation

Pass organization and project as explicit props from the parent (or from TagRow) down to DeploymentNamespace rather than reaching up with useParams({ strict: false }). This keeps the component's dependencies explicit and removes the unsafe cast.

@jog1t jog1t changed the base branch from 05-26-fix_fontend_less_monoscaped to graphite-base/5104 May 26, 2026 22:26
@jog1t jog1t changed the base branch from graphite-base/5104 to main May 26, 2026 22:27
@jog1t jog1t force-pushed the 05-26-fix_frontend_clickable_namespaces_in_deployments_table branch from 2457bc9 to f7c1b84 Compare May 26, 2026 22:28
@jog1t jog1t merged commit 343be6a into main May 26, 2026
8 of 11 checks passed
@jog1t jog1t deleted the 05-26-fix_frontend_clickable_namespaces_in_deployments_table branch May 26, 2026 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant