feat: adds custom sort function feature flag for table widget#39649
feat: adds custom sort function feature flag for table widget#39649rahulbarwal merged 17 commits intoreleasefrom
Conversation
WalkthroughThis pull request introduces a new feature flag ( Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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
Documentation and Community
|
rahulbarwal
changed the title
rahulbarwal
added
Query Widgets & IDE Pod
…and height/maxHeight props
…prove error handling
…n extraction and add edge case handling
…y using custom sort function data
…o rahulbarwal/custom-sort-functionality-implementation
|
/build-deploy-preview skip-tests=true |
rahulbarwal
requested review from
vivek-appsmith
and removed request for
a team
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13854545176. |
![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 (2)
app/client/src/widgets/TableWidgetV2/widget/index.tsx (1)
1233-1238: Consider adding validation for non-array customSortFunctionDataThe implementation silently ignores customSortFunctionData if it's not an array. Consider adding validation or logging to help developers troubleshoot when they provide invalid data.
let data = filteredTableData; if (customSortFunctionData && Array.isArray(customSortFunctionData)) { data = customSortFunctionData; + } else if (customSortFunctionData && !Array.isArray(customSortFunctionData)) { + console.warn("customSortFunction must be an array to be applied for table sorting"); } const transformedData = this.transformData(data, tableColumns);app/client/src/components/propertyControls/TableCustomSortControl.test.tsx (1)
34-117: Thorough unit tests for getInputComputedValue
This block effectively covers arrow functions, function bodies, and edge cases. As a minor enhancement, consider adding a multi-line arrow function test with parentheses for completeness.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/client/src/ce/entities/FeatureFlag.ts(2 hunks)app/client/src/components/propertyControls/TableCustomSortControl.test.tsx(1 hunks)app/client/src/components/propertyControls/TableCustomSortControl.tsx(1 hunks)app/client/src/components/propertyControls/index.ts(3 hunks)app/client/src/widgets/TableWidgetV2/constants.ts(2 hunks)app/client/src/widgets/TableWidgetV2/widget/index.tsx(2 hunks)app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (17)
app/client/src/widgets/TableWidgetV2/constants.ts (2)
70-70: Addition of customSortFunction property looks goodThis optional property allows for specifying a custom sort function as a string in the TableWidgetProps interface.
252-253: Properly implemented feature flag constantThe constant is correctly defined to get its value from the feature flag, matching the pattern used for other feature-controlled functionality.
app/client/src/ce/entities/FeatureFlag.ts (2)
58-59: Feature flag naming follows conventionThe feature flag name follows the established naming convention for table features.
106-106: Default value set to falseDefault value is correctly set to false, which is appropriate for a new feature that should be enabled explicitly.
app/client/src/components/propertyControls/index.ts (3)
80-82: Clean import of new control componentImport statements for the TableCustomSortControl component and its props type are well-structured.
136-136: Component properly registered in PropertyControlsThe TableCustomSortControl is correctly added to the PropertyControls object.
166-166: Type definition extended correctlyThe PropertyControlPropsType union type is properly extended to include the new control's props type.
app/client/src/widgets/TableWidgetV2/widget/index.tsx (1)
1219-1219: Property renamed for clarityRenaming customSortFunction to customSortFunctionData makes the variable purpose clearer in implementation.
app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts (2)
10-13: No concerns with new imports
These feature flags (ALLOW_TABLE_WIDGET_SERVER_SIDE_FILTERINGandCUSTOM_SORT_FUNCTION_ENABLED) are properly imported and ready for use.
438-438: No code presentapp/client/src/components/propertyControls/TableCustomSortControl.test.tsx (3)
1-33: Preliminary setup is clear
The imports, mock functions, and required parameters are well-structured.
118-163: Good coverage of instance methods
Tests check main flows, including non-binding and empty-value scenarios. No issues found.
164-169: Control type test
Validates the returned control type accurately. Looks good.app/client/src/components/propertyControls/TableCustomSortControl.tsx (4)
1-60: Imports and styled components
The modular approach and styled usage are clean. No concerns.
61-119: Reusable InputText component
Encapsulates the code editor with relevant props, placeholders, and hinting. Implementation is straightforward.
121-196: Logical merging and error handling
ThegetTableCustomSortBindingfunction merges data carefully, handles errors gracefully, and preserves the original structure. Implementation looks solid.
197-373: Robust custom sort control
Therender,getInputComputedValue, andgetComputedValuemethods provide flexibility and solid error handling for user-defined logic. Overall design is consistent, no obvious issues found.
app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts
Show resolved
Hide resolved
|
Deploy-Preview-URL: https://ce-39649.dp.appsmith.com |
…o rahulbarwal/custom-sort-functionality-implementation
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13918872181. |
|
Deploy-Preview-URL: https://ce-39649.dp.appsmith.com |
app/client/src/components/propertyControls/TableCustomSortControl.tsx
Outdated
Show resolved
Hide resolved
app/client/src/components/propertyControls/TableCustomSortControl.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/client/src/components/propertyControls/TableCustomSortControl.test.tsx (3)
109-114: Consider extracting utility functions to a shared location.The
normalizeWhitespacefunction could be useful in other tests. Consider moving it to a test utility file if similar normalization is needed elsewhere.
124-147: Consider breaking down large test cases.This test checks multiple aspects of the output in a single test case. Consider breaking it into smaller, more focused tests for better isolation and easier diagnosis when failures occur.
1-191: Consider adding tests for error handling scenarios.While the test coverage is good, consider adding tests for handling malformed inputs, null values, or undefined inputs to ensure robust error handling.
it("should handle null or undefined inputs gracefully", () => { const nullResult = TableCustomSortControl.getInputComputedValue(null as unknown as string); const undefinedResult = TableCustomSortControl.getInputComputedValue(undefined as unknown as string); expect(nullResult).toBe(""); expect(undefinedResult).toBe(""); }); it("should handle malformed binding strings", () => { const malformedString = "{{incomplete"; const result = TableCustomSortControl.getInputComputedValue(malformedString); // Verify expected behavior with malformed input expect(result).toBe(malformedString); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
app/client/src/components/propertyControls/TableCustomSortControl.test.tsx(1 hunks)app/client/src/components/propertyControls/TableCustomSortControl.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/components/propertyControls/TableCustomSortControl.tsx
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / rts-build / build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (5)
app/client/src/components/propertyControls/TableCustomSortControl.test.tsx (5)
1-3: Imports are appropriate for the test file.The imports correctly include the
EditorThemeand the component under testTableCustomSortControl.
4-32: Test setup is comprehensive and well-structured.The mock parameters provide all necessary props for the component under test, including widget properties, control properties, and Jest mock functions for callbacks.
34-116: Test coverage for getInputComputedValue is thorough.Good test cases covering arrow functions, function bodies, non-matching strings, empty strings, and round-trip conversions. The test structure follows best practices with clear assertions.
118-185: Test cases for instance methods are comprehensive.Tests thoroughly verify the
getComputedValuemethod handles various inputs correctly and generates the expected output with all necessary components for table data manipulation.
187-191: Test for getControlType is sufficient.Simple verification of the control type returned by the static method.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/client/src/components/propertyControls/TableCustomSortControl.tsx (3)
163-164: Consider adding null check for row parameterWhen mapping filtered data, ensure the row parameter is valid before accessing properties.
- return {...row, __original__: originalDataMap[row[primaryId]] || {}}; + return row ? {...row, __original__: originalDataMap[row?.[primaryId]] || {}} : row;
204-209: Consider simplifying the nested ternary operatorThe nested ternary operator for determining the value could be simplified for better readability.
- const value = - propertyValue && isDynamicValue(propertyValue) - ? TableCustomSortControl.getInputComputedValue(propertyValue) - : propertyValue - ? propertyValue - : defaultValue; + let value = defaultValue; + if (propertyValue) { + value = isDynamicValue(propertyValue) + ? TableCustomSortControl.getInputComputedValue(propertyValue) + : propertyValue; + }
227-231: Consider using more type-safe initialization for additionalDynamicDataThe current approach of casting empty objects to specific record types might not be the most type-safe approach.
- additionalDynamicData={{ - tableData: {} as Record<string, unknown>, - column: {} as Record<string, unknown>, - order: {} as Record<string, unknown>, - }} + additionalDynamicData={{ + tableData: Object.create(null) as Record<string, unknown>, + column: Object.create(null) as Record<string, unknown>, + order: Object.create(null) as Record<string, unknown>, + }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
app/client/src/components/propertyControls/TableCustomSortControl.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (5)
app/client/src/components/propertyControls/TableCustomSortControl.tsx (5)
107-111: Appropriate implementation of original data accessThe implementation correctly provides access to original data via the
__original__property, which aligns with the previous feedback in the PR.
125-187: Well-structured table custom sort binding implementationThe
getTableCustomSortBindingmethod is comprehensive and handles all key scenarios:
- Merged data management
- Error handling
- Empty dataset handling
- Data integrity preservation
The code is well-documented with clear explanations of the various states and processing steps.
174-178: Good implementation of cleanup to remove temporary propertiesThe code correctly cleans up the
__original__property from the sorted data before returning it.
182-185: Error handling with fallback to filtered dataProperly handles errors in the sorting logic by falling back to the original filtered data.
246-307: Robust parsing of different function signaturesThe
getInputComputedValuemethod effectively handles two different function signature formats:
((tableData, column, order) => {((tableData, column, order) => (The implementation could be simplified, but it handles the parsing requirements well.
rahulbarwal
deleted the
rahulbarwal/custom-sort-functionality-implementation
branch
…thorg#39649) ## Description ### Custom Sort Function for Table Widget This PR introduces a new custom sort function feature for the Table Widget, allowing users to define their own sorting logic for table data. **Feature Flag Implementation** - Added a new feature flag `release_table_custom_sort_function_enabled` to control the availability of this feature - Set the default value to `false` to allow for controlled rollout **Table Widget Updates** - Added `customSortFunction` property to the TableWidgetV2 props interface - Updated property configuration to include the custom sort function control - Added helper text and placeholder to guide users on proper implementation - Implemented visibility condition based on the feature flag and sortable state **Control Updates** - Enhanced `TableCustomSortControl` to support the new custom sorting logic - Improved error handling in the sort function implementation - Updated the binding structure to provide access to original data with `original_` prefix - Modified the function signature to use `(tableData, column, order)` parameters **Testing** - Updated test cases to verify the extraction and computation of custom sort expressions - Implemented round-trip testing to ensure proper function parsing and generation - Added tests for edge cases like empty strings and non-matching formats This feature enables users to implement complex sorting logic beyond the default column-based sorting, such as multi-column sorting, custom collation, or business-specific ordering rules. Fixes #https://github.com/appsmithorg/appsmith-ee/issues/6503 _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Table, @tag.Sanity, @tag.Datasource" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/13986693183> > Commit: b64c291 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13986693183&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Table, @tag.Sanity, @tag.Datasource` > Spec: > <hr>Fri, 21 Mar 2025 08:35:48 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced custom table sorting, enabling users to define and apply their own sorting logic in table displays. - Added a new control interface for configuring custom sort functions, offering enhanced flexibility in data presentation. - Integrated an option to enable or disable the custom sort functionality based on feature flags. - Enhanced the table widget to support custom sorting function data, allowing for dynamic data presentation. - Added a new property configuration for custom sorting functions within the table widget. - **Tests** - Implemented comprehensive tests to verify custom sorting input processing and ensure accurate data computation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Description
Custom Sort Function for Table Widget
This PR introduces a new custom sort function feature for the Table Widget, allowing users to define their own sorting logic for table data.
Feature Flag Implementation
release_table_custom_sort_function_enabledto control the availability of this featurefalseto allow for controlled rolloutTable Widget Updates
customSortFunctionproperty to the TableWidgetV2 props interfaceControl Updates
TableCustomSortControlto support the new custom sorting logicoriginal_prefix(tableData, column, order)parametersTesting
This feature enables users to implement complex sorting logic beyond the default column-based sorting, such as multi-column sorting, custom collation, or business-specific ordering rules.
Fixes #https://github.com/appsmithorg/appsmith-ee/issues/6503
or
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Table, @tag.Sanity, @tag.Datasource"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13986693183
Commit: b64c291
Cypress dashboard.
Tags:
@tag.Table, @tag.Sanity, @tag.DatasourceSpec:
Fri, 21 Mar 2025 08:35:48 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests