From 3d589887ceaef00346655e2bdf816cd8962fba71 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Thu, 1 Mar 2018 11:15:20 +0000
Subject: [PATCH] remove useless properties from user model

we don't need them to mask private variables if we're not doing anything unusual.
---
 app/notify_client/models.py       | 86 ++++---------------------------
 tests/app/main/views/test_send.py |  2 +-
 tests/conftest.py                 |  2 +-
 3 files changed, 11 insertions(+), 79 deletions(-)

diff --git a/app/notify_client/models.py b/app/notify_client/models.py
index e1ae9e024..091f3a68a 100644
--- a/app/notify_client/models.py
+++ b/app/notify_client/models.py
@@ -52,19 +52,19 @@ def translate_permissions_from_admin_roles_to_db(permissions):
 
 class User(UserMixin):
     def __init__(self, fields, max_failed_login_count=3):
-        self._id = fields.get('id')
-        self._name = fields.get('name')
-        self._email_address = fields.get('email_address')
-        self._mobile_number = fields.get('mobile_number')
-        self._password_changed_at = fields.get('password_changed_at')
+        self.id = fields.get('id')
+        self.name = fields.get('name')
+        self.email_address = fields.get('email_address')
+        self.mobile_number = fields.get('mobile_number')
+        self.password_changed_at = fields.get('password_changed_at')
         self._set_permissions(fields.get('permissions', {}))
-        self._auth_type = fields.get('auth_type')
-        self._failed_login_count = fields.get('failed_login_count')
-        self._state = fields.get('state')
+        self.auth_type = fields.get('auth_type')
+        self.failed_login_count = fields.get('failed_login_count')
+        self.state = fields.get('state')
         self.max_failed_login_count = max_failed_login_count
         self.platform_admin = fields.get('platform_admin')
         self.current_session_id = fields.get('current_session_id')
-        self._organisations = fields.get('organisations', [])
+        self.organisations = fields.get('organisations', [])
 
     def _set_permissions(self, permissions_by_service):
         """
@@ -103,54 +103,6 @@ class User(UserMixin):
             super(User, self).is_authenticated
         )
 
-    @property
-    def id(self):
-        return self._id
-
-    @id.setter
-    def id(self, id):
-        self._id = id
-
-    @property
-    def name(self):
-        return self._name
-
-    @name.setter
-    def name(self, name):
-        self._name = name
-
-    @property
-    def email_address(self):
-        return self._email_address
-
-    @email_address.setter
-    def email_address(self, email_address):
-        self._email_address = email_address
-
-    @property
-    def mobile_number(self):
-        return self._mobile_number
-
-    @mobile_number.setter
-    def mobile_number(self, mobile_number):
-        self._mobile_number = mobile_number
-
-    @property
-    def password_changed_at(self):
-        return self._password_changed_at
-
-    @password_changed_at.setter
-    def password_changed_at(self, password_changed_at):
-        self._password_changed_at = password_changed_at
-
-    @property
-    def state(self):
-        return self._state
-
-    @state.setter
-    def state(self, state):
-        self._state = state
-
     @property
     def permissions(self):
         return self._permissions
@@ -178,22 +130,6 @@ class User(UserMixin):
     def has_permission_for_service(self, service_id, permission):
         return permission in self._permissions.get(service_id, [])
 
-    @property
-    def auth_type(self):
-        return self._auth_type
-
-    @auth_type.setter
-    def auth_type(self, auth_type):
-        self._auth_type = auth_type
-
-    @property
-    def failed_login_count(self):
-        return self._failed_login_count
-
-    @failed_login_count.setter
-    def failed_login_count(self, num):
-        self._failed_login_count += num
-
     def is_locked(self):
         return self.failed_login_count >= self.max_failed_login_count
 
@@ -217,10 +153,6 @@ class User(UserMixin):
     def set_password(self, pwd):
         self._password = pwd
 
-    @property
-    def organisations(self):
-        return self._organisations
-
 
 class InvitedUser(object):
 
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
index dd6bc9703..866ac2e55 100644
--- a/tests/app/main/views/test_send.py
+++ b/tests/app/main/views/test_send.py
@@ -1948,7 +1948,7 @@ def test_check_messages_shows_too_many_messages_errors(
 ):
     # csv with 100 phone numbers
     mocker.patch('app.main.views.send.s3download', return_value=',\n'.join(
-        ['phone number'] + ([mock_get_users_by_service(None)[0]._mobile_number] * 100)
+        ['phone number'] + ([mock_get_users_by_service(None)[0].mobile_number] * 100)
     ))
     mocker.patch('app.service_api_client.get_detailed_service_for_today', return_value={
         'data': {
diff --git a/tests/conftest.py b/tests/conftest.py
index 88cdd919d..643c30aef 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1432,7 +1432,7 @@ def mock_get_user_by_email(mocker, user=None):
         user = api_user_active(fake_uuid())
 
     def _get_user(email_address):
-        user._email_address = email_address
+        user.email_address = email_address
         return user
 
     return mocker.patch('app.user_api_client.get_user_by_email', side_effect=_get_user)