Skip to content

feat(aci): set up monitor Disable action #96716

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ameliahsu
Copy link
Member

connected Disable button to detector PUT endpoint

since we already have individual components for each action button now (file here), i figured we could take out the EditDetectorActions component and place the buttons into EditDetectorLayout directly

@ameliahsu ameliahsu requested a review from a team as a code owner July 29, 2025 23:11
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 29, 2025
cursor[bot]

This comment was marked as outdated.

Copy link

codecov bot commented Jul 29, 2025

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
11088 2 11086 10
View the top 2 failed test(s) by shortest run time
DetectorEdit EditDetectorActions calls update mutation when enabling/disabling automation
Stack Traces | 1.01s run time
Error: Unable to find role="link" and name "detector"

Ignored nodes: comments, script, style
...
    at waitForWrapper (.../sentry/node_modules/.pnpm/@testing-library+dom@10.4.0/node_modules/@.../dom/dist/wait-for.js:163:27)
    at .../sentry/node_modules/.pnpm/@testing-library+dom@10.4.0/node_modules/@.../dom/dist/query-helpers.js:86:33
    at Object.findByRole (.../views/detectors/edit.spec.tsx:144:22)
    at Promise.finally.completed (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:1559:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:1499:10)
    at _callCircusTest (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:1009:40)
    at _runTest (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:949:3)
    at _runTestsForDescribeBlock (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:839:13)
    at _runTestsForDescribeBlock (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:829:11)
    at _runTestsForDescribeBlock (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:829:11)
    at run (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:757:3)
    at runAndTransformResultsToJestFormat (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:1920:21)
    at jestAdapter (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/runner.js:101:19)
    at runTestInternal (.../sentry/node_modules/.pnpm/jest-runner@30.0..../jest-runner/build/testWorker.js:272:16)
    at runTest (.../sentry/node_modules/.pnpm/jest-runner@30.0..../jest-runner/build/testWorker.js:340:7)
    at Object.worker (.../sentry/node_modules/.pnpm/jest-runner@30.0..../jest-runner/build/testWorker.js:494:12)
DetectorEdit EditDetectorActions calls delete mutation when deletion is confirmed
Stack Traces | 1.06s run time
Error: Unable to find role="link" and name "detector"

Ignored nodes: comments, script, style
...
    at waitForWrapper (.../sentry/node_modules/.pnpm/@testing-library+dom@10.4.0/node_modules/@.../dom/dist/wait-for.js:163:27)
    at .../sentry/node_modules/.pnpm/@testing-library+dom@10.4.0/node_modules/@.../dom/dist/query-helpers.js:86:33
    at Object.findByRole (.../views/detectors/edit.spec.tsx:109:22)
    at Promise.finally.completed (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:1559:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:1499:10)
    at _callCircusTest (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:1009:40)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at _runTest (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:949:3)
    at _runTestsForDescribeBlock (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:839:13)
    at _runTestsForDescribeBlock (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:829:11)
    at _runTestsForDescribeBlock (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:829:11)
    at run (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:757:3)
    at runAndTransformResultsToJestFormat (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/jestAdapterInit.js:1920:21)
    at jestAdapter (.../sentry/node_modules/.pnpm/jest-circus@30.0.4_babel-plugin-macros@3.1..../jest-circus/build/runner.js:101:19)
    at runTestInternal (.../sentry/node_modules/.pnpm/jest-runner@30.0..../jest-runner/build/testWorker.js:272:16)
    at runTest (.../sentry/node_modules/.pnpm/jest-runner@30.0..../jest-runner/build/testWorker.js:340:7)
    at Object.worker (.../sentry/node_modules/.pnpm/jest-runner@30.0..../jest-runner/build/testWorker.js:494:12)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

return (
<Button size="sm" onClick={() => {}}>
{detector.disabled ? t('Enable') : t('Disable')}
<Button size="sm" onClick={toggleDisabled} busy={isUpdating}>
Copy link
Member

Choose a reason for hiding this comment

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

I think you still need to set disabled={isUpdating} so that the user can't click it twice, I'm not sure if busy does that for you

Copy link
Member Author

Choose a reason for hiding this comment

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

this is handled by busy!

const handleClick = (e: React.MouseEvent<HTMLButtonElement | HTMLAnchorElement>) => {
// Don't allow clicks when disabled or busy
if (props.disabled || props.busy) {
e.preventDefault();
e.stopPropagation();
return;
}

export function DeleteDetectorAction({detector}: {detector: Detector}) {
const organization = useOrganization();
const navigate = useNavigate();
const {mutate: deleteDetector, isPending: isDeleting} = useDeleteDetectorMutation();
Copy link
Member

Choose a reason for hiding this comment

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

If you use mutateAsync instead for the onConfirm, the modal will stay open until the request finishes which is slightly better UX

Copy link
Member Author

Choose a reason for hiding this comment

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

shoot i forgot this again 🤦🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants