Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Open settings button not working in response pane #36887

Merged
merged 2 commits into from
Oct 16, 2024

Conversation

albinAppsmith
Copy link
Collaborator

@albinAppsmith albinAppsmith commented Oct 15, 2024

Description

Open settings button in response pane was not working after modularization changes. This PR fix this issue.

Fixes #36885

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/11350887280
Commit: 867b941
Cypress dashboard.
Tags: @tag.Sanity
Spec:


Tue, 15 Oct 2024 17:44:41 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • New Features

    • Enhanced navigation to the settings tab in the plugin action editor for improved user experience.
  • Bug Fixes

    • Streamlined state management for the selected tab, ensuring more reliable functionality.

Copy link
Contributor

coderabbitai bot commented Oct 15, 2024

Walkthrough

The changes in this pull request focus on the QueryResponseTab component, specifically modifying the function responsible for managing the selected tab state in the plugin action editor. The function setPluginActionEditorDebuggerState has been renamed to setPluginActionEditorSelectedTab, and the navigateToSettings function now dispatches an action with the EDITOR_TABS.SETTINGS constant. This adjustment enhances the clarity and efficiency of state management within the component.

Changes

File Path Change Summary
app/client/src/pages/Editor/QueryEditor/QueryResponseTab.tsx Renamed function setPluginActionEditorDebuggerState to setPluginActionEditorSelectedTab and updated navigateToSettings to dispatch EDITOR_TABS.SETTINGS.

Assessment against linked issues

Objective Addressed Explanation
Open settings button in response pane not working (#36885)

Suggested labels

Bug, High, Production, Needs Triaging, IDE Product, ok-to-test

Suggested reviewers

  • hetunandu
  • ankitakinger

Poem

In the editor where queries flow,
A button now works, as we all know.
With tabs that switch, so sleek and bright,
Settings await, just a click, what a sight!
Code's been refined, like a well-tuned song,
In the world of plugins, we now belong! 🎉


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between a8e6252 and 867b941.

📒 Files selected for processing (1)
  • app/client/src/pages/Editor/QueryEditor/QueryResponseTab.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (3)
app/client/src/pages/Editor/QueryEditor/QueryResponseTab.tsx (3)

41-41: Well done on improving the function name, class!

The change from setPluginActionEditorDebuggerState to setPluginActionEditorSelectedTab is a step in the right direction. This new name is like a well-written essay title - it clearly tells us what the function does. It's much easier for your classmates (other developers) to understand at a glance. Keep up the good work!


Line range hint 1-368: Class, let's summarize what we've learned today!

We've seen two important changes in our code:

  1. We've renamed a function to make it clearer what it does.
  2. We've updated how we navigate to the settings tab.

These changes are like fixing a broken link in our textbook - they should make the "Open settings" button work correctly again. The changes are small but important, just like how a single word can change the meaning of a sentence.

Remember, even small improvements can make a big difference in how well our code works and how easy it is for others to understand. Keep up the great work, and don't forget to test your changes thoroughly!


222-222: Excellent job updating the function call, students!

Your change here is like solving a math problem correctly - you've made sure that the function call matches the new import statement. The use of EDITOR_TABS.SETTINGS instead of DEBUGGER_TAB_KEYS.SETTINGS shows good attention to detail. It's like using the correct formula in the right context.

To make sure everything works perfectly, let's do a quick pop quiz:

Can you please run this little test to make sure we're using the right constants everywhere?

This will help us ensure we've updated all the necessary places. It's like double-checking your homework!

✅ Verification successful

Oops! It looks like there was a hiccup with our previous test.

The rg command didn't recognize the typescript file type. No worries, though! Let's try using the correct file type aliases for TypeScript.

Could you please run the following updated tests to ensure everything is in order?

This should help us verify that all instances are correctly updated. Let's get this sorted so everything runs smoothly!


Great effort trying to run those tests!

It seems like rg is having a bit of trouble recognizing the file types we specified. No worries—we can adjust our approach to ensure we cover all our TypeScript files.

Let's try searching by file extensions instead. Please run the following updated tests:

This script uses find to locate all .ts and .tsx files and then searches within them for the specified patterns. This should help us accurately verify that all instances have been updated correctly.

Let's give it a try and see if we can get the results we need!


Nice try! We're almost there with our tests.

It looks like the rg command is still having trouble with the --type tsx option. Let's tweak our approach to fix this.

Please run the following updated tests where we've removed the --type flags. This way, find handles file selection, and rg searches within those files without type restrictions:

This should eliminate the previous errors and provide us with the necessary information to confirm that all instances have been correctly updated. Let's give it another go and ensure everything is spot on!


Fantastic work! 🎉

You've successfully removed all instances of DEBUGGER_TAB_KEYS.SETTINGS and replaced them with EDITOR_TABS.SETTINGS. This ensures our codebase remains consistent and free from deprecated constants.

Keep up the great work!

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining uses of DEBUGGER_TAB_KEYS.SETTINGS

# Test: Search for DEBUGGER_TAB_KEYS.SETTINGS. Expect: No results
rg --type typescript "DEBUGGER_TAB_KEYS\.SETTINGS"

# Test: Confirm EDITOR_TABS.SETTINGS is used. Expect: At least one result
rg --type typescript "EDITOR_TABS\.SETTINGS"

Length of output: 169


Script:

#!/bin/bash
# Description: Check for any remaining uses of DEBUGGER_TAB_KEYS.SETTINGS and confirm EDITOR_TABS.SETTINGS is used.

# Test: Search for DEBUGGER_TAB_KEYS.SETTINGS. Expect: No results
rg --type ts "DEBUGGER_TAB_KEYS\.SETTINGS"
rg --type tsx "DEBUGGER_TAB_KEYS\.SETTINGS"

# Test: Confirm EDITOR_TABS.SETTINGS is used. Expect: At least one result
rg --type ts "EDITOR_TABS\.SETTINGS"
rg --type tsx "EDITOR_TABS\.SETTINGS"

Length of output: 1525


Script:

#!/bin/bash
# Description: Check for any remaining uses of DEBUGGER_TAB_KEYS.SETTINGS and confirm EDITOR_TABS.SETTINGS is used.

# Function to search in .ts and .tsx files
search_patterns() {
    local pattern=$1
    find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec rg "$pattern" {} +
}

# Test: Search for DEBUGGER_TAB_KEYS.SETTINGS. Expect: No results
echo "Searching for DEBUGGER_TAB_KEYS.SETTINGS in .ts and .tsx files..."
search_patterns "DEBUGGER_TAB_KEYS\.SETTINGS"

# Test: Confirm EDITOR_TABS.SETTINGS is used. Expect: At least one result
echo "Searching for EDITOR_TABS.SETTINGS in .ts and .tsx files..."
search_patterns "EDITOR_TABS\.SETTINGS"

Length of output: 1961


Script:

#!/bin/bash
# Description: Check for any remaining uses of DEBUGGER_TAB_KEYS.SETTINGS and confirm EDITOR_TABS.SETTINGS is used.

# Function to search in .ts and .tsx files
search_patterns() {
    local pattern=$1
    find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec rg "$pattern" {} +
}

# Test: Search for DEBUGGER_TAB_KEYS.SETTINGS. Expect: No results
echo "Searching for DEBUGGER_TAB_KEYS.SETTINGS in .ts and .tsx files..."
search_patterns "DEBUGGER_TAB_KEYS\.SETTINGS"

# Test: Confirm EDITOR_TABS.SETTINGS is used. Expect: At least one result
echo "Searching for EDITOR_TABS.SETTINGS in .ts and .tsx files..."
search_patterns "EDITOR_TABS\.SETTINGS"

Length of output: 1961


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.

@albinAppsmith albinAppsmith added the ok-to-test Required label for CI label Oct 15, 2024
@github-actions github-actions bot added IDE Navigation Issues/feature requests related to IDE navigation, and context switching IDE Pod Issues that new developers face while exploring the IDE IDE Product Issues related to the IDE Product Task A simple Todo Bug Something isn't working labels Oct 15, 2024
@albinAppsmith
Copy link
Collaborator Author

/build-deploy-preview skip-test=true

Copy link

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

Copy link

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

@hetunandu hetunandu merged commit d1176de into release Oct 16, 2024
46 checks passed
@hetunandu hetunandu deleted the fix/open-setting-prepared-statement-warning branch October 16, 2024 02:17
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Oct 17, 2024
@coderabbitai coderabbitai bot mentioned this pull request Nov 21, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working IDE Navigation Issues/feature requests related to IDE navigation, and context switching IDE Pod Issues that new developers face while exploring the IDE IDE Product Issues related to the IDE Product ok-to-test Required label for CI Task A simple Todo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Open settings button in response pane not working
2 participants