fix(metadata-view): show error state when exception is thrown

[greg-in-a-box]

Summary by CodeRabbit

• Bug Fixes

  • Error-state handling for Metadata View V2 now shows the empty state without a loading indicator, improving clarity; non-V2 and other views unchanged.

• UI

  • Increased minimum width for the primary name column in metadata tables to improve readability.

• Tests

  • Added/updated tests to verify EmptyView behavior in V2 error and non-error views.

[coderabbitai]

Walkthrough

I pity the fool who misses this: when contentExplorer.metadataViewV2 is enabled and the view is VIEW_ERROR, Content.tsx now renders EmptyView with isLoading=false; other behaviors remain unchanged. Tests and a column width tweak were also added.

Changes

Cohort / File(s)	Summary of Changes
V2 error-state rendering src/elements/content-explorer/Content.tsx	Added conditional branch: if isMetadataViewV2Feature AND view === VIEW_ERROR, render EmptyView with isLoading=false. Preserves prior non-V2 behavior.
Tests for V2 error-state src/elements/content-explorer/__tests__/Content.test.tsx	Renamed suite; added tests verifying EmptyView renders for VIEW_ERROR with V2 (no loading message) and does not render for non-error views.
Column layout tweak src/elements/content-explorer/MetadataViewContainer.tsx	Increased FIELD_ITEM_NAME column minWidth from 250 to 300; removed prior max/min 250 constraint.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Content
  participant Flag as metadataViewV2
  participant View as CurrentView
  participant Empty as EmptyView

  User->>Content: open content explorer
  Content->>Flag: check isMetadataViewV2Feature
  Content->>View: read current view
  alt Flag enabled AND View == VIEW_ERROR
    Note over Content,Empty: New branch — V2 error handling
    Content->>Empty: render EmptyView (isLoading=false)
  else Other conditions
    Content-->>User: render existing UI paths (unchanged)
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• feat(metadata-view): Added Metadata V2 button #4174 — touches Content.tsx feature-flag logic; overlaps gating/branch decisions.
• feat(metadata-view): Add MetadataView V2 #4191 — modifies Content.tsx rendering for metadataViewV2 and error/empty states.

Suggested labels

ready-to-merge

Suggested reviewers

• jpan-box
• tjuanitas

Poem

I pity the fool who fears the blink and spin,
V2 says "stand down" — no loader in the din.
Error shows still, but calm and clear,
Tests stand guard so no bug sneaks near.
Ship it, champ — the UI's sincere. 🏁

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	I pity the fool who thinks this PR description is sufficient: it only shows an image and copy-pastes merge instructions without any summary of the changes, motivation, or test plan required by the template. This omission leaves reviewers guessing about the purpose and verification steps for the metadata-view error state update. The description needs to be fleshed out with context, change details, and testing instructions.	Expand the description to include a concise summary of the fix, explain why the error state update is necessary, and provide steps to test and verify the changes using the repository's standard sections for motivation, changes, and testing so no fool is left guessing. Also attach any relevant screenshots or logs to illustrate the new error state behavior. Use the description template comments as a guide to structure the new content.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	I pity the fool who tries to misinterpret this title: it clearly calls out fixing the metadata view to show an error state when an exception occurs, matching the core change in Content.tsx. It avoids extraneous mentions like column width tweaks which are peripheral. The phrasing is concise, conventional, and fully aligned with the primary intent.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4bc1d7c and 1d192be.

📒 Files selected for processing (3)

• src/elements/content-explorer/Content.tsx (1 hunks)
• src/elements/content-explorer/MetadataViewContainer.tsx (1 hunks)
• src/elements/content-explorer/__tests__/Content.test.tsx (2 hunks)

✅ Files skipped from review due to trivial changes (1)

• src/elements/content-explorer/MetadataViewContainer.tsx

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Summary
• GitHub Check: lint_test_build

🔇 Additional comments (2)

src/elements/content-explorer/Content.tsx (1)

82-84: Restore the EmptyView for other views, fool!

When metadataViewV2 is on, the !isMetadataViewV2Feature guard kills the classic EmptyView, so empty folders/recents (and any non-error empties) render a blank slab. That’s a straight-up regression. Scope the V2-only branch to metadata errors and keep the legacy EmptyView for everything else.

-            {!isMetadataViewV2Feature && isViewEmpty && <EmptyView view={view} isLoading={percentLoaded !== 100} />}
-            {isMetadataViewV2Feature && view === VIEW_ERROR && <EmptyView view={view} isLoading={false} />}
+            {isMetadataViewV2Feature && isMetadataBasedView && view === VIEW_ERROR && (
+                <EmptyView view={view} isLoading={false} />
+            )}
+            {(!isMetadataViewV2Feature || !isMetadataBasedView) && isViewEmpty && (
+                <EmptyView view={view} isLoading={percentLoaded !== 100} />
+            )}

src/elements/content-explorer/__tests__/Content.test.tsx (1)

127-150: Solid error-state assertions, fool!

I pity the fool who forgets to keep that loading copy out of the error screen—these checks lock it down tight. Keep it rollin’.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

src/elements/content-explorer/__tests__/Content.test.tsx (3)

93-126: Solid coverage for V2 metadata path, fool!

Good assertions that V2 disables the legacy MetadataBasedItemList and renders the new container. You could merge the two tests into one to cut redundancy, but it's fine as-is.

---

127-149: Brittle check for loading state — don’t trust copy strings, sucka.

Asserting absence of the string “Please wait while the items load...” couples the test to UI copy. Prefer verifying the EmptyView prop instead by mocking the component and capturing isLoading.

Example:

+jest.mock('../common/empty-view', () => ({
+  __esModule: true,
+  default: (props: any) => <div data-testid="empty-view-props">{JSON.stringify(props)}</div>,
+}));
...
- expect(screen.queryByText('Please wait while the items load...')).not.toBeInTheDocument();
+ const props = JSON.parse(screen.getByTestId('empty-view-props').textContent!);
+ expect(props.isLoading).toBe(false);

---

150-159: Test name lies — tighten it up, fool.

The test name says “does not render EmptyView,” but you only assert the error message isn’t present. Either assert on the component or rename and add a stronger check (e.g., ProgressBar present).

- test('does not render EmptyView when metadataViewV2 feature is enabled but view is not VIEW_ERROR', () => {
+ test('does not render the error EmptyView message when metadataViewV2 feature is enabled but view is not VIEW_ERROR', () => {
...
- expect(
-     screen.queryByText('A network error has occurred while trying to load.'),
- ).not.toBeInTheDocument();
+ expect(screen.queryByText('A network error has occurred while trying to load.')).not.toBeInTheDocument();
+ expect(screen.getByRole('progressbar')).toBeInTheDocument();

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 141cf6e and 8a75dd7.

📒 Files selected for processing (2)

• src/elements/content-explorer/Content.tsx (1 hunks)
• src/elements/content-explorer/__tests__/Content.test.tsx (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: lint_test_build
• GitHub Check: Summary

🔇 Additional comments (1)

src/elements/content-explorer/Content.tsx (1)

83-83: Nice: explicit V2 error-state EmptyView with isLoading=false.

This aligns the error UX for metadata view V2. Clean and targeted.

[mergify]

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

• ❌ lint_pull_request

You can check the last failing draft PR here: #4291.

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

[mergify]

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

• ❌ lint_pull_request

You can check the last failing draft PR here: #4292.

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.