Skip to content

Conversation

@cathteng
Copy link
Member

Adds organization_slug to the url for the MonitorStats endpoint. Keeps the original orgless URL for now.

Also refactors the test files for MonitorCheckIns and MonitorDetails.

For HC-514


@pending_silo_endpoint
@region_silo_endpoint
class MonitorStatsEndpoint(MonitorEndpoint, StatsMixin):
Copy link
Member Author

Choose a reason for hiding this comment

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

there's no test file for this endpoint so there is no test file to convert

Copy link
Contributor

Choose a reason for hiding this comment

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

The logic looks pretty substantial. Maybe this is a good time to add one?

(To be clear, I'm fine either way. Up to you whether it's worth pausing other work, or if you just want a change of pace.)

Copy link
Member Author

Choose a reason for hiding this comment

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

i was going to attempt to write a test file, but the API appears to be broken given what the frontend is passing to it (see any monitor in https://sentry.io/organizations/sentry/monitors/) so i'm not even sure how it works

Copy link
Member Author

Choose a reason for hiding this comment

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

i broke it but david helped rearrange the urls

@cathteng cathteng marked this pull request as ready for review December 15, 2022 17:52
@cathteng cathteng requested a review from a team as a code owner December 15, 2022 17:52
@cathteng cathteng merged commit 2b9d93c into master Dec 15, 2022
@cathteng cathteng deleted the cathy/hc/convert-orgless-monitor-stats-api branch December 15, 2022 18:58
@github-actions github-actions bot locked and limited conversation to collaborators Jan 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants