From 38e9e84f77f65112c74e51d39ba92ba05dc8861f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 28 Oct 2020 10:03:02 +0000 Subject: [PATCH] Raise exception when overriding a custom property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a subclass of `JSONModel` defines a property then we shouldn’t try to override it with the value from the underlying dictionary. Rather than silently fail we should raise an exception because it will help keep our list of `ALLOWED_PROPERTIES` nice and tidy. --- app/models/__init__.py | 2 +- app/models/job.py | 1 - tests/app/models/test_base_model.py | 9 ++++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/models/__init__.py b/app/models/__init__.py index c42bb028a..791c4a0c8 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -13,7 +13,7 @@ class JSONModel(SerialisedModel): # in the case of a bad request _dict may be `None` self._dict = _dict or {} for property in self.ALLOWED_PROPERTIES: - if property in self._dict and not hasattr(self, property): + if property in self._dict: setattr(self, property, self._dict[property]) def __bool__(self): diff --git a/app/models/job.py b/app/models/job.py index e08a8a87a..65679f3aa 100644 --- a/app/models/job.py +++ b/app/models/job.py @@ -28,7 +28,6 @@ class Job(JSONModel): 'template_version', 'original_file_name', 'created_at', - 'processing_started', 'notification_count', 'created_by', 'template_type', diff --git a/tests/app/models/test_base_model.py b/tests/app/models/test_base_model.py index 48500b0c7..5ccabc850 100644 --- a/tests/app/models/test_base_model.py +++ b/tests/app/models/test_base_model.py @@ -11,7 +11,7 @@ def test_looks_up_from_dict(): assert Custom({'foo': 'bar'}).foo == 'bar' -def test_prefers_property_to_dict(): +def test_raises_when_overriding_custom_properties(): class Custom(JSONModel): @@ -19,9 +19,12 @@ def test_prefers_property_to_dict(): @property def foo(self): - return 'bar' + pass - assert Custom({'foo': 'NOPE'}).foo == 'bar' + with pytest.raises(AttributeError) as e: + Custom({'foo': 'NOPE'}) + + assert str(e.value) == "can't set attribute" @pytest.mark.parametrize('json_response', (