[Response Ops][Rule Form V2] Prepare Rule Form V2 Action Form Dependencies #186490
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JiaweiWu
added
release_note:skip
|
Pinging @elastic/response-ops (Team:ResponseOps) |
1 task
cnasikas
reviewed
Jul 10, 2024
There was a problem hiding this comment.
LGTM! I left some comments. I noticed that the following files are missing tests. How would you like to handle that?
- packages/kbn-alerts-ui-shared/src/action_variables/get_available_action_variables.ts
- packages/kbn-alerts-ui-shared/src/action_variables/transforms.ts
- packages/kbn-alerts-ui-shared/src/common/apis/fetch_connector_types/transform_connector_types_response.ts
- packages/kbn-alerts-ui-shared/src/common/apis/fetch_connectors/transform_connectors_response.ts
- packages/kbn-alerts-ui-shared/src/common/hooks/use_load_connector_types.ts
- packages/kbn-alerts-ui-shared/src/common/hooks/use_load_connectors.ts
- packages/kbn-alerts-ui-shared/src/common/hooks/use_load_rule_type_aad_template_fields.ts
About the new hooks, is there any plan to use them in the flyout or they are dedicated to the new rule form?
x-pack/examples/triggers_actions_ui_example/public/application.tsx
Outdated
Show resolved
Hide resolved
...d/src/common/apis/fetch_rule_type_aad_template_fields/fetch_rule_type_aad_template_fields.ts
Outdated
Show resolved
Hide resolved
...d/src/common/apis/fetch_rule_type_aad_template_fields/fetch_rule_type_aad_template_fields.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
super nit: Wdyt about deleting this file and importing the function in the files that need it? Same for x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/connectors.ts.
There was a problem hiding this comment.
I think it's a bit annoying to do this now because other plugins are importing this from triggers actions UI (x-pack/packages/kbn-elastic-assistant/impl/connectorland/use_load_action_types/index.tsx) for example
| return fetchConnectorTypes({ http, includeSystemActions }); | ||
| }; | ||
|
|
||
| const { data, isLoading, isFetching, isInitialLoading } = useQuery({ |
There was a problem hiding this comment.
In our last sync, @umbopepato suggested returning the whole value of the useQuery hook instead of choosing specific attributes to return. There are a lot of benefits like consistency in types, automatic type inference from the hook, chain query objects, and the ability to select which loading state you would like to check. For example, when the enable property is false the isLoading is true but the isFetching is false. What do you think?
Same for packages/kbn-alerts-ui-shared/src/common/hooks/use_load_connectors.ts and packages/kbn-alerts-ui-shared/src/common/hooks/use_load_rule_type_aad_template_fields.ts.
There was a problem hiding this comment.
Yea I dont really mind, i think we had a bug where if we just check isLoading, it doesn't tell us 100% if it was loading, so we had to check isLoading || isFetching. I just use that now to be safe.
packages/kbn-alerts-ui-shared/src/common/hooks/use_load_rule_type_aad_template_fields.ts
Outdated
Show resolved
Hide resolved
those tests already exist
I think we can do this when we phase out the old form |
|
@elasticmachine merge upstream |
cnasikas
approved these changes
Jul 16, 2024
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| test('should call API endpoint with the correct parameters', async () => { |
There was a problem hiding this comment.
The test description says it verifies if the endpoint was called with correct params but we do not check that in the tests. Should we add it along with the response check we already have?
|
|
||
| expect(result.current.data).toEqual([ | ||
| { | ||
| description: '', |
There was a problem hiding this comment.
Shouldn't the description have the description value based on the HTTP response?
| import { DataViewField } from '@kbn/data-views-plugin/common'; | ||
| import { BASE_RAC_ALERTS_API_PATH, EMPTY_AAD_FIELDS } from '../../constants'; | ||
|
|
||
| export const getDescription = (fieldName: string, ecsFlat: Record<string, EcsMetadata>) => { |
There was a problem hiding this comment.
nit: would be nice to have basic test for getDescription .
| return fetchConnectors({ http, includeSystemActions }); | ||
| }; | ||
|
|
||
| const { data, isLoading, isFetching, isInitialLoading } = useQuery({ |
There was a problem hiding this comment.
I might not have full idea here so might be stupid question: how do we handle error scenarios for all these hooks?
There was a problem hiding this comment.
Haha yea we don't really handle errors because if the endpoint fails then something quite wrong has happened
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
History
|
JiaweiWu
added a commit
that referenced
this pull request
Sep 27, 2024
…Form (#187434) ## Summary Issue: #179105 Related PR: #180539 Final PR of the rule actions V2 PR (2/2 of the actions PRs). This PR contains the actions modal and actions form. This PR depends on #186490. I have also created a example plugin to demonstrate this PR. To access: 1. Run the branch with yarn start --run-examples 2. Navigate to http://localhost:5601/app/triggersActionsUiExample/rule/create/<ruleTypeId> (I use .es-query) 3. Create a rule 4. Navigate to http://localhost:5601/app/triggersActionsUiExample/rule/edit/<ruleId> with the rule you just created to edit the rule <img width="1236" alt="Screenshot 2024-07-02 at 5 15 51 PM" src="https://github.com/elastic/kibana/assets/74562234/1dc5d2a9-804a-4861-94ba-814de73dc3ab">  <img width="1087" alt="Screenshot 2024-07-02 at 5 15 58 PM" src="https://github.com/elastic/kibana/assets/74562234/903e66b5-f9a1-4d09-b121-b1dcecdff72c"> ### Checklist - [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 --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Sep 27, 2024
…Form (elastic#187434) ## Summary Issue: elastic#179105 Related PR: elastic#180539 Final PR of the rule actions V2 PR (2/2 of the actions PRs). This PR contains the actions modal and actions form. This PR depends on elastic#186490. I have also created a example plugin to demonstrate this PR. To access: 1. Run the branch with yarn start --run-examples 2. Navigate to http://localhost:5601/app/triggersActionsUiExample/rule/create/<ruleTypeId> (I use .es-query) 3. Create a rule 4. Navigate to http://localhost:5601/app/triggersActionsUiExample/rule/edit/<ruleId> with the rule you just created to edit the rule <img width="1236" alt="Screenshot 2024-07-02 at 5 15 51 PM" src="https://github.com/elastic/kibana/assets/74562234/1dc5d2a9-804a-4861-94ba-814de73dc3ab">  <img width="1087" alt="Screenshot 2024-07-02 at 5 15 58 PM" src="https://github.com/elastic/kibana/assets/74562234/903e66b5-f9a1-4d09-b121-b1dcecdff72c"> ### Checklist - [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 --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 54659e8)
kibanamachine
added a commit
that referenced
this pull request
Sep 27, 2024
…tions Form (#187434) (#194254) # Backport This will backport the following commits from `main` to `8.x`: - [[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form (#187434)](#187434) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Jiawei Wu","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-27T06:34:27Z","message":"[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form (#187434)\n\n## Summary\r\nIssue: https://github.com/elastic/kibana/issues/179105\r\nRelated PR: https://github.com/elastic/kibana/pull/180539\r\n\r\nFinal PR of the rule actions V2 PR (2/2 of the actions PRs). This PR\r\ncontains the actions modal and actions form. This PR depends on\r\nhttps://github.com//pull/186490.\r\n\r\nI have also created a example plugin to demonstrate this PR. To access:\r\n\r\n1. Run the branch with yarn start --run-examples\r\n2. Navigate to\r\nhttp://localhost:5601/app/triggersActionsUiExample/rule/create/<ruleTypeId>\r\n(I use .es-query)\r\n3. Create a rule\r\n4. Navigate to\r\nhttp://localhost:5601/app/triggersActionsUiExample/rule/edit/<ruleId>\r\nwith the rule you just created to edit the rule\r\n\r\n<img width=\"1236\" alt=\"Screenshot 2024-07-02 at 5 15 51 PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/74562234/1dc5d2a9-804a-4861-94ba-814de73dc3ab\">\r\n\r\n\r\n\r\n<img width=\"1087\" alt=\"Screenshot 2024-07-02 at 5 15 58 PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/74562234/903e66b5-f9a1-4d09-b121-b1dcecdff72c\">\r\n\r\n\r\n### Checklist\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"54659e8ae002aa68be3ee472ef12b3d3f546a1ea","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.16.0"],"title":"[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form","number":187434,"url":"https://github.com/elastic/kibana/pull/187434","mergeCommit":{"message":"[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form (#187434)\n\n## Summary\r\nIssue: https://github.com/elastic/kibana/issues/179105\r\nRelated PR: https://github.com/elastic/kibana/pull/180539\r\n\r\nFinal PR of the rule actions V2 PR (2/2 of the actions PRs). This PR\r\ncontains the actions modal and actions form. This PR depends on\r\nhttps://github.com//pull/186490.\r\n\r\nI have also created a example plugin to demonstrate this PR. To access:\r\n\r\n1. Run the branch with yarn start --run-examples\r\n2. Navigate to\r\nhttp://localhost:5601/app/triggersActionsUiExample/rule/create/<ruleTypeId>\r\n(I use .es-query)\r\n3. Create a rule\r\n4. Navigate to\r\nhttp://localhost:5601/app/triggersActionsUiExample/rule/edit/<ruleId>\r\nwith the rule you just created to edit the rule\r\n\r\n<img width=\"1236\" alt=\"Screenshot 2024-07-02 at 5 15 51 PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/74562234/1dc5d2a9-804a-4861-94ba-814de73dc3ab\">\r\n\r\n\r\n\r\n<img width=\"1087\" alt=\"Screenshot 2024-07-02 at 5 15 58 PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/74562234/903e66b5-f9a1-4d09-b121-b1dcecdff72c\">\r\n\r\n\r\n### Checklist\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"54659e8ae002aa68be3ee472ef12b3d3f546a1ea"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/187434","number":187434,"mergeCommit":{"message":"[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form (#187434)\n\n## Summary\r\nIssue: https://github.com/elastic/kibana/issues/179105\r\nRelated PR: https://github.com/elastic/kibana/pull/180539\r\n\r\nFinal PR of the rule actions V2 PR (2/2 of the actions PRs). This PR\r\ncontains the actions modal and actions form. This PR depends on\r\nhttps://github.com//pull/186490.\r\n\r\nI have also created a example plugin to demonstrate this PR. To access:\r\n\r\n1. Run the branch with yarn start --run-examples\r\n2. Navigate to\r\nhttp://localhost:5601/app/triggersActionsUiExample/rule/create/<ruleTypeId>\r\n(I use .es-query)\r\n3. Create a rule\r\n4. Navigate to\r\nhttp://localhost:5601/app/triggersActionsUiExample/rule/edit/<ruleId>\r\nwith the rule you just created to edit the rule\r\n\r\n<img width=\"1236\" alt=\"Screenshot 2024-07-02 at 5 15 51 PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/74562234/1dc5d2a9-804a-4861-94ba-814de73dc3ab\">\r\n\r\n\r\n\r\n<img width=\"1087\" alt=\"Screenshot 2024-07-02 at 5 15 58 PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/74562234/903e66b5-f9a1-4d09-b121-b1dcecdff72c\">\r\n\r\n\r\n### Checklist\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"54659e8ae002aa68be3ee472ef12b3d3f546a1ea"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Jiawei Wu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Issue: #179106
Depends on this PR: #184892
Part 1/2 of the action form portion of the rule form V2. This PR doesn't add any functionality, all it's doing is moving some of the dependencies needed by the second PR to packages so it can be shared.
Checklist