-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: prevent adding components from a different data model to the subform table #14442
base: main
Are you sure you want to change the base?
fix: prevent adding components from a different data model to the subform table #14442
Conversation
📝 WalkthroughWalkthroughThe pull request introduces enhancements to the subform table column editing functionality in the UX editor. The changes focus on improving the component selection logic by adding a new 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/utils/editSubformTableColumnsUtils.ts (2)
40-51
: Consider adding documentation forcomponentsWithTitleAndDefaultDataModel
Adding JSDoc comments or inline documentation to the
componentsWithTitleAndDefaultDataModel
function can improve readability and maintainability by clearly explaining its purpose and logic.
53-56
: Consider adding documentation forgetDefaultDataModel
Providing a brief explanation of the
getDefaultDataModel
function, including its parameters and return value, can enhance understandability for future contributors.frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.tsx (1)
88-89
: Ensure data availability before usingformLayouts
andsubformDefaultDataModel
Consider adding checks to ensure that
formLayouts
andsubformDefaultDataModel
are defined before they are used. This can prevent potential runtime errors if the data is not yet loaded.Apply this diff to add conditional rendering based on data availability:
const { data: formLayouts } = useFormLayoutsQuery(org, app, subformLayout); const { data: layoutSets } = useLayoutSetsQuery(org, app); + if (!formLayouts || !layoutSets) { + return null; // or a loading indicator if preferred + } const subformDefaultDataModel = getDefaultDataModel(layoutSets, subformLayout); const availableComponents = getComponentsForSubformTable(formLayouts, subformDefaultDataModel);frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/utils/editSubformTableColumnsUtils.test.ts (1)
51-52
: Address the use ofas any
in the test codeUsing
as any
can bypass type safety provided by TypeScript. Since there's a TODO comment referencing issue #14441, consider implementing a temporary type or interface to avoid casting toany
, enhancing type safety until the issue is resolved.Do you want me to help create a temporary type definition to replace
as any
, or assist in resolving the underlying type issue?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
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
(3 hunks)frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/utils/editSubformTableColumnsUtils.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
🔇 Additional comments (7)
frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/utils/editSubformTableColumnsUtils.ts (2)
9-10
: Import statements added appropriatelyThe new imports of
LayoutSets
andconvertDataBindingToInternalFormat
are necessary for the added functionalities and are correctly included.
29-32
: UpdatedgetComponentsForSubformTable
function signatureThe inclusion of the
defaultDataModel
parameter enhances the function's ability to filter components based on the specific data model, improving accuracy in component selection.frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.tsx (3)
22-22
: ImportedgetDefaultDataModel
function correctlyThe addition of
getDefaultDataModel
to the imports ensures that the default data model can be retrieved for component filtering.
27-27
: ImporteduseLayoutSetsQuery
hook appropriatelyIncluding
useLayoutSetsQuery
enables fetching of layout sets necessary for determining the default data model.
61-61
: FetchlayoutSets
usinguseLayoutSetsQuery
The use of
useLayoutSetsQuery
to retrievelayoutSets
is appropriate for accessing layout set data within the component.frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/utils/editSubformTableColumnsUtils.test.ts (2)
131-135
: Updated tests to includedefaultDataModel
parameterThe tests for
getComponentsForSubformTable
now correctly include thedefaultDataModel
argument, ensuring that the new logic is adequately tested.
153-156
: Ensure test coverage for components without data model bindingsThe test case appropriately checks that components without data model bindings are excluded when the
defaultDataModel
is provided, confirming the function's filtering logic.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14442 +/- ##
==========================================
- Coverage 95.65% 95.65% -0.01%
==========================================
Files 1886 1886
Lines 24546 24556 +10
Branches 2817 2819 +2
==========================================
+ Hits 23479 23488 +9
Misses 805 805
- Partials 262 263 +1 ☔ View full report in Codecov by Sentry. |
Description
Updated the filter logic for the component selector in the subform table to only allow components that are part of the same data model as the subform.
subform-default-datamodel.mp4
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates