incident_field from version BC check addition (#224)

* incident_field from version BC check addition

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

Co-Authored-By: Shai Yaakovi <[email protected]>

* Update CHANGELOG.md

* add import

* looseversion fix

* looseversion fix2

Co-authored-by: Shai Yaakovi <[email protected]>

CHANGELOG.md

@@ -5,6 +5,7 @@
 [1]: https://pypi.org/project/demisto-sdk/#history
 
 ### 0.3.10
+* Added a BC check for incident fields - changing from version is not allowed.
 * Fixed an issue in create-content-artifacts where scripts in Packs in TestPlaybooks dir were copied with a wrong prefix.
 
 ### 0.3.9

demisto_sdk/commands/common/hook_validations/incident_field.py

@@ -1,6 +1,8 @@
 """
 This module is designed to validate the correctness of incident field entities in content.
 """
+from distutils.version import LooseVersion
+from demisto_sdk.commands.common.constants import Errors
 from demisto_sdk.commands.common.hook_validations.base_validator import BaseValidator
 from demisto_sdk.commands.common.tools import print_error
 from enum import Enum, IntEnum
@@ -166,7 +168,7 @@ def is_backward_compatible(self):
 
  is_bc_broke = any(
  [
- # in the future, add BC checks here
+ self.is_changed_from_version(),
  ]
  )
 
@@ -184,7 +186,7 @@ def is_valid_file(self, validate_rn=True):
  self.is_valid_system_flag(),
  self.is_valid_cliname(),
  self.is_valid_version(),
- self.is_valid_from_version(),
+ self.is_current_valid_from_version(),
  self.is_valid_required()
  ]
  )
@@ -305,22 +307,42 @@ def is_cliname_is_builtin_key(self):
  )
  return is_valid
 
- def is_valid_from_version(self):
+ def is_current_valid_from_version(self):
+ # type: () -> bool
  error_msg = None
  is_valid = True
- try:
- server_base_version = self.current_file.get('fromVersion').split('.')[0]
- if not int(server_base_version) >= 5:
- error_msg = f'{self.file_path}: "fromVersion" mast be at least 5.0.0'
+
+ # if not a new file, will be checked here
+ # if has an old_file, will be checked in BC checks
+ if not self.old_file:
+ try:
+ from_version = self.current_file.get("fromVersion", "0.0.0")
+ if LooseVersion(from_version) < LooseVersion("5.0.0"):
+ error_msg = f'{self.file_path}: fromVersion must be at least 5.0.0'
+ is_valid = False
+ except (AttributeError, ValueError):
+ error_msg = f'{self.file_path}: "fromVersion" has an invalid value.'
  is_valid = False
- except (AttributeError, ValueError):
- error_msg = f'{self.file_path}: "fromVersion" as an invalid value.'
- is_valid = False
 
  if not is_valid:
  print_error(error_msg)
  return is_valid
 
+ def is_changed_from_version(self):
+ # type: () -> bool
+ """Check if fromversion has been changed.
+
+ Returns:
+ bool. Whether fromversion has been changed.
+ """
+ old_from_version = self.old_file.get('fromVersion', None)
+ if old_from_version:
+ current_from_version = self.current_file.get('fromVersion', None)
+ if old_from_version != current_from_version:
+ print_error(Errors.from_version_modified_after_rename())
+ return True
+ return False
+
  def is_valid_required(self):
  # type: () -> bool
  """Validate that the incident field is not required."""

demisto_sdk/commands/common/tests/incident_field_test.py

@@ -243,18 +243,38 @@ def test_is_valid_version(self, version, is_valid):
  ('4.0.0', False),
  ('5.0.1', True),
  ('100.0.0', True),
- (5, False),
- (None, False),
- ('test', False),
+ ('5', False),
+ (None, False)
  ]
 
  @pytest.mark.parametrize('from_version, is_valid', data_is_valid_from_version)
- def test_is_valid_from_version(self, from_version, is_valid):
+ def test_is_current_valid_from_version(self, from_version, is_valid):
  structure = StructureValidator("")
  structure.current_file = {"fromVersion": from_version}
  validator = IncidentFieldValidator(structure)
- assert validator.is_valid_from_version() == is_valid, f'is_valid_from_version({from_version})' \
- f' returns {not is_valid}.'
+ assert validator.is_current_valid_from_version() == is_valid, f'is_valid_from_version({from_version})' \
+ f' returns {not is_valid}.'
+
+ IS_FROM_VERSION_CHANGED_NO_OLD = {}
+ IS_FROM_VERSION_CHANGED_OLD = {"fromVersion": "5.0.0"}
+ IS_FROM_VERSION_CHANGED_NEW = {"fromVersion": "5.0.0"}
+ IS_FROM_VERSION_CHANGED_NO_NEW = {}
+ IS_FROM_VERSION_CHANGED_NEW_HIGHER = {"fromVersion": "5.5.0"}
+ IS_CHANGED_FROM_VERSION_INPUTS = [
+ (IS_FROM_VERSION_CHANGED_NO_OLD, IS_FROM_VERSION_CHANGED_NO_OLD, False),
+ (IS_FROM_VERSION_CHANGED_NO_OLD, IS_FROM_VERSION_CHANGED_NEW, True),
+ (IS_FROM_VERSION_CHANGED_OLD, IS_FROM_VERSION_CHANGED_NEW, False),
+ (IS_FROM_VERSION_CHANGED_NO_OLD, IS_FROM_VERSION_CHANGED_NO_NEW, False),
+ (IS_FROM_VERSION_CHANGED_OLD, IS_FROM_VERSION_CHANGED_NEW_HIGHER, True),
+ ]
+
+ @pytest.mark.parametrize("current_from_version, old_from_version, answer", IS_CHANGED_FROM_VERSION_INPUTS)
+ def test_is_changed_from_version(self, current_from_version, old_from_version, answer):
+ structure = StructureValidator("")
+ structure.old_file = old_from_version
+ structure.current_file = current_from_version
+ validator = IncidentFieldValidator(structure)
+ assert validator.is_changed_from_version() is answer
 
  data_required = [
  (True, False),