Update User Model (#144)

* Update User model

* Cleanup User model + add explicit unit tests

amundsen_application/models/user.py

-from typing import Dict
+from typing import Dict, Optional
 
 from marshmallow import Schema, fields, pre_load, post_load, validates_schema, ValidationError
 
-from flask import Response, jsonify
 from flask import current_app as app
 
 
@@ -44,14 +43,10 @@ class User:
         self.user_id = user_id
         # TODO: Add frequent_used, bookmarked, & owned resources
 
-    def to_json(self) -> Response:
-        user_info = dump_user(self)
-        return jsonify(user_info)
-
 
 class UserSchema(Schema):
-    display_name = fields.Str(allow_none=True)
-    email = fields.Str(allow_none=True)
+    display_name = fields.Str(allow_none=False)
+    email = fields.Str(required=True)
     employee_type = fields.Str(allow_none=True)
     first_name = fields.Str(allow_none=True)
     full_name = fields.Str(allow_none=True)
@@ -63,20 +58,31 @@ class UserSchema(Schema):
     role_name = fields.Str(allow_none=True)
     slack_id = fields.Str(allow_none=True)
     team_name = fields.Str(allow_none=True)
-    user_id = fields.Str(required=True)
+    user_id = fields.Str(allow_none=False)
+
+    def _str_no_value(self, s: Optional[str]) -> bool:
+        # Returns True if the given string is None or empty
+        if not s:
+            return True
+        if len(s.strip()) == 0:
+            return True
+        return False
 
     @pre_load
     def preprocess_data(self, data: Dict) -> Dict:
-        if not data.get('user_id', None):
-            data['user_id'] = data.get('email', None)
+        if self._str_no_value(data.get('user_id')):
+            data['user_id'] = data.get('email')
 
-        if not data.get('profile_url', None):
+        if self._str_no_value(data.get('profile_url')):
             data['profile_url'] = ''
             if app.config['GET_PROFILE_URL']:
                 data['profile_url'] = app.config['GET_PROFILE_URL'](data['user_id'])
 
-        if not data.get('display_name', None):
-            data['display_name'] = data.get('full_name', data.get('email'))
+        if self. _str_no_value(data.get('display_name')):
+            if self._str_no_value(data.get('full_name')):
+                data['display_name'] = data.get('email')
+            else:
+                data['display_name'] = data.get('full_name')
 
         return data
 
@@ -86,11 +92,11 @@ class UserSchema(Schema):
 
     @validates_schema
     def validate_user(self, data: Dict) -> None:
-        if not data.get('display_name', None):
-            raise ValidationError('"display_name" must be provided')
+        if self._str_no_value(data.get('display_name')):
+            raise ValidationError('"display_name", "full_name", or "email" must be provided')
 
-        if not data.get('user_id', None):
-            raise ValidationError('"user_id" must be provided')
+        if self._str_no_value(data.get('user_id')):
+            raise ValidationError('"user_id" or "email" must be provided')
 
 
 def load_user(user_data: Dict) -> User:

tests/unit/models/test_user.py

+import flask
+import unittest
+
+from amundsen_application.models.user import load_user, dump_user, UserSchema
+
+app = flask.Flask(__name__)
+app.config.from_object('amundsen_application.config.LocalConfig')
+
+
+def mock_get_profile_url(app):
+    return 'testUrl'
+
+
+app.config['GET_PROFILE_URL'] = mock_get_profile_url
+
+
+class UserTest(unittest.TestCase):
+    def test_set_user_id_from_email(self) -> None:
+        """
+        Deserialization and serialization sets user_id from email if no user_id
+        :return:
+        """
+        with app.test_request_context():
+            self.assertEqual(dump_user(load_user({'email': 'test@test.com'})).get('user_id'), 'test@test.com')
+
+    def test_set_display_name_from_full_name(self) -> None:
+        """
+        Deserialization and serialization sets display_name from full_name if no display_name and
+        full_name is a non-empty string
+        :return:
+        """
+        test_user = {
+            'email': 'test@test.com',
+            'full_name': 'Test User',
+        }
+        with app.test_request_context():
+            self.assertEqual(dump_user(load_user(test_user)).get('display_name'), 'Test User')
+
+    def test_set_display_name_from_email(self) -> None:
+        """
+        Deserialization and serialization sets display_name from email if no display_name and
+        full_name is None
+        :return:
+        """
+        with app.test_request_context():
+            self.assertEqual(dump_user(load_user({'email': 'test@test.com'})).get('display_name'), 'test@test.com')
+
+    def test_set_display_name_from_email_if_full_name_empty(self) -> None:
+        """
+        Deserialization and serialization sets display_name from email if no display_name and
+        full_name is ''
+        :return:
+        """
+        test_user = {
+            'email': 'test@test.com',
+            'full_name': '',
+        }
+        with app.test_request_context():
+            self.assertEqual(dump_user(load_user(test_user)).get('display_name'), 'test@test.com')
+
+    def test_profile_url(self) -> None:
+        """
+        Deserialization and serialization sets profile_url from app.config['GET_PROFILE_URL']
+        if no profile_url provided'
+        :return:
+        """
+        with app.test_request_context():
+            self.assertEqual(dump_user(load_user({'email': 'test@test.com'})).get('profile_url'), 'testUrl')
+
+    def test_raise_error_if_no_display_name(self) -> None:
+        """
+        Error is raised if deserialization of Dict will not generate a display_name
+        :return:
+        """
+        with app.test_request_context():
+            data, errors = UserSchema().load({})
+            self.assertEqual(len(errors['_schema']), 1)
+
+    def test_raise_error_if_no_user_id(self) -> None:
+        """
+        Error is raised if deserialization of Dict will not generate a user_id
+        :return:
+        """
+        with app.test_request_context():
+            data, errors = UserSchema().load({'display_name': 'Test User'})
+            self.assertEqual(len(errors['_schema']), 1)
+
+    def test_str_no_value(self) -> None:
+        """
+        Test _str_no_value returns True for a string of spaces
+        :return:
+        """
+        self.assertEqual(UserSchema()._str_no_value(' '), True)