[BI Data Mapper] Re-enable array aggregating option in array mappings with type based aggregate options support #879
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces static mapping-menu definitions with type-aware, dynamically generated menu items; adds a numeric-type helper to drive conditional aggregate and array-to-singleton options based on source/target types. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant Widget as MappingOptionsWidget
participant Utils as Type Utils
participant Menu as Menu Generator
UI->>Widget: Open mapping options for mapping
Widget->>Menu: genMenuItems(mappingType, sourcePort, targetPort)
alt ArrayToSingletonAggregate
Menu->>Utils: isNumericType(sourceMemberType)
Utils-->>Menu: boolean
Menu-->>Widget: numeric/string aggregates + common (first,last)
else ArrayToSingleton
Menu->>Utils: compare sourceMemberType vs targetType (isPrimitive?)
Utils-->>Menu: boolean(s)
Menu-->>Widget: Extract-single + optional Aggregate-and-map
else ArrayToArray / Default
Menu-->>Widget: Default menu items
end
Widget-->>UI: Render filtered menu
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-11-13T05:16:29.503ZApplied to files:
🧬 Code graph analysis (1)workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx (1)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx(3 hunks)workspaces/ballerina/data-mapper/src/components/Diagram/utils/type-utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx (1)
workspaces/ballerina/data-mapper/src/components/Diagram/utils/type-utils.ts (3)
isNumericType(94-99)getGenericTypeKind(73-92)isPrimitive(63-71)
🔇 Additional comments (5)
workspaces/ballerina/data-mapper/src/components/Diagram/utils/type-utils.ts (1)
94-99: LGTM! Clean type utility function.The implementation correctly identifies numeric types by normalizing to generic types and checking for Int, Float, or Decimal. This follows the same pattern as the existing
isPrimitivefunction and integrates well with the PR's objective of type-aware filtering.workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx (4)
31-32: LGTM! Imports align with new functionality.The added imports support the type-aware menu generation logic introduced in this PR.
213-224: LGTM! Clean switch logic for menu selection.The function correctly routes to the appropriate menu generator based on mapping type. However, ensure the issues in
genAggregateItems()andgenArrayToSingletonItems()are addressed, as they are called from here.
226-226: LGTM! Dynamic menu generation enables type-aware filtering.This change successfully replaces static menu items with runtime-computed options, fulfilling the PR's objective of showing type-compatible aggregates.
168-186: Add defensive type guards for port attributes to prevent runtime errors.The function casts
link.getSourcePort()without null checks, then immediately accesses nested properties. While the code may execute in a valid context, defensive checks align with patterns used elsewhere in the codebase (e.g.,modification-utils.ts:32-34). Consider applying guards:const genAggregateItems = () => { const aggregateFnsNumeric = ["sum", "avg", "min", "max"]; const aggregateFnsString = ["string:'join"]; const aggregateFnsCommon = ["first", "last"]; const sourcePort = link.getSourcePort() as InputOutputPortModel; + if (!sourcePort?.attributes?.field) { + return []; + } const sourceType = sourcePort.attributes.field.kind;Apply the same pattern to
genArrayToSingletonItems()at lines 197–200.
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR re-enables array aggregation options in the data mapper with type-based aggregate function suggestions. Instead of showing static aggregate options, the menu now dynamically generates contextual suggestions based on the source array element types and target types.
Key changes:
- Added
isNumericTypeutility function to identify numeric types (Int, Float, Decimal) - Refactored static menu items into dynamic generation functions that inspect source/target port types
- Implemented type-aware aggregation: numeric types get sum/avg/min/max, string types get join, and all types get first/last options
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| workspaces/ballerina/data-mapper/src/components/Diagram/utils/type-utils.ts | Added isNumericType utility function to identify numeric types for type-based aggregate selection |
| workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx | Refactored static aggregate menus into dynamic generation functions (genAggregateItems, genArrayToSingletonItems) that provide contextual options based on data types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx
Show resolved
Hide resolved
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx
Outdated
Show resolved
Hide resolved
…gletonItems in MappingOptionsWidget
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx (1)
174-179: Element type must drive aggregate options.
sourcePort.attributes.field.kindis the array type itself (e.g.,TypeKind.Array), soisNumericType(sourceType)always returnsfalse. As a result the numeric/string-specific aggregates never show up, defeating the goal of filtering the menu by element type. Please derive the element kind viafield.member?.kind(falling back tofield.kindfor non-arrays) before running the type checks.- const sourceType = sourcePort.attributes.field.kind; + const sourceType = sourcePort.attributes.field.member?.kind ?? sourcePort.attributes.field.kind; - const aggregateFns = [...(isNumericType(sourceType) ? aggregateFnsNumeric : - getGenericTypeKind(sourceType) === TypeKind.String ? aggregateFnsString : - []), + const aggregateFns = [...(sourceType && isNumericType(sourceType) ? aggregateFnsNumeric : + sourceType && getGenericTypeKind(sourceType) === TypeKind.String ? aggregateFnsString : + []), ...aggregateFnsCommon];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx (1)
workspaces/ballerina/data-mapper/src/components/Diagram/utils/type-utils.ts (3)
isNumericType(94-99)getGenericTypeKind(73-92)isPrimitive(63-71)
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx
Show resolved
Hide resolved
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx
Outdated
Show resolved
Hide resolved
…ions-public into bi-dm-aggr
Purpose
Screen.Recording.2025-11-12.at.12.39.58.PM.mov
Summary by CodeRabbit