From e6aa4663d4208089e5786c4608afc8d77372f24a Mon Sep 17 00:00:00 2001
From: richard-dds <richard-ctr@friends.dds.mil>
Date: Wed, 28 Nov 2018 15:54:22 -0500
Subject: [PATCH] Better error handling for Auditable class

---
 atst/models/invitation.py           |  2 +-
 atst/models/mixins/auditable.py     | 40 +++++++++++++++++------------
 atst/models/request_status_event.py |  2 +-
 tests/domain/test_invitations.py    | 10 ++++----
 4 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/atst/models/invitation.py b/atst/models/invitation.py
index 0b7d6686..ded1d2b3 100644
--- a/atst/models/invitation.py
+++ b/atst/models/invitation.py
@@ -112,4 +112,4 @@ class Invitation(Base, TimestampsMixin, AuditableMixin):
 
     @property
     def workspace_id(self):
-        return self.workspace_role.workspace_id
+        return self.workspace_role.workspace_id if self.workspace_role else None
diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py
index b39c8768..b2ff5635 100644
--- a/atst/models/mixins/auditable.py
+++ b/atst/models/mixins/auditable.py
@@ -13,14 +13,14 @@ class AuditableMixin(object):
     @staticmethod
     def create_audit_event(connection, resource, action):
         user_id = getattr_path(g, "current_user.id")
-        workspace_id = resource.auditable_workspace_id()
-        request_id = resource.auditable_request_id()
-        resource_type = resource.auditable_resource_type()
-        display_name = resource.auditable_displayname()
-        event_details = resource.auditable_event_details()
+        workspace_id = resource.workspace_id
+        request_id = resource.request_id
+        resource_type = resource.resource_type
+        display_name = resource.displayname
+        event_details = resource.event_details
 
         changed_state = (
-            resource.auditable_changed_state() if action == ACTION_UPDATE else None
+            resource.history if action == ACTION_UPDATE else None
         )
 
         audit_event = AuditEvent(
@@ -77,20 +77,26 @@ class AuditableMixin(object):
                 previous_state[attr.key] = [deleted, added]
         return previous_state
 
-    def auditable_changed_state(self):
-        return getattr_path(self, "history")
+    @property
+    def history(self):
+        return None
 
-    def auditable_event_details(self):
-        return getattr_path(self, "event_details")
+    @property
+    def event_details(self):
+        return None
 
-    def auditable_resource_type(self):
+    @property
+    def resource_type(self):
         return camel_to_snake(type(self).__name__)
 
-    def auditable_workspace_id(self):
-        return getattr_path(self, "workspace_id")
+    @property
+    def workspace_id(self):
+        return None
 
-    def auditable_request_id(self):
-        return getattr_path(self, "request_id")
+    @property
+    def request_id(self):
+        return None
 
-    def auditable_displayname(self):
-        return getattr_path(self, "displayname")
+    @property
+    def displayname(self):
+        return None
diff --git a/atst/models/request_status_event.py b/atst/models/request_status_event.py
index 90ec2532..e3367951 100644
--- a/atst/models/request_status_event.py
+++ b/atst/models/request_status_event.py
@@ -43,7 +43,7 @@ class RequestStatusEvent(Base, mixins.TimestampsMixin, mixins.AuditableMixin):
 
     @property
     def displayname(self):
-        return self.new_status.value
+        return self.new_status.value if self.new_status else None
 
     @property
     def log_name(self):
diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py
index ee5021a3..d570f994 100644
--- a/tests/domain/test_invitations.py
+++ b/tests/domain/test_invitations.py
@@ -47,7 +47,7 @@ def test_accept_expired_invitation():
     increment = Invitations.EXPIRATION_LIMIT_MINUTES + 1
     expiration_time = datetime.datetime.now() - datetime.timedelta(minutes=increment)
     invite = InvitationFactory.create(
-        user_id=user.id, expiration_time=expiration_time, status=Status.PENDING
+        user=user, expiration_time=expiration_time, status=Status.PENDING
     )
     with pytest.raises(ExpiredError):
         Invitations.accept(user, invite.token)
@@ -57,14 +57,14 @@ def test_accept_expired_invitation():
 
 def test_accept_rejected_invite():
     user = UserFactory.create()
-    invite = InvitationFactory.create(user_id=user.id, status=Status.REJECTED_EXPIRED)
+    invite = InvitationFactory.create(user=user, status=Status.REJECTED_EXPIRED)
     with pytest.raises(InvitationError):
         Invitations.accept(user, invite.token)
 
 
 def test_accept_revoked_invite():
     user = UserFactory.create()
-    invite = InvitationFactory.create(user_id=user.id, status=Status.REVOKED)
+    invite = InvitationFactory.create(user=user, status=Status.REVOKED)
     with pytest.raises(InvitationError):
         Invitations.accept(user, invite.token)
 
@@ -72,7 +72,7 @@ def test_accept_revoked_invite():
 def test_wrong_user_accepts_invitation():
     user = UserFactory.create()
     wrong_user = UserFactory.create()
-    invite = InvitationFactory.create(user_id=user.id)
+    invite = InvitationFactory.create(user=user)
     with pytest.raises(WrongUserError):
         Invitations.accept(wrong_user, invite.token)
 
@@ -80,7 +80,7 @@ def test_wrong_user_accepts_invitation():
 def test_user_cannot_accept_invitation_accepted_by_wrong_user():
     user = UserFactory.create()
     wrong_user = UserFactory.create()
-    invite = InvitationFactory.create(user_id=user.id)
+    invite = InvitationFactory.create(user=user)
     with pytest.raises(WrongUserError):
         Invitations.accept(wrong_user, invite.token)
     with pytest.raises(InvitationError):