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

add ews-resolve-names command #32613

Merged
merged 44 commits into from Feb 12, 2024
Merged

add ews-resolve-names command #32613

merged 44 commits into from Feb 12, 2024

Conversation

JudahSchwartz
Copy link
Contributor

@JudahSchwartz JudahSchwartz commented Feb 1, 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: https://jira-dc.paloaltonetworks.com/browse/CIAC-9330

Description

Added the resolve-names command

Must have

  • Tests
  • Documentation

@JudahSchwartz JudahSchwartz changed the title added code and yml add ews-resolve-names command Feb 1, 2024
Copy link

github-actions bot commented Feb 1, 2024

Coverage

Coverage Report
FileStmtsMissCoverMissing
Packs/MicrosoftExchangeOnPremise/Integrations/EWSv2
   EWSv2.py2872391%1015, 1030, 1052, 1057–1058, 1069, 1074, 1447–1456, 1816–1817, 1825, 1829, 1833, 2358
TOTAL2872391% 

Tests Skipped Failures Errors Time
46 0 💤 0 ❌ 0 🔥 3.351s ⏱️

Copy link
Contributor

@ShahafBenYakir ShahafBenYakir left a comment

Choose a reason for hiding this comment

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

Looks good!

  1. Let's add UT to switch_hr_headers and format_identifier?
  2. See the rest of my comments.

if isinstance(contact, Contact) and contact.physical_addresses:
contact_dict['physical_addresses'] = list(map(parse_physical_address, contact.physical_addresses))
if isinstance(contact, Contact) and contact.phone_numbers:
contact_dict['phone_numbers'] = [elt for elt in map(parse_phone_number, contact.phone_numbers) if elt]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have we added the if statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if no phone numbers are defined exchangelib was returning
{ 'label' : 'bussiness', 'number' : None } so the context had a bunch of info like

numbers:
label: bussiness
label: personal

without any real numbers, so i switched the logic here that if a number doesnt exist to not return anything

JudahSchwartz and others added 4 commits February 7, 2024 16:24
Co-authored-by: Shahaf Ben Yakir <44666568+ShahafBenYakir@users.noreply.github.com>
Co-authored-by: Shahaf Ben Yakir <44666568+ShahafBenYakir@users.noreply.github.com>
@ShahafBenYakir
Copy link
Contributor

@JudahSchwartz doc review?

@ShirleyDenkberg
Copy link
Contributor

@ShahafBenYakir @amshamah419 Doc review completed.

JudahSchwartz and others added 11 commits February 11, 2024 09:29
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
@JudahSchwartz
Copy link
Contributor Author

adding CommonReports as a dependency for DeveloperTools as per @dantavori's advice,
Theres a limitation in xsoarng build that we dont install core packs every run, and CommonReports is a core pack. One of the tests is failing since it (Rightfully) assumes this pack should be installed.

@JudahSchwartz
Copy link
Contributor Author

forcing because theres an issue that core packs arent installed in the ng build. These will be addressed in https://jira-dc.paloaltonetworks.com/browse/CIAC-9721

@dantavori dantavori merged commit ed0de61 into master Feb 12, 2024
17 of 18 checks passed
@dantavori dantavori deleted the resolve_names_ewsv2 branch February 12, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants