chore: Opening response pane by default on query creation and for page load queries #37245
Conversation
WalkthroughThe Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
IDE Pod
|
/build-deploy-preview skip-tests=true |
github-actions
bot
added
the
skip-changelog
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11685527718. |
![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
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx(2 hunks)
🧰 Additional context used
🪛 Biome
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx
[error] 76-76: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
🔇 Additional comments (8)
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx (8)
11-14: New imports are appropriate
The added imports are necessary and correctly included.
18-18: Destructuring from usePluginActionContext
Correctly retrieves action, actionResponse, and plugin from the context.
21-23: Initialization of plugin-related variables
Variables pluginRequireDatasource and showSchema are properly initialized using the plugin data.
30-31: State variable for response visibility
showResponseOnFirstLoad state is correctly initialized to manage response tab visibility.
33-35: Retrieval of response display format
responseDisplayFormat is appropriately obtained from actionResponseDisplayDataFormats.
38-55: Opening response tab on initial load
The useEffect hook correctly opens the response tab when conditions are met.
62-71: Opening schema tab when applicable
The useEffect hook properly handles opening the schema tab based on showSchema and selectedTab.
75-79: Resetting response visibility flag on action change
Resets showResponseOnFirstLoad when a new action is detected to ensure consistent behavior.
🧰 Tools
🪛 Biome
[error] 76-76: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx
Outdated
Show resolved
Hide resolved
|
Deploy-Preview-URL: https://ce-37245.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx (1)
30-60: Consider simplifying response visibility conditionsThe useEffect contains multiple conditions that could be extracted into a helper function for better readability and maintainability.
Consider refactoring like this:
+ const shouldShowResponseOnLoad = () => { + return responseDisplayFormat?.title && + actionResponse?.isExecutionSuccess && + !showResponseOnFirstLoad; + }; useEffect(() => { - if ( - responseDisplayFormat && - !!responseDisplayFormat?.title && - actionResponse && - actionResponse.isExecutionSuccess && - !showResponseOnFirstLoad - ) { + if (shouldShowResponseOnLoad()) { dispatch( setPluginActionEditorDebuggerState({ open: true, selectedTab: DEBUGGER_TAB_KEYS.RESPONSE_TAB, }), ); setShowResponseOnFirstLoad(true); } }, [responseDisplayFormat, actionResponse, showResponseOnFirstLoad, dispatch]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx(2 hunks)
🔇 Additional comments (4)
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx (4)
Line range hint 1-14: LGTM: Import statements are well-organized
The new imports for hooks and utilities are properly structured and necessary for the component's functionality.
62-71: LGTM: Schema tab handling is well-implemented
The logic for showing the schema tab when appropriate is clear and the dependencies are correctly specified.
73-79: LGTM: Action ID change handling meets requirements
The implementation correctly handles multiple page load queries by resetting the response visibility state when the action changes.
36-60: Verify performance impact of automatic response pane opening
While the implementation meets the requirements, we should verify that automatically opening the response pane doesn't impact performance, especially with multiple page load queries.
Consider implementing a debounce mechanism if multiple state updates occur in quick succession during page load.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11688817257. |
|
Deploy-Preview-URL: https://ce-37245.dp.appsmith.com |
…o chore/show-response-pane-open
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx
Outdated
Show resolved
Hide resolved
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx
Outdated
Show resolved
Hide resolved
| responseDisplayFormat && | ||
| !!responseDisplayFormat?.title && |
There was a problem hiding this comment.
Are these needed? Would they be available when the action has a failed response?
There was a problem hiding this comment.
This is required and basically works only when the response is a success. This is how it works in the normal flow as well.
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx
Outdated
Show resolved
Hide resolved
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx
Outdated
Show resolved
Hide resolved
…o chore/show-response-pane-open
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11721569577. |
|
Deploy-Preview-URL: https://ce-37245.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11723695733. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx (1)
33-55: Simplify conditions and improve naming.The useEffect implementation is good, but there are a few improvements that could be made:
- if ( - !!responseDisplayFormat?.title && - actionResponse?.isExecutionSuccess - ) { + if ( + responseDisplayFormat?.title && + actionResponse?.isExecutionSuccess + ) {Also, consider extracting the condition into a descriptive variable for better readability:
const shouldOpenResponseTab = responseDisplayFormat?.title && actionResponse?.isExecutionSuccess; if (shouldOpenResponseTab) { // ... existing code }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx(2 hunks)
🔇 Additional comments (3)
app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx (3)
Line range hint 1-24: LGTM! Clean implementation of context and utility imports.
The new imports and context usage are well-organized and properly implement the required functionality.
57-69: LGTM! Well-implemented schema tab logic.
The schema tab auto-opening logic is clean and follows best practices with proper dependency tracking.
33-69: Verify response pane behavior for all query scenarios.
The implementation looks good but let's verify it handles all cases:
Also, please verify that this works correctly for:
- New query creation
- Page load queries
- Manual query execution
✅ Verification successful
Based on the analysis of the codebase, I can now provide a final response:
Response tab behavior implementation is correct and comprehensive
The implementation correctly handles all query execution scenarios:
- Page load queries: Opens response tab when execution is successful and response format is available
- Schema tab: Opens when no tab is selected and schema is available
- Error cases: Handled through separate error tab logic in PluginActionSaga
- Manual execution: Response tab state managed through action execution saga
The changes are well-integrated with the existing debugger tab system and follow the established patterns for tab management.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other places where response pane visibility might be controlled
rg -l "setPluginActionEditorDebuggerState.*open.*true" --type ts
Length of output: 282
Script:
#!/bin/bash
# Let's check the implementation in QueryDebuggerTabs.tsx
rg -A 10 "setPluginActionEditorDebuggerState.*open.*true" app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx
# Also check ApiResponseView.tsx
rg -A 10 "setPluginActionEditorDebuggerState.*open.*true" app/client/src/components/editorComponents/ApiResponseView.tsx
# Let's also check if there are any other debugger tab related constants
rg "DEBUGGER_TAB_KEYS" --type ts
Length of output: 9831
Script:
#!/bin/bash
# Let's check how response tab is handled during action execution
rg -A 10 "executePluginAction|runAction.*setPluginActionEditorDebuggerState" --type ts
# Also check if there are any error cases that might affect response tab
rg -A 5 "actionResponse.*isExecutionSuccess.*false" --type ts
Length of output: 17403
|
Deploy-Preview-URL: https://ce-37245.dp.appsmith.com |
…e load queries (appsmithorg#37245) ## Description Opening response pane by default on query creation and for page load queries Fixes [appsmithorg#37216](appsmithorg#37216) ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 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/11723600626> > Commit: 98db89c > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11723600626&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Thu, 07 Nov 2024 13:48:18 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced the `PluginActionResponse` component for improved functionality and dynamic response handling. - Introduced a new state variable to manage response tab visibility on initial load. - Added logic to automatically open the response and schema tabs based on action responses. - **Bug Fixes** - Improved control flow and error handling for displaying responses based on action success. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Ankita Kinger <[email protected]>
Description
Opening response pane by default on query creation and for page load queries
Fixes #37216
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11723600626
Commit: 98db89c
Cypress dashboard.
Tags:
@tag.SanitySpec:
Thu, 07 Nov 2024 13:48:18 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
PluginActionResponsecomponent for improved functionality and dynamic response handling.Bug Fixes