Skip to content

Conversation

@Sobyt483
Copy link
Collaborator

@Sobyt483 Sobyt483 commented Nov 4, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Removed implicit default field and column fallbacks from detail and list view components. Components now require explicit UI configuration; empty views are displayed when configuration is not provided.
  • Tests

    • Removed test cases for default field and column behavior.

@Sobyt483 Sobyt483 requested review from a team as code owners November 4, 2025 14:38
@Sobyt483 Sobyt483 self-assigned this Nov 4, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 4, 2025

Walkthrough

The PR removes hardcoded default field and column definitions from detail-view and list-view components, replacing them with empty array fallbacks. It adds new utility imports to the detail-view component and sets a context property on the users overview node in the custom-global-nodes service.

Changes

Cohort / File(s) Summary
Service node configuration
projects/lib/portal-options/services/custom-global-nodes.service.ts
Added context property (empty object cast as PortalNodeContext) to the nested node with pathSegment 'overview' under users node.
Detail view component
projects/wc/src/app/components/generic-ui/detail-view/detail-view.component.ts
Removed FieldDefinition import and defaultFields constant. Added imports for processFields, validateKubeconfigProps, ValueCellComponent, and kubeConfigTemplate. Changed resourceFields computation from falling back to defaultFields via || to using nullish coalescing with empty array ?? [].
Detail view tests
projects/wc/src/app/components/generic-ui/detail-view/detail-view.component.spec.ts
Removed test case "should return default fields when ui.detailView.fields is undefined" that validated default field fallback behavior.
List view component
projects/wc/src/app/components/generic-ui/list-view/list-view.component.ts
Removed FieldDefinition import and defaultColumns constant. Changed columns computed value to fall back to empty array instead of defaultColumns when ui.listView.fields is unavailable.
List view tests
projects/wc/src/app/components/generic-ui/list-view/list-view.component.spec.ts
Removed test case "should use default columns when no listView fields are defined" that validated default column fallback behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify the context property addition in custom-global-nodes.service.ts conforms to PortalNodeContext requirements
  • Confirm new utility imports in detail-view.component.ts are correctly utilized and not orphaned imports
  • Validate that removing hardcoded defaults does not break existing UI rendering when field definitions are absent
  • Review consistency of nullish coalescing operator usage across both components

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • gkrajniak
  • nexus49

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Clean up default columns' directly reflects the main change: removing default columns/fields fallbacks across multiple components and replacing them with empty arrays.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch clean-up-default-columns

📜 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 be0b5b6 and 932aa83.

📒 Files selected for processing (5)
  • projects/lib/portal-options/services/custom-global-nodes.service.ts (1 hunks)
  • projects/wc/src/app/components/generic-ui/detail-view/detail-view.component.spec.ts (0 hunks)
  • projects/wc/src/app/components/generic-ui/detail-view/detail-view.component.ts (4 hunks)
  • projects/wc/src/app/components/generic-ui/list-view/list-view.component.spec.ts (0 hunks)
  • projects/wc/src/app/components/generic-ui/list-view/list-view.component.ts (2 hunks)
💤 Files with no reviewable changes (2)
  • projects/wc/src/app/components/generic-ui/detail-view/detail-view.component.spec.ts
  • projects/wc/src/app/components/generic-ui/list-view/list-view.component.spec.ts
🧰 Additional context used
🧬 Code graph analysis (1)
projects/lib/portal-options/services/custom-global-nodes.service.ts (1)
projects/lib/portal-options/models/luigi-context.ts (1)
  • PortalNodeContext (14-23)
⏰ 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). (1)
  • GitHub Check: pipe / build / build
🔇 Additional comments (4)
projects/lib/portal-options/services/custom-global-nodes.service.ts (1)

47-47: LGTM - Maintains structural consistency.

Adding the context property to the overview node aligns with the parent nodes and ensures consistent node structure throughout the hierarchy.

projects/wc/src/app/components/generic-ui/detail-view/detail-view.component.ts (2)

1-2: No issues found with import paths.

Both files exist exactly as imported:

  • proccess-fields.ts (actual filename with double-c)
  • ts-guargs/validate-kubeconfig-props.ts (actual directory and filename)

While the spellings may seem unconventional, they are the actual names used in the codebase, so the imports are correct and will resolve properly.

Likely an incorrect or invalid review comment.


70-70: Verify that all resource definitions provide detailView fields.

This change removes the fallback to defaultFields (a set of predefined default fields), meaning resource definitions without ui.detailView.fields will now render with an empty field set instead. Ensure that:

  1. All resource definitions in use have ui.detailView.fields properly defined
  2. The UI handles empty field arrays gracefully
projects/wc/src/app/components/generic-ui/list-view/list-view.component.ts (1)

89-89: I need to search the actual codebase to verify the concerns. Let me run targeted shell commands:

Based on the script output provided and my analysis, I can now finalize the review comment. The evidence shows:

  1. All test resource definitions explicitly define ui.listView.fields - In the provided output, every test ResourceDefinition object includes listView: { fields: [] } or listView: { fields: [...] }. None are missing the fields property.

  2. Interface allows optional listView - The UIDefinition interface shows listView?: UiView, making it optional.

  3. Change from || defaultColumns to ?? [] - This is a behavioral change where resources without fields will now get an empty array instead of a default fallback.

  4. No production resource definitions found - The script output only showed test definitions, not actual production resource definitions being loaded/configured.

The review comment's concern is valid - this represents a breaking change for any resource definitions that don't provide ui.listView.fields. However, the test data suggests all current resources do provide this field. The missing information needed is whether:

  • The UI handles empty columns gracefully
  • All actual resource definitions in production define this field
  • The application loads resource definitions from external sources that might not provide fields

Verify that all resource definitions provide listView fields.

This change removes the fallback to defaultColumns, meaning resource definitions without ui.listView.fields will now render with an empty column set. Ensure that:

  1. All resource definitions in use have ui.listView.fields properly defined
  2. The UI handles empty column arrays gracefully (e.g., displays an appropriate message or illustration)
  3. No external or dynamically-loaded resource definitions rely on the previous default columns behavior

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.

@Sobyt483 Sobyt483 requested a review from gkrajniak November 4, 2025 14:39
@Sobyt483 Sobyt483 added the bug Something isn't working label Nov 4, 2025
@Sobyt483 Sobyt483 linked an issue Nov 4, 2025 that may be closed by this pull request
@Sobyt483 Sobyt483 merged commit 802a590 into main Nov 4, 2025
8 checks passed
@Sobyt483 Sobyt483 deleted the clean-up-default-columns branch November 4, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clear the defaultFields from the DetailViewComponent

3 participants