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

Improve Fetch incidents structure and add append notes funcitonality #33436

Merged

Conversation

DNRRomero
Copy link
Contributor

@DNRRomero DNRRomero commented Mar 19, 2024

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

A few sentences describing the overall goals of the pull request's commits.

Must have

  • Tests
  • Documentation

DNRRomero and others added 4 commits March 15, 2024 15:24
* Simplify fetch_incidents method to avoid duplicate incidents

change alert ids to str to facilitate storage and comparison
add set to avoid duplicates
delete unnecessary timestamp fields in last_run
reduce fetch_incidents to a single http call to alerts endpoint

* Add PR comments on fixing type hints and polishing tests

---------

Co-authored-by: Diego Ramirez <[email protected]>
remove get_alert_attachment_command to avoid confusing customers
@DNRRomero DNRRomero force-pushed the fix_duplicates branch 2 times, most recently from fa13f27 to 533f859 Compare March 20, 2024 13:50
@jbabazadeh jbabazadeh requested review from merit-maita and removed request for jbabazadeh March 21, 2024 09:17
@jbabazadeh jbabazadeh assigned merit-maita and unassigned jbabazadeh Mar 21, 2024
@DNRRomero
Copy link
Contributor Author

Hi @merit-maita we've had some issues with setting the release notes, as the pipeline shows internal errors
could you give us a hand?

Copy link
Contributor

@merit-maita merit-maita left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!
please see my comments, and let's set up a short demo

Packs/ZeroFox/Integrations/ZeroFox/ZeroFox.py Outdated Show resolved Hide resolved
Packs/ZeroFox/Integrations/ZeroFox/ZeroFox.py Outdated Show resolved Hide resolved
Packs/ZeroFox/ReleaseNotes/1_3_1.json Outdated Show resolved Hide resolved
Packs/ZeroFox/ReleaseNotes/1_3_1.md Outdated Show resolved Hide resolved
@merit-maita
Copy link
Contributor

Hi @merit-maita we've had some issues with setting the release notes, as the pipeline shows internal errors could you give us a hand?

it's fine, the rn validator is expected to fail.

@merit-maita merit-maita added the pending-contributor The PR is pending the response of its creator label Mar 25, 2024
@DNRRomero
Copy link
Contributor Author

Hi @merit-maita PR comments are ready, when could you be available for presenting you a demo?

@DNRRomero
Copy link
Contributor Author

Hi @merit-maita we can give you a demo of the changes whenever you prefer between 8am and 4pm ET

@merit-maita
Copy link
Contributor

@DNRRomero can we set the demo for Monday 8:00 am ET?

@merit-maita merit-maita added the pending-demo Demo pending label Mar 31, 2024
@DNRRomero
Copy link
Contributor Author

@merit-maita I did not see this message on time, as we start at 8 am ET, can we do it tomorrow (tuesday) at 10am ET?

@merit-maita
Copy link
Contributor

@merit-maita I did not see this message on time, as we start at 8 am ET, can we do it tomorrow (tuesday) at 10am ET?

i have meeting at 10:00 ET, maybe 9:30 ET?

@dramirez-SR
Copy link

@merit-maita 9:30 works for me!

@merit-maita
Copy link
Contributor

@merit-maita 9:30 works for me!

because of the time difference, we always miss setting up the meeting, so please once you see this comment, set up a meeting at 9:30, today or tomorrow. my email is [email protected]

@merit-maita
Copy link
Contributor

@dramirez-SR the meeting has no link to a zoom or teams meeting.

@merit-maita
Copy link
Contributor

@DNRRomero thank you for the demo!
your pr has a merge conflict, please resolve it so i can go ahead and merge it.

@DNRRomero
Copy link
Contributor Author

DNRRomero commented Apr 9, 2024

Hi @merit-maita ! merge conflict has been resolved

@merit-maita merit-maita merged commit 0ab3835 into demisto:contrib/DNRRomero_fix_duplicates Apr 11, 2024
13 of 20 checks passed
Copy link

Thank you for your contribution. Your external PR has been merged and the changes are now included in an internal PR for further review. The internal PR will be merged to the master branch within 3 business days.

merit-maita added a commit that referenced this pull request Apr 11, 2024
…33912)

* Improve Fetch incidents structure and add append notes funcitonality (#33436)

* fix duplicate alerts (#123)

* Simplify fetch_incidents method to avoid duplicate incidents

change alert ids to str to facilitate storage and comparison
add set to avoid duplicates
delete unnecessary timestamp fields in last_run
reduce fetch_incidents to a single http call to alerts endpoint

* Add PR comments on fixing type hints and polishing tests

---------

Co-authored-by: Diego Ramirez <[email protected]>

* fix pre-commit formatting issues

* remove get_alert_attachment command

remove get_alert_attachment_command to avoid confusing customers

* Add notes append test

* format according to autopep8

* Update app versioning and add release notes

* Change release version to 2.0.0 due to breaking compatibility

* Undo version bump to 2.0.0 as it was not required, move back to 1.3.1 and mark as breaking change

* Restore get_alert_attachments command and mark as deprecated, as per PR comments

* Add sort field for fetching alerts given last_modified_min_date

* pre-commit

* added rn

---------

Co-authored-by: Diego Ramirez R <[email protected]>
Co-authored-by: merit-maita <[email protected]>
Co-authored-by: merit-maita <[email protected]>
pal-xmco pushed a commit to pal-xmco/content that referenced this pull request Jun 19, 2024
…emisto#33912)

* Improve Fetch incidents structure and add append notes funcitonality (demisto#33436)

* fix duplicate alerts (demisto#123)

* Simplify fetch_incidents method to avoid duplicate incidents

change alert ids to str to facilitate storage and comparison
add set to avoid duplicates
delete unnecessary timestamp fields in last_run
reduce fetch_incidents to a single http call to alerts endpoint

* Add PR comments on fixing type hints and polishing tests

---------

Co-authored-by: Diego Ramirez <[email protected]>

* fix pre-commit formatting issues

* remove get_alert_attachment command

remove get_alert_attachment_command to avoid confusing customers

* Add notes append test

* format according to autopep8

* Update app versioning and add release notes

* Change release version to 2.0.0 due to breaking compatibility

* Undo version bump to 2.0.0 as it was not required, move back to 1.3.1 and mark as breaking change

* Restore get_alert_attachments command and mark as deprecated, as per PR comments

* Add sort field for fetching alerts given last_modified_min_date

* pre-commit

* added rn

---------

Co-authored-by: Diego Ramirez R <[email protected]>
Co-authored-by: merit-maita <[email protected]>
Co-authored-by: merit-maita <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution Form Filled Whether contribution form filled or not. Contribution Thank you! Contributions are always welcome! External PR Partner Support Level Indicates that the contribution is for Partner supported pack Partner Partner-Approved pending-contributor The PR is pending the response of its creator pending-demo Demo pending TIM Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants