Repository navigation
Conversation
🤖 AI-Powered Code Review AvailableYou can leverage AI-powered code review to assist with this PR! Available Commands:
|
|
🤖 Analysis started. Please wait for results... |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🤖 Analysis started. Please wait for results... |
|
@marketplace-ai-reviewer start review |
|
🤖 Analysis started. Please wait for results... |
|
Validate summary Verdict: PR can be force merged from validate perspective? ✅ |
🤖 AI 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. |
marketplace-ai-reviewer
left a comment
There was a problem hiding this comment.
Hi there, thanks for your contribution!
Overall, the new command looks great, but we noticed a few areas that need attention before merging:
- Command Logic: Please add input validation for required arguments, safely handle non-string severity values to avoid type errors, and make sure to capture and return the raw API response in
CommandResults. - YAML Standards: Fix schema key casing (e.g.,
defaultvalue) and ensure argument descriptions follow the standard formatting guidelines. - Release Notes & Versioning: Since this introduces a new command, bump the minor version (e.g., to
2.11.0) per semver guidelines and format the command name with bold-italics.
Once these updates are in place, we should be good to go!
Additionally, please address the following file-level notes:
Packs/ServiceNow/ReleaseNotes/2_10_13.md: The release notes describe adding a new command (servicenow-create-event), which represents a feature addition. Under semantic versioning rules, adding new functionality requires a minor version bump (e.g.,2.11.0and2_11_0.md), whereas a revision/patch bump (2.10.13) is reserved only for bug fixes.
Please update currentVersion in Packs/ServiceNow/pack_metadata.json to a minor version bump and rename the release notes file accordingly, or run:
demisto-sdk update-release-notes -i Packs/ServiceNow -u minorFor more information, refer to the Release Notes Documentation.
@marketplace-ai-reviewer, @omerKarkKatz, @hyaffe839 please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.
|
|
||
| client.create_event([event]) | ||
|
|
||
| return CommandResults( |
There was a problem hiding this comment.
Please assign the response from client.create_event([event]) and pass it to CommandResults as raw_response.
response = client.create_event([event])
return CommandResults(
readable_output="Event successfully sent to ServiceNow Event Management.",
raw_response=response,
)| "resolved": "closed", | ||
| } | ||
|
|
||
| node = args.get("node") |
There was a problem hiding this comment.
The docstring specifies that node, type, and time_of_event are required fields and that a DemistoException is raised if any are missing, but the function does not validate their presence before constructing the event. If omitted, None values are sent to ServiceNow.
Consider adding explicit validation:
| node = args.get("node") | |
| node = args.get("node") | |
| event_type = args.get("type") | |
| time_of_event = args.get("time_of_event") | |
| missing = [name for name, val in (("node", node), ("type", event_type), ("time_of_event", time_of_event)) if not val] | |
| if missing: | |
| raise DemistoException(f"Missing required argument(s): {', '.join(missing)}") |
| event[field] = value | ||
|
|
||
| if (severity := args.get("severity")) is not None: | ||
| event["severity"] = severity_map.get(severity.lower(), severity) |
There was a problem hiding this comment.
If severity is provided as a non-string type (such as an integer from playbook context, e.g., severity=1), calling .lower() will raise an AttributeError.
Consider casting to str before calling .lower():
| event["severity"] = severity_map.get(severity.lower(), severity) | |
| event["severity"] = severity_map.get(str(severity).lower(), str(severity)) |
| if (resolution_state := args.get("resolution_state")) is not None: | ||
| event["resolution_state"] = resolution_state_map.get(resolution_state.lower(), resolution_state) | ||
|
|
||
| client.create_event([event]) |
There was a problem hiding this comment.
The docstring states that CommandResults returns the raw ServiceNow response, but the return value of client.create_event([event]) is currently ignored and raw_response is not passed to CommandResults.
Consider capturing the response and including it in CommandResults:
| client.create_event([event]) | |
| response = client.create_event([event]) | |
| return CommandResults( | |
| readable_output="Event successfully sent to ServiceNow Event Management.", | |
| raw_response=response, | |
| ) |
| description: state of task. | ||
| type: Unknown | ||
| - arguments: | ||
| - defaultValue: Cortex |
There was a problem hiding this comment.
| - defaultValue: Cortex | |
| - defaultvalue: Cortex |
According to Cortex XSOAR YAML formatting standards, schema keys are case-sensitive and should use lowercase (e.g., defaultvalue instead of defaultValue). Please update this key here and in the quick-action arguments on line 2052.
| type: Unknown | ||
| - arguments: | ||
| - defaultValue: Cortex | ||
| description: Source of the event. |
There was a problem hiding this comment.
| description: Source of the event. | |
| description: The source of the event. |
Argument descriptions should follow the descriptive form starting with "The " and ending with a period (e.g., "The source of the event."). Please apply this convention across the other newly added argument descriptions as well.
|
|
||
| - arguments: | ||
| - defaultValue: Cortex | ||
| description: Source of the event. Defaults to "Cortex". |
There was a problem hiding this comment.
| description: Source of the event. Defaults to "Cortex". | |
| description: The source of the event. |
Since a default value is defined in the argument's defaultvalue field, remove "Defaults to "Cortex"." from the description. The defaultvalue field serves as the single source of truth. Additionally, start the description with "The " per description guidelines.
|
|
||
| ##### ServiceNow v2 | ||
|
|
||
| - Added the new **servicenow-create-event** command, which creates an event in ServiceNow Event Management. |
There was a problem hiding this comment.
Command names should be formatted in bold-italics (***command-name***) rather than bold per the Entity Styling guidelines.
| - Added the new **servicenow-create-event** command, which creates an event in ServiceNow Event Management. | |
| - Added the new ***servicenow-create-event*** command, which creates an event in ServiceNow Event Management. |
🔍 AI Triage Report AvailableAn automated triage report has been generated for this pipeline. Status: 📋 Triage Report
|
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
Description
Added the
servicenow-create-eventcommand to the ServiceNow v2 integration, enabling creation of events in ServiceNow Event Management via the/api/global/em/jsonv2webhook.What's new:
servicenow-create-eventwith arguments:source(defaults to"Cortex"),node,type,time_of_event(required),resource,severity,description,additional_info,message_key,resolution_state.critical,high,medium,low,info) are automatically mapped to ServiceNow integer values (1–5).new,in progress,resolved) are mapped to ServiceNow values (open,closed).servicenow-create-event-quick-action) is included for use from the Cortex UI.Must have