BloodHound icon indicating copy to clipboard operation
BloodHound copied to clipboard

feat: "Run" button improvements (status indicators, refresh query, cancel search) BP-2119

Open dcairnsspecterops opened this issue 1 month ago • 2 comments

https://github.com/user-attachments/assets/15332a17-6b17-4be5-8a45-b732353603e4

Description

This PR adds status indicators to the cypher run button state.

Additionally, this PR allows the user to click "Run" again to refresh the query, even if the content of the cypher editor hasn't changed.

Finally, this PR allows user to cancel searches in flight, in the event that it seems to be running long, or the user notices they made a typo.

Motivation and Context

Resolves <BP-2119>

Why is this change required? What problem does it solve?

How Has This Been Tested?

Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc.

Screenshots (optional):

Types of changes

  • Chore (a change that does not modify the application functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Database Migrations

Checklist:

  • [ ] I have met the contributing prerequisites
    • Assigned myself to this PR
    • Added the appropriate labels
    • Associated an issue: https://github.com/SpecterOps/BloodHound/issues/672
    • Read the Contributing guide: https://github.com/SpecterOps/BloodHound/wiki/Contributing
  • [ ] I have ensured that related documentation is up-to-date
    • Open API docs
    • Code comments (GoDocs / JSDocs)
  • [ ] I have followed proper test practices
    • Added/updated tests to cover my changes
    • All new and existing tests passed

Summary by CodeRabbit

  • New Features

    • Processing spinner added to Cypher search UI with Run/Abort button and cancellation support.
    • Tooltip and icon enhancements for query controls.
  • Bug Fixes & Improvements

    • Enhanced error/success messaging and URL/editor sync during query runs.
    • Graph exploration hook now supports configurable error callbacks.
  • Refactor

    • Message component updated to a state-driven API (message state + setter).
  • Tests

    • Updated tests to match the new message API.

✏️ Tip: You can customize this high-level summary in your review settings.

dcairnsspecterops avatar Nov 19 '25 02:11 dcairnsspecterops

[!WARNING]

Rate limit exceeded

@dcairnsspecterops has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 12 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between bdd5c83b31811ce1aaf8c5f650915955157f58f3 and a11a9edb6303bc6bd63d2efc73b7a64347ccc70d.

📒 Files selected for processing (1)
  • packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx (8 hunks)

Walkthrough

This pull request refactors message handling in the Cypher search UI from callback and timer-based logic to a state-driven approach. It adds optional error callback configuration to the explore graph hook, extends the ProcessingIndicator component with customizable styling via an optional className prop, and introduces a new MessageState type for managing message visibility and content.

Changes

Cohort / File(s) Summary
ProcessingIndicator Enhancement
packages/javascript/bh-shared-ui/src/components/Animations/ProcessingIndicator.tsx
Added optional className prop and cn utility import; component now merges custom className with default inline-flex styling.
useExploreGraph Hook Enhancement
packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsx
Introduced ExploreGraphQueryOptions parameter type with optional onError callback; hook now forwards remaining options to React Query configuration and invokes error callback when errors occur.
Message State Refactoring
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/CypherSearchMessage.tsx
Replaced timer-based callback logic with state-driven approach; removed useCallback/useEffect/useRef; introduced MessageState export type; updated component to read from messageState and use event handlers for visibility lifecycle management.
CypherSearch UI Integration
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx, SavedQueries/CypherSearchMessage.test.tsx
Integrated useExploreGraph hook for tracking query execution; replaced clearMessage callback with setMessageState prop; Run button now displays ProcessingIndicator while executing and becomes disabled during search; removed timer-based message clearing logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • CypherSearchMessage.tsx: Review state-driven message lifecycle logic and event handler ordering (onAnimationEnd, onTransitionEnd) to ensure messages clear correctly.
  • CypherSearch.tsx: Verify integration of useExploreGraph hook, message state flow through setMessageState, and button disabled/content states during execution.
  • useExploreGraph.tsx: Confirm error callback is invoked at the correct point and doesn't conflict with existing notification behavior.

Possibly related PRs

  • #2104: Introduces ProcessingIndicator component; this PR extends it with className customization support.
  • #2069: Modifies CypherSearch.tsx initialization and UI behavior; overlaps with refactored message handling and button state logic.
  • #2010: Modifies CypherSearch.tsx Run button attributes and behavior; related to the button disabled/loading state changes.

Suggested labels

enhancement, user interface

Suggested reviewers

  • urangel
  • benwaples
  • specter-flq

Poem

🐰 A message state, now clean and bright, No timers tick through endless night! With hooks and flows, we dance with grace, While indicators show their face. ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. Key sections are missing or unfilled: Motivation/Context lacks explanation, How Has This Been Tested is empty, and the entire checklist is unchecked. Complete the Motivation and Context section, provide detailed testing information, fill out the Types of changes checklist, and verify the contributor checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding a running status to the Run button in the Cypher search interface, with the Jira ticket reference.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 19 '25 02:11 coderabbitai[bot]

Granted the cancel button doesn't actually cancel the query in the graph, just tell the API to ignore the response, there's a risk that someone runs a bunch of heavyweight queries and hangs up a system. While this feels less-than-likely, I'd like to flag this for consideration before we ship with the "cancel" functionality included.

StephenHinck avatar Dec 12 '25 21:12 StephenHinck