Skip to content

ref(hybrid-cloud): fix monitor urls using organization_slug#42825

Merged
cathteng merged 2 commits into
masterfrom
cathy/hc/fix-monitor-urls-with-orgslug
Jan 5, 2023
Merged

ref(hybrid-cloud): fix monitor urls using organization_slug#42825
cathteng merged 2 commits into
masterfrom
cathy/hc/fix-monitor-urls-with-orgslug

Conversation

@cathteng

@cathteng cathteng commented Jan 5, 2023

Copy link
Copy Markdown
Contributor

The format of the url should be /api/0/organization/{organization_slug}/monitors/...

For HC-512, HC-513, HC-514

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 5, 2023
@cathteng
cathteng requested review from a team and RyanSkonnord January 5, 2023 19:30
@cathteng
cathteng marked this pull request as ready for review January 5, 2023 19:30
@cathteng
cathteng requested a review from a team as a code owner January 5, 2023 19:30
@JoshFerge

Copy link
Copy Markdown
Contributor

will this break the current monitors API? and would our docs and cli have to be updated?

@JoshFerge
JoshFerge requested a review from a team January 5, 2023 19:36
@cathteng

cathteng commented Jan 5, 2023

Copy link
Copy Markdown
Contributor Author

@JoshFerge i had previously changed the urls and then changed what was being used on the frontend without a problem -- this is like a second pass to correct what i had missed the first time (changing the slugs #42300 #42356 #42384, changing the frontend #42728)

docs and cli may need to be updated though

@JoshFerge
JoshFerge requested a review from davidenwang January 5, 2023 19:41
@JoshFerge

Copy link
Copy Markdown
Contributor

@cathyteng17 kk. will defer to @davidenwang

@davidenwang davidenwang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks for changing these

Comment thread src/sentry/api/urls.py
),
),
url(
r"^organizations/(?P<organization_slug>[^\/]+)/monitors/",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

placement of these seems fine for now since its close to the original monitors urls, but could you add a note here that when we delete the old monitors routes that we should probably include these routes into the larger /organizations/ route tree below?

@cathteng
cathteng merged commit 91cf672 into master Jan 5, 2023
@cathteng
cathteng deleted the cathy/hc/fix-monitor-urls-with-orgslug branch January 5, 2023 23:14
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 21, 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.

4 participants