feat: unified response view component #37897
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12175257559. |
|
Deploy-Preview-URL: https://ce-37897.dp.appsmith.com |
![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
🧹 Outside diff range and nitpick comments (11)
app/client/src/IDE/Components/ResponseView/components/Response/styles.ts (1)
85-91: Consider adding text color variableThe ErrorContent component defines font properties but doesn't specify a text color. Consider using the design system's error text color variable for consistency.
export const ErrorContent = styled.div` display: flex; align-items: flex-start; gap: 4px; font-size: 12px; line-height: 16px; + color: var(--ads-v2-color-fg-error); `;app/client/src/IDE/Components/ResponseView/components/Response/components/ContentTypeSelector/styles.ts (1)
1-15: LGTM with a minor suggestionThe implementation is clean and follows best practices. Consider making the transition more specific by using
transition: opacity 0.25s ease-in-outfor smoother animation.- transition: opacity 0.25s; + transition: opacity 0.25s ease-in-out;app/client/src/IDE/Components/ResponseView/components/Response/Response.tsx (1)
171-185: SimplifyhandleContentTypeChangefunctionConsider refactoring the event handling to use
event.currentTargetwith proper typing or directly accessvalueif available. This can make the code more concise and type-safe.app/client/src/IDE/Components/ResponseView/components/Response/utils/checkForPreparedStatement.ts (1)
3-26: RefactorcheckForPreparedStatementfor clarityYou can simplify the function by combining conditions using logical OR and optional chaining.
Refactored code:
export function checkForPreparedStatement(action: Action) { if (action.pluginType !== PluginType.DB) { return false; } return ( action.actionConfiguration?.pluginSpecifiedTemplates?.[0]?.value === true || action.actionConfiguration?.formData?.preparedStatement?.data === true ); }app/client/src/IDE/Components/ResponseView/components/Response/utils/parseActionResponse.ts (1)
29-29: Use optional chaining to simplify the conditionYou can simplify the check for messages by using optional chaining.
Apply this change:
- if (actionResponse.messages && actionResponse.messages.length) { + if (actionResponse.messages?.length) {🧰 Tools
🪛 Biome (1.9.4)
[error] 29-29: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/IDE/Components/ResponseView/components/Response/components/ContentTypeSelector/ContentTypeSelector.tsx (1)
15-20: Consider adding prop validation for contentTypeOptionsThe interface looks good, but consider adding validation for the contentTypeOptions array to ensure it's not empty.
interface ContentTypeSelectorProps { currentContentType: string; - contentTypeOptions: Array<{ label: string; value: string }>; + contentTypeOptions: Array<{ label: string; value: string }> & { length: number }; isHovered: boolean; handleContentTypeChange: (e?: Event) => void; }app/client/src/IDE/Components/ResponseView/ResponseView.tsx (1)
Line range hint
29-35: Address TODO comment about debugger stateThere's a TODO comment about combining API and Query Debugger state. This should be tracked for future improvement.
Would you like me to create a GitHub issue to track this technical debt?
app/client/src/IDE/Components/ResponseView/components/Response/components/ErrorView/ErrorView.tsx (2)
69-72: Simplify nested conditionals using optional chainingThe error message rendering logic can be simplified using optional chaining.
- {actionResponse && - (actionResponse.pluginErrorDetails ? ( + {actionResponse?.pluginErrorDetails ? ( <> <div data-testid="t--response-error"> {actionResponse.pluginErrorDetails.downstreamErrorMessage || actionResponse.pluginErrorDetails.appsmithErrorMessage} </div> {actionResponse.pluginErrorDetails.downstreamErrorCode && ( <LogAdditionalInfo text={actionResponse.pluginErrorDetails.downstreamErrorCode} /> )} </> ) : ( - actionResponse.body && + actionResponse?.body && action.pluginType === PluginType.DB && ( <div data-testid="t--response-error">{actionResponse.body}</div> ) ))}Also applies to: 73-91
101-108: Use optional chaining for request checkSimplify the request existence check using optional chaining.
- {actionResponse && actionResponse.request && ( + {actionResponse?.request && ( <JsonWrapper className="t--debugger-log-state" onClick={handleJsonWrapperClick} > <ReactJson src={updateTimestamp} {...reactJsonParams} /> </JsonWrapper> )}🧰 Tools
🪛 Biome (1.9.4)
[error] 101-101: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/IDE/Components/ResponseView/components/Response/Response.test.tsx (2)
17-27: Consider adding type assertions for nested objectsWhile the props structure is correct, consider adding type assertions for nested objects to improve type safety.
action: { id: "test-action-id", name: "Test Action", - actionConfiguration: { pluginSpecifiedTemplates: [{ value: true }] }, + actionConfiguration: { + pluginSpecifiedTemplates: [{ value: true }] as Array<{ value: boolean }> + }, userPermissions: ["execute"] as const, } as Action,
133-143: Consider adding more error scenariosWhile the current test cases cover basic prepared statement scenarios, consider adding tests for:
- Network timeouts
- Malformed responses
- Different plugin types
Example test case structure:
it("should handle network timeout gracefully", () => { const props = { ...defaultProps, actionResponse: { isExecutionSuccess: false, readableError: "Network timeout", } as ActionResponse, }; // Add test implementation });Also applies to: 184-192
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (37)
app/client/src/IDE/Components/ResponseView/ResponseView.tsx(1 hunks)app/client/src/IDE/Components/ResponseView/components/ApiFormatSegmentedResponse.tsx(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/Response.test.tsx(6 hunks)app/client/src/IDE/Components/ResponseView/components/Response/Response.tsx(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/components/ContentTypeSelector/ContentTypeSelector.tsx(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/components/ContentTypeSelector/index.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/components/ContentTypeSelector/styles.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/components/ErrorView/ErrorView.tsx(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/components/ErrorView/index.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/components/index.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/constants.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/index.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/styles.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/utils/checkForPreparedStatement.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/utils/index.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/Response/utils/parseActionResponse.ts(1 hunks)app/client/src/IDE/Components/ResponseView/components/ResponseFormatTabs.tsx(1 hunks)app/client/src/IDE/Components/ResponseView/components/Schema/CurrentDataSourceLink.tsx(1 hunks)app/client/src/IDE/Components/ResponseView/index.ts(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/index.ts(0 hunks)app/client/src/PluginActionEditor/index.ts(1 hunks)app/client/src/PluginActionEditor/store/pluginEditorReducer.ts(1 hunks)app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx(3 hunks)app/client/src/ce/navigation/FocusElements/AppIDE.ts(1 hunks)app/client/src/components/editorComponents/ApiResponseView.tsx(3 hunks)app/client/src/components/editorComponents/Debugger/DebuggerTabs.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/Resizer/index.tsx(1 hunks)app/client/src/components/editorComponents/JSResponseView.tsx(3 hunks)app/client/src/pages/Editor/DataSourceEditor/Debugger.tsx(1 hunks)app/client/src/pages/Editor/DatasourceInfo/DatasourceViewModeSchema.tsx(1 hunks)app/client/src/pages/Editor/DatasourceInfo/GoogleSheetSchema.tsx(1 hunks)app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx(1 hunks)app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx(3 hunks)app/client/src/pages/Editor/__tests__/QueryEditorTable.test.tsx(1 hunks)app/client/src/reducers/uiReducers/datasourcePaneReducer.ts(1 hunks)app/client/src/reducers/uiReducers/debuggerReducer.ts(1 hunks)app/client/src/reducers/uiReducers/jsPaneReducer.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/client/src/PluginActionEditor/components/PluginActionResponse/index.ts
✅ Files skipped from review due to trivial changes (20)
- app/client/src/IDE/Components/ResponseView/components/Response/components/ContentTypeSelector/index.ts
- app/client/src/IDE/Components/ResponseView/components/Response/components/ErrorView/index.ts
- app/client/src/IDE/Components/ResponseView/components/Schema/CurrentDataSourceLink.tsx
- app/client/src/pages/Editor/DatasourceInfo/GoogleSheetSchema.tsx
- app/client/src/reducers/uiReducers/datasourcePaneReducer.ts
- app/client/src/components/editorComponents/Debugger/DebuggerTabs.tsx
- app/client/src/reducers/uiReducers/jsPaneReducer.ts
- app/client/src/IDE/Components/ResponseView/index.ts
- app/client/src/IDE/Components/ResponseView/components/Response/utils/index.ts
- app/client/src/components/editorComponents/Debugger/Resizer/index.tsx
- app/client/src/ce/navigation/FocusElements/AppIDE.ts
- app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx
- app/client/src/pages/Editor/tests/QueryEditorTable.test.tsx
- app/client/src/reducers/uiReducers/debuggerReducer.ts
- app/client/src/IDE/Components/ResponseView/components/Response/components/index.ts
- app/client/src/PluginActionEditor/store/pluginEditorReducer.ts
- app/client/src/pages/Editor/DatasourceInfo/DatasourceViewModeSchema.tsx
- app/client/src/pages/Editor/DataSourceEditor/Debugger.tsx
- app/client/src/IDE/Components/ResponseView/components/ApiFormatSegmentedResponse.tsx
- app/client/src/IDE/Components/ResponseView/components/Response/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- app/client/src/components/editorComponents/ApiResponseView.tsx
- app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx
- app/client/src/components/editorComponents/JSResponseView.tsx
- app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/IDE/Components/ResponseView/components/Response/utils/parseActionResponse.ts
[error] 29-29: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/IDE/Components/ResponseView/components/Response/components/ErrorView/ErrorView.tsx
[error] 96-96: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 101-101: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (11)
app/client/src/IDE/Components/ResponseView/components/Response/styles.ts (2)
75-83: LGTM: ErrorContainer styling follows design system
The component correctly uses design system variables for colors and spacing.
93-95: LGTM: ErrorDefaultMessage uses flex-shrink appropriately
The flex-shrink: 0 property ensures the default message maintains its width.
app/client/src/IDE/Components/ResponseView/components/ResponseFormatTabs.tsx (2)
Line range hint 1-65: Clean implementation with proper type safety
The component is well-structured with:
- Clear type definitions
- Proper handling of different response formats
- Good separation of concerns
- Consistent error handling through default case
2-2: Verify the import path change impact
The import path has been updated as part of the response component unification. Let's verify that this change is consistent across the codebase.
✅ Verification successful
Import path changes are consistent
The new import path PluginActionEditor/constants/CommonApiConstants is consistently used across both files that import ResponseDisplayFormats. No old import paths remain in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining old import paths
rg "import.*ResponseDisplayFormats.*from.*constants/CommonApiConstants"
# Search for consistency in new import paths
rg "import.*ResponseDisplayFormats.*from.*PluginActionEditor/constants/CommonApiConstants"
Length of output: 898
app/client/src/IDE/Components/ResponseView/components/Response/components/ContentTypeSelector/styles.ts (1)
7-9: Verify parent container positioning
The absolute positioning requires a relatively positioned parent container to work correctly. Ensure the parent component has position: relative set.
✅ Verification successful
Let me search for the parent container's styles to verify the positioning context.
Let me search for the Response component's styles to verify the positioning context.
Let me try one more search to find any styles in the parent components.
Absolute positioning is correctly implemented
The parent DataContainer component has position: relative set, which provides the correct positioning context for the absolutely positioned ContentTypeSelector. The implementation is safe and follows best practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the parent component's styles
rg -A 5 "ContentTypeSelector" --type ts
Length of output: 3582
Script:
#!/bin/bash
# Search for styles in the Response component
rg -A 10 "const.*Styled" app/client/src/IDE/Components/ResponseView/components/Response/Response.tsx
# Search for any styled components in Response.tsx
ast-grep --pattern 'styled.$_`$$$`'
Length of output: 65793
Script:
#!/bin/bash
# Search for styled components in Response.tsx
rg -A 5 "styled\." app/client/src/IDE/Components/ResponseView/components/Response/Response.tsx
# Search for any container styles in Response.tsx
rg -A 5 "const.*Container" app/client/src/IDE/Components/ResponseView/components/Response/Response.tsx
Length of output: 201
Script:
#!/bin/bash
# Search for the Response component's content
cat app/client/src/IDE/Components/ResponseView/components/Response/Response.tsx
# Also check for any DataContainer styles since it might be the parent
rg -A 5 "DataContainer" app/client/src/IDE/Components/ResponseView/components/Response/
Length of output: 11703
app/client/src/IDE/Components/ResponseView/components/Response/Response.tsx (1)
135-137: Verify the condition for displaying the prepared statement warning
The condition checkForPreparedStatement(action) && errorMessage might prevent the warning from displaying when there is no error message. Ensure that the warning should only appear when both conditions are true.
app/client/src/IDE/Components/ResponseView/components/Response/constants.ts (1)
1-19: Constants are well-defined and enhance readability
The defined constants improve code clarity and maintainability.
app/client/src/PluginActionEditor/index.ts (1)
8-8: Export path for PluginActionResponse updated correctly
The export statement now correctly points to the new ResponseView component.
app/client/src/IDE/Components/ResponseView/components/Response/components/ContentTypeSelector/ContentTypeSelector.tsx (1)
37-46: LGTM! Good accessibility implementation
The Fab component implementation includes proper aria-label and data-testid attributes.
app/client/src/IDE/Components/ResponseView/ResponseView.tsx (1)
6-7: Verify import path changes across codebase
The import paths have been updated to reflect the new structure. Let's verify all references are updated.
Also applies to: 11-11
✅ Verification successful
The previous search might have been too specific. Let's broaden our search to find all occurrences of these imports and verify they are using the correct path structure.
Import paths are correctly updated across the codebase
All references to setPluginActionEditorDebuggerState and getPluginActionDebuggerState are using the correct import path from "PluginActionEditor/store". The imports in ResponseView.tsx are consistent with the codebase's usage pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for old import paths that might have been missed
rg -l "import.*from.*store.*pluginActionEditorDebuggerState" | grep -v "PluginActionEditor/store"
Length of output: 97
Script:
#!/bin/bash
# Let's try a more comprehensive search for both imports
rg "setPluginActionEditorDebuggerState|getPluginActionDebuggerState" -A 1
# Also search for any potential old paths
rg "pluginActionEditorDebuggerState" -A 1
Length of output: 7795
app/client/src/IDE/Components/ResponseView/components/Response/Response.test.tsx (1)
5-6: LGTM: Import statements are properly updated
The imports correctly reflect the transition to the unified Response component.
Also applies to: 11-12
app/client/src/IDE/Components/ResponseView/components/Response/Response.test.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
app/client/src/IDE/Components/ResponseView/components/Response/Response.tsx (3)
117-121: Use 1024 for kilobyte conversionWhen calculating the response size, dividing by 1024 is standard for converting bytes to kilobytes.
Apply this diff to adjust the size calculation:
- `${(Number(actionResponse.size) / 1000).toFixed(1)}kb`, + `${(Number(actionResponse.size) / 1024).toFixed(1)}KB`,
84-84: DefaultrecordCountto 0 instead of 1If
responseis undefined, settingrecordCountto 0 may be more accurate than defaulting to 1.Apply this diff to update the default value:
- const recordCount = response?.length ?? 1; + const recordCount = response?.length ?? 0;
159-174: ExcludedispatchfromuseMemodependency arrayThe
dispatchfunction from Redux is stable and doesn't need to be included in the dependency array.Apply this diff to refine the dependency array:
- }, [dispatch, isActionRedesignEnabled]); + }, [isActionRedesignEnabled]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
app/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
app/client/package.json(2 hunks)app/client/src/IDE/Components/ResponseView/components/Response/Response.tsx(1 hunks)
🔇 Additional comments (1)
app/client/package.json (1)
170-170: LGTM! Dependencies look good.
The addition of pluralize and its type definitions is appropriate for the unified response view component. Note that while pluralize@8.0.0 is the latest stable version, @types/pluralize@0.0.33 is quite old but it's the latest available type definition package.
Let's verify if there are any newer versions or known vulnerabilities:
Also applies to: 275-275
✅ Verification successful
Dependencies are up-to-date and secure
The verification confirms that both packages are using their latest available versions (pluralize@8.0.0 and @types/pluralize@0.0.33) with no known security vulnerabilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest versions and security advisories for pluralize
# Check npm registry for latest versions
echo "Latest versions from npm registry:"
curl -s https://registry.npmjs.org/pluralize | jq '.["dist-tags"].latest'
curl -s https://registry.npmjs.org/@types/pluralize | jq '.["dist-tags"].latest'
# Check for known vulnerabilities
echo "Checking for vulnerabilities:"
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "pluralize") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 666
alex-golovanov
force-pushed
the
feat/37759-unified-response-view
branch
from
89ee20e to
8e96f45
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/Response.tsx (1)
176-190: Specify the correct event type inhandleContentTypeChangeThe event
eis currently typed asEvent, which may not provide accurate type definitions. Updating it toReact.MouseEvent<HTMLElement>enhances type safety.Apply this diff to update the event type and remove unnecessary optional chaining:
-const handleContentTypeChange = useEventCallback((e?: Event) => { - if (e?.target && e.target instanceof HTMLElement) { +const handleContentTypeChange = useEventCallback((e: React.MouseEvent<HTMLElement>) => { + if (e.target instanceof HTMLElement) { const { value } = e.target.dataset; if (typeof value === "string") { dispatch( setActionResponseDisplayFormat({ id: action?.id || "", field: "responseDisplayFormat", value, }), ); } } });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
app/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
app/client/package.json(2 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/Response.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/package.json
🔇 Additional comments (1)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/Response.tsx (1)
57-307: Consider refactoring the Response component into smaller units
The Response component handles multiple responsibilities and could be split into smaller, single-responsibility components for better maintainability.
# Conflicts: # app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/styles.ts (2)
76-84: LGTM! Consider using design system spacing tokens.The ErrorContainer styling is well-structured and follows design system color conventions. However, the hardcoded padding and gap values could be replaced with design system spacing tokens for better consistency.
- padding: 8px 16px; - gap: 8px; + padding: var(--ads-v2-spaces-3) var(--ads-v2-spaces-4); + gap: var(--ads-v2-spaces-3);
86-92: LGTM! Consider using typography tokens.The ErrorContent component is well-structured. Consider using design system typography tokens for font-size and line-height if available.
- font-size: 12px; - line-height: 16px; + font-size: var(--ads-v2-font-size-3); + line-height: var(--ads-v2-line-height-3);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
app/client/cypress/support/Pages/DataSources.ts(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/styles.ts(1 hunks)app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx(3 hunks)app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx
- app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/support/Pages/DataSources.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
🔇 Additional comments (2)
app/client/cypress/support/Pages/DataSources.ts (1)
157-157: LGTM! Verify usage of the new error selector.
The change from t--query-error to t--response-error aligns with the PR's unification goals.
Let's verify the usage of this selector across the codebase:
✅ Verification successful
Selector change is correctly implemented
The old selector t--query-error has been completely removed from the codebase, and the new selector t--response-error is consistently used in both the test file and the actual component implementation in ErrorView.tsx.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining usages of the old selector and verify the new one is used consistently
# Check for old selector usage
echo "Checking for old selector usage..."
rg "t--query-error"
# Check for new selector usage
echo "Checking for new selector usage..."
rg "t--response-error"
Length of output: 678
app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/styles.ts (1)
94-96: LGTM!
The ErrorDefaultMessage component is simple and serves its purpose well.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/cypress/support/Pages/DebuggerHelper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/support/Pages/DebuggerHelper.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
## Description Common component for response tab view. Fixes appsmithorg#37759 ## Automation /ok-to-test tags="@tag.All" ### 🔍 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/12242471397> > Commit: 5263092 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12242471397&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Mon, 09 Dec 2024 21:00:53 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new `Response` component to handle and display action responses in the plugin action editor. - Streamlined response handling by consolidating components and updating props for better performance and user experience. - **Bug Fixes** - Enhanced error handling and response display logic to improve clarity and user interaction. - **Documentation** - Updated import paths and component references to reflect the new structure and naming conventions. - **Tests** - Modified Cypress tests to utilize the new `runQueryAndVerifyResponseViews` method for improved readability and maintainability. - Updated tests to replace direct interactions with UI elements by using methods from the `BottomTabs` module for selecting response types. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Description
Common component for response tab view.
Fixes #37759
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12242471397
Commit: 5263092
Cypress dashboard.
Tags:
@tag.AllSpec:
Mon, 09 Dec 2024 21:00:53 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Responsecomponent to handle and display action responses in the plugin action editor.Bug Fixes
Documentation
Tests
runQueryAndVerifyResponseViewsmethod for improved readability and maintainability.BottomTabsmodule for selecting response types.