From 0da023e7bf6af1251d54ed0b766122fff641fbcd Mon Sep 17 00:00:00 2001 From: Bar Katzir <37335599+bakatzir@users.noreply.github.com> Date: Thu, 27 Feb 2020 13:48:58 +0200 Subject: [PATCH] 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 <30797606+yaakovi@users.noreply.github.com> * Update CHANGELOG.md * add import * looseversion fix * looseversion fix2 Co-authored-by: Shai Yaakovi <30797606+yaakovi@users.noreply.github.com> --- CHANGELOG.md | 1 + .../common/hook_validations/incident_field.py | 42 ++++++++++++++----- .../common/tests/incident_field_test.py | 32 +++++++++++--- 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b574ebb0c26..bd648d1141d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/demisto_sdk/commands/common/hook_validations/incident_field.py b/demisto_sdk/commands/common/hook_validations/incident_field.py index 42d82db26f7..ebead3336d1 100644 --- a/demisto_sdk/commands/common/hook_validations/incident_field.py +++ b/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.""" diff --git a/demisto_sdk/commands/common/tests/incident_field_test.py b/demisto_sdk/commands/common/tests/incident_field_test.py index 4c92990e204..474568a3f09 100644 --- a/demisto_sdk/commands/common/tests/incident_field_test.py +++ b/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),