Skip to content

fix:query response count on error in QueryDebuggerTabs#35363

Merged
yatinappsmith merged 8 commits intoappsmithorg:releasefrom
raushan3737:fix/query-response-error-count
Nov 4, 2024
Merged

fix:query response count on error in QueryDebuggerTabs#35363
yatinappsmith merged 8 commits intoappsmithorg:releasefrom
raushan3737:fix/query-response-error-count

Conversation

@raushan3737
Copy link
Contributor

@raushan3737 raushan3737 commented Aug 2, 2024

PR Description:

  • Description: Added the conditional rendering of the record count or error state based on the isExecutionSuccess property which is present in props actionResponse.
  • Attached the video explaning the fix and demoing: Video Url
  • isExecutionSuccess: boolean contains the response of the query whether that query was successful or not on the server.
  • Added the unit test case.

Fixes #33813

Summary by CodeRabbit

  • New Features

    • Enhanced error handling in the Query Debugger, displaying error messages in red when an action fails.
    • Improved user feedback by conditionally rendering output length based on the success of the action.
  • Tests

    • Added test cases to ensure correct component behavior for both successful and failed API responses.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 2, 2024

Walkthrough

The recent updates enhance the QueryDebuggerTabs component by introducing thorough testing and improving user feedback based on API responses. Mock responses for success and failure scenarios have been implemented, allowing the tests to validate component behavior accurately. Additionally, the rendering logic within the component has been refined to display either output length or error messages based on the execution result, significantly enhancing user experience and reliability.

Changes

Files Change Summary
app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.test.tsx Added mock response objects for testing and introduced new test cases for handling successful and failed API responses.
app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx Implemented conditional rendering to show output length on success or an error message on failure, improving user feedback.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant QueryDebuggerTabs
    participant API

    User->>QueryDebuggerTabs: Trigger Query
    QueryDebuggerTabs->>API: Send Request
    API-->>QueryDebuggerTabs: Success Response
    QueryDebuggerTabs->>User: Display Output Length

    User->>QueryDebuggerTabs: Trigger Query
    QueryDebuggerTabs->>API: Send Request
    API-->>QueryDebuggerTabs: Failed Response
    QueryDebuggerTabs->>User: Display Error Message
Loading

Poem

In the tabs of query’s dance,
Success shines, giving us a chance.
But should it fail, with color bright,
An error glows, a guiding light.
Testing strong, our code does sing,
Embracing change, oh what joy it brings! 🌟


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a0f2ee1 and 3d47dbc.

Files selected for processing (2)
  • app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.test.tsx (2 hunks)
  • app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx (1 hunks)
Additional comments not posted (5)
app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.test.tsx (4)

17-28: Well-structured mock response for success scenario.

The mockSuccessResponse object correctly simulates a successful API response and aligns with the ActionResponse type.


30-41: Well-structured mock response for failure scenario.

The mockFailedResponse object correctly simulates a failed API response and aligns with the ActionResponse type.


118-144: Good test case for successful response scenario.

The test case correctly verifies that the component displays the record count when a successful response is received.


146-172: Good test case for failure response scenario.

The test case correctly verifies that the component displays an error message when a failed response is received.

app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx (1)

245-257: Effective conditional rendering logic.

The conditional rendering logic correctly displays the result count or an error message based on the isExecutionSuccess property, enhancing user feedback.

@raushan3737
Copy link
Contributor Author

Hi @rohan-arthur , Could you please trigger the CI/CD and assign the reviewer for it.

@rohan-arthur
Copy link
Contributor

@raushan3737 please pick issues from this list, as they generally have a clear solution: https://github.com/appsmithorg/appsmith/issues?q=is%3Aopen+is%3Aissue+label%3A%22Inviting+Contribution%22

For this one, would you please attach a video explaining the solution?

@raushan3737
Copy link
Contributor Author

Hi @rohan-arthur , I have attached the video explaning the fix and demoing: Video Url

Please review and assign the reviewer for it.

Thank You.

@github-actions
Copy link

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions bot added the Stale label Aug 21, 2024
@raushan3737
Copy link
Contributor Author

Hi @rohan-arthur , @NilanshBansal , Could you please review the PR. I have attached the video expaining the fix.

@NilanshBansal NilanshBansal requested a review from a team August 22, 2024 10:31
@NilanshBansal NilanshBansal added Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Community Contributor Meant to track issues that are assigned to external contributors labels Aug 22, 2024
@github-actions github-actions bot removed the Stale label Aug 22, 2024
@github-actions
Copy link

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions bot added the Stale label Aug 31, 2024
@raushan3737
Copy link
Contributor Author

Hi @sagar-qa007, Could you please assign the reviewer & get it reviewed this PR.
Thank you.

@NilanshBansal
Copy link
Contributor

@raushan3737 do you have any updates on this PR as suggested by @AmanAgarwal041?

@raushan3737
Copy link
Contributor Author

@raushan3737 do you have any updates on this PR as suggested by @AmanAgarwal041?

Yes, I have updated the code as per @AmanAgarwal041 suggestion. Please review once.

Thank you.

@AmanAgarwal041
Copy link
Contributor

Hey @raushan3737 , there are some client build errors. Can you please update your branch to latest release and check for the build errors ?
Screenshot 2024-09-25 at 2 44 40 PM

@raushan3737
Copy link
Contributor Author

raushan3737 commented Sep 25, 2024

Hey @raushan3737 , there are some client build errors. Can you please update your branch to latest release and check for the build errors ? Screenshot 2024-09-25 at 2 44 40 PM

Updated the code. Please check once.

Thank you.

@NilanshBansal
Copy link
Contributor

/build-deploy-preview skip-tests=true

@github-actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11046827143.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 35363.
recreate: .

@github-actions
Copy link

Deploy-Preview-URL: https://ce-35363.dp.appsmith.com

@AmanAgarwal041
Copy link
Contributor

Tested for the error and successful response:

Success:
Screenshot 2024-09-26 at 3 48 05 PM

Error:
Screenshot 2024-09-26 at 3 48 55 PM

@AmanAgarwal041
Copy link
Contributor

@raushan3737 Everything seems just prettier fixes :
Screenshot 2024-09-26 at 5 06 04 PM
Screenshot 2024-09-26 at 5 06 26 PM
Can you please fix them ?

@raushan3737
Copy link
Contributor Author

Hi @AmanAgarwal041, I have fixed the prettier issue. Please check once.

Thank you.

@AmanAgarwal041
Copy link
Contributor

@raushan3737 There are some cypress failures. Can you please check ?
Screenshot 2024-10-03 at 11 30 25 AM

@AmanAgarwal041
Copy link
Contributor

@raushan3737 A gentle reminder, we will wait until 17 Oct, and move this back to the backlog if there are no further updates about the cypress failures.

@raushan3737
Copy link
Contributor Author

raushan3737 commented Oct 16, 2024

@raushan3737 There are some cypress failures. Can you please check ? Screenshot 2024-10-03 at 11 30 25 AM

Yes sure @AmanAgarwal041 apology for the delay. Actually the above cypress is not altered by me & even it is failing with the previous src code also. I have tried to run the cypress with both the code(previous & current PR changes) but it is failing in both cases & oftenly i am getting error after some time as below:
image

Observation:

  1. Assertion was failing due in beforeAll of mysql2_spec.ts due to missing underscore _ in connection name.
    Code Link

If possible could you please provide the test method names so that i can investigate further?

Thank you.

@AmanAgarwal041
Copy link
Contributor

@raushan3737 The test cases have passed but there seems to be another linting error
Screenshot 2024-10-23 at 1 11 19 PM

@yatinappsmith yatinappsmith merged commit c42be90 into appsmithorg:release Nov 4, 2024
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Nov 20, 2024
…5363)

### PR Description:
- Description: Added the conditional rendering of the record count or
error state based on the `isExecutionSuccess` property which is present
in props actionResponse.
- Attached the video explaning the fix and demoing: [Video
Url](https://www.loom.com/share/c40e624688114c1c9c005ab3e8f5de69?sid=bb927dc4-a9a2-476b-861a-26bbffda51b0)
- `isExecutionSuccess`: boolean contains the response of the query
whether that query was successful or not on the server.
- Added the unit test case.

Fixes appsmithorg#33813

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Enhanced error handling in the Query Debugger, displaying error
messages in red when an action fails.
- Improved user feedback by conditionally rendering output length based
on the success of the action.

- **Tests**
- Added test cases to ensure correct component behavior for both
successful and failed API responses.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community Contributor Meant to track issues that are assigned to external contributors Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Integrations Pod Query & JS Pod Issues related to the query & JS Pod

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: query response counts error as one record

6 participants