Skip to content
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

SetGridField fixes #31318

Merged
merged 29 commits into from
Dec 12, 2023
Merged

SetGridField fixes #31318

merged 29 commits into from
Dec 12, 2023

Conversation

sapirshuker
Copy link
Contributor

@sapirshuker sapirshuker commented Dec 5, 2023

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

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: https://jira-dc.paloaltonetworks.com/browse/XSUP-30932
https://jira-dc.paloaltonetworks.com/browse/XSUP-31038
https://jira-dc.paloaltonetworks.com/browse/XSUP-30050

Description

Fix an issue that the script to raise an uninformative error when trying to run SetGridField from the playground.

Must have

  • Tests
  • Documentation

@sapirshuker sapirshuker marked this pull request as ready for review December 7, 2023 13:32
Copy link
Contributor

@GuyAfik GuyAfik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sapirshuker LGTM, nice work

@@ -362,24 +369,27 @@ def main(): # pragma: no cover
keys_from_nested=argToList(args.get('keys_from_nested'))
)
# Execute automation 'setIncident` which change the Context data in the incident
res = demisto.executeCommand("setIncident", {
res_set = demisto.executeCommand("setIncident", {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why changing the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The res was overwritten by the line:
res = demisto.executeCommand("getIncidents", {"id": demisto.incident().get("id")})
and I would like to save it so I can raise an error if necessary

Copy link
Contributor

@eyalpalo eyalpalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@content-bot
Copy link
Collaborator

This PR was automatically updated by a GitHub Action

  • CommonScripts pack version was bumped to 1.12.57.

To stop automatic version bumps, add the ignore-auto-bump-version label to the github PR.

@sapirshuker sapirshuker requested a review from eyalpalo December 11, 2023 12:58
custom_fields = data[0].get("CustomFields") if data and data[0].get("CustomFields") else {}
if (not is_playground) and table and grid_id not in custom_fields:
raise ValueError(get_error_message(grid_id))
if is_error(res_set):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add is_error to the getIncidents command as well in line 392

Copy link
Contributor

@ilaner ilaner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! see small comment

Copy link
Contributor

@eyalpalo eyalpalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Add what @ilaner asked and merge

@sapirshuker sapirshuker merged commit 10954e3 into master Dec 12, 2023
@sapirshuker sapirshuker deleted the XSUP-30932/SetGridField branch December 12, 2023 14:54
sharonfi99 pushed a commit that referenced this pull request Dec 12, 2023
* SetGridField fixes

* update docker

* add support for incident_id

* do only needed change

* revert yml

* docker image

* revert ruff changes

* revert ruff changes

* fixes

* fixes

* fixes

* RN conflicts

* Update SetGridField.py

* add tests

* fix test

* add tests

* Update SetGridField_test.py

* fixes CR

* fix CR review,update docker

* Bump pack from version CommonScripts to 1.12.57.

* fix CR review

* fix CR review

---------

Co-authored-by: Content Bot <bot@demisto.com>
tkatzir pushed a commit that referenced this pull request Dec 20, 2023
* SetGridField fixes

* update docker

* add support for incident_id

* do only needed change

* revert yml

* docker image

* revert ruff changes

* revert ruff changes

* fixes

* fixes

* fixes

* RN conflicts

* Update SetGridField.py

* add tests

* fix test

* add tests

* Update SetGridField_test.py

* fixes CR

* fix CR review,update docker

* Bump pack from version CommonScripts to 1.12.57.

* fix CR review

* fix CR review

---------

Co-authored-by: Content Bot <bot@demisto.com>
sapirshuker added a commit that referenced this pull request Dec 21, 2023
* SetGridField fixes

* update docker

* add support for incident_id

* do only needed change

* revert yml

* docker image

* revert ruff changes

* revert ruff changes

* fixes

* fixes

* fixes

* RN conflicts

* Update SetGridField.py

* add tests

* fix test

* add tests

* Update SetGridField_test.py

* fixes CR

* fix CR review,update docker

* Bump pack from version CommonScripts to 1.12.57.

* fix CR review

* fix CR review

---------

Co-authored-by: Content Bot <bot@demisto.com>
maimorag pushed a commit that referenced this pull request Dec 31, 2023
* SetGridField fixes

* update docker

* add support for incident_id

* do only needed change

* revert yml

* docker image

* revert ruff changes

* revert ruff changes

* fixes

* fixes

* fixes

* RN conflicts

* Update SetGridField.py

* add tests

* fix test

* add tests

* Update SetGridField_test.py

* fixes CR

* fix CR review,update docker

* Bump pack from version CommonScripts to 1.12.57.

* fix CR review

* fix CR review

---------

Co-authored-by: Content Bot <bot@demisto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants