feat(aurora-portal): object storage — file actions, container management improvements, and bug fixes#654
Open
mark-karnaukh-extern-sap wants to merge 29 commits intomainfrom
Open
feat(aurora-portal): object storage — file actions, container management improvements, and bug fixes#654mark-karnaukh-extern-sap wants to merge 29 commits intomainfrom
mark-karnaukh-extern-sap wants to merge 29 commits intomainfrom
Conversation
mark-karnaukh-extern-sap
added
enhancement
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…to previous approach
…-segments variants
…to apply correct Swift params
… and folder creation
…rve folder separators
…ter test assertions
…ser and rename support
…anged from source
…ject action modals
… DeleteObjectModal
…native type detection
…in DeleteObjectModal
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
.../accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.test.tsx
Fixed
Show fixed
Hide fixed
mark-karnaukh-extern-sap
force-pushed
the
mark-swift-objects-file-actions
branch
from
9d9c1af to
200eb74
Compare
…l and MoveRenameObjectModal
…al when container appears empty
…ngui interpolation warning
mark-karnaukh-extern-sap
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.tsx (1)
41-79: Optional: reduce repetitive toast handler boilerplate.The per-operation handlers are clear, but they repeat a lot of identical
setToastData(...)scaffolding. A tiny helper/factory would shrink this surface and make future operations easier to add.🤖 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.tsx around lines 41 - 79, Refactor the repetitive toast handlers by creating a small factory/helper that returns standardized success/error handlers which call setToastData with the appropriate toast creator; replace functions like handleCreateSuccess, handleCreateError, handleEmptySuccess, handleEmptyError, handleDeleteSuccess, handleDeleteError, handlePropertiesSuccess, handlePropertiesError, handleAclSuccess, and handleAclError with handlers produced by that factory and keep using the existing toast factories (getContainerCreatedToast, getContainerCreateErrorToast, getContainerEmptiedToast, getContainerEmptyErrorToast, getContainerDeletedToast, getContainerDeleteErrorToast, getContainerUpdatedToast, getContainerUpdateErrorToast, getContainerAclUpdatedToast, getContainerAclUpdateErrorToast) and the shared onDismiss handler (handleToastDismiss) so each generated handler simply calls setToastData(...) with the correct toast creator and arguments.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerTableView.test.tsx (1)
404-490: Optional: extract a shared helper for menu-action flows.The callback tests repeat the same “open row menu → click action → click simulate button” pattern. A shared helper would reduce repetition and improve test readability.
🤖 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/ContainerTableView.test.tsx around lines 404 - 490, Tests duplicate the "open row menu → click action → click simulate button" flow; extract a helper to reduce repetition. Create a helper (e.g., interactWithRowAction) that accepts parameters like containerId ("alpha"), actionTestId ("empty-action-alpha" | "delete-action-alpha" | "properties-action-alpha" | "access-control-action-alpha") and simulateButtonName ("SimulateEmptySuccess", etc.), uses userEvent.setup(), finds the row via screen.getByTestId("container-row-<id>"), clicks the menu button, clicks the action test id, then clicks the simulate button; update each test (which currently calls renderView and repeats those clicks) to call renderView(...) and then the new helper with the correct params before asserting the corresponding callback (onEmptySuccess, onDeleteError, onPropertiesSuccess, onAclError, etc.).apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsx (1)
81-113: Several of these new tests are not asserting the wiring they claim to cover.
expect(getObjectCopiedToast).toBeDefined()and the current container test will still pass ifSwiftObjectsstops passingcontaineror never calls the toast factories. Capture the relevant props from theObjectsTableViewmock, invoke the callbacks, and assert the factory call arguments instead.Sketch of a stronger approach
-let capturedOnDeleteFolderSuccess: ((folderName: string, deletedCount: number) => void) | undefined +let capturedTableProps: + | { + container?: string + onDeleteFolderSuccess?: (folderName: string, deletedCount: number) => void + onDownloadError?: (objectName: string, errorMessage: string) => void + onCopyObjectSuccess?: (objectName: string, targetContainer: string, targetPath: string) => void + onCopyObjectError?: (objectName: string, errorMessage: string) => void + onMoveObjectSuccess?: (objectName: string, targetContainer: string, targetPath: string) => void + onMoveObjectError?: (objectName: string, errorMessage: string) => void + } + | undefined ObjectsTableView: vi.fn( ({ + container, rows, searchTerm, onDeleteFolderSuccess, onDeleteFolderError, onDownloadError, onDeleteObjectSuccess, onDeleteObjectError, onCopyObjectSuccess, onCopyObjectError, onMoveObjectSuccess, onMoveObjectError, }) => { - capturedOnDeleteFolderSuccess = onDeleteFolderSuccess + capturedTableProps = { + container, + onDeleteFolderSuccess, + onDownloadError, + onCopyObjectSuccess, + onCopyObjectError, + onMoveObjectSuccess, + onMoveObjectError, + } return ( <div data-testid="objects-table-view" + data-container={container}Then drive the callbacks directly in the tests and assert
getObject*Toastwas called with the expected args.Also applies to: 281-356
🤖 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 81 - 113, The ObjectsTableView mock in the tests is only asserting existence of toast factory functions and container but not that SwiftObjects actually wires and calls the callbacks; update the ObjectsTableView mock to capture each callback prop (e.g., onDeleteFolderSuccess, onDeleteFolderError, onDownloadError, onDeleteObjectSuccess, onDeleteObjectError, onCopyObjectSuccess, onCopyObjectError, onMoveObjectSuccess, onMoveObjectError and the container prop) into uniquely named variables (similar to capturedOnDeleteFolderSuccess), then in the test invoke those captured callbacks with representative args and assert the corresponding toast factory functions (getObjectCopiedToast, getObjectDeletedToast, etc.) were called with the expected arguments and that the container was forwarded to the toast factories; focus changes around the ObjectsTableView mock and the SwiftObjects test assertions so the tests verify actual wiring and invocation rather than only presence.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/DeleteObjectModal.tsx (1)
64-69: React lint warning:deleteObjectMutationmissing from dependency array.The
useEffectreferencesdeleteObjectMutation.reset()but doesn't include the mutation in dependencies. While this works because the mutation object reference is stable, adding it to the array (or using the recommended pattern of calling reset in the close handler only) would silence lint warnings and improve clarity.The current implementation is functionally correct since
useMutationreturns a stable reference, so this is a minor lint hygiene issue.♻️ Optional: suppress or satisfy the lint rule
useEffect(() => { if (!isOpen) { deleteObjectMutation.reset() setKeepSegments(false) } + // eslint-disable-next-line react-hooks/exhaustive-deps }, [isOpen])Or consolidate reset logic entirely in
handleClose:- useEffect(() => { - if (!isOpen) { - deleteObjectMutation.reset() - setKeepSegments(false) - } - }, [isOpen]) - const handleClose = () => { deleteObjectMutation.reset() + setKeepSegments(false) onClose() }🤖 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/DeleteObjectModal.tsx around lines 64 - 69, The useEffect that calls deleteObjectMutation.reset() is missing deleteObjectMutation in its dependency array; either add deleteObjectMutation to the dependencies or, better, move the reset() call into the existing handleClose function (which already runs when the modal closes) and remove it from the effect to satisfy the linter and keep reset logic consolidated; update useEffect to only depend on isOpen if you move reset to handleClose, or add deleteObjectMutation to the array if you keep it in the effect.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx (1)
116-159: Consider consolidatinghandleDownloadwithhandlePreviewOrDownload.
handleDownloadandhandlePreviewOrDownloadshare nearly identical logic (streaming chunks, assembling blob, error handling). The only difference is thathandleDownloadalways triggers a download anchor click, whilehandlePreviewOrDownloadconditionally opens in a new tab.You could consolidate these into a single function with a
forceDownloadparameter to reduce ~40 lines of duplication.♻️ Suggested consolidation
- const handleDownload = async (row: ObjectRow) => { - setDownloadingRow(row) - try { - const chunks: Uint8Array[] = [] - let contentType = "application/octet-stream" - let filename = row.displayName - - const iterable = await trpcClient.storage.swift.downloadObject.mutate({ - container, - object: row.name, - filename: row.displayName, - ...(account ? { account } : {}), - }) - - for await (const { chunk, contentType: ct, filename: fn, downloaded, total } of iterable) { - if (ct) contentType = ct - if (fn) filename = fn - chunks.push(Uint8Array.from(atob(chunk), (c) => c.charCodeAt(0))) - setDownloadProgress({ downloaded, total }) - } - - const totalLength = chunks.reduce((sum, c) => sum + c.length, 0) - const merged = new Uint8Array(totalLength) - let offset = 0 - for (const c of chunks) { - merged.set(c, offset) - offset += c.length - } - - const blob = new Blob([merged], { type: contentType }) - const url = URL.createObjectURL(blob) - const anchor = document.createElement("a") - anchor.href = url - anchor.download = filename - document.body.appendChild(anchor) - anchor.click() - document.body.removeChild(anchor) - URL.revokeObjectURL(url) - } catch (err) { - onDownloadError(row.displayName, err instanceof Error ? err.message : String(err)) - } finally { - setDownloadingRow(null) - setDownloadProgress(null) - } - } - - const handlePreviewOrDownload = async (row: ObjectRow) => { + const handlePreviewOrDownload = async (row: ObjectRow, forceDownload = false) => { const previewing = isBrowserPreviewable(row.content_type) - if (previewing) { + if (previewing && !forceDownload) { setPreviewingRow(row) } else { setDownloadingRow(row) } // ... rest unchanged, just use (previewing && !forceDownload) for the branchThen update the Download menu item:
- onClick={() => handleDownload(row as ObjectRow)} + onClick={() => handlePreviewOrDownload(row as ObjectRow, true)}🤖 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.tsx around lines 116 - 159, handleDownload duplicates most of handlePreviewOrDownload (streaming, chunk assembly, blob creation and error/cleanup handling); consolidate both into a single helper (e.g., downloadOrPreviewObject(container, object, filename, { account?, forceDownload:boolean })) that contains the shared async streaming loop, chunk merging, blob/url creation, error handling via onDownloadError, and cleanup (setDownloadingRow/setDownloadProgress). Keep the existing behaviors by using the forceDownload flag: if true perform the anchor click download, otherwise open the blob URL in a new tab for preview; replace calls to handleDownload and handlePreviewOrDownload with the new function and remove the duplicated old function body. Ensure to preserve and pass account, row.displayName/row.name, and update the Download menu item to call the new function with forceDownload: true.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectToastNotifications.tsx (1)
160-181: Consider formatting for copy/move success messages when path is empty.When
targetPathis empty (root copy/move), the message renders as:"report.pdf" was successfully copied to backup-container.
The period directly follows the container name, which is correct. However, when
targetPathhas a value:"report.pdf" was successfully copied to backup-containerarchive/.
The container and path concatenate without a separator. Consider adding a slash or space between them for clarity.
♻️ Optional: Add separator between container and path
description={ <Trans> - "{objectName}" was successfully copied to {targetContainer} - {targetPath}. + "{objectName}" was successfully copied to {targetContainer} + {targetPath ? `/${targetPath}` : ""}. </Trans> }Same change for
getObjectMovedToast.Also applies to: 200-221
🤖 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/ObjectToastNotifications.tsx around lines 160 - 181, The success message concatenates targetContainer and targetPath without a separator; update getObjectCopiedToast (and mirror the same change in getObjectMovedToast) to build a displayTarget variable that, when targetPath is non-empty, joins them with a "/" (ensuring you don’t duplicate a slash if targetPath already starts with one), and when targetPath is empty uses only targetContainer, then use displayTarget in the description JSX so messages render correctly for both root and nested targets.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/MoveRenameObjectModal.tsx (2)
234-253: Consider adding duplicate name check at destination.The
validateObjectNamefunction validates format but doesn't check if an object with the same name already exists at the destination path. Swift will silently overwrite existing objects. Consider warning users if they're about to overwrite an existing file.💡 Suggested enhancement
const validateObjectName = useCallback( (name: string): boolean => { const trimmed = name.trim() if (!trimmed) { setNewObjectNameError(t`Object name is required`) return false } if (trimmed.includes("/")) { setNewObjectNameError(t`Object name cannot contain slashes`) return false } if (trimmed !== name) { setNewObjectNameError(t`Object name cannot have leading or trailing whitespace`) return false } + // Check if destination already exists (would overwrite) + const destExists = rows.some( + (r) => r.kind === "object" && r.displayName === trimmed + ) + if (destExists && trimmed !== object?.displayName) { + setNewObjectNameError(t`An object with this name already exists at the destination`) + return false + } setNewObjectNameError(null) return true }, - [t] + [t, rows, object?.displayName] )🤖 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/MoveRenameObjectModal.tsx around lines 234 - 253, validateObjectName currently only checks format; add a duplicate-name check against the destination so users are warned before overwriting. After the existing synchronous checks in validateObjectName (or by creating an async wrapper like checkDestinationCollision), call the Swift/Storage API to test if `${destinationPrefix}/${trimmed}` exists (use the existing client helper such as checkObjectExists, headObject or listObjects used elsewhere in this file), and if it exists setNewObjectNameError(t`An object with this name already exists at the destination`) or surface a confirmation/overwrite flag to the move/rename handler; ensure the UI path uses this async check before performing the move in the function that calls validateObjectName (e.g., the submit/move handler) so you don't silently overwrite objects.
55-66: Effect dependency may cause unintended resets.The effect resets state when
targetContainerchanges, buttargetContaineris also in the dependency array. This means changing the target container will resetlocalFolders, losing any folders the user created in the previous container. This might be intentional, but consider if users expect their locally-created folders to persist when switching between containers.🤖 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/MoveRenameObjectModal.tsx around lines 55 - 66, The effect invoked in useEffect resets UI state (setCurrentPrefix, setLocalFolders, setNewFolderName, setNewFolderError, setShowNewFolderInput, setContainerSearch, setDebouncedSearch, setNewObjectNameError) whenever either isOpen or targetContainer changes; because targetContainer is in the dependency array this will wipe localFolders when switching containers. To fix, decide whether state should persist across containers: if it should persist, remove targetContainer from the dependency array so the reset only happens on isOpen changes (keep useEffect([isOpen]) and leave the existing setters), or if per-container state is required, change localFolders to be keyed by targetContainer (e.g., a map) and update setLocalFolders to read/write only that container’s entry instead of clearing the whole map when targetContainer changes.
🤖 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/Containers/ManageContainerAccessModal.tsx:
- Around line 58-68: Remove the dead branch that checks entry.startsWith(".r:-")
and the subsequent entry.startsWith(".r:") block in ManageContainerAccessModal
(the parser that inspects the entry variable), since an earlier condition
already matches both `.r:` and `.r:-` patterns; delete these redundant if blocks
(the ones creating labels "Denied referrer: ..." and "Referrer: ...") so only
the primary handling logic remains.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/CopyObjectModal.test.tsx:
- Around line 399-412: The test "shows error when folder name has leading
whitespace" is weak and creates a separate userEvent instance with
userEvent.setup().paste(" leading") which doesn't target the input; either
remove the test if behavior is undefined or change it to assert the expected
validation: use the same user instance from userEvent.setup() used earlier (or
reuse the existing "user" variable), paste/type into the input element directly
(e.g., user.paste(input, " leading") or user.type(input, " leading")), click the
Create button via getByRole, then assert the expected outcome (e.g., expect an
error message about leading whitespace or expect the folder to be created) and
remove the ambiguous comment.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/CopyObjectModal.tsx:
- Around line 68-79: The effect that resets modal state (in the CopyObjectModal
component) currently only depends on isOpen but reads sourceContainer, so when
the route changes the modal can keep an outdated targetContainer; update the
effect in useEffect to also depend on sourceContainer (or add a separate effect
watching sourceContainer) and call setTargetContainer(sourceContainer) when
sourceContainer changes while the modal is open/closed as appropriate, ensuring
targetContainer is resynced; reference the useEffect that sets targetContainer,
setTargetContainer, sourceContainer and isOpen.
- Around line 132-142: The mutation's onSettled is closing the modal regardless
of outcome; remove onSettled and move handleClose into the success path so the
dialog only closes on success. Edit the
trpcReact.storage.swift.copyObject.useMutation block: delete the onSettled
handler, and in the onSuccess handler (where
utils.storage.swift.listObjects.invalidate() and onSuccess?.(...) are called)
call handleClose() after those actions; do not call handleClose() in onError so
the modal remains open for retries. Ensure references to
submittedNameRef.current, targetContainer, and currentPrefix remain unchanged.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/MoveRenameObjectModal.tsx:
- Around line 75-88: The effect that resets modal state (useEffect) reads
sourceContainer but only lists isOpen in its dependency array, so
targetContainer won't reset to a changed sourceContainer when the modal reopens;
update the dependency array of that useEffect to include sourceContainer and
ensure the reset logic (calls to setTargetContainer, setCurrentPrefix,
setLocalFolders, setNewFolderName, setNewFolderError, setShowNewFolderInput,
setContainerSearch, setDebouncedSearch, setNewObjectName, setNewObjectNameError)
runs whenever either isOpen or sourceContainer changes.
In `@apps/aurora-portal/src/server/Storage/types/swift.ts`:
- Around line 394-398: The downloadObjectInputSchema currently requires filename
but callers may omit it; update downloadObjectInputSchema (which extends
baseContainerInputSchema) to make the filename property optional (e.g., use
z.string().optional()) so swiftRouter.downloadObject can receive absent
filenames and handle them as designed; ensure any downstream type references
accept undefined.
---
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Containers/ContainerTableView.test.tsx:
- Around line 404-490: Tests duplicate the "open row menu → click action → click
simulate button" flow; extract a helper to reduce repetition. Create a helper
(e.g., interactWithRowAction) that accepts parameters like containerId
("alpha"), actionTestId ("empty-action-alpha" | "delete-action-alpha" |
"properties-action-alpha" | "access-control-action-alpha") and
simulateButtonName ("SimulateEmptySuccess", etc.), uses userEvent.setup(), finds
the row via screen.getByTestId("container-row-<id>"), clicks the menu button,
clicks the action test id, then clicks the simulate button; update each test
(which currently calls renderView and repeats those clicks) to call
renderView(...) and then the new helper with the correct params before asserting
the corresponding callback (onEmptySuccess, onDeleteError, onPropertiesSuccess,
onAclError, etc.).
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Containers/index.tsx:
- Around line 41-79: Refactor the repetitive toast handlers by creating a small
factory/helper that returns standardized success/error handlers which call
setToastData with the appropriate toast creator; replace functions like
handleCreateSuccess, handleCreateError, handleEmptySuccess, handleEmptyError,
handleDeleteSuccess, handleDeleteError, handlePropertiesSuccess,
handlePropertiesError, handleAclSuccess, and handleAclError with handlers
produced by that factory and keep using the existing toast factories
(getContainerCreatedToast, getContainerCreateErrorToast,
getContainerEmptiedToast, getContainerEmptyErrorToast, getContainerDeletedToast,
getContainerDeleteErrorToast, getContainerUpdatedToast,
getContainerUpdateErrorToast, getContainerAclUpdatedToast,
getContainerAclUpdateErrorToast) and the shared onDismiss handler
(handleToastDismiss) so each generated handler simply calls setToastData(...)
with the correct toast creator and arguments.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/DeleteObjectModal.tsx:
- Around line 64-69: The useEffect that calls deleteObjectMutation.reset() is
missing deleteObjectMutation in its dependency array; either add
deleteObjectMutation to the dependencies or, better, move the reset() call into
the existing handleClose function (which already runs when the modal closes) and
remove it from the effect to satisfy the linter and keep reset logic
consolidated; update useEffect to only depend on isOpen if you move reset to
handleClose, or add deleteObjectMutation to the array if you keep it in the
effect.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/index.test.tsx:
- Around line 81-113: The ObjectsTableView mock in the tests is only asserting
existence of toast factory functions and container but not that SwiftObjects
actually wires and calls the callbacks; update the ObjectsTableView mock to
capture each callback prop (e.g., onDeleteFolderSuccess, onDeleteFolderError,
onDownloadError, onDeleteObjectSuccess, onDeleteObjectError,
onCopyObjectSuccess, onCopyObjectError, onMoveObjectSuccess, onMoveObjectError
and the container prop) into uniquely named variables (similar to
capturedOnDeleteFolderSuccess), then in the test invoke those captured callbacks
with representative args and assert the corresponding toast factory functions
(getObjectCopiedToast, getObjectDeletedToast, etc.) were called with the
expected arguments and that the container was forwarded to the toast factories;
focus changes around the ObjectsTableView mock and the SwiftObjects test
assertions so the tests verify actual wiring and invocation rather than only
presence.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/MoveRenameObjectModal.tsx:
- Around line 234-253: validateObjectName currently only checks format; add a
duplicate-name check against the destination so users are warned before
overwriting. After the existing synchronous checks in validateObjectName (or by
creating an async wrapper like checkDestinationCollision), call the
Swift/Storage API to test if `${destinationPrefix}/${trimmed}` exists (use the
existing client helper such as checkObjectExists, headObject or listObjects used
elsewhere in this file), and if it exists setNewObjectNameError(t`An object with
this name already exists at the destination`) or surface a
confirmation/overwrite flag to the move/rename handler; ensure the UI path uses
this async check before performing the move in the function that calls
validateObjectName (e.g., the submit/move handler) so you don't silently
overwrite objects.
- Around line 55-66: The effect invoked in useEffect resets UI state
(setCurrentPrefix, setLocalFolders, setNewFolderName, setNewFolderError,
setShowNewFolderInput, setContainerSearch, setDebouncedSearch,
setNewObjectNameError) whenever either isOpen or targetContainer changes;
because targetContainer is in the dependency array this will wipe localFolders
when switching containers. To fix, decide whether state should persist across
containers: if it should persist, remove targetContainer from the dependency
array so the reset only happens on isOpen changes (keep useEffect([isOpen]) and
leave the existing setters), or if per-container state is required, change
localFolders to be keyed by targetContainer (e.g., a map) and update
setLocalFolders to read/write only that container’s entry instead of clearing
the whole map when targetContainer changes.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx:
- Around line 116-159: handleDownload duplicates most of handlePreviewOrDownload
(streaming, chunk assembly, blob creation and error/cleanup handling);
consolidate both into a single helper (e.g., downloadOrPreviewObject(container,
object, filename, { account?, forceDownload:boolean })) that contains the shared
async streaming loop, chunk merging, blob/url creation, error handling via
onDownloadError, and cleanup (setDownloadingRow/setDownloadProgress). Keep the
existing behaviors by using the forceDownload flag: if true perform the anchor
click download, otherwise open the blob URL in a new tab for preview; replace
calls to handleDownload and handlePreviewOrDownload with the new function and
remove the duplicated old function body. Ensure to preserve and pass account,
row.displayName/row.name, and update the Download menu item to call the new
function with forceDownload: true.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectToastNotifications.tsx:
- Around line 160-181: The success message concatenates targetContainer and
targetPath without a separator; update getObjectCopiedToast (and mirror the same
change in getObjectMovedToast) to build a displayTarget variable that, when
targetPath is non-empty, joins them with a "/" (ensuring you don’t duplicate a
slash if targetPath already starts with one), and when targetPath is empty uses
only targetContainer, then use displayTarget in the description JSX so messages
render correctly for both root and nested targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b26b705-be19-4f61-9eff-741cf4c33961
📒 Files selected for processing (32)
apps/aurora-portal/docs/006_swift_object_storage_bff.mdapps/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/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.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/CopyObjectModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/CopyObjectModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/DeleteObjectModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/DeleteObjectModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/MoveRenameObjectModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/MoveRenameObjectModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectToastNotifications.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectToastNotifications.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.tsxapps/aurora-portal/src/client/trpcClient.tsapps/aurora-portal/src/locales/de/messages.poapps/aurora-portal/src/locales/en/messages.poapps/aurora-portal/src/locales/en/messages.tsapps/aurora-portal/src/server/Storage/routers/swiftRouter.test.tsapps/aurora-portal/src/server/Storage/routers/swiftRouter.tsapps/aurora-portal/src/server/Storage/types/swift.test.tsapps/aurora-portal/src/server/Storage/types/swift.ts
Comment on lines
+58
to
+68
| // .r:-<referrer> — deny a specific referrer | ||
| if (entry.startsWith(".r:-")) { | ||
| const referrer = entry.slice(4) | ||
| return { raw: entry, label: `Denied referrer: ${referrer}`, description: "", requiresToken: false } | ||
| } | ||
|
|
||
| // .r:<referrer> — grant access to a specific referrer | ||
| if (entry.startsWith(".r:")) { | ||
| const referrer = entry.slice(3) | ||
| return { raw: entry, label: `Referrer: ${referrer}`, description: "", requiresToken: false } | ||
| } |
There was a problem hiding this comment.
Remove unreachable dead code.
Lines 58-68 are unreachable because the condition at line 46 (entry.startsWith(".r:")) already matches both .r:<referrer> and .r:-<referrer> patterns. The logic at lines 45-56 correctly handles both cases, making this code dead.
🧹 Proposed fix to remove dead code
return {
raw: entry,
label: isDeny ? `Denied referer: ${host}` : `Specific referer: ${host}`,
description: "",
requiresToken: false,
}
}
- // .r:-<referrer> — deny a specific referrer
- if (entry.startsWith(".r:-")) {
- const referrer = entry.slice(4)
- return { raw: entry, label: `Denied referrer: ${referrer}`, description: "", requiresToken: false }
- }
-
- // .r:<referrer> — grant access to a specific referrer
- if (entry.startsWith(".r:")) {
- const referrer = entry.slice(3)
- return { raw: entry, label: `Referrer: ${referrer}`, description: "", requiresToken: false }
- }
-
if (entry === ".rlistings") {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // .r:-<referrer> — deny a specific referrer | |
| if (entry.startsWith(".r:-")) { | |
| const referrer = entry.slice(4) | |
| return { raw: entry, label: `Denied referrer: ${referrer}`, description: "", requiresToken: false } | |
| } | |
| // .r:<referrer> — grant access to a specific referrer | |
| if (entry.startsWith(".r:")) { | |
| const referrer = entry.slice(3) | |
| return { raw: entry, label: `Referrer: ${referrer}`, description: "", requiresToken: false } | |
| } |
🤖 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/ManageContainerAccessModal.tsx
around lines 58 - 68, Remove the dead branch that checks
entry.startsWith(".r:-") and the subsequent entry.startsWith(".r:") block in
ManageContainerAccessModal (the parser that inspects the entry variable), since
an earlier condition already matches both `.r:` and `.r:-` patterns; delete
these redundant if blocks (the ones creating labels "Denied referrer: ..." and
"Referrer: ...") so only the primary handling logic remains.
Comment on lines
+399
to
+412
| test("shows error when folder name has leading whitespace", async () => { | ||
| const user = userEvent.setup() | ||
| renderModal() | ||
| await user.click(screen.getByRole("button", { name: /New Folder/i })) | ||
| const input = screen.getByPlaceholderText(/new-folder-name/i) | ||
| // Type a name with a leading space by manipulating the input value directly | ||
| await user.type(input, "valid") | ||
| // Clear and retype with leading space using fireEvent to bypass userEvent trimming | ||
| await user.clear(input) | ||
| // Use paste to insert a value with leading space | ||
| await userEvent.setup().paste(" leading") | ||
| await user.click(screen.getByRole("button", { name: /^Create$/i })) | ||
| // Either whitespace error or just passes — either way no crash | ||
| }) |
There was a problem hiding this comment.
Test has a weak assertion and may not verify intended behavior.
This test doesn't make a specific assertion about the expected outcome. The comment "either whitespace error or just passes — either way no crash" indicates uncertainty about the expected behavior. Additionally, userEvent.setup().paste(" leading") creates a new userEvent instance, which won't be properly connected to the input element.
Consider either:
- Removing this test if the behavior is undefined
- Fixing it to properly test leading whitespace validation
🔧 Suggested fix if whitespace validation is expected
test("shows error when folder name has leading whitespace", async () => {
const user = userEvent.setup()
renderModal()
await user.click(screen.getByRole("button", { name: /New Folder/i }))
const input = screen.getByPlaceholderText(/new-folder-name/i)
- // Type a name with a leading space by manipulating the input value directly
- await user.type(input, "valid")
- // Clear and retype with leading space using fireEvent to bypass userEvent trimming
- await user.clear(input)
- // Use paste to insert a value with leading space
- await userEvent.setup().paste(" leading")
- await user.click(screen.getByRole("button", { name: /^Create$/i }))
- // Either whitespace error or just passes — either way no crash
+ // Use clipboard paste to insert value with leading space
+ await user.click(input)
+ await user.paste(" leading")
+ await user.click(screen.getByRole("button", { name: /^Create$/i }))
+ expect(screen.getByText(/cannot have leading or trailing whitespace/i)).toBeInTheDocument()
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test("shows error when folder name has leading whitespace", async () => { | |
| const user = userEvent.setup() | |
| renderModal() | |
| await user.click(screen.getByRole("button", { name: /New Folder/i })) | |
| const input = screen.getByPlaceholderText(/new-folder-name/i) | |
| // Type a name with a leading space by manipulating the input value directly | |
| await user.type(input, "valid") | |
| // Clear and retype with leading space using fireEvent to bypass userEvent trimming | |
| await user.clear(input) | |
| // Use paste to insert a value with leading space | |
| await userEvent.setup().paste(" leading") | |
| await user.click(screen.getByRole("button", { name: /^Create$/i })) | |
| // Either whitespace error or just passes — either way no crash | |
| }) | |
| test("shows error when folder name has leading whitespace", async () => { | |
| const user = userEvent.setup() | |
| renderModal() | |
| await user.click(screen.getByRole("button", { name: /New Folder/i })) | |
| const input = screen.getByPlaceholderText(/new-folder-name/i) | |
| // Use clipboard paste to insert value with leading space | |
| await user.click(input) | |
| await user.paste(" leading") | |
| await user.click(screen.getByRole("button", { name: /^Create$/i })) | |
| expect(screen.getByText(/cannot have leading or trailing whitespace/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/CopyObjectModal.test.tsx
around lines 399 - 412, The test "shows error when folder name has leading
whitespace" is weak and creates a separate userEvent instance with
userEvent.setup().paste(" leading") which doesn't target the input; either
remove the test if behavior is undefined or change it to assert the expected
validation: use the same user instance from userEvent.setup() used earlier (or
reuse the existing "user" variable), paste/type into the input element directly
(e.g., user.paste(input, " leading") or user.type(input, " leading")), click the
Create button via getByRole, then assert the expected outcome (e.g., expect an
error message about leading whitespace or expect the folder to be created) and
remove the ambiguous comment.
Comment on lines
+68
to
+79
| useEffect(() => { | ||
| if (!isOpen) { | ||
| setTargetContainer(sourceContainer) | ||
| setCurrentPrefix("") | ||
| setLocalFolders({}) | ||
| setNewFolderName("") | ||
| setNewFolderError(null) | ||
| setShowNewFolderInput(false) | ||
| setContainerSearch("") | ||
| setDebouncedSearch("") | ||
| } | ||
| }, [isOpen]) |
There was a problem hiding this comment.
Resync targetContainer when the route container changes.
This effect reads sourceContainer but only depends on isOpen. If the user navigates to another $containerName while this component remains mounted, the modal can reopen with the previous container still selected.
Proposed fix
- }, [isOpen])
+ }, [isOpen, sourceContainer])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| if (!isOpen) { | |
| setTargetContainer(sourceContainer) | |
| setCurrentPrefix("") | |
| setLocalFolders({}) | |
| setNewFolderName("") | |
| setNewFolderError(null) | |
| setShowNewFolderInput(false) | |
| setContainerSearch("") | |
| setDebouncedSearch("") | |
| } | |
| }, [isOpen]) | |
| useEffect(() => { | |
| if (!isOpen) { | |
| setTargetContainer(sourceContainer) | |
| setCurrentPrefix("") | |
| setLocalFolders({}) | |
| setNewFolderName("") | |
| setNewFolderError(null) | |
| setShowNewFolderInput(false) | |
| setContainerSearch("") | |
| setDebouncedSearch("") | |
| } | |
| }, [isOpen, sourceContainer]) |
🤖 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/CopyObjectModal.tsx
around lines 68 - 79, The effect that resets modal state (in the CopyObjectModal
component) currently only depends on isOpen but reads sourceContainer, so when
the route changes the modal can keep an outdated targetContainer; update the
effect in useEffect to also depend on sourceContainer (or add a separate effect
watching sourceContainer) and call setTargetContainer(sourceContainer) when
sourceContainer changes while the modal is open/closed as appropriate, ensuring
targetContainer is resynced; reference the useEffect that sets targetContainer,
setTargetContainer, sourceContainer and isOpen.
Comment on lines
+132
to
+142
| const copyMutation = trpcReact.storage.swift.copyObject.useMutation({ | ||
| onSuccess: () => { | ||
| utils.storage.swift.listObjects.invalidate() | ||
| onSuccess?.(submittedNameRef.current, targetContainer, currentPrefix) | ||
| }, | ||
| onError: (error) => { | ||
| onError?.(submittedNameRef.current, error.message) | ||
| }, | ||
| onSettled: () => { | ||
| handleClose() | ||
| }, |
There was a problem hiding this comment.
Keep the dialog open on copy failures.
onSettled always calls handleClose(), so the error branch never stays mounted long enough to render the inline <Message> and the user cannot correct the destination and retry. Close only on success; leave the modal open on error.
Proposed fix
const copyMutation = trpcReact.storage.swift.copyObject.useMutation({
onSuccess: () => {
utils.storage.swift.listObjects.invalidate()
onSuccess?.(submittedNameRef.current, targetContainer, currentPrefix)
+ handleClose()
},
onError: (error) => {
onError?.(submittedNameRef.current, error.message)
},
- onSettled: () => {
- handleClose()
- },
})🤖 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/CopyObjectModal.tsx
around lines 132 - 142, The mutation's onSettled is closing the modal regardless
of outcome; remove onSettled and move handleClose into the success path so the
dialog only closes on success. Edit the
trpcReact.storage.swift.copyObject.useMutation block: delete the onSettled
handler, and in the onSuccess handler (where
utils.storage.swift.listObjects.invalidate() and onSuccess?.(...) are called)
call handleClose() after those actions; do not call handleClose() in onError so
the modal remains open for retries. Ensure references to
submittedNameRef.current, targetContainer, and currentPrefix remain unchanged.
Comment on lines
+75
to
+88
| useEffect(() => { | ||
| if (!isOpen) { | ||
| setTargetContainer(sourceContainer) | ||
| setCurrentPrefix("") | ||
| setLocalFolders({}) | ||
| setNewFolderName("") | ||
| setNewFolderError(null) | ||
| setShowNewFolderInput(false) | ||
| setContainerSearch("") | ||
| setDebouncedSearch("") | ||
| setNewObjectName("") | ||
| setNewObjectNameError(null) | ||
| } | ||
| }, [isOpen]) |
There was a problem hiding this comment.
Missing sourceContainer in effect dependency array.
The effect uses sourceContainer (line 77) but it's not in the dependency array. If sourceContainer changes while the modal is closed, targetContainer won't be reset to the new value on the next open.
🔧 Suggested fix
useEffect(() => {
if (!isOpen) {
setTargetContainer(sourceContainer)
setCurrentPrefix("")
setLocalFolders({})
setNewFolderName("")
setNewFolderError(null)
setShowNewFolderInput(false)
setContainerSearch("")
setDebouncedSearch("")
setNewObjectName("")
setNewObjectNameError(null)
}
- }, [isOpen])
+ }, [isOpen, sourceContainer])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| if (!isOpen) { | |
| setTargetContainer(sourceContainer) | |
| setCurrentPrefix("") | |
| setLocalFolders({}) | |
| setNewFolderName("") | |
| setNewFolderError(null) | |
| setShowNewFolderInput(false) | |
| setContainerSearch("") | |
| setDebouncedSearch("") | |
| setNewObjectName("") | |
| setNewObjectNameError(null) | |
| } | |
| }, [isOpen]) | |
| useEffect(() => { | |
| if (!isOpen) { | |
| setTargetContainer(sourceContainer) | |
| setCurrentPrefix("") | |
| setLocalFolders({}) | |
| setNewFolderName("") | |
| setNewFolderError(null) | |
| setShowNewFolderInput(false) | |
| setContainerSearch("") | |
| setDebouncedSearch("") | |
| setNewObjectName("") | |
| setNewObjectNameError(null) | |
| } | |
| }, [isOpen, sourceContainer]) |
🤖 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/MoveRenameObjectModal.tsx
around lines 75 - 88, The effect that resets modal state (useEffect) reads
sourceContainer but only lists isOpen in its dependency array, so
targetContainer won't reset to a changed sourceContainer when the modal reopens;
update the dependency array of that useEffect to include sourceContainer and
ensure the reset logic (calls to setTargetContainer, setCurrentPrefix,
setLocalFolders, setNewFolderName, setNewFolderError, setShowNewFolderInput,
setContainerSearch, setDebouncedSearch, setNewObjectName, setNewObjectNameError)
runs whenever either isOpen or sourceContainer changes.
Comment on lines
+394
to
+398
| // Download object input schema | ||
| export const downloadObjectInputSchema = baseContainerInputSchema.extend({ | ||
| object: z.string().min(1), | ||
| filename: z.string(), | ||
| }) |
There was a problem hiding this comment.
filename should be optional in the download schema.
swiftRouter.downloadObject already handles an absent filename and only forwards it in the first yielded chunk when provided. Requiring it here forces every caller to invent a name and blocks legitimate cases that only need the stream.
Proposed fix
-export const downloadObjectInputSchema = baseContainerInputSchema.extend({
- object: z.string().min(1),
- filename: z.string(),
-})
+export const downloadObjectInputSchema = baseObjectInputSchema.extend({
+ filename: z.string().optional(),
+})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/aurora-portal/src/server/Storage/types/swift.ts` around lines 394 - 398,
The downloadObjectInputSchema currently requires filename but callers may omit
it; update downloadObjectInputSchema (which extends baseContainerInputSchema) to
make the filename property optional (e.g., use z.string().optional()) so
swiftRouter.downloadObject can receive absent filenames and handle them as
designed; ensure any downstream type references accept undefined.
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
1 participant
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.
📋 Overview
This PR completes the Swift object storage UI epic with full file action support (copy, move/rename, delete with SLO/DLO handling, file preview), major container management improvements (toast refactor, ACL editor fixes, sync delay messaging), and a range of bug fixes and code quality improvements across the object storage UI.
🔀 What's Changed
📁 File Actions (Objects)
📋 Copy object
freshMetadataflag)initialPrefixderived from object path)✏️ Move / Rename object
🗑️ Delete object
multipartManifest=deleteDelete (Keep Segments)menu item removed — replaced by the in-modal checkboxdisplayNameRefrename fromsubmittedNameRef👁️ File preview
BROWSER_PREVIEWABLE_TYPES: images, video, audio, PDF, text/* subtypescontent_typein the Swift listing — no extra fetchwindow.open; revoked after 10s🔧 Lingui `t`` interpolation fixes
row.displayName→rowDisplayName(extracted beforet\...``)Math.round(...)→progressPctconst in<Trans>(ObjectsTableView)metadataError.message→metadataErrorMessageconst beforereturn(DeleteObjectModal)🗄️ Container Management
🔔 Toast notifications refactored to page level
ContainerTableViewno longer owns toast stateonCreateSuccess/Error,onEmptySuccess/Error,onDeleteSuccess/Error,onPropertiesSuccess/Error,onAclSuccess/Error) lifted to containersindex.tsx🔄 Empty container — cache invalidation fix
listObjectsnow invalidated on empty success (scoped to the container)listContainerswas invalidated⏳ Delete container — sync delay info message
container.count > 0but listing returns 0 objects, an info note explains the Swift eventual consistency delay🔐 ACL preview fixes (ManageContainerAccessModal)
e9141fb24eee...) no longer break layout — raw ACL shown in abreak-allcode block below the labeltitleattribute added to truncated label text.r:<referrer>entries now correctly parsed as "Specific referer: {host}" instead of falling through to the project/user branch and producing "User ... from project .r".r:-<referrer>parsed as "Denied referer: {host}"⚡ Object Browser (Copy / Move modals)
🚀 Virtualization
CopyObjectModalandMoveRenameObjectModalvirtualised with@tanstack/react-virtualuseVirtualizerdeclared before early return to satisfy Rules of HooksestimateSize: 32,overscan: 5; scroll container keepsmax-h-56📸 Screenshots
✅ Checklist
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation