Skip to content

feat(aurora-portal): add object details and metadata editing#710

Draft
mark-karnaukh-extern-sap wants to merge 4 commits intomark-swift-objects-file-sharingfrom
mark-swift-objects-file-metadata
Draft

feat(aurora-portal): add object details and metadata editing#710
mark-karnaukh-extern-sap wants to merge 4 commits intomark-swift-objects-file-sharingfrom
mark-swift-objects-file-metadata

Conversation

@mark-karnaukh-extern-sap
Copy link
Copy Markdown
Contributor

@mark-karnaukh-extern-sap mark-karnaukh-extern-sap commented Apr 16, 2026

Summary

Implements the object properties panel for the Swift object browser. Users can now view full object metadata and edit custom X-Object-Meta-* headers directly from the Aurora Portal without leaving the object list.

What's changed

  • New EditObjectMetadataModal — opens from the Properties action in each object row's menu
  • Read-only properties section: content type, MD5 checksum, size, last modified
  • Editable Expires at field with debounced regexp validation and inline hint text
  • Full custom metadata table — add, edit, delete, save, discard with inline row editing
  • SLO and DLO detection via getObjectMetadata HEAD request with informational banners
  • Key validation: RFC 7230 token allowlist + alphanumeric requirement (blocks e.g. ----)
  • Same key validation applied to EditContainerMetadataModal for consistency
  • Focus-loss bug fixed in both modals — row key prop now uses stable index
  • getObjectMetadataUpdatedToast and getObjectMetadataUpdateErrorToast added to notifications
  • 57 new unit tests for the modal; existing tests updated across 4 files

Screenshots

Properties modal — empty metadata state

image

Properties modal — with existing metadata entries

image

Expires at field — debounced validation hint while typing

image

SLO manifest — informational banner

image

Notes

  • Swift's POST metadata update is a full replace — no explicit removeMetadata needed; omitted keys are cleared server-side
  • The deleteAt Unix timestamp returned by the API is converted to YYYY-MM-DD HH:MM:SS for display and back to Unix on submit
  • The DataGridRow key fix (index-only) resolves focus loss on key-field edit — applied to both object and container metadata modals

Checklist

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

Summary by CodeRabbit

Release Notes

  • New Features

    • Added object properties editor enabling users to view and update custom metadata, set expiration timestamps, and manage metadata key-value pairs.
    • Implemented metadata key validation requiring alphanumeric characters and RFC 7230 token-safe characters only.
    • Added toast notifications for successful and failed metadata updates.
  • Tests

    • Comprehensive test coverage for metadata editing workflows, validation rules, and success/error notifications.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 16, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c1fdc3de-0d98-45b9-9518-a706cab2d9da

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mark-swift-objects-file-metadata

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mark-karnaukh-extern-sap mark-karnaukh-extern-sap changed the title Mark swift objects file metadata feat(aurora-portal): add object details and metadata editing Apr 16, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (5)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx (1)

433-436: Optional: align data-testid with the new label.

The menu label is now Edit Metadata but data-testid is still properties-action-${row.name}. It won't break anything (tests use the same testid), but renaming to e.g. edit-metadata-action-${row.name} keeps grep/refactor and analytics selectors consistent with the UI.

✏️ Proposed rename
             <PopupMenuItem
               label={t`Edit Metadata`}
               onClick={() => setEditMetadataTarget(row as ObjectRow)}
-              data-testid={`properties-action-${row.name}`}
+              data-testid={`edit-metadata-action-${row.name}`}
             />

The ObjectsTableView.test.tsx Properties (Edit metadata) modal suite would need the corresponding getByTestId / queryByTestId updates.

🤖 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 433 - 436, Update the data-testid on the Edit Metadata menu item to
match the new label by changing `data-testid={'properties-action-${row.name}'}`
to something like `edit-metadata-action-${row.name}` in ObjectsTableView (where
setEditMetadataTarget is called) and then update any tests (e.g.,
ObjectsTableView.test.tsx "Properties (Edit metadata) modal" suite) to use the
new `getByTestId`/`queryByTestId` selector.
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectToastNotifications.tsx (1)

257-289: Consider tightening toast titles for clarity.

The operation updates metadata/properties only; "Object Updated" / "Failed to Update Object" can be read as "the object's content was written". Titles like "Object Metadata Updated" and "Failed to Update Object Metadata" match the description text ("Properties of ... were successfully updated") and disambiguate from future upload/replace flows.

✏️ Proposed wording tweak
-      title={<Trans>Object Updated</Trans>}
+      title={<Trans>Object Metadata Updated</Trans>}
       description={<Trans>Properties of "{objectName}" were successfully updated.</Trans>}
-      title={<Trans>Failed to Update Object</Trans>}
+      title={<Trans>Failed to Update Object Metadata</Trans>}

Tests asserting "Object Updated" / "Failed to Update Object" strings would need to follow.

🤖 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 257 - 289, The toast titles in getObjectMetadataUpdatedToast and
getObjectMetadataUpdateErrorToast are ambiguous; update the NotificationText
title props (within the Trans wrappers) to read "Object Metadata Updated" and
"Failed to Update Object Metadata" respectively so they match the description
about properties/metadata, and update any tests that assert the old strings.
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/EditObjectMetadataModal.tsx (2)

232-251: Inconsistent trimming between add-new and edit-save paths.

handleAddNew stores newKey.trim() (line 284), but handleSaveEdit stores entry.key as-is — so an edited key can silently contain trailing whitespace that then becomes part of the X-Object-Meta-* header name on submit. Apply the same .trim() (and ideally the same normalization for value) in both places, or move trimming into validateMetaKey / a single normalization helper.

Also applies to: 272-289

🤖 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/EditObjectMetadataModal.tsx
around lines 232 - 251, The edit path is not normalizing/triming metadata like
the add-new path, so handleSaveEdit should trim and normalize the key and value
before storing; update handleSaveEdit (and similarly handleAddNew if needed) to
call .trim() on entry.key (and apply the same normalization used in handleAddNew
for entry.value) or better yet move the normalization into a single helper
(e.g., normalizeMetaEntry or inside validateMetaKey) and call that from
handleSaveEdit, handleAddNew, and any submit paths so all stored metadata
keys/values are consistently trimmed and normalized.

406-411: Nit: placeholder and helptext use different example timestamps.

Line 406 uses "2026-05-16 18:14:57", line 409 uses "2026-05-16 18:05:52". Worth harmonizing so translators don't end up with two near-duplicate catalog entries and users don't see the value mysteriously change after they start typing.

🤖 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/EditObjectMetadataModal.tsx
around lines 406 - 411, The placeholder and helptext example timestamps in the
EditObjectMetadataModal component are inconsistent; make them the same to avoid
duplicate translations and confusing UX by using a single example timestamp
string for both the placeholder prop and the conditional helptext (the t`...`
template literals that reference the placeholder and helptext, using the
expiresAt variable to choose whether to show helptext).
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/EditObjectMetadataModal.test.tsx (1)

644-652: Assert the actual deleteAt value, not just its presence.

expect.any(Number) will pass even if the conversion is off by an entire timezone offset (which is exactly the bug flagged in the component). Lock in the exact expected Unix seconds for "2026-05-16 18:14:57" interpreted as UTC so this test acts as a regression guard for the submit path.

🧪 Suggested tightening
-await user.type(screen.getByLabelText(/Expires at/i), "2026-05-16 18:14:57")
-await user.click(getModalUpdateButton())
-expect(mockMutate).toHaveBeenCalledWith(expect.objectContaining({ deleteAt: expect.any(Number) }))
+await user.type(screen.getByLabelText(/Expires at/i), "2026-05-16 18:14:57")
+await user.click(getModalUpdateButton())
+// 2026-05-16T18:14:57Z → 1779473697
+expect(mockMutate).toHaveBeenCalledWith(
+  expect.objectContaining({ deleteAt: Date.UTC(2026, 4, 16, 18, 14, 57) / 1000 })
+)
🤖 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/EditObjectMetadataModal.test.tsx
around lines 644 - 652, The test "calls mutate with deleteAt when expires at is
set" currently asserts mockMutate was called with expect.any(Number); change
that to assert the exact Unix seconds for "2026-05-16 18:14:57" interpreted as
UTC (use mockMutate expectation containing deleteAt: 1778955297) so the test
validates the precise conversion path in the submit logic (locate the assertion
in the test file around the mockMutate call).
🤖 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/EditObjectMetadataModal.test.tsx:
- Around line 310-325: The test "shows invalid state after debounce when format
is wrong" only asserts the input value; update it to assert the actual invalid
UI state after the debounce by waiting for the input to reflect invalid (e.g.,
expect(screen.getByLabelText(/Expires at/i)).toHaveAttribute('aria-invalid',
'true') or expect an error icon/class applied by the TextInput component). Keep
the same user.type and waitFor usage but replace or add the final assertion to
check the invalid indicator (aria-invalid, error icon selector, or the CSS class
the `invalid` prop produces) so the test fails if validation/debounce stops
working.
- Around line 301-308: The test only validates a date shape and masks a timezone
bug in EditObjectMetadataModal; update the test in
EditObjectMetadataModal.test.tsx to assert the exact UTC-formatted string for
Unix 1747400000 (compute expected via new Date(1747400000 * 1000).toISOString()
and convert to the same "YYYY-MM-DD HH:mm:ss" format the component should
render) instead of using a regex, and if necessary modify
EditObjectMetadataModal (the formatting logic around deleteAt) to ensure it
formats dates in UTC before rendering so the test is deterministic across
timezones.

In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/EditObjectMetadataModal.tsx:
- Line 333: The Trans components in EditObjectMetadataModal are using member
expressions (metaError?.message and updateMutation.error.message) which breaks
Lingui extraction; fix by computing plain local variables (e.g., const
metaErrorMessage = metaError?.message ?? "" and const updateErrorMessage =
updateMutation?.error?.message ?? "") above the JSX and then use those bare
identifiers inside <Trans> (replace {metaError?.message ?? ""} and
{updateMutation.error.message} with {metaErrorMessage} and
{updateErrorMessage}). Ensure the computed names are declared in the same scope
as the JSX so the Trans components reference simple variables.
- Around line 199-202: The metadataRecord construction silently overwrites
duplicate keys (metadata -> metadataRecord), so add duplicate detection before
building the payload: scan the metadata array for duplicate entry.key values
(e.g., in the component state update/submit handler inside
EditObjectMetadataModal) and set a validation error tied to validateMetaKey or
the component's saveEnabled flag; surface the error inline for the offending
key(s) and prevent Save (disable the Save button in the save handler/Save button
binding) until duplicates are resolved so the payload never silently drops
values.
- Around line 44-54: TIMESTAMP_RE only validates format but allows semantically
invalid dates; update the validation flow that uses TIMESTAMP_RE to parse the
string with new Date(...) and then reject the value if parsed.getTime() is NaN
or if the parsed date components (year, month, day, hour, minute, second) don’t
round-trip back to the original string (i.e., reconstructing from the Date
should equal the matched parts). Reference TIMESTAMP_RE and perform this extra
check immediately after regex match/parsing so invalid inputs like "2026-13-45
25:99:99" are rejected.
- Around line 65-70: The displayed "Expires at (UTC)" value is incorrectly
generated from local-time getters in formatUnixToTimestamp; update the function
(formatUnixToTimestamp) to use UTC getters (getUTCFullYear, getUTCMonth,
getUTCDate, getUTCHours, getUTCMinutes, getUTCSeconds) so the formatted
"YYYY-MM-DD HH:MM:SS" is in UTC; keep the existing pad helper and string format
unchanged so the UI shows a true UTC timestamp that will round-trip correctly
with the corresponding parser.
- Around line 204-210: The submit handler uses new Date(expiresAt.trim()) which
parses the timestamp as local time; change the deleteAt computation in the
updateMutation.mutate call so it parses expiresAt as UTC (matching
formatUnixToTimestamp) by extracting year/month/day/hour/minute/second via regex
and computing Math.floor(Date.UTC(year, month-1, day, hour, minute, second) /
1000) when expiresAt.trim() is present; keep the conditional spread logic intact
and ensure the new UTC parser is used wherever deleteAt is computed so the
round-trip with formatUnixToTimestamp is stable.

---

Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/EditObjectMetadataModal.test.tsx:
- Around line 644-652: The test "calls mutate with deleteAt when expires at is
set" currently asserts mockMutate was called with expect.any(Number); change
that to assert the exact Unix seconds for "2026-05-16 18:14:57" interpreted as
UTC (use mockMutate expectation containing deleteAt: 1778955297) so the test
validates the precise conversion path in the submit logic (locate the assertion
in the test file around the mockMutate call).

In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/EditObjectMetadataModal.tsx:
- Around line 232-251: The edit path is not normalizing/triming metadata like
the add-new path, so handleSaveEdit should trim and normalize the key and value
before storing; update handleSaveEdit (and similarly handleAddNew if needed) to
call .trim() on entry.key (and apply the same normalization used in handleAddNew
for entry.value) or better yet move the normalization into a single helper
(e.g., normalizeMetaEntry or inside validateMetaKey) and call that from
handleSaveEdit, handleAddNew, and any submit paths so all stored metadata
keys/values are consistently trimmed and normalized.
- Around line 406-411: The placeholder and helptext example timestamps in the
EditObjectMetadataModal component are inconsistent; make them the same to avoid
duplicate translations and confusing UX by using a single example timestamp
string for both the placeholder prop and the conditional helptext (the t`...`
template literals that reference the placeholder and helptext, using the
expiresAt variable to choose whether to show helptext).

In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.tsx:
- Around line 433-436: Update the data-testid on the Edit Metadata menu item to
match the new label by changing `data-testid={'properties-action-${row.name}'}`
to something like `edit-metadata-action-${row.name}` in ObjectsTableView (where
setEditMetadataTarget is called) and then update any tests (e.g.,
ObjectsTableView.test.tsx "Properties (Edit metadata) modal" suite) to use the
new `getByTestId`/`queryByTestId` selector.

In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectToastNotifications.tsx:
- Around line 257-289: The toast titles in getObjectMetadataUpdatedToast and
getObjectMetadataUpdateErrorToast are ambiguous; update the NotificationText
title props (within the Trans wrappers) to read "Object Metadata Updated" and
"Failed to Update Object Metadata" respectively so they match the description
about properties/metadata, and update any tests that assert the old strings.
🪄 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: 7e144c36-ccf2-41d1-845a-cdc18ebd46ce

📥 Commits

Reviewing files that changed from the base of the PR and between 9392d6b and 16293c8.

📒 Files selected for processing (10)
  • apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/EditContainerMetadataModal.test.tsx
  • apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/EditContainerMetadataModal.tsx
  • apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/EditObjectMetadataModal.test.tsx
  • apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/EditObjectMetadataModal.tsx
  • apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectToastNotifications.test.tsx
  • apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectToastNotifications.tsx
  • apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/ObjectsTableView.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/-components/Swift/Objects/index.test.tsx
  • apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx

Comment on lines +301 to +308
test("field is populated with formatted timestamp when deleteAt is set", async () => {
// Unix 1747400000 → "2025-05-16 ..."
mockObjectMetadata = makeObjectMetadata({ deleteAt: 1747400000 })
renderModal()
await flushEffects()
const input = screen.getByLabelText(/Expires at/i)
expect(input.getAttribute("value")).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/)
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

This assertion only checks shape, masking a timezone bug.

The test comments "Unix 1747400000 → '2025-05-16 ...'" but only asserts the regex pattern, so the test passes regardless of the machine's timezone — which is what lets the local-vs-UTC mismatch flagged on EditObjectMetadataModal.tsx (lines 66-70, 204-210) slip through. Lock down the exact expected UTC string so any future timezone regression fails here:

🧪 Suggested tightening
-mockObjectMetadata = makeObjectMetadata({ deleteAt: 1747400000 })
+mockObjectMetadata = makeObjectMetadata({ deleteAt: 1747400000 })
 renderModal()
 await flushEffects()
 const input = screen.getByLabelText(/Expires at/i)
-expect(input.getAttribute("value")).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/)
+// 1747400000 in UTC
+expect(input).toHaveValue("2025-05-16 13:13:20")

(Confirm the exact expected minute/second from new Date(1747400000 * 1000).toISOString() after the component is fixed to format in UTC.)

📝 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("field is populated with formatted timestamp when deleteAt is set", async () => {
// Unix 1747400000 → "2025-05-16 ..."
mockObjectMetadata = makeObjectMetadata({ deleteAt: 1747400000 })
renderModal()
await flushEffects()
const input = screen.getByLabelText(/Expires at/i)
expect(input.getAttribute("value")).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/)
})
test("field is populated with formatted timestamp when deleteAt is set", async () => {
// Unix 1747400000 → "2025-05-16 ..."
mockObjectMetadata = makeObjectMetadata({ deleteAt: 1747400000 })
renderModal()
await flushEffects()
const input = screen.getByLabelText(/Expires at/i)
// 1747400000 in UTC
expect(input).toHaveValue("2025-05-16 13:13:20")
})
🤖 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/EditObjectMetadataModal.test.tsx
around lines 301 - 308, The test only validates a date shape and masks a
timezone bug in EditObjectMetadataModal; update the test in
EditObjectMetadataModal.test.tsx to assert the exact UTC-formatted string for
Unix 1747400000 (compute expected via new Date(1747400000 * 1000).toISOString()
and convert to the same "YYYY-MM-DD HH:mm:ss" format the component should
render) instead of using a regex, and if necessary modify
EditObjectMetadataModal (the formatting logic around deleteAt) to ensure it
formats dates in UTC before rendering so the test is deterministic across
timezones.

Comment on lines +310 to +325
test("shows invalid state after debounce when format is wrong", async () => {
mockObjectMetadata = makeObjectMetadata()
const user = userEvent.setup()
renderModal()
await flushEffects()
// Type an invalid value and wait longer than the 600ms debounce
await user.type(screen.getByLabelText(/Expires at/i), "not-a-date")
// The invalid prop is set by the debounce — verify the input exists and has a value
await waitFor(
() => {
const input = screen.getByLabelText(/Expires at/i)
expect(input).toHaveValue("not-a-date")
},
{ timeout: 2000 }
)
}, 10000)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Test does not actually assert the invalid state it claims to cover.

The test is named "shows invalid state after debounce when format is wrong", but the only assertion is that the input retains the typed value — that would pass even if debounced validation never ran. Assert on the UI reflecting invalidity (e.g., aria-invalid, the error icon, or a class the invalid prop applies) so a regression in the debounce/validation path would actually fail this test.

🧪 Suggested tightening
 await user.type(screen.getByLabelText(/Expires at/i), "not-a-date")
-// The invalid prop is set by the debounce — verify the input exists and has a value
 await waitFor(
   () => {
     const input = screen.getByLabelText(/Expires at/i)
     expect(input).toHaveValue("not-a-date")
+    expect(input).toHaveAttribute("aria-invalid", "true")
   },
   { timeout: 2000 }
 )

Adjust the attribute to whatever invalid surfaces in the juno TextInput DOM.

📝 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 invalid state after debounce when format is wrong", async () => {
mockObjectMetadata = makeObjectMetadata()
const user = userEvent.setup()
renderModal()
await flushEffects()
// Type an invalid value and wait longer than the 600ms debounce
await user.type(screen.getByLabelText(/Expires at/i), "not-a-date")
// The invalid prop is set by the debounce — verify the input exists and has a value
await waitFor(
() => {
const input = screen.getByLabelText(/Expires at/i)
expect(input).toHaveValue("not-a-date")
},
{ timeout: 2000 }
)
}, 10000)
test("shows invalid state after debounce when format is wrong", async () => {
mockObjectMetadata = makeObjectMetadata()
const user = userEvent.setup()
renderModal()
await flushEffects()
// Type an invalid value and wait longer than the 600ms debounce
await user.type(screen.getByLabelText(/Expires at/i), "not-a-date")
await waitFor(
() => {
const input = screen.getByLabelText(/Expires at/i)
expect(input).toHaveValue("not-a-date")
expect(input).toHaveAttribute("aria-invalid", "true")
},
{ timeout: 2000 }
)
}, 10000)
🤖 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/EditObjectMetadataModal.test.tsx
around lines 310 - 325, The test "shows invalid state after debounce when format
is wrong" only asserts the input value; update it to assert the actual invalid
UI state after the debounce by waiting for the input to reflect invalid (e.g.,
expect(screen.getByLabelText(/Expires at/i)).toHaveAttribute('aria-invalid',
'true') or expect an error icon/class applied by the TextInput component). Keep
the same user.type and waitFor usage but replace or add the final assertion to
check the invalid indicator (aria-invalid, error icon selector, or the CSS class
the `invalid` prop produces) so the test fails if validation/debounce stops
working.

Comment on lines +44 to +54
// Expected format: "YYYY-MM-DD HH:MM:SS"
const TIMESTAMP_RE = /^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/

type MetaKeyError = "required" | "invalid-chars" | "no-alnum"

const validateMetaKey = (key: string): MetaKeyError | null => {
if (!key.trim()) return "required"
if (!VALID_KEY_RE.test(key)) return "invalid-chars"
if (!/[a-zA-Z0-9]/.test(key)) return "no-alnum"
return null
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

TIMESTAMP_RE accepts semantically invalid dates.

The regex only checks shape, so values like 2026-13-45 25:99:99 or 2026-02-30 10:00:00 pass validation and then silently roll over through new Date(...) (e.g., Feb 30 → Mar 2). Consider re-validating after parsing — reject when isNaN(parsed.getTime()) or when the parts don't round-trip back to the input.

🤖 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/EditObjectMetadataModal.tsx
around lines 44 - 54, TIMESTAMP_RE only validates format but allows semantically
invalid dates; update the validation flow that uses TIMESTAMP_RE to parse the
string with new Date(...) and then reject the value if parsed.getTime() is NaN
or if the parsed date components (year, month, day, hour, minute, second) don’t
round-trip back to the original string (i.e., reconstructing from the Date
should equal the matched parts). Reference TIMESTAMP_RE and perform this extra
check immediately after regex match/parsing so invalid inputs like "2026-13-45
25:99:99" are rejected.

Comment on lines +65 to +70
// Converts a Unix timestamp (seconds) to the "YYYY-MM-DD HH:MM:SS" field format
const formatUnixToTimestamp = (unix: number): string => {
const d = new Date(unix * 1000)
const pad = (n: number) => String(n).padStart(2, "0")
return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}`
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Bug: "UTC" label shown but local-time conversion used.

The Expires at (UTC) input claims UTC, but formatUnixToTimestamp formats using local-time getters (getFullYear, getMonth, getDate, getHours, getMinutes, getSeconds). The value a user sees for a given deleteAt differs by their timezone, so two users looking at the same object see different timestamps, and a user typing back what they see will round-trip to a different Unix time. This pairs with the submit bug at line 208 where new Date("YYYY-MM-DD HH:MM:SS") is also parsed as local time (see separate comment).

🐛 Proposed fix — use UTC getters
-// Converts a Unix timestamp (seconds) to the "YYYY-MM-DD HH:MM:SS" field format
+// Converts a Unix timestamp (seconds) to the "YYYY-MM-DD HH:MM:SS" field format in UTC
 const formatUnixToTimestamp = (unix: number): string => {
   const d = new Date(unix * 1000)
   const pad = (n: number) => String(n).padStart(2, "0")
-  return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}`
+  return `${d.getUTCFullYear()}-${pad(d.getUTCMonth() + 1)}-${pad(d.getUTCDate())} ${pad(d.getUTCHours())}:${pad(d.getUTCMinutes())}:${pad(d.getUTCSeconds())}`
 }
📝 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
// Converts a Unix timestamp (seconds) to the "YYYY-MM-DD HH:MM:SS" field format
const formatUnixToTimestamp = (unix: number): string => {
const d = new Date(unix * 1000)
const pad = (n: number) => String(n).padStart(2, "0")
return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}`
}
// Converts a Unix timestamp (seconds) to the "YYYY-MM-DD HH:MM:SS" field format in UTC
const formatUnixToTimestamp = (unix: number): string => {
const d = new Date(unix * 1000)
const pad = (n: number) => String(n).padStart(2, "0")
return `${d.getUTCFullYear()}-${pad(d.getUTCMonth() + 1)}-${pad(d.getUTCDate())} ${pad(d.getUTCHours())}:${pad(d.getUTCMinutes())}:${pad(d.getUTCSeconds())}`
}
🤖 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/EditObjectMetadataModal.tsx
around lines 65 - 70, The displayed "Expires at (UTC)" value is incorrectly
generated from local-time getters in formatUnixToTimestamp; update the function
(formatUnixToTimestamp) to use UTC getters (getUTCFullYear, getUTCMonth,
getUTCDate, getUTCHours, getUTCMinutes, getUTCSeconds) so the formatted
"YYYY-MM-DD HH:MM:SS" is in UTC; keep the existing pad helper and string format
unchanged so the UI shows a true UTC timestamp that will round-trip correctly
with the corresponding parser.

Comment on lines +199 to +202
const metadataRecord: Record<string, string> = {}
for (const entry of metadata) {
metadataRecord[entry.key] = entry.value
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Silent duplicate-key collisions in the submitted payload.

Building metadataRecord from the list discards any duplicates without warning. Two existing entries with the same key after an edit (or an added key that matches an existing one) will send only the last value to Swift with no user feedback. Consider either (a) detecting duplicates in validateMetaKey/save handlers and surfacing an inline error, or (b) at minimum disabling Save when a duplicate key is entered.

🤖 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/EditObjectMetadataModal.tsx
around lines 199 - 202, The metadataRecord construction silently overwrites
duplicate keys (metadata -> metadataRecord), so add duplicate detection before
building the payload: scan the metadata array for duplicate entry.key values
(e.g., in the component state update/submit handler inside
EditObjectMetadataModal) and set a validation error tied to validateMetaKey or
the component's saveEnabled flag; surface the error inline for the offending
key(s) and prevent Save (disable the Save button in the save handler/Save button
binding) until duplicates are resolved so the payload never silently drops
values.

Comment on lines +204 to +210
updateMutation.mutate({
container: containerName,
object: object.name,
metadata: metadataRecord,
...(expiresAt.trim() ? { deleteAt: Math.floor(new Date(expiresAt.trim()).getTime() / 1000) } : {}),
})
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Bug: submit parses the "UTC" input as local time.

new Date("YYYY-MM-DD HH:MM:SS") (no timezone suffix) is parsed as local time in every browser that follows ES2015+, so a user in e.g. UTC+2 entering 2026-05-16 18:14:57 in the field labeled UTC sends a deleteAt that is 2 hours too early. Combined with the local-time formatter at line 69, the field is effectively "local time" mislabeled as UTC.

🐛 Proposed fix — parse as UTC
-      ...(expiresAt.trim() ? { deleteAt: Math.floor(new Date(expiresAt.trim()).getTime() / 1000) } : {}),
+      ...(expiresAt.trim()
+        ? { deleteAt: Math.floor(Date.parse(expiresAt.trim().replace(" ", "T") + "Z") / 1000) }
+        : {}),

Or build it explicitly via Date.UTC(y, m-1, d, hh, mm, ss) after parsing the regex groups. Either way, the parser must match formatUnixToTimestamp's direction so the round-trip is stable.

📝 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
updateMutation.mutate({
container: containerName,
object: object.name,
metadata: metadataRecord,
...(expiresAt.trim() ? { deleteAt: Math.floor(new Date(expiresAt.trim()).getTime() / 1000) } : {}),
})
}
updateMutation.mutate({
container: containerName,
object: object.name,
metadata: metadataRecord,
...(expiresAt.trim()
? { deleteAt: Math.floor(Date.parse(expiresAt.trim().replace(" ", "T") + "Z") / 1000) }
: {}),
})
}
🤖 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/EditObjectMetadataModal.tsx
around lines 204 - 210, The submit handler uses new Date(expiresAt.trim()) which
parses the timestamp as local time; change the deleteAt computation in the
updateMutation.mutate call so it parses expiresAt as UTC (matching
formatUnixToTimestamp) by extracting year/month/day/hour/minute/second via regex
and computing Math.floor(Date.UTC(year, month-1, day, hour, minute, second) /
1000) when expiresAt.trim() is present; keep the conditional spread logic intact
and ensure the new UTC parser is used wherever deleteAt is computed so the
round-trip with formatUnixToTimestamp is stable.

</Stack>
) : isMetaError ? (
<Message variant="danger">
<Trans>Failed to load object metadata: {metaError?.message ?? ""}</Trans>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Lingui lint: use plain variables inside <Trans>, not member expressions.

Static analysis is flagging ${metaError?.message ?? ""} and ${updateMutation.error.message} inside <Trans>. The extractor expects bare identifiers so the catalog keys stay stable and interpolation is unambiguous. Bind to locals first:

🛠️ Proposed fix
-    <Message variant="danger">
-      <Trans>Failed to load object metadata: {metaError?.message ?? ""}</Trans>
-    </Message>
+    {(() => {
+      const message = metaError?.message ?? ""
+      return (
+        <Message variant="danger">
+          <Trans>Failed to load object metadata: {message}</Trans>
+        </Message>
+      )
+    })()}
-  {updateMutation.isError && (
-    <Message variant="danger">
-      <Trans>Failed to update object: {updateMutation.error.message}</Trans>
-    </Message>
-  )}
+  {updateMutation.isError && (() => {
+    const message = updateMutation.error.message
+    return (
+      <Message variant="danger">
+        <Trans>Failed to update object: {message}</Trans>
+      </Message>
+    )
+  })()}

(Or compute the two messages above the JSX and reference them.)

Also applies to: 576-576

🧰 Tools
🪛 GitHub Check: lint

[warning] 333-333:
Should be ${variable}, not ${object.property} or ${myFunction()}

🤖 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/EditObjectMetadataModal.tsx
at line 333, The Trans components in EditObjectMetadataModal are using member
expressions (metaError?.message and updateMutation.error.message) which breaks
Lingui extraction; fix by computing plain local variables (e.g., const
metaErrorMessage = metaError?.message ?? "" and const updateErrorMessage =
updateMutation?.error?.message ?? "") above the JSX and then use those bare
identifiers inside <Trans> (replace {metaError?.message ?? ""} and
{updateMutation.error.message} with {metaErrorMessage} and
{updateErrorMessage}). Ensure the computed names are declared in the same scope
as the JSX so the Trans components reference simple variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

object-storage-ui: ObjectDetails - Object information and metadata management

1 participant