chore: Refactor derived properties #37390
Conversation
WalkthroughThe changes involve the removal of multiple Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning Rate limit exceeded@jsartisan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 4 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (7)
app/client/src/widgets/MultiSelectTreeWidget/widget/index.tsx (1)
645-652: Consider caching the parsed derived properties.The
parseDerivedPropertiesis called on every invocation ofgetDerivedPropertiesMap. Consider caching the result in a class field to avoid unnecessary parsing.static getDerivedPropertiesMap() { - const parsedDerivedProperties = parseDerivedProperties(derivedPropertyFns); + // Cache the parsed properties + if (!this.parsedDerivedProperties) { + this.parsedDerivedProperties = parseDerivedProperties(derivedPropertyFns); + } + const parsedDerivedProperties = this.parsedDerivedProperties; return { value: `{{this.selectedOptionValues}}`, isValid: `{{(()=>{${parsedDerivedProperties.getIsValid}})()}}`, flattenedOptions: `{{(()=>{${parsedDerivedProperties.getFlattenedOptions}})()}}`, selectedOptionValues: `{{(()=>{${parsedDerivedProperties.getSelectedOptionValues}})()}}`, selectedOptionLabels: `{{(()=>{${parsedDerivedProperties.getSelectedOptionLabels}})()}}`, }; }app/client/src/widgets/WidgetUtils.ts (2)
1024-1028: Strengthen regex patterns for better reliabilityThe current regex patterns for extracting function body and parameters could be more robust.
- const functionBody = functionString.match(/(?<=\{)(.|\n)*(?=\})/)?.[0]; + const functionBody = functionString.match(/(?<=\{)([\s\S]*?)(?=\}(?![^{]*\}))/)?.[0]; - const paramMatch = functionString.match(/\((.*?),/); + const paramMatch = functionString.match(/^\s*(?:function\s*\w*\s*)?\((.*?)(?:,|\))/); const propsParam = paramMatch ? paramMatch[1].trim() : "props";
992-1040: Consider security implications of regex on user-provided codeThe function processes user-provided code using regex which could be exploited. Consider using a proper AST parser for more secure code analysis.
Consider using a proper AST parser like
@babel/parserfor safer code analysis instead of regex-based parsing.app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx (2)
Line range hint
809-819: Consider simplifying the type checking logic.The type checking for string array can be simplified using Array.prototype.every() for better readability.
- if ( - this.props.defaultOptionValue && - this.props.defaultOptionValue.some( - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (value: any) => isString(value) || isFinite(value), - ) - ) { - isStringArray = true; - } + isStringArray = Array.isArray(this.props.defaultOptionValue) && + this.props.defaultOptionValue.every( + (value): value is string | number => isString(value) || isFinite(value) + );
Line range hint
820-834: Consider extracting comparison logic to a separate method.The comparison logic for defaultOptionValue changes is complex and would benefit from being extracted into a separate method for better maintainability.
+ private hasDefaultValueChanged( + current: string[] | OptionValue[], + previous: string[] | OptionValue[], + isStringArray: boolean + ): boolean { + return isStringArray + ? xorWith(current as string[], previous as string[], equal).length > 0 + : xorWith(current as OptionValue[], previous as OptionValue[], equal).length > 0; + } componentDidUpdate(prevProps: MultiSelectWidgetProps): void { // ... type checking logic ... - const hasChanges = isStringArray - ? xorWith( - this.props.defaultOptionValue as string[], - prevProps.defaultOptionValue as string[], - equal, - ).length > 0 - : xorWith( - this.props.defaultOptionValue as OptionValue[], - prevProps.defaultOptionValue as OptionValue[], - equal, - ).length > 0; + const hasChanges = this.hasDefaultValueChanged( + this.props.defaultOptionValue, + prevProps.defaultOptionValue, + isStringArray + ); if (hasChanges && this.props.isDirty) { this.props.updateWidgetMetaProperty("isDirty", false); } }app/client/src/widgets/ListWidgetV2/widget/index.tsx (1)
287-291: Consider using a more descriptive variable name.While the implementation is good, consider renaming
parsedDerivedPropertiesto something more specific likeparsedChildAutoCompletePropsto better reflect its usage.- const parsedDerivedProperties = parseDerivedProperties(derivedPropertyFns); + const parsedChildAutoCompleteProps = parseDerivedProperties(derivedPropertyFns); return { - childAutoComplete: `{{(() => {${parsedDerivedProperties.getChildAutoComplete}})()}}`, + childAutoComplete: `{{(() => {${parsedChildAutoCompleteProps.getChildAutoComplete}})()}}`, };app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/widget/index.tsx (1)
206-209: Nitpick: Remove unnecessary spaces in template literalsThere are extra spaces inside the template literals that can be removed for consistency.
Apply this diff:
- filteredTableData: `{{(()=>{ ${parsedDerivedProperties.getFilteredTableData}})()}}`, - updatedRows: `{{(()=>{ ${parsedDerivedProperties.getUpdatedRows}})()}}`, - updatedRowIndices: `{{(()=>{ ${parsedDerivedProperties.getUpdatedRowIndices}})()}}`, - updatedRow: `{{(()=>{ ${parsedDerivedProperties.getUpdatedRow}})()}}`, + filteredTableData: `{{(()=>{${parsedDerivedProperties.getFilteredTableData}})()}}`, + updatedRows: `{{(()=>{${parsedDerivedProperties.getUpdatedRows}})()}}`, + updatedRowIndices: `{{(()=>{${parsedDerivedProperties.getUpdatedRowIndices}})()}}`, + updatedRow: `{{(()=>{${parsedDerivedProperties.getUpdatedRow}})()}}`,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (28)
app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/parsedDerivedProperties.ts(0 hunks)app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/widget/parsedDerivedProperties.ts(0 hunks)app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/widget/index.tsx(3 hunks)app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/CurrencyInputWidget/widget/parsedDerivedProperties.ts(0 hunks)app/client/src/widgets/DatePickerWidget2/widget/index.tsx(2 hunks)app/client/src/widgets/DatePickerWidget2/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/InputWidgetV2/widget/parsedDerivedProperties.ts(0 hunks)app/client/src/widgets/ListWidget/widget/index.tsx(2 hunks)app/client/src/widgets/ListWidget/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/ListWidgetV2/widget/index.tsx(2 hunks)app/client/src/widgets/ListWidgetV2/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/MultiSelectTreeWidget/widget/index.tsx(2 hunks)app/client/src/widgets/MultiSelectTreeWidget/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx(2 hunks)app/client/src/widgets/MultiSelectWidgetV2/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/PhoneInputWidget/widget/parsedDerivedProperties.ts(0 hunks)app/client/src/widgets/SelectWidget/widget/index.tsx(2 hunks)app/client/src/widgets/SelectWidget/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/SingleSelectTreeWidget/widget/index.tsx(2 hunks)app/client/src/widgets/SingleSelectTreeWidget/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/TableWidget/widget/index.tsx(2 hunks)app/client/src/widgets/TableWidget/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/TableWidgetV2/widget/index.tsx(2 hunks)app/client/src/widgets/TableWidgetV2/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/TabsWidget/widget/index.tsx(2 hunks)app/client/src/widgets/TabsWidget/widget/parseDerivedProperties.ts(0 hunks)app/client/src/widgets/WidgetUtils.ts(1 hunks)
💤 Files with no reviewable changes (16)
- app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/parsedDerivedProperties.ts
- app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/widget/parsedDerivedProperties.ts
- app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/widget/parseDerivedProperties.ts
- app/client/src/widgets/CurrencyInputWidget/widget/parsedDerivedProperties.ts
- app/client/src/widgets/DatePickerWidget2/widget/parseDerivedProperties.ts
- app/client/src/widgets/InputWidgetV2/widget/parsedDerivedProperties.ts
- app/client/src/widgets/ListWidget/widget/parseDerivedProperties.ts
- app/client/src/widgets/ListWidgetV2/widget/parseDerivedProperties.ts
- app/client/src/widgets/MultiSelectTreeWidget/widget/parseDerivedProperties.ts
- app/client/src/widgets/MultiSelectWidgetV2/widget/parseDerivedProperties.ts
- app/client/src/widgets/PhoneInputWidget/widget/parsedDerivedProperties.ts
- app/client/src/widgets/SelectWidget/widget/parseDerivedProperties.ts
- app/client/src/widgets/SingleSelectTreeWidget/widget/parseDerivedProperties.ts
- app/client/src/widgets/TableWidget/widget/parseDerivedProperties.ts
- app/client/src/widgets/TableWidgetV2/widget/parseDerivedProperties.ts
- app/client/src/widgets/TabsWidget/widget/parseDerivedProperties.ts
🔇 Additional comments (21)
app/client/src/widgets/TabsWidget/widget/index.tsx (2)
20-21: LGTM! Import changes align with derived properties refactor.
541-544: LGTM! getDerivedPropertiesMap implementation correctly uses the new utility.
The change maintains the existing functionality while adopting the new derived properties parsing mechanism.
app/client/src/widgets/DatePickerWidget2/widget/index.tsx (2)
23-24: LGTM: Import changes align with the refactoring objective.
The replacement of direct derived properties with the new utility function improves code maintainability.
636-639: LGTM: Clean implementation of the derived properties refactoring.
The use of parseDerivedProperties centralizes the property parsing logic while maintaining the same functionality. The template literal syntax for the isValid property is correctly implemented.
app/client/src/widgets/SingleSelectTreeWidget/widget/index.tsx (2)
25-26: LGTM! Clean import refactoring.
The imports have been updated to use the new derived properties parsing mechanism.
633-640: Verify derived properties parsing.
The implementation looks correct, but let's verify that all derived property functions are being parsed correctly.
app/client/src/widgets/MultiSelectTreeWidget/widget/index.tsx (1)
26-27: LGTM! Import changes align with the derived properties refactoring.
app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx (2)
23-24: LGTM: Import changes align with the refactoring.
The replacement of direct derived properties with the new utility function improves code organization.
758-765: LGTM: Derived properties refactoring looks good.
The refactoring improves maintainability by centralizing the derived properties parsing logic.
app/client/src/widgets/SelectWidget/widget/index.tsx (2)
30-31: LGTM: Import changes align with the refactoring objective.
The replacement of direct derived properties with the new utility function improves code modularity.
776-783: Verify the derived properties parsing implementation.
The implementation looks correct, but we should ensure that all derived properties are properly parsed and maintain their original functionality.
✅ Verification successful
Let me verify the implementation details of both old and new approaches to ensure consistency.
Let me check one final aspect to verify the consistency of derived property functions across widgets.
Derived properties implementation is correctly structured
Based on the verification results:
- The
parseDerivedPropertiesutility function inWidgetUtils.tsproperly handles function parsing and parameter extraction - Multiple widgets (Tabs, Table, Select, etc.) have adopted the new implementation
- The implementation pattern is consistent across widgets, wrapping derived properties in self-executing functions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the derived properties implementation across the codebase
# Check if any other widgets still use the old implementation
# Search for other widgets using old derivedProperties
rg -l "derivedProperties" "app/client/src/widgets"
# Search for new implementation adoption
rg -l "parseDerivedProperties.*derivedPropertyFns" "app/client/src/widgets"
Length of output: 890
Script:
#!/bin/bash
# Check the implementation of parseDerivedProperties function
rg -A 10 "function parseDerivedProperties" app/client/src/widgets
# Check how derivedProperties are used in files with old implementation
rg -A 5 "derivedProperties\s*=|derivedProperties:" app/client/src/widgets/PhoneInputWidget/widget/index.tsx app/client/src/widgets/InputWidgetV2/widget/index.tsx app/client/src/widgets/CurrencyInputWidget/widget/index.tsx
# Check the pattern of usage in files with new implementation
rg -A 5 "parseDerivedProperties\(derivedPropertyFns\)" app/client/src/widgets/SelectWidget/widget/index.tsx app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx
Length of output: 2688
Script:
#!/bin/bash
# Check the derivedPropertyFns implementation in SelectWidget and other widgets
ast-grep --pattern 'const derivedPropertyFns = {
$$$
}' app/client/src/widgets/SelectWidget/widget/index.tsx
# Compare with another widget using the new implementation
ast-grep --pattern 'const derivedPropertyFns = {
$$$
}' app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx
Length of output: 232
app/client/src/widgets/ListWidgetV2/widget/index.tsx (2)
10-11: LGTM! Good separation of concerns.
The split between derived property functions and parsing utility improves modularity.
287-291: Verify consistent usage of parseDerivedProperties across widgets.
Let's verify that other widgets follow the same pattern for derived properties.
✅ Verification successful
Changes follow consistent pattern across widgets
The implementation of parseDerivedProperties in ListWidgetV2 follows the established pattern used by other widgets in the codebase. Several widgets like TableWidgetV2, TabsWidget, MultiSelectTreeWidget, and DatePickerWidget2 use the same approach of parsing derived properties with the utility function.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of parseDerivedProperties across widgets
# Search for getDerivedPropertiesMap implementations
ast-grep --pattern 'static getDerivedPropertiesMap() {
$$$
}'
# Search for parseDerivedProperties usage
rg "parseDerivedProperties\(" --type ts --type tsx
Length of output: 146
Script:
#!/bin/bash
# Search for getDerivedPropertiesMap implementations with correct file type
rg "static getDerivedPropertiesMap\(\)" -t ts -t typescript
# Search for parseDerivedProperties usage with correct file type
rg "parseDerivedProperties\(" -t ts -t typescript
# Find all widget files to understand the context
fd "widget" -e ts -e tsx
Length of output: 20617
Script:
#!/bin/bash
# Search for files in the ListWidgetV2 directory
fd . app/client/src/widgets/ListWidgetV2/
# Look for getDerivedPropertiesMap and parseDerivedProperties in the widget files
rg -g "*.{ts,tsx}" "getDerivedPropertiesMap|parseDerivedProperties" app/client/src/widgets/ListWidgetV2/
Length of output: 1616
Script:
#!/bin/bash
# Check the implementation of parseDerivedProperties utility
rg -g "*.{ts,tsx}" "export.*parseDerivedProperties" -A 10 app/client/src/widgets/WidgetUtils.ts
# Look for other widgets using getDerivedPropertiesMap
rg -g "*.{ts,tsx}" "static getDerivedPropertiesMap\(\)" -A 5 app/client/src/widgets/
Length of output: 24932
app/client/src/widgets/TableWidget/widget/index.tsx (2)
42-43: LGTM! Clean import refactoring.
The imports have been updated to use the new centralized utility for parsing derived properties.
429-439: LGTM! Clean implementation of getDerivedPropertiesMap.
The method now uses the centralized parseDerivedProperties utility while maintaining the same derived properties functionality. The implementation is more maintainable and consistent with other widgets.
app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/widget/index.tsx (1)
197-198: Good use of parseDerivedProperties
The use of parseDerivedProperties with derivedPropertyFns is correct and helps in managing derived properties efficiently.
app/client/src/widgets/ListWidget/widget/index.tsx (2)
70-71: Imports look good.
The new imports are correctly added and renamed as per the refactoring.
610-616: Refactored derived properties are correctly applied.
The getDerivedPropertiesMap method has been updated to use parsedDerivedProperties, which aligns with the new utility function implementation.
app/client/src/widgets/TableWidgetV2/widget/index.tsx (3)
66-67: Imports updated correctly
The imports now include derivedPropertyFns and parseDerivedProperties, reflecting the new parsing mechanism for derived properties.
489-502: Derived properties map updated to use parsed functions
The getDerivedPropertiesMap method now utilizes parsedDerivedProperties, ensuring that derived properties are processed consistently.
486-487: Verify all parsed derived properties are defined and exported
Ensure all required functions are present in derivedPropertyFns and are correctly exported.
Run the following script to verify that all necessary functions are exported:
✅ Verification successful
All required derived properties are correctly defined and exported
The verification confirms that all necessary derived properties are properly defined in the derived.js file and are being correctly parsed by parseDerivedProperties. The file includes all the required functions:
- getSelectedRow
- getTriggeredRow
- getSelectedRows
- getPageSize
- getProcessedTableData
- getOrderedTableColumns
- getFilteredTableData
- getUpdatedRows
- getUpdatedRowIndices
- getUpdatedRow
- getPageOffset
- getEditableCellValidity
- getTableHeaders
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all necessary functions are exported
# List of required functions
required_functions=(
"getSelectedRow"
"getTriggeredRow"
"getSelectedRows"
"getPageSize"
"getProcessedTableData"
"getOrderedTableColumns"
"getFilteredTableData"
"getUpdatedRows"
"getUpdatedRowIndices"
"getUpdatedRow"
"getPageOffset"
"getEditableCellValidity"
"getTableHeaders"
)
# Use ast-grep to find exported function declarations
for func in "${required_functions[@]}"; do
ast-grep --pattern "export function $func() { $$$ }" || echo "Function $func is missing or not exported"
done
Length of output: 1715
Script:
#!/bin/bash
# Let's try a different approach to find the derived properties
# First, let's find where derivedPropertyFns is defined
rg -l "derivedPropertyFns" --type ts
# Then let's look for any export statements related to these functions
rg "export const|export function" --type ts | grep -E "getSelected|getTriggered|getPage|getProcessed|getOrdered|getFiltered|getUpdated|getEditable|getTable"
# Also check for object-style exports that might contain these functions
ast-grep --pattern 'export const derivedPropertyFns = {
$$$
}'
Length of output: 8036
Script:
#!/bin/bash
# Let's look at the specific file content to see how derivedPropertyFns is defined
cat app/client/src/widgets/TableWidgetV2/widget/index.tsx
# Also check for any derived properties in nearby files
rg -l "getSelected|getTriggered|getPage|getProcessed|getOrdered|getFiltered|getUpdated|getEditable|getTable" app/client/src/widgets/TableWidgetV2/
Length of output: 96715
Script:
#!/bin/bash
# Let's check the derived.js file content since it contains the derived properties
cat app/client/src/widgets/TableWidgetV2/widget/derived.js
Length of output: 35022
| export function parseDerivedProperties(propertyFns: Record<string, unknown>) { | ||
| const derivedProperties: Record<string, string> = {}; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and error handling
The function should validate the input object and handle potential errors gracefully.
export function parseDerivedProperties(propertyFns: Record<string, unknown>) {
+ if (!propertyFns || typeof propertyFns !== 'object') {
+ throw new Error('Invalid propertyFns: Expected a non-null object');
+ }
const derivedProperties: Record<string, string> = {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function parseDerivedProperties(propertyFns: Record<string, unknown>) { | |
| const derivedProperties: Record<string, string> = {}; | |
| export function parseDerivedProperties(propertyFns: Record<string, unknown>) { | |
| if (!propertyFns || typeof propertyFns !== 'object') { | |
| throw new Error('Invalid propertyFns: Expected a non-null object'); | |
| } | |
| const derivedProperties: Record<string, string> = {}; | |
| const modifiedBody = functionBody | ||
| .trim() | ||
| .replace(new RegExp(`${propsParam}\\.`, "g"), "this."); | ||
|
|
||
| derivedProperties[key] = modifiedBody; |
There was a problem hiding this comment.
💡 Codebase verification
Add validation to prevent destructured props usage in derived properties
The code currently lacks validation to enforce the documented rule against destructuring props. Add a simple regex check in the parseDerivedProperties function:
// Add before modifiedBody assignment
if (/const.*?=.*?props/.test(functionBody)) {
throw new Error(`Destructuring props is not allowed in derived properties. Use props.value instead of const { value } = props`);
}🔗 Analysis chain
Add validation for destructured props usage
The comment mentions not to destructure props, but there's no validation to enforce this.
Let me analyze the code further to verify if validation for destructured props usage is needed.
Let me check if there's any validation being performed on the function body.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for destructured props usage in derived properties
# Search for destructuring patterns in derived property functions
rg -U "(?s)getDerivedPropertiesMap.*?\{.*?const.*?=.*?props.*?\}" --type ts
Length of output: 259937
Script:
#!/bin/bash
# Search for any existing validation logic around destructuring in the parseDerivedProperties function
rg -A 10 -B 10 "parseDerivedProperties" app/client/src/widgets/WidgetUtils.ts
Length of output: 861
Script:
#!/bin/bash
# Search for validation logic in the function body processing
rg -A 15 "functionBody" app/client/src/widgets/WidgetUtils.ts
Length of output: 523
| pageOffset: `{{(()=>{${derivedProperties.getPageOffset}})()}}`, | ||
| isEditableCellsValid: `{{(()=>{ ${derivedProperties.getEditableCellValidity}})()}}`, | ||
| tableHeaders: `{{(()=>{${derivedProperties.getTableHeaders}})()}}`, | ||
| processedTableData: `{{(()=>{${parsedDerivedProperties.getProcessedTableData}})()}}`, |
There was a problem hiding this comment.
Fix: Cannot access this.onRowSelected in a static method
The reference to this.onRowSelected inside the static method getDerivedPropertiesMap is invalid since this is undefined in static context.
Apply this diff to fix the issue:
-static getDerivedPropertiesMap() {
+getDerivedPropertiesMap() {
...
triggerRowSelection: "{{!!this.onRowSelected}}",
...
}Consider moving triggerRowSelection to an instance method or refactoring the code to avoid using this in a static context.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/index.tsx (1)
Line range hint
71-119: Consider addressing type assertions in property configurationsMultiple TODO comments and type assertions to 'any' indicate a need for proper typing. Consider creating proper type definitions for property configurations to improve type safety.
interface PropertyConfig { propertyName: string; placeholderText: string; // Add other necessary properties }app/client/src/widgets/CurrencyInputWidget/widget/index.tsx (2)
Line range hint
52-54: Remove TODO comments and fix eslint issuesThe function has multiple TODO comments about fixing eslint-disable. Consider addressing these TypeScript type issues now.
- // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - value: any, + value: string | number | undefined | null,Also applies to: 61-63
Line range hint
486-503: Handle potential edge cases in formatTextThe number formatting logic should handle edge cases like NaN or Infinity. Consider adding explicit checks.
formatText() { if (!!this.props.text && !this.isTextFormatted()) { try { const floatVal = parseFloat(this.props.text); + if (!Number.isFinite(floatVal)) { + return; + } const formattedValue = Intl.NumberFormat(getLocale(), {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/index.tsx(3 hunks)app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/widget/index.tsx(2 hunks)app/client/src/widgets/CurrencyInputWidget/widget/index.tsx(2 hunks)app/client/src/widgets/InputWidgetV2/widget/index.tsx(2 hunks)app/client/src/widgets/PhoneInputWidget/widget/index.tsx(2 hunks)
🧰 Additional context used
🪛 Biome
app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/widget/index.tsx
[error] 58-58: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
app/client/src/widgets/InputWidgetV2/widget/index.tsx
[error] 646-646: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🔇 Additional comments (9)
app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/widget/index.tsx (2)
10-11: LGTM: Import changes align with derived properties refactor
The import changes correctly support the transition to the new derived properties implementation.
56-60: LGTM: Verify derived properties behavior
The implementation correctly uses the new parseDerivedProperties utility while maintaining the inheritance chain.
Let's verify the derived properties implementation across the widget hierarchy:
✅ Verification successful
Implementation follows established widget patterns
The implementation correctly follows the widget hierarchy pattern:
- WDSInputWidget properly extends WDSBaseInputWidget
- parseDerivedProperties usage is consistent with other widgets in the codebase (found in 15 other widget implementations)
- The inheritance chain through super.getDerivedPropertiesMap() is maintained correctly
The pattern matches the established widget architecture and is safe to approve.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent derived properties implementation across widget hierarchy
# Find the base widget implementation
echo "Checking base widget implementation..."
rg -l "class WDSBaseInputWidget" --type typescript
# Check for similar derived properties refactoring pattern
echo "Checking derived properties pattern..."
rg "parseDerivedProperties\(derivedPropertyFns\)" --type typescript
Length of output: 366
Script:
#!/bin/bash
# Search for base widget implementation and derived properties pattern without typescript extension
echo "Checking base widget implementation..."
rg -l "class WDSBaseInputWidget"
echo "Checking derived properties pattern..."
rg "parseDerivedProperties\(derivedPropertyFns\)"
# Search for getDerivedPropertiesMap implementations
echo "Checking getDerivedPropertiesMap implementations..."
ast-grep --pattern 'getDerivedPropertiesMap() {
$$$
}'
# Find all files containing WDSInputWidget
echo "Finding WDSInputWidget related files..."
fd "WDSInputWidget"
Length of output: 2910
Script:
#!/bin/bash
# Check the implementation of getDerivedPropertiesMap in WDSBaseInputWidget
echo "Checking WDSBaseInputWidget getDerivedPropertiesMap..."
rg "getDerivedPropertiesMap" app/client/src/modules/ui-builder/ui/wds/WDSBaseInputWidget/widget/index.tsx -A 5
# Check the inheritance chain
echo "Checking inheritance chain..."
rg "class WDSInputWidget extends" app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/widget/index.tsx -A 2
Length of output: 530
🧰 Tools
🪛 Biome
[error] 58-58: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/index.tsx (1)
9-9: LGTM: Import changes align with derived properties refactoring
Also applies to: 22-22, 24-24
app/client/src/widgets/PhoneInputWidget/widget/index.tsx (2)
17-18: LGTM: Clean import refactor
The import changes align with the new derived properties pattern being implemented across widgets.
299-302: Verify derived property behavior remains consistent
The implementation looks correct, but we should ensure the parsed isValid property maintains the same validation behavior as before.
✅ Verification successful
isValid property implementation verified and consistent
The derived property implementation maintains the same validation behavior. The isValid function in derived.js properly handles:
- Required field validation
- Empty value checks
- Custom validation prop
- Regex validation with proper parsing of regex patterns and flags
The extensive test coverage in derived.test.ts confirms all these validation scenarios work as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare the implementation of isValid property before and after the refactor
# Search for previous implementation
rg -p "isValid.*PhoneInputWidget" --type ts
# Search for new implementation in derived.ts
fd -e ts -e tsx derived.ts -x cat {} \; | grep -A 10 "isValid"
Length of output: 111
Script:
#!/bin/bash
# Let's try a broader search to find the implementation
# Search for isValid property in the PhoneInputWidget directory
rg -p "isValid" "app/client/src/widgets/PhoneInputWidget" -t ts -t tsx
# Search for derived properties implementation
rg -p "derivedPropertyFns|parseDerivedProperties" "app/client/src/widgets/PhoneInputWidget" -t ts -t tsx -A 5
Length of output: 234
Script:
#!/bin/bash
# Let's try without file type specifications since tsx isn't recognized
# Search for isValid property in the PhoneInputWidget directory
rg -p "isValid" "app/client/src/widgets/PhoneInputWidget"
# Search for derived properties implementation
rg -p "derivedPropertyFns|parseDerivedProperties" "app/client/src/widgets/PhoneInputWidget"
# Also search for the derived properties file
fd derived app/client/src/widgets/PhoneInputWidget -x cat {}
Length of output: 4509
app/client/src/widgets/CurrencyInputWidget/widget/index.tsx (2)
16-17: LGTM: Clean import refactoring
The imports align with the centralized derived properties approach.
426-430: Verify derived properties behavior remains consistent
While the implementation looks correct, we should ensure the derived properties behavior remains unchanged after the refactoring.
✅ Verification successful
Derived properties behavior remains consistent
The implementation maintains consistent behavior:
- The
parseDerivedPropertiesutility correctly transforms the derived property functions into string expressions - The derived property functions (
isValidandvalue) maintain their original logic - The wrapping of derived properties in IIFE
{{(()=>{...})()}}ensures proper evaluation context
The refactoring successfully preserves the widget's derived properties functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other references to the old derived properties implementation
# and verify the consistency of the new implementation
# Check for any remaining old implementation files
fd -g "*parsedDerivedProperties.ts" -g "*parseDerivedProperties.ts"
# Check for any direct references to derived properties in the widget
rg -l "isValid:|value:" "app/client/src/widgets/CurrencyInputWidget"
Length of output: 648
Script:
#!/bin/bash
# Let's check the implementation details of derived properties in both old and new files
rg -A 10 "parseDerivedProperties" app/client/src/widgets/CurrencyInputWidget/widget/
# Check the test files to understand the expected behavior
cat app/client/src/widgets/CurrencyInputWidget/widget/derived.test.ts
# Check if there are any other widgets using similar derived properties pattern
rg "parseDerivedProperties" --type ts --type tsx
Length of output: 3742
Script:
#!/bin/bash
# Let's check the implementation of parseDerivedProperties in WidgetUtils
cat app/client/src/widgets/WidgetUtils.ts
# Let's check the derived.js file that contains the actual derived property logic
cat app/client/src/widgets/CurrencyInputWidget/widget/derived.js
# Check for similar patterns in other widgets with correct file type flags
rg -t typescript -t javascriptreact -t typescriptreact "parseDerivedProperties"
Length of output: 33784
app/client/src/widgets/InputWidgetV2/widget/index.tsx (2)
42-43: LGTM: Import changes align with derived properties refactoring
644-648: LGTM: Derived properties implementation
The implementation correctly uses the new parsing mechanism while preserving the inheritance chain through merge.
🧰 Tools
🪛 Biome
[error] 646-646: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
| const parsedDerivedProperties = parseDerivedProperties(derivedPropertyFns); | ||
|
|
||
| return { | ||
| isValid: `{{(() => {${derivedProperties.isValid}})()}}`, | ||
| isValid: `{{(() => {${parsedDerivedProperties.isValid}})()}}`, | ||
| }; |
There was a problem hiding this comment.
💡 Codebase verification
Derived properties implementation is incomplete
The implementation is missing the derived.ts file that should contain the isValid property function. This file needs to be created to complete the derived properties migration.
- Create
derived.tsin theapp/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/directory - Implement the
isValidderived property function
🔗 Analysis chain
Verify derived properties migration
The implementation looks good, using the new parseDerivedProperties utility. Let's verify the derived properties implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the derived properties implementation
# Check if the derivedPropertyFns module exports the expected properties
# Look for isValid implementation in derived.ts
fd -e ts -e tsx -x grep -l "isValid.*export" {}
Length of output: 2514
Script:
#!/bin/bash
# Let's try a different approach to find the derived properties implementation
# First, let's find the derived.ts file in the WDSCurrencyInputWidget directory
fd -t f "derived.ts" app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget
# Then let's examine its content if found
fd -t f "derived.ts" app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget -x cat {}
# Also check for any other files that might contain the derived properties implementation
rg -l "derivedPropertyFns" app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget
Length of output: 353
Script:
#!/bin/bash
# Let's examine the content of index.tsx to understand the derivedPropertyFns implementation
cat app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/index.tsx
# Also search for any parseDerivedProperties utility implementation
rg -g '!*.d.ts' -l "parseDerivedProperties" app/client/src/modules/ui-builder/ui/wds
Length of output: 11154
Script:
#!/bin/bash
# Let's check the derived.ts file content to verify the isValid implementation
cat app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/derived.ts
# Also check the parseDerivedProperties utility implementation
rg -g '!*.d.ts' "export.*parseDerivedProperties" -A 5
Length of output: 767
| const parsedDerivedProperties = parseDerivedProperties(derivedPropertyFns); | ||
|
|
||
| return merge(super.getDerivedPropertiesMap(), { | ||
| isValid: `{{(() => {${derivedProperties.isValid}})()}}`, | ||
| isValid: `{{(() => {${parsedDerivedProperties.isValid}})()}}`, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace super with class name in static context
Using super in a static context can be confusing. Consider using the class name instead.
static getDerivedPropertiesMap(): DerivedPropertiesMap {
const parsedDerivedProperties = parseDerivedProperties(derivedPropertyFns);
- return merge(super.getDerivedPropertiesMap(), {
+ return merge(BaseInputWidget.getDerivedPropertiesMap(), {
isValid: `{{(() => {${parsedDerivedProperties.isValid}})()}}`,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const parsedDerivedProperties = parseDerivedProperties(derivedPropertyFns); | |
| return merge(super.getDerivedPropertiesMap(), { | |
| isValid: `{{(() => {${derivedProperties.isValid}})()}}`, | |
| isValid: `{{(() => {${parsedDerivedProperties.isValid}})()}}`, | |
| }); | |
| const parsedDerivedProperties = parseDerivedProperties(derivedPropertyFns); | |
| return merge(BaseInputWidget.getDerivedPropertiesMap(), { | |
| isValid: `{{(() => {${parsedDerivedProperties.isValid}})()}}`, | |
| }); |
🧰 Tools
🪛 Biome
[error] 646-646: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/index.tsx (3)
9-9: Remove unnecessary blank line in imports sectionimport { CurrencyInputComponent } from "../component"; - import {
Line range hint
183-195: Consider using specific error types for better error trackingWhile the error handling is good, using specific error types would help with better error tracking and debugging.
- } catch (e) { + } catch (e: Error) { formattedValue = value; - log.error(e); + log.error(`Currency formatting error: ${e.message}`); Sentry.captureException(e); }
Line range hint
208-267: Consider breaking down onFocusChange into smaller functionsThe method handles multiple responsibilities (focus state, formatting/deformatting, error handling). Consider extracting the formatting logic into separate methods for better maintainability.
Example refactor:
private handleFocus() { const text = this.props.parsedText || ""; const deFormattedValue = this.deformatValue(text); this.updateTextAndTriggerEvent(deFormattedValue, true); } private handleBlur() { if (this.props.parsedText) { const formattedValue = this.formatValue(this.props.parsedText); this.updateTextAndTriggerEvent(formattedValue, false); } } private deformatValue(text: string): string { return text.replace( new RegExp("\\" + getLocaleThousandSeparator(), "g"), "", ); } private formatValue(text: string): string { return formatCurrencyNumber(this.props.decimals, text); } private updateTextAndTriggerEvent(value: string, isFocused: boolean) { this.props.updateWidgetMetaProperty("parsedText", value); this.props.updateWidgetMetaProperty("isFocused", isFocused, { triggerPropertyName: isFocused ? "onFocus" : "onBlur", dynamicString: isFocused ? this.props.onFocus : this.props.onBlur, event: { type: isFocused ? EventType.ON_FOCUS : EventType.ON_BLUR, }, }); }app/client/src/widgets/CurrencyInputWidget/widget/index.tsx (1)
Line range hint
703-709: Consider consolidating decimal-related propertiesThe interface has both
noOfDecimalsanddecimalsproperties. This could lead to confusion and potential inconsistencies. Consider:
- Using only one property for decimal places
- Documenting the difference if both properties serve different purposes
export interface CurrencyInputWidgetProps extends BaseInputWidgetProps { countryCode?: string; currencyCode?: string; - noOfDecimals?: number; allowCurrencyChange?: boolean; decimals?: number; defaultText?: number; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/index.tsx(3 hunks)app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/widget/index.tsx(2 hunks)app/client/src/widgets/CurrencyInputWidget/widget/index.tsx(2 hunks)app/client/src/widgets/InputWidgetV2/widget/index.tsx(2 hunks)app/client/src/widgets/PhoneInputWidget/widget/index.tsx(2 hunks)
🧰 Additional context used
🪛 Biome
app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/widget/index.tsx
[error] 58-58: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
app/client/src/widgets/InputWidgetV2/widget/index.tsx
[error] 646-646: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🔇 Additional comments (10)
app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/widget/index.tsx (2)
10-11: LGTM: Import changes align with derived properties refactor
The updated imports follow the new pattern for handling derived properties across widgets.
56-60: Implementation looks good, but consider static method best practices
The derived properties implementation correctly uses the new utility while maintaining inheritance behavior. However, there's a minor consideration about using super in a static context.
Let's verify this pattern is consistent across other widgets:
🧰 Tools
🪛 Biome
[error] 58-58: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/index.tsx (1)
125-129: Verify derivedPropertyFns implementation
The refactoring looks good, but let's ensure the imported derivedPropertyFns contains the correct implementation for isValid.
app/client/src/widgets/PhoneInputWidget/widget/index.tsx (2)
17-18: LGTM: Clean import refactoring
The import changes align with the centralized derived properties parsing approach.
299-302: LGTM: Clean derived properties implementation
The getDerivedPropertiesMap implementation correctly uses the new centralized parsing utility while maintaining the same functionality.
app/client/src/widgets/CurrencyInputWidget/widget/index.tsx (2)
16-17: LGTM! Import changes align with derived properties refactoring
426-430: LGTM! Verify derived properties implementation across usages
The implementation correctly uses the new parseDerivedProperties utility. Let's verify the derived properties are working as expected.
app/client/src/widgets/InputWidgetV2/widget/index.tsx (3)
42-43: LGTM: Import changes align with derived properties refactoring
646-646: Static analysis warning can be safely ignored
The use of super in this static context is a common and accepted pattern for widget inheritance in this codebase.
🧰 Tools
🪛 Biome
[error] 646-646: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
644-647: LGTM: Clean implementation of derived properties
The implementation correctly uses the new parseDerivedProperties utility while maintaining the inheritance chain.
Let's verify that all widget implementations follow this pattern:
🧰 Tools
🪛 Biome
[error] 646-646: Using super in a static context can be confusing.
super refers to a parent class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/src/modules/ui-builder/ui/wds/WDSPhoneInputWidget/widget/index.tsx(2 hunks)
🔇 Additional comments (1)
app/client/src/modules/ui-builder/ui/wds/WDSPhoneInputWidget/widget/index.tsx (1)
111-114: LGTM: Clean implementation of derived properties
The implementation correctly uses the new parseDerivedProperties utility and properly wraps the derived property in an IIFE.
| @@ -15,7 +15,8 @@ import { WDSBaseInputWidget } from "modules/ui-builder/ui/wds/WDSBaseInputWidget | |||
| import { AsYouType, parseIncompletePhoneNumber } from "libphonenumber-js"; | |||
| import { EventType } from "constants/AppsmithActionConstants/ActionConstants"; | |||
| import type { KeyDownEvent } from "modules/ui-builder/ui/wds/WDSBaseInputWidget/component/types"; | |||
| import derivedProperties from "modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/parsedDerivedProperties"; | |||
| import derivedPropertyFns from "modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/derived"; | |||
There was a problem hiding this comment.
Fix incorrect import path for derivedPropertyFns
The derivedPropertyFns import appears to be from WDSCurrencyInputWidget instead of the current widget's path.
Apply this fix:
-import derivedPropertyFns from "modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/derived";
+import derivedPropertyFns from "./derived";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import derivedPropertyFns from "modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/derived"; | |
| import derivedPropertyFns from "./derived"; |
| const parsedDerivedProperties = parseDerivedProperties(derivedPropertyFns); | ||
|
|
||
| return { | ||
| isValid: `{{(() => {${derivedProperties.isValid}})()}}`, | ||
| isValid: `{{(() => {${parsedDerivedProperties.isValid}})()}}`, |
There was a problem hiding this comment.
💡 Codebase verification
Potential issue with derived properties import
The widget is importing derived properties from WDSCurrencyInputWidget instead of having its own implementation:
import derivedPropertyFns from "modules/ui-builder/ui/wds/WDSCurrencyInputWidget/widget/derived";This appears to be incorrect as phone input validation would differ from currency input validation.
🔗 Analysis chain
Verify complete migration of derived properties
Let's ensure no derived properties were missed during the migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining old derived properties implementation
rg -l "getDerivedPropertiesMap.*derivedProperties" "app/client/src/modules/ui-builder/ui/wds"
# Search for consistency in new implementation
rg -l "getDerivedPropertiesMap.*parseDerivedProperties" "app/client/src/modules/ui-builder/ui/wds"
Length of output: 192
Script:
#!/bin/bash
# Search for derived properties in the WDSPhoneInputWidget directory
rg -A 5 "derivedProperties|parseDerivedProperties" "app/client/src/modules/ui-builder/ui/wds/WDSPhoneInputWidget"
# Search for getDerivedPropertiesMap implementation
ast-grep --pattern 'getDerivedPropertiesMap() {
$$$
}'
Length of output: 1550
Script:
#!/bin/bash
# Search for all derived properties in the widget
ast-grep --pattern 'const derivedPropertyFns = {
$$$
}'
# Check for any other derived properties related functions
rg -A 3 "getDerivedProperties|derivedPropertyFns" "app/client/src/modules/ui-builder/ui/wds/WDSPhoneInputWidget"
Length of output: 1297
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
/ok-to-test tags="@tag.Widget"
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11836752809
Commit: e895c38
Cypress dashboard.
Tags: @tag.Widget
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/Apps/CommunityIssues_Spec.ts
- cypress/e2e/Regression/Apps/PromisesApp_spec.js
- cypress/e2e/Regression/ClientSide/BugTests/AllWidgets_Reset_Spec.ts
- cypress/e2e/Regression/ClientSide/BugTests/Moment_Spec.ts
- cypress/e2e/Regression/ClientSide/BugTests/SelectWidget_Bug9334_Spec.ts
- cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.ts
- cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/OneClickBindingMysql_spec.ts
- cypress/e2e/Regression/ClientSide/OtherUIFeatures/Widget_Error_spec.js
- cypress/e2e/Regression/ClientSide/SetProperty/WidgetPropertySetters1_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/Button/Button_onClickAction_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/CurrencyInput/CurrencyInputDynamicCurrencyCode_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker_Toggle_js_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Dropdown/DropDownWidget_value_reset_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Dropdown/Dropdown_onOptionChange_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Dropdown/Dropdown_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Filepicker/FilePickerV2_CSV_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Form/FormData_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Form/FormReset_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Form/FormWidget_Select_TreeSelect_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Form/FormWidget_With_Input_Number.js
- cypress/e2e/Regression/ClientSide/Widgets/Input/Input2_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/Input/Input_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Input/Inputv2_inside_List_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Input/Inputv2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONFrom_Modal_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/List/List4_1_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/List/List4_2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Childwigets/List_Select_Widgets_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Childwigets/List_Table_Widgets_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_NestedList_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_nested_List_widget_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicChildWidgetInteraction_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_Meta_Hydration_ClientSide_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_Meta_Hydration_ServerSide_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_autocomplete_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_onItemClick_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Modal/Modal_On_Table_Filter_Pane_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/Modal/Modal_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect1_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect4_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect5_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect_label_value.ts
- cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiTreeSelect_2_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiTreeSelect_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Multiselect/Multi_Select_Tree_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/PhoneInput/PhoneInputDynamicValue_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Radio/Radio_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Select/Select2_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/Select/Select3_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/Select/Select_TreeSelect_MultiSelect_OnFocus_OnBlur_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Select/Select_Validation_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Select/Select_label_value.ts
- cypress/e2e/Regression/ClientSide/Widgets/Select/Select_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Select/select_Widget_Bug_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Switch/Switch2_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/Switch/Switch_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/TableBugs_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/TableFilter1_1_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/TableFilter1_2_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/TableFilter2_1_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/TableFilter2_2_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Button_Icon_validation_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Color_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Derived_Column_Data_validation_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_EmptyRow_Color_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_FilteredTableData_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_GeneralProperty_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_MultiRowSelect_dataUpdation_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_MultiRowSelect_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Number_column_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_PropertyPane_1_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_PropertyPane_2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_PropertyPane_IconName_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Property_JsonUpdate_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Switch_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Widget_Add_button_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Widget_Default_Row_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Widget_Derived_Column_Computed_value_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Widget_Selected_row_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_tabledata_schema_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV1/table_with_text_selRowIndices_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow3_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Custom_column_alias_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Date_column_editing_1_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Date_column_editing_2_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Date_column_types_validation_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Edge_case_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Image_resize_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Inline_editing_1_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Inline_editing_2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Inline_editing_3_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_1_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_2_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_1_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Button_Icon_validation_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Color_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Column_Order_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Derived_Column_Data_validation_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_DisplayText_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_FilteredTableData_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_GeneralProperty_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_MultiRowSelect_dataUpdation_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_MultiRowSelect_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_PropertyPane_1_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_PropertyPane_2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_PropertyPane_IconName_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Property_JsonUpdate_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Switch_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Url_Column_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Widget_Add_button_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Widget_Default_Row_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Widget_Derived_Column_Computed_value_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Widget_Selected_row_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_header_menu_visibility_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_misc.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_pagination_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_tabledata_schema_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/Text_wrapping_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/VirtualRow_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/ColumnTypeMenubutton_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/ColumnTypesSelect2_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Table_select_validation_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/checkboxCell_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/currency_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/switchCell_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/freeze_column1_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/freeze_column2_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/freeze_column3_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/freeze_column_query_change_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/inline_editing_validations_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/non_ascii_column_name_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/server_side_filtering_spec_1.ts
- cypress/e2e/Regression/ClientSide/Widgets/TableV2/table_data_change_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Single_Select_Tree_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Tree_Select_2_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Tree_Select_spec.ts
List of identified flaky tests.Thu, 14 Nov 2024 13:56:38 UTC
Summary by CodeRabbit
New Features
parseDerivedPropertiesfor better handling of derived properties across various widgets.noOfDecimalsto theCurrencyInputWidgetfor managing decimal precision.WDSCurrencyInputWidgetandWDSPhoneInputWidget.Bug Fixes
Documentation
WDSCurrencyInputWidgetfor clearer user guidance.Refactor