-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add alerts initial_arr_flow_completed optional filter #42575
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 alerts initial_arr_flow_completed optional filter #42575
Conversation
* Add alerts initial_arr_flow_completed optional filter * Update Packs/Claroty/Integrations/Claroty/Claroty.py * Update Packs/Claroty/Integrations/Claroty/Claroty.py * Update Packs/Claroty/Integrations/Claroty/Claroty.yml --------- Co-authored-by: merit-maita <49760643+merit-maita@users.noreply.github.com>
Coverage Report
|
||||||||||||||||||||||||||||||
…itial-arr-flow-completed-filter
|
🤖 Content AI Reviewer: Analysis started. Please wait for results... |
|
Validate summary Verdict: PR can be force merged from validate perspective? ✅ |
🤖 Content-bot Review DisclaimerThis review was generated by an AI-powered tool and may contain inaccuracies. Please be advised, and we extend our sincere apologies for any inconvenience this may cause. |
content-bot
left a comment
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.
Thanks for the updates to the Claroty integration! I have a few suggestions to help polish the code and documentation.
Please consider using argToBoolean for consistency, adding type hints, and including a test case for fetch_incidents. There are also minor fixes needed in the README regarding output types and capitalization in the Release Notes.
Great work overall
@merit-maita please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.
| if bool(demisto.params().get("exclude_resolved_alerts", False)): | ||
| extra_filters_list = _add_exclude_resolved_alerts_filters(extra_filters_list) | ||
|
|
||
| if argToBoolean(demisto.params().get("include_only_arr_completed_alerts", False)): |
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.
Consistency suggestion: Update line 156 to use argToBoolean.
| return filters | ||
|
|
||
|
|
||
| def _add_include_only_arr_completed_alerts_filters(filters: list[Filter]): |
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.
- Consider simplifying by removing this helper function.
- Add a return type hint
-> list[Filter].
| assert response["objects"][0]["alert_indicators"] | ||
|
|
||
|
|
||
| def test_query_with_arr_filter__filter_is_applied(mocker, requests_mock): |
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.
Consider adding a test case for fetch_incidents to verify the filter application.
| | Claroty.Alert.ResourceID | String | The alert resource ID (AlertID-SiteID). | | ||
| | Claroty.Alert.Severity | String | The alert severity. | | ||
| | Claroty.Alert.Category | String | The alert category. | | ||
| | Claroty.Alert.InitialArrFlowCompleted | Number | Whether the alert has completed its initial Automated Resolution Rules flow. | |
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.
The output type should be Boolean to match the YAML definition.
|
|
||
| ##### Claroty | ||
|
|
||
| - Added support for **Include only ARR completed alerts** parameter that includes only alerts that have completed their initial automated resolution rules flow. |
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.
Consider capitalizing 'Automated Resolution Rules' to match the feature name and the YAML description.
Original External PR
external pull request
Contributor
@Royee-Topper
Contributing to Cortex XSOAR Content
Make sure to register your contribution by filling the contribution registration form
The Pull Request will be reviewed only after the contribution registration form is filled.
Status
Related Issues
fixes: https://jira-dc.paloaltonetworks.com/browse/CIAC-15385
Description
A few sentences describing the overall goals of the pull request's commits.
Must have