fix: bug run button not disabled in response pane#36864
fix: bug run button not disabled in response pane#36864prasad-madine wants to merge 1 commit intoappsmithorg:releasefrom
Conversation
WalkthroughThe pull request introduces updates to the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/src/pages/Editor/QueryEditor/QueryResponseTab.test.tsx (1)
198-223: A gold star for adding this crucial test case!I'm thrilled to see you've added a test to check if the run button is disabled when the query is empty. This is an important edge case that needed coverage. Your setup of the test props and rendering of the component is spot on!
However, I have a small suggestion to make this test even better:
Instead of using
screen.getByRole('button'), which might be ambiguous if there are multiple buttons, let's be more specific. Consider using a test ID or a more specific role+name combination. For example:const runButton = screen.getByRole('button', { name: /run/i });or
const runButton = screen.getByTestId('run-query-btn');This will ensure we're always testing the correct button, even if the component structure changes in the future.
Keep up the excellent work in improving our test coverage!
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- app/client/src/pages/Editor/QueryEditor/QueryResponseTab.test.tsx (2 hunks)
- app/client/src/pages/Editor/QueryEditor/QueryResponseTab.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (4)
app/client/src/pages/Editor/QueryEditor/QueryResponseTab.test.tsx (1)
2-3: Excellent addition of testing utilities, class!I'm pleased to see you've imported
screenfrom@testing-library/reactand included@testing-library/jest-dom/extend-expect. These are valuable tools in our testing toolkit!The
screenobject will allow you to query rendered components more idiomatically, making your tests easier to read and write. Thejest-domextension adds custom matchers that will make your assertions on DOM elements more expressive.Keep up the good work in enhancing our testing capabilities!
app/client/src/pages/Editor/QueryEditor/QueryResponseTab.tsx (3)
99-99: Well done, class! Let's examine this new variable.The addition of
isQueryEmptyis a smart move. It's like checking if a student has written anything on their test paper before they submit it. This variable will help us determine if there's actually a query to run.
349-349: Excellent modification! Let's break it down for the class.This change is like adding an extra check before allowing a student to submit their test. Not only do we make sure they have permission to take the test (
isExecutePermitted), but we also ensure they've actually written something (!isQueryEmpty). This will prevent students... I mean, users, from submitting empty queries.
99-99: Class, let's summarize today's lesson!These changes work together like a well-oiled machine to solve our run button problem. By introducing
isQueryEmptyand using it in theisRunDisabledcondition, we've ensured that our query "test paper" can't be submitted if it's blank. This is a textbook example of how small, focused changes can have a significant impact on user experience.Remember, children, in the world of coding, it's not just about writing code, but writing code that prevents users from making mistakes. You've all done very well today!
Also applies to: 349-349
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
Hello @sagar-qa007 @rohan-arthur Could you please review this PR. |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
Hello @sagar-qa007 , Please review this PR. |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
Hello @sagar-qa007 , Please review this PR. |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
Hello @sagar-qa007 , Please review this PR. |
|
@rahulbarwal FYI. Please check this. @PrasadMadine Please resolved the conflict. |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
This PR has been closed because of inactivity. |
2 participants
Description:
When no query is added in the query input box, the run button on the top query page is getting disabled. But the same behaviour is not maintained for the run button in the response pane.
Issue Link
Screenshots:
Before resoving bug:
After resoving bug:
Summary by CodeRabbit
New Features
Bug Fixes