From cae2cdaad892304c3d1b57fa1cb54356e70885e0 Mon Sep 17 00:00:00 2001
From: AlanCoding <arominge@redhat.com>
Date: Mon, 7 Nov 2016 08:45:50 -0500
Subject: [PATCH] check related method small fixups

---
 awx/main/access.py                 | 12 ++++--------
 awx/main/tests/unit/test_access.py |  6 +++---
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/awx/main/access.py b/awx/main/access.py
index 8f77698791..1c907f67fa 100644
--- a/awx/main/access.py
+++ b/awx/main/access.py
@@ -201,7 +201,6 @@ class BaseAccess(object):
          - editing an existing resource, user must have permission to resource
            in `data`, as well as existing related resource on `obj`
 
-        If obj.field is null, this does not block the action
         If `mandatory` is set, new resources require the field and
                                existing field will always be checked
         '''
@@ -242,7 +241,6 @@ class BaseAccess(object):
 
         if (not new) and (not obj) and mandatory:
             # Restrict ability to create resource without required field
-            print ' superuser '
             return self.user.is_superuser
 
         def user_has_resource_access(resource):
@@ -253,13 +251,11 @@ class BaseAccess(object):
                 return self.user.can_access(type(resource), access_method_type, resource, None)
             return self.user in role
 
-        if new and changed:
-            if not user_has_resource_access(new):
-                return False  # User lacks access to provided resource
+        if new and changed and (not user_has_resource_access(new)):
+            return False  # User lacks access to provided resource
 
-        if current and (changed or mandatory):
-            if not user_has_resource_access(current):
-                return False  # User lacks access to existing resource
+        if current and (changed or mandatory) and (not user_has_resource_access(current)):
+            return False  # User lacks access to existing resource
 
         return True  # User has access to both, permission check passed
 
diff --git a/awx/main/tests/unit/test_access.py b/awx/main/tests/unit/test_access.py
index 10b2328033..728b73954e 100644
--- a/awx/main/tests/unit/test_access.py
+++ b/awx/main/tests/unit/test_access.py
@@ -53,12 +53,12 @@ class TestRelatedFieldAccess:
         data = {'related': resource_bad}
         assert not access.check_related('related', mocker.MagicMock, data)
 
-    def test_new_with_bad_data(self, access, resource_bad, mocker):
+    def test_new_with_bad_data(self, access, mocker):
         data = {'related': 3.1415}
         with pytest.raises(ParseError):
             access.check_related('related', mocker.MagicMock, data)
 
-    def test_new_mandatory_fail(self, access, resource_bad, mocker):
+    def test_new_mandatory_fail(self, access, mocker):
         access.user.is_superuser = False
         assert not access.check_related(
             'related', mocker.MagicMock, {}, mandatory=True)
@@ -101,7 +101,7 @@ class TestRelatedFieldAccess:
         assert not access.check_related(
             'related', mocker.MagicMock, data, obj=resource_bad)
 
-    def test_existing_not_null_null(self, access, bad_role, mocker):
+    def test_existing_not_null_null(self, access, mocker):
         resource = mocker.MagicMock(related=None)
         data = {'related': None}
         # Not changing anything by giving null when it is already-null