Fix groupMode toggle

[ehconitin]

groupMode should be undefined if no groupby is set on the secondary axis!
This default value is also the reason -- all the logic -- where groupMode was checked for conditional rendering -- for eg
, negative data labels when there is no groupby -- the labels should appear below wasn't happening -- since there was always a default to groupMode!
changes -

• getting rid of the default value on the bar chart DTO for groupMode
• on front -- make sure groupMode is only set when the secondary axis gets groupBy id and proper cleanup

[greptile-apps]

Greptile Overview

Greptile Summary

This PR attempts to fix the groupMode toggle behavior for bar charts by centralizing the logic for managing the groupMode field when secondary axis fields are selected or cleared.

Key changes:

• Created buildBarChartGroupByConfigUpdate() utility to handle groupMode updates when selecting/clearing secondary axis fields
• Created getEffectiveGroupMode() utility to determine the rendered group mode based on whether a secondary axis field exists
• Removed default value for groupMode in the DTO to properly support undefined state
• Refactored ChartGroupByFieldSelectionDropdownContentBase to use the new utility function
• Simplified useUpdateChartSettingToggle by removing complex conditional logic

Critical issue found:
The new buildBarChartGroupByConfigUpdate() function always resets groupMode to STACKED when selecting a secondary axis field, discarding the user's previous GROUPED preference. This creates a poor UX where users lose their group mode setting whenever they change the secondary axis field.

Confidence Score: 2/5

• This PR has a critical logic bug that will cause user preferences to be lost
• While the PR improves code organization by extracting utilities and removing the default value from the DTO, it introduces a significant bug where the groupMode is always reset to STACKED when selecting a secondary axis field, ignoring any previous user preference for GROUPED mode
• Pay close attention to buildBarChartGroupByConfigUpdate.ts - the logic needs to preserve existing groupMode values

Important Files Changed

File Analysis

Filename	Score	Overview
packages/twenty-front/src/modules/command-menu/pages/page-layout/utils/buildBarChartGroupByConfigUpdate.ts	2/5	New utility function that builds config updates for secondary axis field selection. Critical logic issue: always resets groupMode to STACKED when selecting a field, ignoring user's previous GROUPED preference
packages/twenty-front/src/modules/command-menu/pages/page-layout/components/dropdown-content/ChartGroupByFieldSelectionDropdownContentBase.tsx	4/5	Refactored to use new buildBarChartGroupByConfigUpdate utility in three places. Changes are consistent but inherit the logic bug from the utility function
packages/twenty-server/src/engine/core-modules/page-layout/dtos/bar-chart-configuration.dto.ts	5/5	Removed defaultValue: BarChartGroupMode.STACKED from groupMode field, making it truly optional. This correctly allows undefined values

Sequence Diagram

sequenceDiagram
    participant User
    participant Dropdown as ChartGroupByFieldSelection
    participant Builder as buildBarChartGroupByConfigUpdate
    participant Hook as useUpdateChartSettingToggle
    participant Renderer as GraphWidgetBarChartRenderer
    participant Util as getEffectiveGroupMode

    Note over User,Util: Scenario 1: Selecting Secondary Axis Field
    User->>Dropdown: Select field for secondary axis
    Dropdown->>Builder: buildBarChartGroupByConfigUpdate(fieldId, null, config)
    Builder->>Builder: Check if fieldId is defined
    Builder->>Builder: Set groupMode = STACKED (always!)
    Builder-->>Dropdown: Return config with groupMode: STACKED
    Dropdown->>Renderer: Update widget configuration
    Renderer->>Util: getEffectiveGroupMode(STACKED, true)
    Util-->>Renderer: Return 'stacked'
    Renderer-->>User: Display stacked chart

    Note over User,Util: Scenario 2: Toggling Stacked/Grouped
    User->>Hook: Toggle STACKED_BARS setting
    Hook->>Hook: Read current groupMode (e.g., STACKED)
    Hook->>Hook: Switch to GROUPED
    Hook->>Renderer: Update config with groupMode: GROUPED
    Renderer->>Util: getEffectiveGroupMode(GROUPED, true)
    Util-->>Renderer: Return 'grouped'
    Renderer-->>User: Display grouped chart

    Note over User,Util: Scenario 3: Bug - Reselecting Field Loses Preference
    User->>Dropdown: Clear secondary axis field
    Dropdown->>Builder: buildBarChartGroupByConfigUpdate(null, null, config)
    Builder->>Builder: Set groupMode = undefined
    Builder-->>Dropdown: Return config with groupMode: undefined
    User->>Dropdown: Select new secondary axis field
    Dropdown->>Builder: buildBarChartGroupByConfigUpdate(newFieldId, null, config)
    Note right of Builder: BUG: Ignores existing<br/>config.groupMode (GROUPED)
    Builder->>Builder: Set groupMode = STACKED (hardcoded!)
    Builder-->>Dropdown: Return config with groupMode: STACKED
    Note over User: User's GROUPED preference lost!

Loading

_{6 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[github-actions]

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:63106

This environment will automatically shut down when the PR is closed or after 5 hours.

[ehconitin]

@greptileai

[greptile-apps]

Greptile Overview

Greptile Summary

Fixed groupMode to be properly undefined when no secondary axis groupBy is set, preventing incorrect default behavior.

Key changes:

• Removed defaultValue: BarChartGroupMode.STACKED from backend DTO, making groupMode truly nullable
• Added getEffectiveGroupMode utility to return undefined when no secondary axis groupBy exists
• Updated ChartGroupByFieldSelectionDropdownContentBase to set groupMode to null when removing secondary axis field
• Chart renderer now correctly handles undefined groupMode for proper conditional rendering (e.g., negative data labels positioning)

The fix ensures that groupMode-dependent UI logic only executes when a secondary axis groupBy is actually configured.

Confidence Score: 4/5

• This PR is safe to merge with minor considerations around state management
• The changes are well-structured and fix a legitimate bug where groupMode had an unwanted default value. The logic is clean and the new utility function provides good separation of concerns. Score of 4 (not 5) because: (1) the buildConfigUpdate function uses null to clear groupMode, but the type system expects undefined - this inconsistency should be verified with runtime testing, and (2) there's potential for the preserved groupMode value (via configuration.groupMode ?? BarChartGroupMode.STACKED) to cause issues if a user had previously selected GROUPED mode, then removed and re-added a secondary axis - the mode should ideally be preserved correctly.
• Pay close attention to ChartGroupByFieldSelectionDropdownContentBase.tsx for the groupMode state preservation logic

Important Files Changed

File Analysis

Filename	Score	Overview
packages/twenty-server/src/engine/core-modules/page-layout/dtos/bar-chart-configuration.dto.ts	5/5	Removed defaultValue from groupMode field, making it properly nullable (no issues)
packages/twenty-front/src/modules/page-layout/widgets/graph/graphWidgetBarChart/utils/getEffectiveGroupMode.ts	5/5	New utility function to determine effective groupMode, returns undefined when no secondary axis groupBy (no issues)
packages/twenty-front/src/modules/command-menu/pages/page-layout/components/dropdown-content/ChartGroupByFieldSelectionDropdownContentBase.tsx	4/5	Added buildConfigUpdate function to manage groupMode when selecting/deselecting secondary axis fields; sets groupMode to null when removing secondary axis

Sequence Diagram

sequenceDiagram
    participant User
    participant Dropdown as ChartGroupByFieldSelectionDropdownContentBase
    participant Update as useUpdateCurrentWidgetConfig
    participant Renderer as GraphWidgetBarChartRenderer
    participant Utils as getEffectiveGroupMode
    participant Chart as GraphWidgetBarChart

    User->>Dropdown: Select secondary axis field
    Dropdown->>Dropdown: buildConfigUpdate(fieldId, null)
    Note over Dropdown: isSecondaryAxis = true<br/>groupMode = configuration.groupMode ?? STACKED
    Dropdown->>Update: updateCurrentWidgetConfig({<br/>secondaryAxisGroupByFieldMetadataId: fieldId,<br/>groupMode: STACKED})
    
    User->>Dropdown: Remove secondary axis (select "None")
    Dropdown->>Dropdown: buildConfigUpdate(null, null)
    Note over Dropdown: isSecondaryAxis = true<br/>groupMode = null (cleanup)
    Dropdown->>Update: updateCurrentWidgetConfig({<br/>secondaryAxisGroupByFieldMetadataId: null,<br/>groupMode: null})
    
    Update->>Renderer: Re-render with new configuration
    Renderer->>Utils: getEffectiveGroupMode(groupMode, hasGroupByOnSecondaryAxis)
    
    alt No secondary axis groupBy
        Utils-->>Renderer: undefined
        Note over Renderer: No groupMode applied to chart
    else Has secondary axis groupBy
        Utils-->>Renderer: 'stacked' | 'grouped'
        Note over Renderer: groupMode applied to chart
    end
    
    Renderer->>Chart: Render with groupMode
    Chart-->>User: Display chart with correct grouping behavior

Loading

_{5 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[lucasbordeau]

LGTM

[github-actions]

Thanks @ehconitin for your contribution!
This marks your 242nd PR on the repo. You're top 1% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter