diff --git a/app/models/__init__.py b/app/models/__init__.py index f73531f12..791c4a0c8 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -12,6 +12,9 @@ class JSONModel(SerialisedModel): def __init__(self, _dict): # 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: + setattr(self, property, self._dict[property]) def __bool__(self): return self._dict != {} @@ -19,37 +22,9 @@ class JSONModel(SerialisedModel): def __hash__(self): return hash(self.id) - def __dir__(self): - return super().__dir__() + list(sorted(self.ALLOWED_PROPERTIES)) - def __eq__(self, other): return self.id == other.id - def __getattribute__(self, attr): - # Eventually we should remove this custom implementation in - # favour of looping over self.ALLOWED_PROPERTIES in __init__ - - try: - return super().__getattribute__(attr) - except AttributeError as e: - # Re-raise any `AttributeError`s that are not directly on - # this object because they indicate an underlying exception - # that we don’t want to swallow - if str(e) != "'{}' object has no attribute '{}'".format( - self.__class__.__name__, attr - ): - raise e - - if attr in super().__getattribute__('ALLOWED_PROPERTIES'): - return super().__getattribute__('_dict')[attr] - - raise AttributeError(( - "'{}' object has no attribute '{}' and '{}' is not a field " - "in the underlying JSON" - ).format( - self.__class__.__name__, attr, attr - )) - def _get_by_id(self, things, id): try: return next(thing for thing in things if thing['id'] == str(id)) diff --git a/app/models/job.py b/app/models/job.py index adda565d5..65679f3aa 100644 --- a/app/models/job.py +++ b/app/models/job.py @@ -25,11 +25,9 @@ class Job(JSONModel): ALLOWED_PROPERTIES = { 'id', 'service', - 'template', 'template_version', 'original_file_name', 'created_at', - 'processing_started', 'notification_count', 'created_by', 'template_type', diff --git a/app/models/service.py b/app/models/service.py index c8e973bc2..ede212422 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -31,11 +31,9 @@ class Service(JSONModel): ALLOWED_PROPERTIES = { 'active', 'contact_link', - 'email_branding', 'email_from', 'id', 'inbound_api', - 'letter_branding', 'message_limit', 'rate_limit', 'name', diff --git a/app/models/user.py b/app/models/user.py index 0c27aff2e..d3fd5a3aa 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -39,7 +39,6 @@ class User(JSONModel, UserMixin): 'mobile_number', 'password_changed_at', 'permissions', - 'platform_admin', 'state', } diff --git a/tests/app/models/test_base_model.py b/tests/app/models/test_base_model.py index 2be06759e..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', ( @@ -39,10 +42,7 @@ def test_model_raises_for_unknown_attributes(json_response): with pytest.raises(AttributeError) as e: model.foo - assert str(e.value) == ( - "'Custom' object has no attribute 'foo' and 'foo' is not a " - "field in the underlying JSON" - ) + assert str(e.value) == "'Custom' object has no attribute 'foo'" def test_model_raises_keyerror_if_item_missing_from_dict(): @@ -50,10 +50,10 @@ def test_model_raises_keyerror_if_item_missing_from_dict(): class Custom(JSONModel): ALLOWED_PROPERTIES = {'foo'} - with pytest.raises(KeyError) as e: + with pytest.raises(AttributeError) as e: Custom({}).foo - assert str(e.value) == "'foo'" + assert str(e.value) == "'Custom' object has no attribute 'foo'" @pytest.mark.parametrize('json_response', ( @@ -79,4 +79,6 @@ def test_dynamic_properties_are_introspectable(): class Custom(JSONModel): ALLOWED_PROPERTIES = {'foo', 'bar', 'baz'} - assert dir(Custom({}))[-3:] == ['bar', 'baz', 'foo'] + model = Custom({'foo': None, 'bar': None, 'baz': None}) + + assert dir(model)[-3:] == ['bar', 'baz', 'foo']