-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[RAM][HTTP Versioning] Version Public Rule Update Route #179587
[RAM][HTTP Versioning] Version Public Rule Update Route #179587
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
@elasticmachine merge upstream |
@@ -63,7 +63,7 @@ describe('updateAlertRoute', () => { | |||
|
|||
expect(config.path).toMatchInlineSnapshot(`"/api/alerts/alert/{id}"`); | |||
|
|||
rulesClient.update.mockResolvedValueOnce(mockedResponse); | |||
rulesClient.update.mockResolvedValueOnce(mockedResponse as unknown as SanitizedRule); |
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.
nit: how about using only one type assertion when mockedResponse is declared?
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.
LGTM!
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.
Initial code changes LGTM, ping me when tests are fixed and I can give a final approval
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
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.
LGTM
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
Summary
Issue: #179476
Parent issue: #157883
This PR versions the update (
PUT '/api/alerting/rule/{id}'
) route. Still usingconfig-schema
for now, even though we will eventually switch tozod
when core is ready with openapi doc generation support in the versioned router.We are now validating update data using the update data schema when calling
rulesClient->update
. We are also validating (but not throwing) the updated rule as well.Checklist