Skip to content

Conversation

@Raubzeug
Copy link
Contributor

@Raubzeug Raubzeug commented Dec 12, 2025

Greptile Overview

Greptile Summary

Fixed parseNodeGroupsToPreparedVersions to always include a default color when version data is missing, preventing undefined color values in the UI.

  • Imported DEFAULT_COLOR constant from getVersionsColors
  • Added explicit color fallback: color: data?.color ?? DEFAULT_COLOR before spreading data
  • Ensures VersionsBar component always receives valid color values for rendering version indicators

Issue: The sibling function parseNodesToPreparedVersions (lines 23-34) has the same problem but was not fixed. Both functions return PreparedVersion objects consumed by VersionsBar, which expects the color property to be defined.

Confidence Score: 3/5

  • Safe to merge but incomplete - same issue exists in sibling function
  • The fix correctly addresses the color fallback issue for parseNodeGroupsToPreparedVersions, but the identical function parseNodesToPreparedVersions has the same bug and was not fixed. Both functions are used in production and should have consistent behavior.
  • Check parseNodesToPreparedVersions function in src/utils/versions/parseNodesToVersionsValues.ts:23-34

Important Files Changed

File Analysis

Filename Score Overview
src/utils/versions/parseNodesToVersionsValues.ts 3/5 Fixed default color for parseNodeGroupsToPreparedVersions, but parseNodesToPreparedVersions has same issue

Sequence Diagram

sequenceDiagram
    participant Versions as Versions Component
    participant Utils as useGetPreparedVersions
    participant Parser as parseNodeGroupsToPreparedVersions
    participant Colors as getVersionsColors
    participant VersionsBar as VersionsBar Component
    
    Versions->>Utils: Request prepared versions
    Utils->>Utils: Get versionsDataMap
    alt ClusterInfo V2
        Utils->>Parser: parseNodeGroupsToPreparedVersions(groups, versionsDataMap)
    else Nodes with tableGroups
        Utils->>Parser: parseNodeGroupsToPreparedVersions(tableGroups, versionsDataMap)
    else Nodes array
        Utils->>Parser: parseNodesToPreparedVersions(nodes, versionsDataMap)
    end
    
    Parser->>Colors: getMinorVersion(version)
    Colors-->>Parser: minorVersion
    Parser->>Parser: Get color from versionsDataMap
    Parser->>Parser: Apply DEFAULT_COLOR fallback (NEW)
    Parser-->>Utils: PreparedVersion[] with color
    Utils-->>Versions: preparedVersions
    Versions->>VersionsBar: Render with preparedVersions
    VersionsBar->>VersionsBar: Use item.color for styling
Loading

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total Passed Failed Flaky Skipped
384 380 0 2 2
Test Changes Summary ⏭️2

⏭️ Skipped Tests (2)

  1. Scroll to row, get shareable link, navigate to URL and verify row is scrolled into view (tenant/diagnostics/tabs/queries.test.ts)
  2. Copy result button copies to clipboard (tenant/queryEditor/queryEditor.test.ts)

Bundle Size: ✅

Current: 62.50 MB | Main: 62.50 MB
Diff: +0.17 KB (0.00%)

✅ Bundle size unchanged.

ℹ️ CI Information
  • Test recordings for failed tests are available in the full report.
  • Bundle size is measured for the entire 'dist' directory.
  • 📊 indicates links to detailed reports.
  • 🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

Copilot AI review requested due to automatic review settings December 12, 2025 08:30
@Raubzeug Raubzeug enabled auto-merge December 12, 2025 08:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where version colors could be undefined by ensuring a default color is always provided. The fix adds color: data?.color ?? DEFAULT_COLOR before spreading the data object, preventing the spread from overwriting the default with undefined.

Key Changes

  • Imported DEFAULT_COLOR constant from getVersionsColors
  • Added explicit color assignment with fallback to DEFAULT_COLOR in parseNodeGroupsToPreparedVersions

Comment on lines +52 to 53
color: data?.color ?? DEFAULT_COLOR,
...data,
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The fix applies the default color before spreading the data object to ensure color is always defined. However, the same issue exists in the parseNodesToPreparedVersions function above (lines 27-33). That function also spreads data without setting a default color first, which means it can have the same problem where color might be undefined. Consider applying the same fix pattern there for consistency.

Suggested change
color: data?.color ?? DEFAULT_COLOR,
...data,
...data,
color: data?.color ?? DEFAULT_COLOR,

Copilot uses AI. Check for mistakes.
@Raubzeug Raubzeug added this pull request to the merge queue Dec 12, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. src/utils/versions/parseNodesToVersionsValues.ts, line 23-34 (link)

    logic: parseNodesToPreparedVersions should also explicitly set default color like parseNodeGroupsToPreparedVersions does. Without this, versions without color data will have undefined color property.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Merged via the queue into main with commit f7594fb Dec 12, 2025
13 checks passed
@Raubzeug Raubzeug deleted the versions branch December 12, 2025 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants