-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix(detectors): Navigate to detector type list page after deletion #103562
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(detectors): Navigate to detector type list page after deletion #103562
Conversation
| onConfirm: async () => { | ||
| await deleteDetector(detector.id); | ||
| navigate(makeMonitorBasePathname(organization.slug)); | ||
| navigate(makeMonitorTypePathname(organization.slug, detector.type)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The makeMonitorTypePathname function is imported and called in actions.tsx but is not defined, leading to build or runtime errors.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The makeMonitorTypePathname function is imported in static/app/views/detectors/components/details/common/actions.tsx but is not defined in static/app/views/detectors/pathnames.tsx or anywhere else in the codebase. This function is actively called at line 104 within the DeleteDetectorAction component. This will lead to a build-time failure due to a TypeScript/module resolution error or a runtime error when the DeleteDetectorAction component attempts to execute, preventing users from deleting detectors. The PR appears to be incomplete, as it introduces the usage of this function without defining it.
💡 Suggested Fix
Define the makeMonitorTypePathname function in static/app/views/detectors/pathnames.tsx to correctly map detector types to their corresponding list pages.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/views/detectors/components/details/common/actions.tsx#L104
Potential issue: The `makeMonitorTypePathname` function is imported in
`static/app/views/detectors/components/details/common/actions.tsx` but is not defined in
`static/app/views/detectors/pathnames.tsx` or anywhere else in the codebase. This
function is actively called at line 104 within the `DeleteDetectorAction` component.
This will lead to a build-time failure due to a TypeScript/module resolution error or a
runtime error when the `DeleteDetectorAction` component attempts to execute, preventing
users from deleting detectors. The PR appears to be incomplete, as it introduces the
usage of this function without defining it.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2785176
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
part of a diff PR
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
When deleting a monitor, navigate to the detector type-specific list page (e.g., /monitors/crons/, /monitors/uptime/, /monitors/metrics/) instead of the base monitors page. This provides a predictable navigation experience that takes users back to the relevant monitor type they were working with. Fixes [ID-1106: Deleting a monitor should return to previous list page](https://linear.app/getsentry/issue/ID-1106/deleting-a-monitor-should-return-to-previous-list-page)
b789bfa to
76e0add
Compare
When deleting a monitor, navigate to the detector type-specific list
page (e.g., /monitors/crons/, /monitors/uptime/, /monitors/metrics/)
instead of the base monitors page. This provides a predictable
navigation experience that takes users back to the relevant monitor
type they were working with.
Fixes ID-1106: Deleting a monitor should return to previous list page