-
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
[Cases] API to return a case's connectors information #147295
[Cases] API to return a case's connectors information #147295
Conversation
@@ -61,6 +61,36 @@ const existsSchema = s.object({ | |||
}) | |||
), | |||
}); | |||
|
|||
const rangeSchema = s.object({ |
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.
These changes are needed for a filter used here: https://github.com/elastic/kibana/pull/147295/files#diff-94dd88fd0956fc92263b02e7ea005e640da97008a8c22d7acd8a13fe2eb11c70R161
), | ||
}); | ||
|
||
const termValueSchema = s.object({ |
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.
These changes are needed for the filter used here: https://github.com/elastic/kibana/pull/147295/files#diff-94dd88fd0956fc92263b02e7ea005e640da97008a8c22d7acd8a13fe2eb11c70R174
term: s.recordOf(s.string(), s.object({ value: s.string() })), | ||
}); | ||
|
||
const nestedSchema = s.object({ |
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.
These changes are needed for the filter here: https://github.com/elastic/kibana/pull/147295/files#diff-94dd88fd0956fc92263b02e7ea005e640da97008a8c22d7acd8a13fe2eb11c70R168
@@ -86,7 +86,7 @@ export async function deleteAll( | |||
concurrency: MAX_CONCURRENT_SEARCHES, | |||
}); | |||
|
|||
await userActionService.bulkCreateAttachmentDeletion({ | |||
await userActionService.creator.bulkCreateAttachmentDeletion({ |
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.
All methods related to creating a user action or audit logging were moved to a new class. This class is accessible through the original user action service class via the creator
public accessor method.
@@ -119,8 +120,11 @@ export class CasesClientFactory { | |||
excludedExtensions: [SECURITY_EXTENSION_ID], | |||
}); | |||
|
|||
const savedObjectsSerializer = savedObjectsService.createSerializer(); |
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.
We now use top hits aggregations that return a raw elasticsearch version of a saved object document. To interact with that document we need to convert it to a saved object. This serializer does the conversion for us so we're passing it through to the user action service.
@@ -0,0 +1,428 @@ | |||
/* |
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.
Shouldn't be any new functionality in here just the class structure and copied methods from the user action service.
@@ -126,3 +134,24 @@ export type CommonBuilderArguments = CommonArguments & { | |||
export interface BuilderDeps { | |||
persistableStateAttachmentTypeRegistry: PersistableStateAttachmentTypeRegistry; | |||
} | |||
|
|||
export interface ServiceContext { |
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.
Refactored the parameters to the user action service into this context interface
@@ -52,6 +52,7 @@ | |||
"@kbn/safer-lodash-set", | |||
"@kbn/logging-mocks", | |||
"@kbn/ecs", | |||
"@kbn/core-saved-objects-base-server-mocks", |
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.
I think this is so we can get the mock for the serializer? I didn't add it explicitly, maybe vs code did 🤷♂️
@@ -0,0 +1,324 @@ | |||
/* |
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.
Moved all the connector related utility functions for the integrations tests to this file
@@ -0,0 +1,676 @@ | |||
/* |
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.
Integration tests for the new API are here
…ana into cases-connector-api
Pinging @elastic/response-ops (Team:ResponseOps) |
Pinging @elastic/response-ops-cases (Feature:Cases) |
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.
Code LGTM! Tested and is working as expected 🚀 . I left some nit comments.
const enrichedConnectors: EnrichedConnector[] = []; | ||
|
||
for (const aggregationConnector of connectors) { | ||
const connectorDetails = await actionsClient.get({ id: aggregationConnector.connectorId }); |
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.
To my understanding the actionsClient
do not provide a bulk get operation. Do you think it makes sense to do a Promise.all
or it is too early to optimize? Another suggestion would be to not do the call at all and rely to the pushed info to get the name. This has the downside that the name would not get updated if the connector's name is updated (on the next push it will be updated). Also, could you please open an issue about adding the bulk get operation to the actionsClient
?
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.
Oh great point, looks like they do have a bulk get 🙌 I'll use that.
if (pushRequest.length <= 0) { | ||
return new Map(); | ||
} |
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: should we move this check inside getConnectorFieldsUsedInPushes
?
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.
Great idea 👍
type CreatePayloadFunction<Item, ActionType extends ActionTypeValues> = ( | ||
items: Item[] | ||
) => UserActionParameters<ActionType>['payload']; | ||
public async getConnectorFieldsUsedInPushes( |
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: I found the name a bit confusing. To my understanding, this function returns the connector's fields before the latest push for each connector. What do you think about getConnectorFieldsBeforeLatestPush
?
|
||
await checkConnectorsAuthorization({ authorization, connectors, latestUserAction }); | ||
|
||
const enrichedConnectors = await enrichConnectors({ |
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.
I am wondering if it makes sense for the enrichConnectors
to return the final response (GetCaseConnectorsResponseRt
). What do you think?
💚 Build Succeeded
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: |
## Summary This PR updates the UI to use the new connector's API introduced by this PR #147295. In this PR I refactor a bit some of the logic we have for the connectors. I left comments to explain the reasoning. ## Testing Please test the following: - The fields of each connector are preserved correctly when changing connectors - The following callout is shown when you do not have selected any connector <img width="784" alt="Screenshot 2023-01-23 at 2 21 33 PM" src="https://user-images.githubusercontent.com/7871006/214534706-5d447472-a40b-4fb4-aa4f-733bdcc76d9e.png"> - The edit connector button (pencil) is not shown if you do not have access to use connectors. Also, the following text should be shown. <img width="784" alt="Screenshot 2023-01-23 at 2 20 30 PM" src="https://user-images.githubusercontent.com/7871006/214535027-4b6b17a6-9d2c-4634-b78a-e966d70ae966.png"> - You see the following callout if a) select a connector to a case and b) then delete the connector <img width="778" alt="Screenshot 2023-01-23 at 2 11 49 PM" src="https://user-images.githubusercontent.com/7871006/214535408-ad72ed87-d9ea-4449-8340-4b17d63ed7b4.png"> - The text of the connector's push button is shown correctly. It should be `Push <connector_nane>` for the first push and `Updated Push <connector_nane>` for any other push. <img width="214" alt="Screenshot 2023-01-25 at 12 09 29 PM" src="https://user-images.githubusercontent.com/7871006/214536143-72e8f5f7-bb9d-4e4f-b2e7-66186ac76cdb.png"> <img width="292" alt="Screenshot 2023-01-25 at 12 09 17 PM" src="https://user-images.githubusercontent.com/7871006/214536169-765a682f-f4d9-4389-ad25-ec8184f99040.png"> - The connector's push button is disabled correctly. - When the connector's push button is disabled a tooltip is shown <img width="291" alt="Screenshot 2023-01-25 at 12 11 27 PM" src="https://user-images.githubusercontent.com/7871006/214536498-4058f1bd-2548-4a87-8e7c-589e1717e0a2.png"> - The push user action label is displayed correctly. It should be `pushed as new incident...` for the first push and `updated incident...` for any other push. <img width="630" alt="Screenshot 2023-01-25 at 9 54 23 AM" src="https://user-images.githubusercontent.com/7871006/214533317-ed29f938-298c-4d5f-94f9-9f11821f9365.png"> <img width="614" alt="Screenshot 2023-01-25 at 11 56 56 AM" src="https://user-images.githubusercontent.com/7871006/214533604-5be88d51-a021-4095-ab70-1938d82c6bdc.png"> - The push arrow indicators are shown correctly. The top arrow should be displayed only if it is the latest push. The bottom arrow should be displayed only if it is the latest push and new changes need to be pushed. Changes are considered the update of text or description, the addition or edit of a comment, and the addition or removal of a case. <img width="371" alt="Screenshot 2023-01-11 at 1 38 37 PM" src="https://user-images.githubusercontent.com/7871006/214534308-29da895a-2dff-4ae7-ace5-31ff7574feac.png"> <img width="322" alt="Screenshot 2023-01-11 at 1 37 31 PM" src="https://user-images.githubusercontent.com/7871006/214534312-9e28a398-ae64-4cc2-9f07-0ed72546e337.png"> - The "View <incident_name>" shows the latest push incident of each connector when changing connectors. <img width="229" alt="Screenshot 2023-01-23 at 2 25 30 PM" src="https://user-images.githubusercontent.com/7871006/214039111-0d4692c3-e7c3-4b7b-825b-bd86a0f03a96.png"> Depends on: #149451, #149535 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Jonathan Buttner <[email protected]>
## Summary This PR updates the UI to use the new connector's API introduced by this PR elastic#147295. In this PR I refactor a bit some of the logic we have for the connectors. I left comments to explain the reasoning. ## Testing Please test the following: - The fields of each connector are preserved correctly when changing connectors - The following callout is shown when you do not have selected any connector <img width="784" alt="Screenshot 2023-01-23 at 2 21 33 PM" src="https://user-images.githubusercontent.com/7871006/214534706-5d447472-a40b-4fb4-aa4f-733bdcc76d9e.png"> - The edit connector button (pencil) is not shown if you do not have access to use connectors. Also, the following text should be shown. <img width="784" alt="Screenshot 2023-01-23 at 2 20 30 PM" src="https://user-images.githubusercontent.com/7871006/214535027-4b6b17a6-9d2c-4634-b78a-e966d70ae966.png"> - You see the following callout if a) select a connector to a case and b) then delete the connector <img width="778" alt="Screenshot 2023-01-23 at 2 11 49 PM" src="https://user-images.githubusercontent.com/7871006/214535408-ad72ed87-d9ea-4449-8340-4b17d63ed7b4.png"> - The text of the connector's push button is shown correctly. It should be `Push <connector_nane>` for the first push and `Updated Push <connector_nane>` for any other push. <img width="214" alt="Screenshot 2023-01-25 at 12 09 29 PM" src="https://user-images.githubusercontent.com/7871006/214536143-72e8f5f7-bb9d-4e4f-b2e7-66186ac76cdb.png"> <img width="292" alt="Screenshot 2023-01-25 at 12 09 17 PM" src="https://user-images.githubusercontent.com/7871006/214536169-765a682f-f4d9-4389-ad25-ec8184f99040.png"> - The connector's push button is disabled correctly. - When the connector's push button is disabled a tooltip is shown <img width="291" alt="Screenshot 2023-01-25 at 12 11 27 PM" src="https://user-images.githubusercontent.com/7871006/214536498-4058f1bd-2548-4a87-8e7c-589e1717e0a2.png"> - The push user action label is displayed correctly. It should be `pushed as new incident...` for the first push and `updated incident...` for any other push. <img width="630" alt="Screenshot 2023-01-25 at 9 54 23 AM" src="https://user-images.githubusercontent.com/7871006/214533317-ed29f938-298c-4d5f-94f9-9f11821f9365.png"> <img width="614" alt="Screenshot 2023-01-25 at 11 56 56 AM" src="https://user-images.githubusercontent.com/7871006/214533604-5be88d51-a021-4095-ab70-1938d82c6bdc.png"> - The push arrow indicators are shown correctly. The top arrow should be displayed only if it is the latest push. The bottom arrow should be displayed only if it is the latest push and new changes need to be pushed. Changes are considered the update of text or description, the addition or edit of a comment, and the addition or removal of a case. <img width="371" alt="Screenshot 2023-01-11 at 1 38 37 PM" src="https://user-images.githubusercontent.com/7871006/214534308-29da895a-2dff-4ae7-ace5-31ff7574feac.png"> <img width="322" alt="Screenshot 2023-01-11 at 1 37 31 PM" src="https://user-images.githubusercontent.com/7871006/214534312-9e28a398-ae64-4cc2-9f07-0ed72546e337.png"> - The "View <incident_name>" shows the latest push incident of each connector when changing connectors. <img width="229" alt="Screenshot 2023-01-23 at 2 25 30 PM" src="https://user-images.githubusercontent.com/7871006/214039111-0d4692c3-e7c3-4b7b-825b-bd86a0f03a96.png"> Depends on: elastic#149451, elastic#149535 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Jonathan Buttner <[email protected]>
This PR implements a new internal API to return the information relating to all the connectors used throughout a case's lifespan.
Fixes: #134346
cURL example
Response
Notable changes:
UserActionPersister
CaseUserActionService
class to pull the saved object client, logger, and other fields passed to each function via parameters to be wrapped in acontext
member field within the classsavedObjectsService.createSerializer
to transform a raw elasticsearch document into the saved object representation_connectors
route andgetConnectors
client functionNeeds to be pushed algorithm
To determine whether a case needs to be pushed for a certain connector we follow this algorithm:
connector
user action or if the connector was configured only once when the case was initially created it'll be on thecreate_case
user action