feat(UI): timeout limit checkbox to remove query limits - BED-7032#2436
Open
feat(UI): timeout limit checkbox to remove query limits - BED-7032#2436
Conversation
catsiller
added
the
user interface
coderabbitai
bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/javascript/bh-shared-ui/src/views/Explore/providers/index.ts (1)
17-17: Prefer direct provider imports over expanding barrel exports.Line 17 adds another provider barrel export; this can increase circular dependency risk in provider modules. Consider importing
DisableQueryLimitProviderdirectly where needed unless this must be part of the package’s stable public surface.Based on learnings: In the BloodHound codebase, prefer direct imports over barrel exports to avoid circular dependencies, especially for provider modules like SavedQueriesProvider.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/Explore/providers/index.ts` at line 17, Remove the barrel export for DisableQueryLimitProvider from the providers index (the export statement exporting './DisableQueryLimitProvider') to avoid expanding the barrel and potential circular deps; instead, delete that export line from the file and update any modules that currently import it from the providers barrel to import DisableQueryLimitProvider directly from its module path (./DisableQueryLimitProvider) so only direct imports reference the provider.packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsx (1)
94-103:useUserSettingscan be simplified to a pure conditional return.Current object mutation +
deleteworks, but a direct return is clearer and easier to maintain.♻️ Suggested refactor
export const useUserSettings = () => { const { isDisableQueryLimit } = useDisableQueryLimitContext(); - - const settings: UserSettings = { - headers: { Prefer: '' }, - }; - - if (isDisableQueryLimit) { - settings.headers = { Prefer: 'wait=-1' }; - } else { - delete settings.headers; - } - - return settings; + return isDisableQueryLimit ? { headers: { Prefer: 'wait=-1' } } : {}; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsx` around lines 94 - 103, The current settings object is created and then mutated/deleted; simplify by returning the appropriate UserSettings directly based on isDisableQueryLimit: if isDisableQueryLimit return { headers: { Prefer: 'wait=-1' } } otherwise return {} (or undefined) as the UserSettings value; update the code around the settings: UserSettings declaration and usages (referenced by settings and the isDisableQueryLimit flag) to remove the mutation and delete flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/ui/src/ducks/global/reducer.ts`:
- Line 29: The reducer globalViewReducer currently initializes timeoutSetting
but never handles actions of type types.GLOBAL_SET_TIMEOUT_SETTING, so calls to
setTimeoutSetting(...) are ignored; add a case for
types.GLOBAL_SET_TIMEOUT_SETTING in globalViewReducer that returns a new state
with timeoutSetting set to the action payload (ensuring you read the boolean
from action.payload) so dispatched toggles update the store; reference the
globalViewReducer switch and the action type types.GLOBAL_SET_TIMEOUT_SETTING
and update state.timeoutSetting accordingly.
In `@cmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.test.tsx`:
- Around line 265-269: The test is a false negative because it calls setup()
before updating serverState and also renders the default (non‑Cypher) tab, so
the config‑gated "Disable query timeout" checkbox is never exercised; fix by
initializing serverState with setInitialServerState(savedConfigurationValue)
before calling setup() or by ensuring the Cypher tab is selected after setup()
so the checkbox render path runs (referencing the setup() helper, serverState
variable, and setInitialServerState(savedConfigurationValue) call in this test).
In
`@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx`:
- Around line 142-144: The refetchFlag boolean is never cleared, so the if
(refetchFlag && cypherQuery !== '') { refetch(); } in CypherSearch.tsx continues
to cause extra refetches; after calling refetch() reset/clear refetchFlag (e.g.,
call the corresponding setter like setRefetchFlag(false) or invoke an
onRefetchHandled callback) so the flag only triggers a single refetch, and keep
the existing cypherQuery check (consider using cypherQuery.trim() !== '' if
needed).
- Around line 354-355: The visibility check for the timeout limit UI currently
uses a falsy check on timeoutLimitEnabled which shows the checkbox when the
config is undefined; update the condition to explicitly check for false (e.g.,
timeoutLimitEnabled === false) so only an explicit disabled setting renders the
element; locate the JSX around the timeoutLimitEnabled usage in the CypherSearch
component and replace the falsy conditional with the explicit equality check to
ensure undefined does not trigger visibility.
---
Nitpick comments:
In
`@packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsx`:
- Around line 94-103: The current settings object is created and then
mutated/deleted; simplify by returning the appropriate UserSettings directly
based on isDisableQueryLimit: if isDisableQueryLimit return { headers: { Prefer:
'wait=-1' } } otherwise return {} (or undefined) as the UserSettings value;
update the code around the settings: UserSettings declaration and usages
(referenced by settings and the isDisableQueryLimit flag) to remove the mutation
and delete flow.
In `@packages/javascript/bh-shared-ui/src/views/Explore/providers/index.ts`:
- Line 17: Remove the barrel export for DisableQueryLimitProvider from the
providers index (the export statement exporting './DisableQueryLimitProvider')
to avoid expanding the barrel and potential circular deps; instead, delete that
export line from the file and update any modules that currently import it from
the providers barrel to import DisableQueryLimitProvider directly from its
module path (./DisableQueryLimitProvider) so only direct imports reference the
provider.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 7ee0ed68-62dc-4bc3-895a-d5b49eb7efc3
📒 Files selected for processing (25)
cmd/ui/src/ducks/global/actions.tscmd/ui/src/ducks/global/reducer.tscmd/ui/src/ducks/global/types.tscmd/ui/src/store.tscmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.test.tsxcmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.tsxpackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/aclInheritanceSearch.tspackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/compositionSearch.tspackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/cypherSearch.tspackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/nodeSearch.tspackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/pathfindingSearch.tspackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/relationshipSearch.tspackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/utils.tspackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.test.tsxpackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsxpackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.test.tsxpackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsxpackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/QuerySearchFilter.test.tsxpackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/types.tspackages/javascript/bh-shared-ui/src/views/Explore/providers/DisableQueryLimitProvider/DisableQueryLimitContext.tspackages/javascript/bh-shared-ui/src/views/Explore/providers/DisableQueryLimitProvider/DisableQueryLimitContextProvider.tsxpackages/javascript/bh-shared-ui/src/views/Explore/providers/DisableQueryLimitProvider/index.tspackages/javascript/bh-shared-ui/src/views/Explore/providers/index.tspackages/javascript/js-client-library/src/client.tspackages/javascript/js-client-library/src/utils/config.ts
💤 Files with no reviewable changes (1)
- packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/QuerySearchFilter.test.tsx
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx
Show resolved
Hide resolved
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.test.tsx`:
- Around line 260-263: The test currently asserts the config-gated checkbox
synchronously which can race the async config fetch in
useTimeoutLimitConfiguration()/useGetConfiguration(); update the assertion in
the test that calls setup('cypher') to await the checkbox with
screen.findByRole('checkbox', { name: /Disable query timeout/i }) instead of
screen.getByRole so the test waits for the element to appear after the async
config resolves.
In
`@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx`:
- Around line 135-145: The handler handleCypherSearch can call both
performSearch() and refetch() in one click when refetchFlag is true, causing
duplicate queries; change the control flow so only one execution path runs:
check the refetchFlag branch first and perform refetch() (and
setRefetchFlag(false)) when refetchFlag && cypherQuery !== '', otherwise call
performSearch() only when cypherQuery is set; update logic around performSearch,
refetchFlag, refetch, and setRefetchFlag to be mutually exclusive (e.g., if
(refetchFlag && cypherQuery !== '') { refetch(); setRefetchFlag(false); } else
if (cypherQuery) { performSearch(); }).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: fb99e667-8aec-48e3-99f4-288e9a067a2d
📒 Files selected for processing (2)
cmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.test.tsxpackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx
Comment on lines
135
to
+145
| const handleCypherSearch = () => { | ||
| if (cypherQuery) { | ||
| performSearch(); | ||
| } | ||
|
|
||
| // If query is selected and "Run", user refreshes page and clicks "Run" again (disable query limit checkbox checked or unchecked) | ||
| // If user presses "Run" and then checks or unchecks disable query limit checkbox and presses "Run" again | ||
| if (refetchFlag && cypherQuery !== '') { | ||
| refetch(); | ||
| setRefetchFlag(false); | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid issuing two query executions in one Run action.
Line 136 and Line 142 can both execute on the same click when refetchFlag is true, which risks duplicate requests and inconsistent final results.
🔧 Proposed fix (single execution path)
const handleCypherSearch = () => {
- if (cypherQuery) {
- performSearch();
- }
-
- // If query is selected and "Run", user refreshes page and clicks "Run" again (disable query limit checkbox checked or unchecked)
- // If user presses "Run" and then checks or unchecks disable query limit checkbox and presses "Run" again
- if (refetchFlag && cypherQuery !== '') {
- refetch();
- setRefetchFlag(false);
- }
+ if (cypherQuery !== '') {
+ if (refetchFlag) {
+ refetch();
+ setRefetchFlag(false);
+ } else {
+ performSearch();
+ }
+ }
setMessageState((prev) => ({
...prev,
showMessage: false,
}));
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx`
around lines 135 - 145, The handler handleCypherSearch can call both
performSearch() and refetch() in one click when refetchFlag is true, causing
duplicate queries; change the control flow so only one execution path runs:
check the refetchFlag branch first and perform refetch() (and
setRefetchFlag(false)) when refetchFlag && cypherQuery !== '', otherwise call
performSearch() only when cypherQuery is set; update logic around performSearch,
refetchFlag, refetch, and setRefetchFlag to be mutually exclusive (e.g., if
(refetchFlag && cypherQuery !== '') { refetch(); setRefetchFlag(false); } else
if (cypherQuery) { performSearch(); }).
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx (1)
135-149:⚠️ Potential issue | 🟠 MajorMake the Run flow mutually exclusive to avoid double execution.
Line 136 runs
performSearch(), and Line 142/Line 147 can also runrefetch()in the same click path. That can execute two query paths for one Run action.🔧 Proposed fix
const handleCypherSearch = () => { - if (cypherQuery) { - performSearch(); - } - - // If query is selected and "Run", user refreshes page and clicks "Run" again (disable query limit checkbox checked or unchecked) - if (refetchFlag === false && cypherQuery !== '') { - refetch(); - } - - // If user presses "Run" and then checks or unchecks disable query limit checkbox and presses "Run" again (without updating the cypher) - if (refetchFlag && cypherQuery !== '') { - refetch(); - setRefetchFlag(false); - } + if (cypherQuery !== '') { + if (refetchFlag) { + refetch(); + setRefetchFlag(false); + } else { + performSearch(); + } + } setMessageState((prev) => ({ ...prev, showMessage: false, })); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx` around lines 135 - 149, The current handleCypherSearch can call both performSearch() and refetch() on one click, causing duplicate executions; change the control flow so the actions are mutually exclusive: first guard that cypherQuery is non-empty, then branch on refetchFlag using an if/else (or early return) so that when refetchFlag is true you call refetch() and setRefetchFlag(false), and when refetchFlag is false you call performSearch(); update handleCypherSearch to remove the overlapping calls and only invoke either performSearch or refetch (references: handleCypherSearch, performSearch, refetchFlag, refetch, setRefetchFlag, cypherQuery).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx`:
- Around line 135-149: The current handleCypherSearch can call both
performSearch() and refetch() on one click, causing duplicate executions; change
the control flow so the actions are mutually exclusive: first guard that
cypherQuery is non-empty, then branch on refetchFlag using an if/else (or early
return) so that when refetchFlag is true you call refetch() and
setRefetchFlag(false), and when refetchFlag is false you call performSearch();
update handleCypherSearch to remove the overlapping calls and only invoke either
performSearch or refetch (references: handleCypherSearch, performSearch,
refetchFlag, refetch, setRefetchFlag, cypherQuery).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1c0d26a0-edc0-4d5b-b6a5-248dc2d8040c
📒 Files selected for processing (2)
cmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.test.tsxpackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.test.tsx
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx (1)
135-155:⚠️ Potential issue | 🟠 MajorMultiple query executions on single Run click.
The current logic can execute both
performSearch()andrefetch()in the same handler invocation:
- Line 137:
performSearch()runs whencypherQueryis truthy- Line 142:
refetch()runs whenrefetchFlag === false && cypherQuery !== ''On a normal Run click (where
refetchFlagis false and query exists), both execute, causing duplicate requests.🔧 Proposed fix (mutually exclusive execution paths)
const handleCypherSearch = () => { - if (cypherQuery) { - performSearch(); - } - - // If query is selected and "Run", user refreshes page and clicks "Run" again (disable query limit checkbox checked or unchecked) - if (refetchFlag === false && cypherQuery !== '') { - refetch(); - } - - // If user presses "Run" and then checks or unchecks disable query limit checkbox and presses "Run" again (without updating the cypher) - if (refetchFlag && cypherQuery !== '') { - refetch(); - setRefetchFlag(false); - } + if (cypherQuery !== '') { + if (refetchFlag) { + // Re-run with updated settings (e.g., disable query limit toggled) + refetch(); + setRefetchFlag(false); + } else { + performSearch(); + } + } setMessageState((prev) => ({ ...prev, showMessage: false, })); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx` around lines 135 - 155, The handler sometimes calls both performSearch() and refetch(); make the branches mutually exclusive inside handleCypherSearch: first check if cypherQuery is empty and return early; then use an if/else (or switch) on refetchFlag so only one action runs (e.g., if refetchFlag === false -> call refetch(); else -> call performSearch() and, if needed, call setRefetchFlag(false) after). Update the conditions around performSearch, refetch, and setRefetchFlag so they cannot all execute on one Run click (refer to handleCypherSearch, performSearch, refetch, setRefetchFlag).
🧹 Nitpick comments (3)
packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsx (2)
69-83: Unnecessary spread ofuserSettingsintouseQueryoptions.Line 82 spreads
userSettings(which containsheaders) into theuseQueryoptions object. However,react-query'suseQuerydoesn't use aheadersproperty—headers are already correctly passed to the API client viaqueryFnincypherSearchGraphQuery. This spread has no effect and may cause confusion.♻️ Remove unnecessary spread
return useQuery({ ...queryConfig, onError: (error: any) => { const { message, key } = query.getErrorMessage(error); if (onError) { onError(message); } addNotification(message, key, { autoHideDuration: SNACKBAR_DURATION_LONG, }); }, ...rest, - ...userSettings, }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsx` around lines 69 - 83, The spread of userSettings into the useQuery options is unnecessary and potentially confusing because react-query's useQuery does not consume a headers property; remove the spread of userSettings from the returned useQuery call in useExploreGraph so that useQuery({...queryConfig, onError: ..., ...rest}) is used instead, leaving headers handling to cypherSearchGraphQuery/queryFn where the API client already receives userSettings; update the return in useExploreGraph (the useQuery invocation) to omit ...userSettings.
92-107: SimplifyuseUserSettingsby conditionally constructing the object.Creating an object with
headers, then deleting it is less efficient and harder to read than conditionally building the object.♻️ Cleaner implementation
export const useUserSettings = () => { const { isDisableQueryLimit } = useDisableQueryLimitContext(); const timeoutLimitEnabled = useTimeoutLimitConfiguration(); - const settings: UserSettings = { - headers: { Prefer: '' }, - }; - - if (isDisableQueryLimit && timeoutLimitEnabled === false) { - settings.headers = { Prefer: 'wait=-1' }; - } else { - delete settings.headers; - } - - return settings; + if (isDisableQueryLimit && timeoutLimitEnabled === false) { + return { headers: { Prefer: 'wait=-1' } }; + } + + return {}; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsx` around lines 92 - 107, The useUserSettings function currently creates a settings object with headers and then deletes headers conditionally; instead, construct the UserSettings object only with headers when needed: inside useUserSettings (which uses useDisableQueryLimitContext and useTimeoutLimitConfiguration) build and return either { headers: { Prefer: 'wait=-1' } } when isDisableQueryLimit && timeoutLimitEnabled === false, or an empty object (or {} as UserSettings) otherwise, ensuring the returned type matches UserSettings and removing the initial assignment + delete flow.packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx (1)
127-133: Consider simplifying the conditional.The if-else can be reduced to a single assignment since
disableQueryLimitis already a boolean.♻️ Simplified version
- if (disableQueryLimit) { - setIsDisableQueryLimit(true); - } else { - setIsDisableQueryLimit(false); - } + setIsDisableQueryLimit(disableQueryLimit);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx` around lines 127 - 133, The if/else that sets state based on disableQueryLimit is verbose; replace the conditional block with a direct assignment using setIsDisableQueryLimit(disableQueryLimit) (or Boolean(disableQueryLimit) if you want to coerce), inside the same effect where disableQueryLimit and setIsDisableQueryLimit are referenced, removing the redundant if-else.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx`:
- Around line 135-155: The handler sometimes calls both performSearch() and
refetch(); make the branches mutually exclusive inside handleCypherSearch: first
check if cypherQuery is empty and return early; then use an if/else (or switch)
on refetchFlag so only one action runs (e.g., if refetchFlag === false -> call
refetch(); else -> call performSearch() and, if needed, call
setRefetchFlag(false) after). Update the conditions around performSearch,
refetch, and setRefetchFlag so they cannot all execute on one Run click (refer
to handleCypherSearch, performSearch, refetch, setRefetchFlag).
---
Nitpick comments:
In
`@packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsx`:
- Around line 69-83: The spread of userSettings into the useQuery options is
unnecessary and potentially confusing because react-query's useQuery does not
consume a headers property; remove the spread of userSettings from the returned
useQuery call in useExploreGraph so that useQuery({...queryConfig, onError: ...,
...rest}) is used instead, leaving headers handling to
cypherSearchGraphQuery/queryFn where the API client already receives
userSettings; update the return in useExploreGraph (the useQuery invocation) to
omit ...userSettings.
- Around line 92-107: The useUserSettings function currently creates a settings
object with headers and then deletes headers conditionally; instead, construct
the UserSettings object only with headers when needed: inside useUserSettings
(which uses useDisableQueryLimitContext and useTimeoutLimitConfiguration) build
and return either { headers: { Prefer: 'wait=-1' } } when isDisableQueryLimit &&
timeoutLimitEnabled === false, or an empty object (or {} as UserSettings)
otherwise, ensuring the returned type matches UserSettings and removing the
initial assignment + delete flow.
In
`@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx`:
- Around line 127-133: The if/else that sets state based on disableQueryLimit is
verbose; replace the conditional block with a direct assignment using
setIsDisableQueryLimit(disableQueryLimit) (or Boolean(disableQueryLimit) if you
want to coerce), inside the same effect where disableQueryLimit and
setIsDisableQueryLimit are referenced, removing the redundant if-else.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c1417762-7b63-4590-a040-755c1f6cd199
📒 Files selected for processing (3)
packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/cypherSearch.tspackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/useExploreGraph.tsxpackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx
specter-flq
reviewed
Mar 11, 2026
Comment on lines
+119
to
+122
| console.log(timeoutLimitEnabled); | ||
|
|
||
| console.log(disableQueryLimit); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
ah good catch, removed!
specter-flq
reviewed
Mar 11, 2026
Comment on lines
+119
to
+122
| console.log(timeoutLimitEnabled); | ||
|
|
||
| console.log(disableQueryLimit); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
ah good catch, removed!
specter-flq
reviewed
Mar 11, 2026
| disabled={cypherSearchIsRunning} | ||
| aria-label='Run cypher query' | ||
| className={cn({ | ||
| 'bg-slate-600 max-w-[83px] hover:bg-slate-700': cypherSearchIsRunning, |
Contributor
There was a problem hiding this comment.
We might not have the exact color in the themes, but wonder if we should get the closest tokenized one from our theme design/system instead of using a color utility class directly
Contributor
Author
There was a problem hiding this comment.
i dont see a similar variant in the DUI colors 🤔 maybe a good idea to get that added into the design system and come back/replace this
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Required behaviors:
enabled: false, the Explore page Cypher query tab should display a checkbox allowing the user to remove query limits for that query execution.Motivation and Context
Resolves https://specterops.atlassian.net/browse/BED-7032
Why is this change required? What problem does it solve?
How Has This Been Tested?
Added unit tests
Manual testing
Reviewer: make sure timeout limit parameter is enabled: false in the DB
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Behavior
Tests