Skip to content

Conversation

@lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Nov 12, 2025

  • .to.throw; does nothing. You have to call it like .to.throw();. We'll try adding a lint rule for this.
  • The logic was only hiding the action if if it is readonly and you CAN delete a database.

@lerouxb lerouxb requested a review from a team as a code owner November 12, 2025 09:54
@lerouxb lerouxb requested review from Copilot and nbbeeken November 12, 2025 09:54
@github-actions github-actions bot added the fix label Nov 12, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where the "Drop database" action was not properly disabled in read-only mode in the database context menu. The fix corrects the logical condition from using AND (&&) to OR (||) when checking whether to hide the drop database action, ensuring it's disabled when either write actions are disabled OR the user lacks permission to delete the database.

Key changes:

  • Fixed logical operator in the condition controlling "Drop database" visibility
  • Reformatted test assertions for better readability

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/compass-connections-navigation/src/item-actions.ts Changed condition from hasWriteActionsDisabled && canDeleteDatabase to `hasWriteActionsDisabled
packages/compass-connections-navigation/src/connections-navigation-tree.spec.tsx Reformatted test assertions to improve code readability

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lerouxb lerouxb merged commit 3fbcb02 into main Nov 12, 2025
87 of 90 checks passed
@lerouxb lerouxb deleted the readonly-drop-db branch November 12, 2025 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants