[Fix] Added case insensitivity to playbook condition validation (#296)

* added case insensivity to playbook condition validation

* improved UT

* pep8 changes

* changelog update

* Update demisto_sdk/commands/common/hook_validations/playbook.py

Co-Authored-By: Itay Keren <[email protected]>

* docstring

Co-authored-by: Bar Katzir <[email protected]>
Co-authored-by: Itay Keren <[email protected]>

CHANGELOG.md

@@ -5,6 +5,7 @@
 [1]: https://pypi.org/project/demisto-sdk/#history
 ### 0.4.8
 * Added the *max* field to the Playbook schema, allowing to define it in tasks loop.
+* Fixed an issue in *validate* where Condition branches checks were case sensitive.
 
 ### 0.4.7
 * Added the *slareminder* field to the Playbook schema.

demisto_sdk/commands/common/hook_validations/playbook.py

@@ -95,6 +95,7 @@ def is_condition_branches_handled(self): # type: () -> bool
 
  def is_builtin_condition_task_branches_handled(self, task: Dict) -> bool:
  """Checks whether a builtin conditional task branches are handled properly
+ NOTE: The function uses str.upper() on branches to be case insensitive
 
  Args:
  task (dict): task json loaded from a yaml
@@ -103,22 +104,26 @@ def is_builtin_condition_task_branches_handled(self, task: Dict) -> bool:
  bool. if the task handles all condition branches correctly.
  """
  is_all_condition_branches_handled: bool = True
+ # ADD all possible conditions to task_condition_labels (UPPER)
  # #default# condition should always exist in a builtin condition
- task_condition_labels = {'#default#'}
+ task_condition_labels = {'#DEFAULT#'}
  for condition in task.get('conditions', []):
  label = condition.get('label')
  if label:
- task_condition_labels.add(label)
+ task_condition_labels.add(label.upper())
+
+ # REMOVE all used condition branches from task_condition_labels (UPPER)
  next_tasks: Dict = task.get('nexttasks', {})
- for next_task_branch, next_task_ids in next_tasks.items():
+ for next_task_branch in next_tasks.keys():
  try:
- if next_task_ids:
- task_condition_labels.remove(next_task_branch)
+ if next_task_branch:
+ task_condition_labels.remove(next_task_branch.upper())
  except KeyError:
  print_error(f'Playbook conditional task with id:{task.get("id")} has task with unreachable '
  f'next task condition "{next_task_branch}". Please remove this task or add '
  f'this condition to condition task with id:{task.get("id")}.')
  self.is_valid = is_all_condition_branches_handled = False
+
  # if there are task_condition_labels left then not all branches are handled
  if task_condition_labels:
  try:
@@ -135,6 +140,7 @@ def is_builtin_condition_task_branches_handled(self, task: Dict) -> bool:
 
  def is_ask_condition_branches_handled(self, task: Dict) -> bool:
  """Checks whether a builtin conditional task branches are handled properly
+ NOTE: The function uses str.upper() on branches to be case insensitive
 
  Args:
  task (dict): task json loaded from a yaml
@@ -147,17 +153,22 @@ def is_ask_condition_branches_handled(self, task: Dict) -> bool:
  # if default is handled, then it means all branches are being handled
  if '#default#' in next_tasks:
  return is_all_condition_branches_handled
- unhandled_reply_options = set(task.get('message', {}).get('replyOptions'))
+
+ # ADD all replyOptions to unhandled_reply_options (UPPER)
+ unhandled_reply_options = set(map(str.upper, task.get('message', {}).get('replyOptions', [])))
+
+ # Remove all nexttasks from unhandled_reply_options (UPPER)
  next_tasks: Dict = task.get('nexttasks', {})
  for next_task_branch, next_task_id in next_tasks.items():
  try:
  if next_task_id:
- unhandled_reply_options.remove(next_task_branch)
+ unhandled_reply_options.remove(next_task_branch.upper())
  except KeyError:
  print_error(f'Playbook conditional Ask task with id:{task.get("id")} has task with unreachable '
  f'next task condition "{next_task_branch}". Please remove this task or add '
  f'this condition to condition task with id:{task.get("id")}.')
  self.is_valid = is_all_condition_branches_handled = False
+
  if unhandled_reply_options:
  print_error(f'Playbook conditional Ask task with id:{task.get("id")} has unhandled condition: '
  f'{",".join(map(lambda x: f"{str(x)}", unhandled_reply_options))}')

demisto_sdk/commands/common/tests/playbook_test.py

@@ -68,6 +68,11 @@ class TestPlaybookValidator:
  {'1': {'type': 'condition',
  'conditions': [{'label': 'yes'}],
  'nexttasks': {'#default#': ['2'], 'yes': ['3']}}}}
+ CONDITION_EXIST_FULL_CASE_DIF = {"id": "Intezer - scan host", "version": -1,
+ "tasks":
+ {'1': {'type': 'condition',
+ 'conditions': [{'label': 'YES'}],
+ 'nexttasks': {'#default#': ['2'], 'yes': ['3']}}}}
  CONDITIONAL_ASK_EXISTS_NO_REPLY_OPTS = {"id": "Intezer - scan host", "version": -1,
  "tasks":
  {'1': {'type': 'condition',
@@ -90,6 +95,11 @@ class TestPlaybookValidator:
  'message': {'replyOptions': ['yes']},
  'nexttasks': {'yes': ['1']}}}
  }
+ CONDITIONAL_ASK_EXISTS_WITH_NXT_TASK_CASE_DIF = {"id": "Intezer - scan host", "version": -1,
+ "tasks":
+ {'1': {'type': 'condition',
+ 'message': {'replyOptions': ['yes']},
+ 'nexttasks': {'YES': ['1']}}}}
  CONDITIONAL_SCRPT_WITHOUT_NXT_TASK = {"id": "Intezer - scan host", "version": -1,
  "tasks":
  {'1': {'type': 'condition',
@@ -113,13 +123,15 @@ class TestPlaybookValidator:
  (CONDITION_EXIST_PARTIAL_3, False),
  (CONDITION_EXIST_FULL_NO_TASK_ID, False),
  (CONDITION_EXIST_FULL, True),
+ (CONDITION_EXIST_FULL_CASE_DIF, True),
  (CONDITIONAL_ASK_EXISTS_NO_REPLY_OPTS, True),
  (CONDITIONAL_ASK_EXISTS_NO_NXT_TASK, False),
  (CONDITIONAL_ASK_EXISTS_WITH_DFLT_NXT_TASK, True),
  (CONDITIONAL_ASK_EXISTS_WITH_NXT_TASK, True),
+ (CONDITIONAL_ASK_EXISTS_WITH_NXT_TASK_CASE_DIF, True),
  (CONDITIONAL_SCRPT_WITHOUT_NXT_TASK, False),
  (CONDITIONAL_SCRPT_WITH_DFLT_NXT_TASK, False),
- (CONDITIONAL_SCRPT_WITH_MULTI_NXT_TASK, True)
+ (CONDITIONAL_SCRPT_WITH_MULTI_NXT_TASK, True),
  ]
 
  TASKS_NOT_EXIST = ROLENAME_NOT_EXIST