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

Updated CommonServerPython with new fields for field restructure #18051

Merged
merged 80 commits into from
May 10, 2022

Conversation

Ni-Knight
Copy link
Contributor

@Ni-Knight Ni-Knight commented Mar 10, 2022

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: link to the issue

Description

Added the needed attributes to indicator Classes under the Common class.

Minimum version of Cortex XSOAR

  • 6.0.0
  • 6.1.0
  • 6.2.0
  • 6.5.0

Does it break backward compatibility?

  • Yes
    • Further details:
  • No

Must have

  • Tests
  • Documentation

@Ni-Knight Ni-Knight changed the title Updated with new fields for field restructure Updated CommonServerPython with new fields for field restructure Mar 13, 2022
Ni-Knight and others added 8 commits April 3, 2022 11:21
# Conflicts:
#	Packs/Base/ReleaseNotes/1_18_18.md
#	Packs/Base/pack_metadata.json
Co-authored-by: Shai Yaakovi <30797606+yaakovi@users.noreply.github.com>
Co-authored-by: Shai Yaakovi <30797606+yaakovi@users.noreply.github.com>
Co-authored-by: Shai Yaakovi <30797606+yaakovi@users.noreply.github.com>
…es-Updates' into CommonServerPython-Indicator-Types-Updates
@Ni-Knight Ni-Knight requested review from yaakovi and removed request for Itay4 April 3, 2022 09:16
@lgtm-com
Copy link

lgtm-com bot commented Apr 3, 2022

This pull request introduces 1 alert when merging fad7c30 into 2c6cacf - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

Switched the new class back to `certificates` as the class `certificate` already exists
There is always at least 1 hash, so list is never empty.
@lgtm-com
Copy link

lgtm-com bot commented Apr 4, 2022

This pull request introduces 1 alert when merging 4d7fb88 into 4a6a939 - view on LGTM.com

new alerts:

  • 1 for Syntax error

@ShacharKidor ShacharKidor self-requested a review April 28, 2022 17:54
Copy link
Contributor

@ShacharKidor ShacharKidor left a comment

Choose a reason for hiding this comment

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

Hi @Ni-Knight,
Nice work!

I added unit tests and a few little corrections.
Please see the correction I added to the Account class that deals with the CommunityNotes field in the 'to_context' function.

In addition please see review comments.

Let me know if you have any questions :)

Co-authored-by: Judah Schwartz <JudahSchwartz@users.noreply.github.com>
@content-bot
Copy link
Collaborator

@ShacharKidor ShacharKidor merged commit 1428de3 into master May 10, 2022
@ShacharKidor ShacharKidor deleted the CommonServerPython-Indicator-Types-Updates branch May 10, 2022 06:42
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.

7 participants