feat(aurora-portal): swift objects list with file browser interface#593
Merged
mark-karnaukh-extern-sap merged 27 commits intomainfrom Mar 25, 2026
Merged
Conversation
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR restructures object storage routing from a generic splat-based path to explicit container and object-browsing routes, adds new UI components for Swift object storage browsing with breadcrumb navigation and virtualized tables, and implements service availability validation with intelligent fallback redirects. Changes
Sequence DiagramsequenceDiagram
actor User
participant Router
participant ContainersRoute as Containers Route
participant ObjectsRoute as Objects Route
participant API as tRPC API
participant UI as UI Components
User->>Router: Navigate to /storage/$provider/containers/
Router->>ContainersRoute: Load containers route
ContainersRoute->>API: Check service availability
API-->>ContainersRoute: Available services
ContainersRoute->>UI: Render SwiftContainers
UI-->>User: Display container list
User->>UI: Click container name
UI->>Router: Navigate to /storage/$provider/containers/$containerName/objects/
Router->>ObjectsRoute: Load objects route
ObjectsRoute->>API: Check service availability
API-->>ObjectsRoute: Available services
ObjectsRoute->>API: listObjects query (container, prefix)
API-->>ObjectsRoute: Object summaries
ObjectsRoute->>UI: Render SwiftObjects
UI->>UI: Build rows (folders + objects)
UI-->>User: Display breadcrumbs + objects table
User->>UI: Click folder in table
UI->>Router: Update search params with new prefix
Router->>API: Re-query with new prefix
API-->>ObjectsRoute: Filtered object summaries
UI-->>User: Update breadcrumbs + table content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
12 tasks
7 tasks
mark-karnaukh-extern-sap
force-pushed
the
mark-swift-objects-file-browser-interface
branch
from
10a7163 to
1fce367
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ManageContainerAccessModal.tsx (1)
205-212:⚠️ Potential issue | 🟡 MinorReset
showPreviewin the external-close path too.When the modal is closed via parent state (
isOpen = false) instead ofhandleClose,showPreviewis not cleared. On reopen, preview state can be stale.Proposed patch
useEffect(() => { if (!isOpen) { setReadAcl("") setWriteAcl("") setPublicRead(false) + setShowPreview(false) resetMutationRef.current() } }, [isOpen])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ManageContainerAccessModal.tsx around lines 205 - 212, The useEffect that clears state when the modal is closed via parent (useEffect with dependency [isOpen]) misses resetting the preview flag, causing stale previews on reopen; update that effect to also reset the preview state by invoking the showPreview state setter (e.g., call setShowPreview(false) or equivalent) alongside setReadAcl, setWriteAcl, setPublicRead, and resetMutationRef.current(), so both external-close (useEffect) and handleClose paths clear showPreview consistently.
🧹 Nitpick comments (3)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/index.tsx (1)
107-113: Unusedprojectvariable in StorageDashboard.The
projectvariable extracted fromuseParamsis only used for a truthy check but never used for its actual value. Consider renaming to clarify intent or removing if not needed.💡 Suggested clarification
If you only need to check if project exists:
function StorageDashboard() { - const { project, provider } = useParams({ + const { hasProject, provider } = useParams({ from: "/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/", select: (params) => { - return { project: params.projectId, provider: params.provider } + return { hasProject: Boolean(params.projectId), provider: params.provider } }, })Or simply use
projectIdconsistently if you might need the value later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/$provider/containers/index.tsx around lines 107 - 113, The destructured variable project from the useParams call in StorageDashboard is unused except for a truthy check; update the useParams usage to either rename project to something like hasProject (if you only need existence) or remove it and check for provider/projectId existence directly, or replace it with projectId to use the actual value consistently; locate the useParams call in StorageDashboard and adjust the select return or subsequent checks so there is no unused variable left.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ContainerTableView.test.tsx (1)
15-41: Router mock is duplicated across test files.This router mock is identical to the one in
ContainerList.test.tsx. Consider extracting it to a shared test utility to reduce duplication and ensure consistency.💡 Suggested shared mock location
Create a shared test utility, e.g.,
src/client/test-utils/routerMocks.ts:import { vi } from "vitest" import React from "react" export const mockTanStackRouter = () => { vi.mock("@tanstack/react-router", async () => { const actual = await vi.importActual("@tanstack/react-router") return { ...actual, useParams: vi.fn(() => ({ accountId: "test-account", projectId: "test-project", provider: "swift", })), Link: vi.fn(({ children, to, ...props }) => ( <a href={to} {...props}>{children}</a> )), } }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ContainerTableView.test.tsx around lines 15 - 41, Extract the duplicated router mock from ContainerTableView.test.tsx (and ContainerList.test.tsx) into a shared test utility function (e.g., mockTanStackRouter) and export it from a new module (suggested name routerMocks or test-utils/routerMocks); move the vi.mock implementation there and ensure it returns the same mocked useParams and Link as the original, then replace the in-file vi.mock calls in both tests with an import of mockTanStackRouter and a call to it at the top of each test file so both tests reuse the single shared mock implementation.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/index.test.tsx (1)
7-25: Same redirect mock pattern duplicated across test files.This redirect mock is identical to the one in
$.test.tsx. Consider extracting to a shared test utility.💡 Suggested shared mock
// src/client/test-utils/redirectMock.ts export const createRedirectMock = () => vi.fn((args) => { if (args.href) { throw new Error(`Redirect to: ${args.href}`) } let resolvedPath: string = args.to if (args.params) { for (const [key, value] of Object.entries(args.params as Record<string, string>)) { resolvedPath = resolvedPath.replace(`$${key}`, value ?? "") } } throw new Error(`Redirect to: ${resolvedPath}`) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/$provider/containers/index.test.tsx around lines 7 - 25, Extract the duplicated redirect mock into a shared test utility (e.g. export createRedirectMock from src/client/test-utils/redirectMock) and replace the inline mock in the vi.mock("@tanstack/react-router") blocks with an import of that helper; update the vi.mock return to spread the actual module and set redirect: createRedirectMock(), keeping the same behavior of checking args.href, interpolating args.params into args.to, and throwing the "Redirect to: ..." Error so tests that assert thrown messages keep working (refer to the existing vi.mock in the test and the suggested createRedirectMock helper name).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ObjectList.tsx:
- Around line 287-293: Replace the template-literal interpolation inside the
<Trans> component so i18n extraction works: in the error-handling branch of
ObjectList (the block that checks `if (error)`), remove `{error.message}` from
the literal and pass the runtime message to <Trans> via a values prop or as a
child placeholder (e.g., use a message key like "Error Loading Objects:
{message}" and pass `{ message: error.message }` to the <Trans> component) so
translation extraction tools can pick up the static string while still rendering
the dynamic `error.message`.
- Around line 26-36: The current encodePrefix/decodePrefix use btoa/atob which
throw on non-Latin-1 input; update encodePrefix to first UTF-8 encode the string
(TextEncoder -> Uint8Array) and then base64-encode that byte buffer (convert
bytes to a binary string and btoa it), and update decodePrefix to atob the
base64 into a binary string, convert that to a Uint8Array and decode it with
TextDecoder to produce the original Unicode string; keep the existing undefined
check and wrap both functions' operations in try/catch to return empty string on
failure, referencing the functions encodePrefix and decodePrefix.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/$.tsx:
- Around line 101-114: The loader and beforeLoad both call
trpcClient.auth.getAvailableServices, causing duplicate network requests;
refactor so loader fetches availableServices once and beforeLoad reuses it: have
loader return availableServices (keeping client) and modify beforeLoad to accept
or read that loader data instead of calling trpcClient again, then pass the
returned availableServices into checkServiceAvailability(availableServices,
params) (use the loader-returned availableServices and params in beforeLoad
rather than invoking trpcClient.auth.getAvailableServices twice).
- Around line 98-100: The notFoundComponent currently returns plain English
text; wrap the string in the Lingui translation component (e.g., replace
<p>Storage container not found</p> with <p><Trans>Storage container not
found</Trans></p>) and add the necessary import for Trans if missing so the
container 404 copy is localized; the change targets the notFoundComponent
returned JSX in the route component.
- Around line 92-97: The errorComponent currently renders raw error.message for
Error instances, leaking internals; update the errorComponent to always render
the shared app error UI (use ErrorComponent / the app ErrorBoundary/ErrorBoundry
recovery component) instead of returning error.message so both Error and
non-Error cases display the unified error page. Locate the errorComponent in the
route (symbol: errorComponent) and replace the conditional branch that returns
<div>{error.message}</div> with a call to the shared error UI (e.g., return
<ErrorComponent error={error} />) so all errors go through the same recovery UX.
- Around line 127-139: The setPageTitle calls are being invoked directly in the
component body (via Route.useRouteContext() -> setPageTitle) which triggers
document/title updates on every render; move this logic into a useEffect so it
only runs when the computed pageTitle changes. Compute pageTitle from provider
(using useLingui()'s t for "Object Storage" vs "Storage Overview") and then call
setPageTitle inside a useEffect with [pageTitle, setPageTitle] as dependencies,
leaving the rest of the render logic unchanged.
- Around line 144-149: The ErrorBoundary wrapping SwiftObjects doesn't include
resetKeys, so once SwiftObjects throws the fallback remains until a full reload;
extract the prefix search param and add resetKeys={[project, provider,
containerName, prefix]} to the ErrorBoundary component (referencing
ErrorBoundary and SwiftObjects) so the boundary resets when any of project,
provider, containerName or prefix change.
---
Outside diff comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ManageContainerAccessModal.tsx:
- Around line 205-212: The useEffect that clears state when the modal is closed
via parent (useEffect with dependency [isOpen]) misses resetting the preview
flag, causing stale previews on reopen; update that effect to also reset the
preview state by invoking the showPreview state setter (e.g., call
setShowPreview(false) or equivalent) alongside setReadAcl, setWriteAcl,
setPublicRead, and resetMutationRef.current(), so both external-close
(useEffect) and handleClose paths clear showPreview consistently.
---
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ContainerTableView.test.tsx:
- Around line 15-41: Extract the duplicated router mock from
ContainerTableView.test.tsx (and ContainerList.test.tsx) into a shared test
utility function (e.g., mockTanStackRouter) and export it from a new module
(suggested name routerMocks or test-utils/routerMocks); move the vi.mock
implementation there and ensure it returns the same mocked useParams and Link as
the original, then replace the in-file vi.mock calls in both tests with an
import of mockTanStackRouter and a call to it at the top of each test file so
both tests reuse the single shared mock implementation.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/$provider/containers/index.test.tsx:
- Around line 7-25: Extract the duplicated redirect mock into a shared test
utility (e.g. export createRedirectMock from src/client/test-utils/redirectMock)
and replace the inline mock in the vi.mock("@tanstack/react-router") blocks with
an import of that helper; update the vi.mock return to spread the actual module
and set redirect: createRedirectMock(), keeping the same behavior of checking
args.href, interpolating args.params into args.to, and throwing the "Redirect
to: ..." Error so tests that assert thrown messages keep working (refer to the
existing vi.mock in the test and the suggested createRedirectMock helper name).
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/$provider/containers/index.tsx:
- Around line 107-113: The destructured variable project from the useParams call
in StorageDashboard is unused except for a truthy check; update the useParams
usage to either rename project to something like hasProject (if you only need
existence) or remove it and check for provider/projectId existence directly, or
replace it with projectId to use the actual value consistently; locate the
useParams call in StorageDashboard and adjust the select return or subsequent
checks so there is no unused variable left.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 246f3106-e12c-46a4-afcf-a75b6467d2e0
📒 Files selected for processing (11)
apps/aurora-portal/src/client/routeTree.gen.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/$.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/$.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/index.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/index.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ContainerList.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ContainerList.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ContainerTableView.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ContainerTableView.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ManageContainerAccessModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/SwiftObjectStorage/ObjectList.tsx
...es/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx
Outdated
Show resolved
Hide resolved
...es/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx
Show resolved
Hide resolved
...$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/index.tsx
Show resolved
Hide resolved
...$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/index.tsx
Show resolved
Hide resolved
...$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/index.tsx
Show resolved
Hide resolved
...nts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/$.tsx
Outdated
Show resolved
Hide resolved
...$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/index.tsx
Show resolved
Hide resolved
…tion in object browser
…bjectsFileNavigation
…and Objects subdirectories
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx (2)
16-23:⚠️ Potential issue | 🟡 MinorUnicode prefixes can break with
btoa/atob.On Line 16 and Line 22, direct
btoa/atobis not UTF-8 safe; non‑Latin-1 folder names can fail encoding/decoding.Proposed fix
-const encodePrefix = (prefix: string): string => btoa(prefix) +const encodePrefix = (prefix: string): string => { + const bytes = new TextEncoder().encode(prefix) + let bin = "" + bytes.forEach((b) => (bin += String.fromCharCode(b))) + return btoa(bin) +} const decodePrefix = (encoded: string | undefined): string => { if (!encoded) return "" try { - return atob(encoded) + const bin = atob(encoded) + const bytes = Uint8Array.from(bin, (ch) => ch.charCodeAt(0)) + return new TextDecoder().decode(bytes) } catch { return "" } }Does the browser btoa()/atob() API support Unicode input directly, and what is the recommended UTF-8 safe encoding pattern?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx around lines 16 - 23, The current encodePrefix/decodePrefix use btoa/atob which break on Unicode; update encodePrefix to UTF-8 encode the string (use TextEncoder to get bytes, convert bytes to a binary string, then base64-encode that) and update decodePrefix to base64-decode to a binary string (via atob), convert that binary string to a Uint8Array and UTF-8 decode with TextDecoder, preserving non‑Latin-1 characters; modify the functions named encodePrefix and decodePrefix accordingly so all callers keep the same signatures.
231-231:⚠️ Potential issue | 🟡 MinorUse Lingui-compatible interpolation in
<Trans>for extraction.On Line 231, interpolating
error.messagedirectly as{error.message}in<Trans>is the pattern already flagged by lint and can break extraction semantics.Proposed fix
- <Trans>Error Loading Objects: {error.message}</Trans> + <Trans id="swift.objects.errorLoading" message="Error Loading Objects: {message}" values={{ message: error.message }} />In Lingui v5, what is the recommended pattern for passing dynamic values into `<Trans>` so message extraction works correctly?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx at line 231, Replace the direct JSX interpolation inside <Trans> with Lingui's values interpolation: change the fragment that currently renders <Trans>Error Loading Objects: {error.message}</Trans> to use a named placeholder like <Trans values={{ errorMessage: error.message }}>Error Loading Objects: {errorMessage}</Trans>; this keeps extraction working and avoids direct {error.message} inside the <Trans> content in the Swift Objects component (index.tsx).
🧹 Nitpick comments (1)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.test.tsx (1)
39-65: Consider centralizing the TanStack router mock used in Swift container tests.This mock pattern is effectively duplicated in
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerTableView.test.tsx(Line 15-42). A shared test helper would reduce drift risk when route params evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Containers/index.test.tsx around lines 39 - 65, Extract the duplicated vi.mock of "@tanstack/react-router" (the useParams and Link mocks) into a single test helper module that exports a setup function (e.g., setupTanstackRouterMock) or the mock implementation itself; replace the inline mocks in index.test.tsx and ContainerTableView.test.tsx with an import and call to that helper. Ensure the helper uses vi.importActual to spread the real module, defines useParams and Link the same way, and exports any types or utilities needed by tests so both tests import the same centralized mock and avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx:
- Around line 71-72: The footer currently uses visibleCount derived from
rowVirtualizer.getVirtualItems().length which reflects the virtualization window
rather than the total filtered dataset; replace visibleCount with the actual
dataset count (e.g., objects.length or filteredObjects.length used for
rendering) when composing the “Showing X of Y items” text, or alternatively
change the label to explicitly say “Showing X visible items” if you intend to
report mounted rows; update the usages around visibleCount and the footer render
block (referenced where rowVirtualizer.getVirtualItems() is called and the
footer lines ~167-169) to ensure the count represents dataset size not
virtualized window.
- Line 138: The folder-row button in ObjectsTableView (the element with
className containing "flex min-w-0 items-center gap-2 text-left hover:underline
focus:outline-none") currently removes the focus indicator; remove
"focus:outline-none" and instead add keyboard-visible focus styles like
"focus-visible:ring-2 focus-visible:ring-offset-1 focus-visible:ring-indigo-500"
(or your design system's equivalent) so keyboard users retain a visible focus
state while keeping hover behavior intact.
- Around line 39-44: The formatDate helper (formatDate) currently assumes new
Date(dateString) will throw on bad input, but it returns an Invalid Date and
toLocaleString() yields "Invalid Date"; change formatDate to create the Date,
test its validity with isNaN(date.getTime()) (or Number.isNaN) and return t`N/A`
for invalid dates, otherwise return date.toLocaleString(); apply the same
validity-check fix to the second occurrence referenced at line 155 (likely the
other date formatting helper or inlined usage) so both places return t`N/A`
instead of "Invalid Date".
---
Duplicate comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx:
- Around line 16-23: The current encodePrefix/decodePrefix use btoa/atob which
break on Unicode; update encodePrefix to UTF-8 encode the string (use
TextEncoder to get bytes, convert bytes to a binary string, then base64-encode
that) and update decodePrefix to base64-decode to a binary string (via atob),
convert that binary string to a Uint8Array and UTF-8 decode with TextDecoder,
preserving non‑Latin-1 characters; modify the functions named encodePrefix and
decodePrefix accordingly so all callers keep the same signatures.
- Line 231: Replace the direct JSX interpolation inside <Trans> with Lingui's
values interpolation: change the fragment that currently renders <Trans>Error
Loading Objects: {error.message}</Trans> to use a named placeholder like <Trans
values={{ errorMessage: error.message }}>Error Loading Objects:
{errorMessage}</Trans>; this keeps extraction working and avoids direct
{error.message} inside the <Trans> content in the Swift Objects component
(index.tsx).
---
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Containers/index.test.tsx:
- Around line 39-65: Extract the duplicated vi.mock of "@tanstack/react-router"
(the useParams and Link mocks) into a single test helper module that exports a
setup function (e.g., setupTanstackRouterMock) or the mock implementation
itself; replace the inline mocks in index.test.tsx and
ContainerTableView.test.tsx with an import and call to that helper. Ensure the
helper uses vi.importActual to spread the real module, defines useParams and
Link the same way, and exports any types or utilities needed by tests so both
tests import the same centralized mock and avoid duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5822c4ba-55bd-4043-966b-b940780bfb17
📒 Files selected for processing (23)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/$.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/index.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerLimitsTooltip.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerLimitsTooltip.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerTableView.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerTableView.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerToastNotifications.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerToastNotifications.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/CreateContainerModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/CreateContainerModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/DeleteContainerModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/DeleteContainerModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/EditContainerMetadataModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/EditContainerMetadataModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/EmptyContainerModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/EmptyContainerModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ManageContainerAccessModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ManageContainerAccessModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsFileNavigation.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/index.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/$.tsx
...counts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx
Outdated
Show resolved
Hide resolved
...counts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx
Outdated
Show resolved
Hide resolved
...counts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.test.tsx (2)
1-1: Unnecessary React import with new JSX transform.Same as the previous file - this import is not needed with React 19's JSX transform.
Suggested fix
-import React from "react"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.test.tsx at line 1, Remove the unnecessary top-level import "import React from 'react'" from the ObjectsTableView.test.tsx test file because React 19's automatic JSX transform makes it redundant; update the file by deleting the import statement (ensure no JSX pragma or direct use of the React identifier remains in ObjectsTableView.test.tsx and run tests to confirm).
186-196: Footer assertions may be fragile due to Lingui Trans interpolation.The tests assert
/4 items/iand/2 items/i, but the component renders<Trans>Showing {visibleCount} of {allCount} items</Trans>. Depending on how Lingui handles the translation in test mode, the text may render differently. If tests become flaky, consider asserting on the full pattern or using a more specific query.More robust assertion approach
describe("Footer", () => { test("shows item count in footer", () => { renderView() - expect(screen.getByText(/Showing/i)).toBeInTheDocument() - expect(screen.getByText(/4 items/i)).toBeInTheDocument() + expect(screen.getByText(/Showing 4 of 4 items/i)).toBeInTheDocument() }) test("footer total matches rows length", () => { renderView({ rows: [makeObject("a.txt"), makeObject("b.txt")] }) - expect(screen.getByText(/2 items/i)).toBeInTheDocument() + expect(screen.getByText(/Showing 2 of 2 items/i)).toBeInTheDocument() }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.test.tsx around lines 186 - 196, The footer tests in ObjectsTableView.test.tsx are asserting on partial text like /4 items/i which can break due to Lingui <Trans> interpolation; update the tests (the test cases that call renderView and makeObject) to assert the full localized pattern instead—e.g., use a regex that matches "Showing {visibleCount} of {allCount} items" such as /Showing\s+\d+\s+of\s+\d+\s+items/i or screen.getByText(/Showing.*of.*items/i)—so the assertions target the complete rendered sentence rather than only the trailing "X items".apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsx (3)
73-77: MutabletrpcStatepattern works but consider factory function for isolation.The mutable
trpcStateobject is reassigned inbeforeEach, which works but can be error-prone if tests run in parallel or if a test forgets to reset state. A factory function pattern would provide better isolation.Alternative approach using factory
-let trpcState = { - objects: mockObjects as ObjectSummary[] | undefined, - isLoading: false, - error: null as { message: string } | null, -} +const createTrpcState = () => ({ + objects: mockObjects as ObjectSummary[] | undefined, + isLoading: false, + error: null as { message: string } | null, +}) + +let trpcState = createTrpcState()Then in
beforeEach:- trpcState = { objects: mockObjects, isLoading: false, error: null } + trpcState = createTrpcState()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsx around lines 73 - 77, Replace the mutable shared trpcState object with a factory function that returns a fresh state for each test to ensure isolation: create a function (e.g., makeTrpcState or createTrpcState) that returns the object currently assigned to trpcState (including objects: mockObjects as ObjectSummary[] | undefined, isLoading: false, error: null) and call that factory inside beforeEach to reassign a new local trpcState for each test; update any references to trpcState in the test file (including beforeEach and tests using mockObjects) so they use the freshly created instance instead of a shared mutable variable.
31-42: Relative mock path is fragile and may break on file moves.The mock path
"../../../$provider/containers/$containerName/objects/$"is deeply nested and tightly coupled to the current file location. Consider using a path alias (e.g.,@/client/routes/...) if the project supports it, or document this dependency clearly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsx around lines 31 - 42, The relative mock path string "../../../$provider/containers/$containerName/objects/$" is fragile; update the vi.mock call to reference a stable path alias (for example use "@/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/$") or add a module name mapping in your test config (Vitest/Jest) and use that alias in the vi.mock invocation; ensure the mocked export shape (Route, useSearch, useParams) remains unchanged so tests still stub Route.useParams and Route.useSearch correctly.
1-1: Unnecessary React import with new JSX transform.With the new JSX transform (React 17+), explicit
import React from "react"is not required for JSX. Since this project uses React 19.1.0, this import can be removed.Suggested fix
-import React from "react"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsx at line 1, Remove the unnecessary top-level import statement `import React from "react"` from the test file `index.test.tsx` (the Swift Objects test) — the project uses the new JSX transform (React 17+ / React 19.1.0) so JSX works without importing React; simply delete that import line to clean up the file.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsFileNavigation.test.tsx (1)
1-1: Unnecessary React import with new JSX transform.Same as previous files - this import can be removed with React 19's JSX transform.
Suggested fix
-import React from "react"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsFileNavigation.test.tsx at line 1, Remove the unnecessary top-level "import React from \"react\"" in ObjectsFileNavigation.test.tsx (the import statement at the top of the file) since the project uses the new JSX transform; delete that import line and run the tests to confirm no other code in the file relies on the React identifier (leave other imports and the test code untouched).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsFileNavigation.test.tsx:
- Around line 105-114: The test "last segment is active and does not call
onPrefixClick when clicked" currently only checks that onPrefixClick wasn't
called with "a/b/c/"; update the assertion to ensure onPrefixClick was not
called at all (e.g., use expect(onPrefixClick).not.toHaveBeenCalled() or
expect(onPrefixClick).toHaveBeenCalledTimes(0)) after rendering via renderNav
and clicking the "c" element (cEl) so any accidental calls with other args are
caught; keep the rest of the test (userEvent.setup, renderNav, finding cEl)
unchanged.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.test.tsx:
- Around line 159-163: The test title is inconsistent with its assertion: change
the test description string in ObjectsTableView.test.tsx (the test that calls
renderView with makeObject and checks getByTestId("object-row-file.txt")) to
reflect the actual expected symbol "—" (e.g., "renders — for missing
last_modified on objects") so the name matches the assertion and component
behavior.
---
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsx:
- Around line 73-77: Replace the mutable shared trpcState object with a factory
function that returns a fresh state for each test to ensure isolation: create a
function (e.g., makeTrpcState or createTrpcState) that returns the object
currently assigned to trpcState (including objects: mockObjects as
ObjectSummary[] | undefined, isLoading: false, error: null) and call that
factory inside beforeEach to reassign a new local trpcState for each test;
update any references to trpcState in the test file (including beforeEach and
tests using mockObjects) so they use the freshly created instance instead of a
shared mutable variable.
- Around line 31-42: The relative mock path string
"../../../$provider/containers/$containerName/objects/$" is fragile; update the
vi.mock call to reference a stable path alias (for example use
"@/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/$")
or add a module name mapping in your test config (Vitest/Jest) and use that
alias in the vi.mock invocation; ensure the mocked export shape (Route,
useSearch, useParams) remains unchanged so tests still stub Route.useParams and
Route.useSearch correctly.
- Line 1: Remove the unnecessary top-level import statement `import React from
"react"` from the test file `index.test.tsx` (the Swift Objects test) — the
project uses the new JSX transform (React 17+ / React 19.1.0) so JSX works
without importing React; simply delete that import line to clean up the file.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsFileNavigation.test.tsx:
- Line 1: Remove the unnecessary top-level "import React from \"react\"" in
ObjectsFileNavigation.test.tsx (the import statement at the top of the file)
since the project uses the new JSX transform; delete that import line and run
the tests to confirm no other code in the file relies on the React identifier
(leave other imports and the test code untouched).
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.test.tsx:
- Line 1: Remove the unnecessary top-level import "import React from 'react'"
from the ObjectsTableView.test.tsx test file because React 19's automatic JSX
transform makes it redundant; update the file by deleting the import statement
(ensure no JSX pragma or direct use of the React identifier remains in
ObjectsTableView.test.tsx and run tests to confirm).
- Around line 186-196: The footer tests in ObjectsTableView.test.tsx are
asserting on partial text like /4 items/i which can break due to Lingui <Trans>
interpolation; update the tests (the test cases that call renderView and
makeObject) to assert the full localized pattern instead—e.g., use a regex that
matches "Showing {visibleCount} of {allCount} items" such as
/Showing\s+\d+\s+of\s+\d+\s+items/i or
screen.getByText(/Showing.*of.*items/i)—so the assertions target the complete
rendered sentence rather than only the trailing "X items".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a84fae78-3dfe-4da3-8fdf-ea016c7f4dee
📒 Files selected for processing (3)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsFileNavigation.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsx
...countId/projects/$projectId/storage/-components/Swift/Objects/ObjectsFileNavigation.test.tsx
Show resolved
Hide resolved
...s/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.test.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx (1)
234-238:⚠️ Potential issue | 🟡 MinorUse a plain variable placeholder inside
<Trans>instead oferror.messageaccess.Line 237 still uses property access interpolation inside
<Trans>, which is the same lint/i18n extraction issue flagged earlier.💡 Proposed fix
// Handle error state if (error) { + const message = error.message return ( <Stack className="fixed inset-0" distribution="center" alignment="center" direction="vertical"> - <Trans>Error Loading Objects: {error.message}</Trans> + <Trans>Error Loading Objects: {message}</Trans> </Stack> ) }Use this read-only check to find the same pattern elsewhere:
#!/bin/bash rg -nP --type=tsx '<Trans[^>]*>[^<]*\{[A-Za-z_$][\w$]*\.[^}]+\}[^<]*</Trans>'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx around lines 234 - 238, The <Trans> element currently interpolates a property access ({error.message}) which breaks i18n extraction; extract the message into a local plain variable (e.g., const errorMessage = error?.message ?? '') before the JSX and then use that variable inside <Trans> (e.g., <Trans>Error Loading Objects: {errorMessage}</Trans>), replacing the direct property access; target the JSX block using the <Trans> element and the error variable to apply this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx:
- Around line 234-238: The <Trans> element currently interpolates a property
access ({error.message}) which breaks i18n extraction; extract the message into
a local plain variable (e.g., const errorMessage = error?.message ?? '') before
the JSX and then use that variable inside <Trans> (e.g., <Trans>Error Loading
Objects: {errorMessage}</Trans>), replacing the direct property access; target
the JSX block using the <Trans> element and the error variable to apply this
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a8758d4-caee-436c-b6ad-df114c1a224f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/aurora-portal/package.jsonapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsFileNavigation.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/aurora-portal/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx
…for correct i18n extractionwq
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx (1)
177-205: Memoize row derivation to avoid repeated O(n log n) work per render.
buildRows+ filter + sort are recomputed on every render. For large containers, this can become a visible UI cost even with virtualization.⚙️ Proposed refactor
-import { useState, startTransition } from "react" +import { useMemo, useState, startTransition } from "react" ... - const allRows = buildRows((objects ?? []) as ObjectSummary[], currentPrefix) - - const filteredRows = allRows.filter((row) => row.displayName.toLowerCase().includes(searchTerm.toLowerCase().trim())) - - const sortedRows = !sortSettings.sortBy - ? filteredRows - : [...filteredRows].sort((a, b) => { + const sortedRows = useMemo(() => { + const allRows = buildRows((objects ?? []) as ObjectSummary[], currentPrefix) + const filteredRows = allRows.filter((row) => + row.displayName.toLowerCase().includes(searchTerm.toLowerCase().trim()) + ) + + if (!sortSettings.sortBy) return filteredRows + + return [...filteredRows].sort((a, b) => { let comparison = 0 switch (sortSettings.sortBy) { case "name": comparison = a.displayName.localeCompare(b.displayName) break ... } return sortSettings.sortDirection === "desc" ? -comparison : comparison }) + }, [objects, currentPrefix, searchTerm, sortSettings.sortBy, sortSettings.sortDirection])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx around lines 177 - 205, The rows (buildRows -> filteredRows -> sortedRows) are recomputed every render causing O(n log n) work; wrap the entire derivation (call to buildRows, the search filter using searchTerm, and the sorting using sortSettings) in a useMemo that depends on [objects, currentPrefix, searchTerm, sortSettings] (or the specific keys within sortSettings) so the expensive computation only runs when inputs change; update references to use the memoized value instead of recomputing inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx:
- Around line 213-219: handleSortChange currently normalizes newSort.sortBy
using lossy .toString(), which can yield unsupported keys (e.g., "0" or
"name,bytes") and cause the switch in the sort application logic to no-op;
update handleSortChange to explicitly normalize newSort.sortBy into one of the
supported string keys before calling setSortSettings (use type-safe checks: if
sortBy is a string and is one of the allowed keys use it, if it's a number map
it to the corresponding allowed key, if it's an array pick the first allowed
element or join/map to a canonical key, otherwise default to "name"), and ensure
the SortSettings passed into setSortSettings uses that normalized value instead
of newSort.sortBy?.toString().
---
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx:
- Around line 177-205: The rows (buildRows -> filteredRows -> sortedRows) are
recomputed every render causing O(n log n) work; wrap the entire derivation
(call to buildRows, the search filter using searchTerm, and the sorting using
sortSettings) in a useMemo that depends on [objects, currentPrefix, searchTerm,
sortSettings] (or the specific keys within sortSettings) so the expensive
computation only runs when inputs change; update references to use the memoized
value instead of recomputing inline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62b4cd10-8044-4fd6-88e3-f2d17f337de9
📒 Files selected for processing (1)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx
...es/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx
Show resolved
Hide resolved
…indow size in footer
…ssy toString coercion
…fects during render
…ssing last_modified
mark-karnaukh-extern-sap
added
enhancement
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/index.tsx (1)
121-125: Consider defensive handling foravailableServices.Line 124 uses non-null assertion (
availableServices!) after optional chaining on line 123. IftrpcClientis undefined (or if the query returns undefined unexpectedly), this assertion would pass an invalid value tocheckServiceAvailability, potentially causing a runtime error within that function when accessingavailableServices.reduce(...).Since
beforeLoadruns before the component mounts, a silent failure here could be confusing to debug.🛡️ Proposed defensive check
beforeLoad: async ({ context, params }) => { const { trpcClient } = context const availableServices = await trpcClient?.auth.getAvailableServices.query() + if (!availableServices) { + throw redirect({ + to: "/accounts/$accountId/projects", + params: { accountId: params.accountId }, + }) + } - checkServiceAvailability(availableServices!, params) + checkServiceAvailability(availableServices, params) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/index.tsx around lines 121 - 125, beforeLoad is calling trpcClient?.auth.getAvailableServices.query() and then force-unwrapping availableServices with availableServices! when passing to checkServiceAvailability, which can cause a runtime error if trpcClient is undefined or the query returns undefined; update beforeLoad to defensively handle unavailable services by checking trpcClient and availableServices before calling checkServiceAvailability (e.g., if trpcClient is missing abort/return early or throw a meaningful Error, and if availableServices is undefined pass an empty array or handle the error path), referencing the beforeLoad handler, the trpcClient variable, availableServices, and the checkServiceAvailability(...) call so the call site safely handles undefined results.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/index.tsx:
- Around line 121-125: beforeLoad is calling
trpcClient?.auth.getAvailableServices.query() and then force-unwrapping
availableServices with availableServices! when passing to
checkServiceAvailability, which can cause a runtime error if trpcClient is
undefined or the query returns undefined; update beforeLoad to defensively
handle unavailable services by checking trpcClient and availableServices before
calling checkServiceAvailability (e.g., if trpcClient is missing abort/return
early or throw a meaningful Error, and if availableServices is undefined pass an
empty array or handle the error path), referencing the beforeLoad handler, the
trpcClient variable, availableServices, and the checkServiceAvailability(...)
call so the call site safely handles undefined results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41561a31-d90b-4d0a-847a-12941a4fb1fd
📒 Files selected for processing (12)
apps/aurora-portal/src/client/routeTree.gen.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/index.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/$containerName/objects/index.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/index.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerTableView.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsFileNavigation.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerTableView.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.test.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/$provider/containers/index.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.test.tsx
mark-karnaukh-extern-sap
requested review from
KirylSAP,
TilmanHaupt,
andypf and
vlad-schur-external-sap
vlad-schur-external-sap
approved these changes
Mar 25, 2026
andypf
approved these changes
Mar 25, 2026
mark-karnaukh-extern-sap
deleted the
mark-swift-objects-file-browser-interface
branch
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Object Storage — Swift Object Browser
Implements the Swift object browser for the Aurora Portal. Users can now open a container, browse its contents in a familiar folder-like structure, and navigate through nested prefixes using a breadcrumb trail — with smooth performance even for containers holding thousands of objects.
Related tasks
object-storage-ui: ObjectBrowser - File browser interface with folder navigationobject-storage-ui: Breadcrumb - Navigation breadcrumb component for folder structuresWhat's included
Object browser — clicking a container name opens a file browser view listing its folders and objects at the current prefix level. Folders and files are visually distinct. Clicking a folder navigates into it; the list updates to show only that folder's contents.
Breadcrumb navigation — a breadcrumb trail above the list always reflects the current location. Each segment is clickable and jumps directly to that prefix level. The container name acts as the root; the deepest open folder is shown as the non-interactive active segment.
URL-based prefix state — the current folder path is stored in the URL as a search param, so deep links, browser back/forward, and page reloads all work correctly.
Virtual scrolling — the object list uses TanStack Virtual, consistent with the existing container list, so containers with 1000+ objects remain performant.
Sort and search — objects can be sorted by name, last modified, or size. The default view preserves the API order. A search field filters by name within the current prefix level.
URL-persisted sort and search — sort column, sort direction, and search term are stored in the URL search params for both the container list and object browser. Deep links, browser back/forward, and page reloads all restore the exact sort and filter state.
Container name links — container names in the container list now link directly to the object browser.
Long name handling — long container and object names are truncated with an ellipsis; the full name is shown on hover.
Component organisation — Swift container and object components are now grouped into dedicated
Containers/andObjects/subdirectories, and the object list is split into focused single-responsibility components, mirroring the existing container list structure.Also fixed
ManageContainerAccessModalcaused by a mutation object reference in auseEffectdependency array/Aurora_gold.png) and self-referencing directory entries when listing inside a prefixScreenshots
Container list — clickable names
Object browser — root level (
andreas-pfau)Object browser — inside a folder (
test/)Object browser — deep nesting
Object browser — large container (
--progress_segments)Checklist
Summary by CodeRabbit
New Features
Refactor