Skip to content
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

Add openAPI schema for some internal endpoints #5037

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

Ferril
Copy link
Member

@Ferril Ferril commented Sep 18, 2024

What this PR does

Adds openAPI schema for following endpoints:

  • /escalation_chain
  • /escalation_policy
  • /channel_filter
  • /user_notification_policy

Which issue(s) this PR closes

Related to https://github.com/grafana/oncall-private/issues/2457

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • Added the relevant release notes label (see labels prefixed w/ release:). These labels dictate how your PR will
    show up in the autogenerated release notes.

- /escalation_chain
- /escalation_policy
- /channel_filter
- /user_notification_policy
@Ferril Ferril added pr:no public docs Added to a PR that does not require public documentation updates release:ignore PR will not be added to release notes labels Sep 18, 2024
@Ferril Ferril requested a review from a team as a code owner September 18, 2024 12:06
Copy link
Member

@vadimkerr vadimkerr left a comment

Choose a reason for hiding this comment

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

great job 👍

engine/apps/api/serializers/channel_filter.py Outdated Show resolved Hide resolved
Comment on lines -61 to -70
user_id = self.request.query_params.get("user")
slack_channel_id = self.request.query_params.get("slack_channel")
channel_filter_id = self.request.query_params.get("channel_filter")
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 these are no longer used, can we remove them?

Copy link
Member Author

Choose a reason for hiding this comment

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

They have been already removed

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/grafana/oncall/pull/5037/files#diff-232bde1ff9834bf2d4fea690e3c5c94f77acd148ac0579b4081a8b2f43c37831R46-R56

I mean that frontend/mobile app don't seem to use these filters, can we remove them?

Copy link
Member Author

Choose a reason for hiding this comment

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

got it, yeah, makes sense 👍

engine/common/api_helpers/filters.py Outdated Show resolved Hide resolved
engine/apps/api/views/channel_filter.py Outdated Show resolved Hide resolved
engine/apps/api/serializers/channel_filter.py Outdated Show resolved Hide resolved
Comment on lines +351 to +354
"/channel_filters",
"/escalation_chains",
"/escalation_policies",
"/notification_policies",
Copy link
Member

Choose a reason for hiding this comment

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

might be a good idea to re-generate frontend types and commit them in this PR as well

Copy link
Contributor

@joeyorlando joeyorlando left a comment

Choose a reason for hiding this comment

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

I think we should revert the pnpm generate-types commit and run that in the packages/grafana-oncall-app directory of grafana/irm instead.

cc: @brojd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates release:ignore PR will not be added to release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants