chore(IDataModelBindings): Fix typing of data model bindings #14659
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the data model binding types throughout the codebase. String-based definitions have been replaced with more structured types, such as Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
area/ui-editor
lassopicasso
changed the title
lassopicasso
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
frontend/packages/ux-editor/src/types/global.ts (1)
19-28: LGTM! Well-structured type definitions.The new type definitions provide a clear separation between implicit and explicit bindings while maintaining backward compatibility. The union types are well-designed to handle both cases.
Consider adding JSDoc comments to document the purpose and usage of each type, especially explaining when to use
ImplicitDataModelBindingvsExplicitDataModelBinding. Example:+/** + * Represents a simple string-based data model binding (legacy format) + */ export type ImplicitDataModelBinding = string; + +/** + * Represents a structured data model binding with explicit dataType and field + */ export type ExplicitDataModelBinding = { dataType: string; field: string; };frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/SelectDataModelBinding.tsx (1)
35-41: Consider initializing field with null or undefined.The empty string initialization for the
fieldproperty might not accurately represent an unset state. Consider usingnullorundefinedto be more explicit about the field not being selected yet.const handleDataModelChange = (newDataModel: string) => { const dataModelBinding = { - field: '', + field: undefined, dataType: newDataModel, }; handleBindingChange(dataModelBinding); };frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/SelectDataFieldBinding.tsx (1)
31-33: Add type guard for internalBindingFormat properties.Consider adding type guards to ensure type safety when destructuring
internalBindingFormat.- const { dataType: currentDataModel, field: currentDataModelField } = internalBindingFormat; + const currentDataModel = internalBindingFormat?.dataType ?? ''; + const currentDataModelField = internalBindingFormat?.field ?? '';frontend/packages/ux-editor/src/utils/dataModelUtils.ts (1)
105-114: Consider adding validation for empty strings.While the conversion logic is correct, consider validating empty strings in the field and dataType properties.
return { - field: binding || '', + field: binding && binding.trim() ? binding : '', dataType: '', };frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.test.tsx (1)
336-338: Consider using exact matching instead of RegExp.Using RegExp for matching the field value might be fragile if the field contains special characters. Consider using exact matching:
- name: new RegExp(postPlaceDataField), + name: postPlaceDataField,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
frontend/packages/shared/src/types/ComponentSpecificConfig.ts(1 hunks)frontend/packages/shared/src/types/api/FormLayoutsResponse.ts(2 hunks)frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/ColumnElement.test.tsx(2 hunks)frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/DataModelBindingsCombobox.tsx(2 hunks)frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.test.tsx(7 hunks)frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.tsx(3 hunks)frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/utils/editSubformTableColumnsUtils.test.ts(1 hunks)frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/utils/editSubformTableColumnsUtils.ts(1 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx(1 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/EditBinding.test.tsx(2 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/EditBinding.tsx(2 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/EditBindingButtons.tsx(1 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/SelectDataFieldBinding.tsx(1 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/SelectDataModelBinding.tsx(1 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditDataModelBinding.tsx(1 hunks)frontend/packages/ux-editor/src/hooks/mutations/useUpdateFormComponentMutation.test.ts(2 hunks)frontend/packages/ux-editor/src/types/FormComponent.ts(2 hunks)frontend/packages/ux-editor/src/types/FormContainer.ts(2 hunks)frontend/packages/ux-editor/src/types/global.ts(1 hunks)frontend/packages/ux-editor/src/utils/dataModelUtils.test.ts(1 hunks)frontend/packages/ux-editor/src/utils/dataModelUtils.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
- GitHub Check: CodeQL
🔇 Additional comments (34)
frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditDataModelBinding.tsx (2)
28-28: LGTM!The fallback logic for
bindingKeyis clear and appropriate.
29-31: LGTM! Improved type safety and specificity.The changes correctly target a specific binding using
bindingKeyand safely handle undefined values through optional chaining. This aligns well with the PR's objective of improving type safety in data model bindings.frontend/packages/shared/src/types/ComponentSpecificConfig.ts (9)
10-13: Good import for unified data model bindings.
By importingIDataModelBindingsandIDataModelBindingsKeyValuefrom a single source, you ensure consistency across the codebase.
16-20: Consistent use ofIDataModelBindingsfor address fields.
Switching toIDataModelBindingsfor each field clarifies the expected structure and enforces type safety.
23-23: AdoptingIDataModelBindingsKeyValuefor custom bindings.
This is a logical approach for flexible data fields, providing a more structured type than plain strings.
26-26: Refining group field toIDataModelBindings.
UsingIDataModelBindingsfor the group property further aligns it with the new typed approach.
29-29: Aligning list bindings withIDataModelBindingsKeyValue.
This update provides clarity for list-based data structures while maintaining flexibility.
32-33: Likert fields switching toIDataModelBindings.
Defining bothanswerandquestionsasIDataModelBindingsis consistent and ensures a uniform contract.
37-37: ApplyingIDataModelBindingsfor list usage.
This maintains a standardized approach for handling data model bindings across similar components.
41-42: Added typedmetadatainDataModelBindingsOptionsSimple.
Including an optionalmetadatafield typed asIDataModelBindingsprovides structured extensibility.
46-46: SimplifyingDataModelBindingsSimplewith typed field.
ConvertingsimpleBindingtoIDataModelBindingspromotes consistency with other binding structures.frontend/packages/ux-editor/src/types/FormContainer.ts (2)
1-1: New import aligns with key-value style bindings.
ImportingIDataModelBindingsKeyValuesets the stage for more structured container references.
13-13: UsingIDataModelBindingsKeyValuein the container base.
Replacing the old approach withdataModelBindings?: IDataModelBindingsKeyValueensures typed mappings at the container level.frontend/packages/shared/src/types/api/FormLayoutsResponse.ts (2)
5-5: IntroducingIDataModelBindingsKeyValueimport.
Pulling in the new type underlines the shift from string-based definitions to robust key-value mappings.
24-24: Switching external components toIDataModelBindingsKeyValue.
ReplacingKeyValuePairs<string>ensures a clearer and more enforceable structure for external layout bindings.frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/EditBindingButtons.tsx (1)
6-6:❓ Verification inconclusive
LGTM! Type changes align with the PR objectives.
The change from
InternalBindingFormattoExplicitDataModelBindingimproves type safety and clarity.Let's verify that all usages of
handleBindingChangeare updated consistently:Also applies to: 9-9
🏁 Script executed:
#!/bin/bash # Search for handleBindingChange function calls ast-grep --pattern 'handleBindingChange($$$)'Length of output: 762
Action Required: Verify Consistency of
handleBindingChangeUsageLGTM overall—the change from
InternalBindingFormattoExplicitDataModelBindingimproves type safety and clarity as intended. Our search confirms that in the config edit modal components (in EditBindingButtons.tsx, SelectDataFieldBinding.tsx, and SelectDataModelBinding.tsx), the updated single-parameter usage (orundefinedfor deletion) is applied correctly.However, note that in
•frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.tsx
the function is invoked with two arguments. Please verify that this dual-parameter call is meant to use an overloaded version (or remains unaffected by this PR’s type change) so that it doesn’t conflict with the updated signature.frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx (1)
12-12: LGTM! Type changes maintain consistency.The change to
ExplicitDataModelBindingaligns with the type system improvements and maintains type safety when destructuringfieldanddataType.Also applies to: 17-17
frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/DataModelBindingsCombobox.tsx (1)
4-4: LGTM! Type change improves specificity.The change to
IDataModelBindingsKeyValuebetter represents the key-value structure of the bindings.Also applies to: 9-9
frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/utils/editSubformTableColumnsUtils.ts (1)
44-44: LGTM! Improved null safety with optional chaining.The addition of optional chaining when accessing
dataModelBindings[binding]prevents potential runtime errors.frontend/packages/ux-editor/src/types/FormComponent.ts (1)
2-2: LGTM! Type update aligns with new binding system.The change from
IDataModelBindingstoIDataModelBindingsKeyValueinFormComponentBaseinterface correctly implements the new typing system while maintaining backward compatibility.Also applies to: 17-17
frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/SelectDataFieldBinding.tsx (1)
42-48: LGTM! Proper handling of field updates.The
handleDataModelFieldChangefunction correctly maintains the data model binding structure when updating fields.frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/EditBinding.tsx (1)
19-19: LGTM! Type safety improvements look good.The changes enhance type safety by:
- Using
ExplicitDataModelBindinginstead ofInternalBindingFormat- Consistently applying the type across props and handlers
Also applies to: 27-27, 44-44
frontend/packages/ux-editor/src/utils/dataModelUtils.ts (1)
100-103: LGTM! Well-implemented type guard.The type guard correctly checks for both
dataTypeandfieldproperties, ensuring type safety.frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/ColumnElement.test.tsx (1)
14-14: LGTM! Test updates properly reflect the new typing system.The changes correctly use the conversion function to handle data model bindings in tests.
Also applies to: 20-22, 54-54
frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/utils/editSubformTableColumnsUtils.test.ts (1)
50-50: LGTM! Improved type safety by removing type assertion.The change properly types the
simpleBindingobject without relying on type assertions.frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.tsx (3)
24-24: LGTM! Type import aligns with PR objectives.The change from
IDataModelBindingstoIDataModelBindingsKeyValueimproves type safety by using a more specific type for key-value data model bindings.
58-60: LGTM! Safe handling of optional values.The use of optional chaining (
?.) in accessingdataModelBindingsensures safe handling of potentially undefined values.
69-73: LGTM! Consistent use of new binding format.The
handleBindingChangefunction correctly uses the newIDataModelBindingsKeyValuetype and properly extracts the field usingconvertDataBindingToInternalFormat.frontend/packages/ux-editor/src/hooks/mutations/useUpdateFormComponentMutation.test.ts (1)
19-19: LGTM! Test data updated to use new type.The test data correctly uses the new
IDataModelBindingsKeyValuetype for data model bindings, maintaining consistency with the type system changes.Also applies to: 33-35
frontend/packages/ux-editor/src/utils/dataModelUtils.test.ts (1)
158-160: LGTM! Comprehensive test coverage for binding format conversion.The test cases thoroughly cover:
- Internal format preservation
- Old format conversion
- Undefined value handling
The changes improve type safety while maintaining backward compatibility.
Also applies to: 166-168, 173-174
frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/EditBinding/EditBinding.test.tsx (2)
15-16: LGTM! Appropriate type imports.The imports correctly include both
ExplicitDataModelBindingtype and theconvertDataBindingToInternalFormatutility function.
40-42: LGTM! State management updated for new binding format.The component correctly:
- Uses
ExplicitDataModelBindingfor state type- Utilizes
convertDataBindingToInternalFormatfor binding conversion- Updates state with the extracted field value
Also applies to: 47-49, 52-52
frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.test.tsx (2)
13-13: LGTM! Well-structured data binding conversion.The extraction of data fields using
convertDataBindingToInternalFormatat the top level promotes reusability and aligns with the PR's goal of improving typing.Also applies to: 27-32
138-138: LGTM! Consistent usage of converted data fields.The test cases have been consistently updated to use the converted data fields while maintaining the original test behavior.
Also applies to: 164-164, 275-275, 313-313
...erties/EditSubformTableColumns/ColumnElement/EditColumnElement/DataModelBindingsCombobox.tsx
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14659 +/- ##
=======================================
Coverage 95.76% 95.76%
=======================================
Files 1914 1914
Lines 24961 24961
Branches 2858 2858
=======================================
Hits 23904 23904
Misses 799 799
Partials 258 258 ☔ View full report in Codecov by Sentry. |
Description
This PR fixes the type logic for the union type
IDataModelBindingsthat contains old and new datamodel format (stringand{ dataType: string, field: string }. Also separate it more clearer to have aIDataModelBindingsKeyValueas well.IDataModelBindings: is used for types with specific keys/props, such as address:IDataModelBindingsKeyValue: is used for more "generic/unknown" keys/props, such asDataModelBindingsForCustom:convertDataBindingToInternalFormatwith type guards so that the types is correctly. BeforedataModelBindingsalways ended up as a string according to typescript.Related Issue(s)
IDataModelBindingstype #14441Verification
Documentation
Summary by CodeRabbit