From 0516ef02ca68208c045b98884ef8d25327858136 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Mon, 23 May 2016 17:14:33 -0400
Subject: [PATCH 01/16] adding new factories

---
 awx/main/tests/factories/README.md            |   7 +
 awx/main/tests/factories/__init__.py          |   9 +
 awx/main/tests/factories/tower.py             | 196 ++++++++++++++++++
 .../functional/test_fixture_factories.py      |  25 +++
 4 files changed, 237 insertions(+)
 create mode 100644 awx/main/tests/factories/README.md
 create mode 100644 awx/main/tests/factories/__init__.py
 create mode 100644 awx/main/tests/factories/tower.py
 create mode 100644 awx/main/tests/functional/test_fixture_factories.py

diff --git a/awx/main/tests/factories/README.md b/awx/main/tests/factories/README.md
new file mode 100644
index 0000000000..ed8e9bb115
--- /dev/null
+++ b/awx/main/tests/factories/README.md
@@ -0,0 +1,7 @@
+fixtures
+========
+
+This is a module for defining stand-alone fixtures. Ideally a fixture will implement a single item.
+DO NOT decorate fixtures in this module with the @pytest.fixture. These fixtures are to be combined
+with fixture factories and composition using the `conftest.py` convention. Those composed fixtures
+will be decorated for usage and discovery.
diff --git a/awx/main/tests/factories/__init__.py b/awx/main/tests/factories/__init__.py
new file mode 100644
index 0000000000..2df095f34e
--- /dev/null
+++ b/awx/main/tests/factories/__init__.py
@@ -0,0 +1,9 @@
+from .tower import (
+    create_organization,
+    create_job_template,
+)
+
+__all__ = [
+    'create_organization',
+    'create_job_template',
+]
diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
new file mode 100644
index 0000000000..5d780fb76f
--- /dev/null
+++ b/awx/main/tests/factories/tower.py
@@ -0,0 +1,196 @@
+from collections import namedtuple
+
+from django.contrib.auth.models import User
+from django.conf import settings
+
+from awx.main.models import (
+    Organization,
+    Project,
+    Team,
+    Instance,
+    JobTemplate,
+    Credential,
+    Inventory,
+    Label,
+)
+
+def instance():
+    return Instance.objects.get_or_create(uuid=settings.SYSTEM_UUID, primary=True, hostname="instance.example.org")
+
+def mk_organization(name, desc, persisted=True):
+    org = Organization(name=name, description=desc)
+    if persisted:
+        instance()
+        org.save()
+    return org
+
+def mk_label(name, organization=None, persisted=True):
+    label = Label(name=name, description="%s-desc".format(name))
+    if organization is not None:
+        label.organization = organization
+    if persisted:
+        label.save()
+    return label
+
+def mk_team(name, organization=None, persisted=True):
+    team = Team(name=name)
+    if organization is not None:
+        team.organization = organization
+    if persisted:
+        instance()
+        team.save()
+    return team
+
+
+def mk_user(name, organization=None, team=None, is_superuser=False, persisted=True):
+    user = User(username=name)
+
+    if persisted:
+        user.save()
+        if organization is not None:
+            organization.member_role.members.add(user)
+        if team is not None:
+            team.member_role.members.add(user)
+    return user
+
+
+def mk_project(name, organization=None, persisted=True):
+    project = Project(name=name)
+
+    if organization is not None:
+        project.organization = organization
+    if persisted:
+        project.save()
+    return project
+
+
+def mk_credential(name, *args, **kwargs):
+    return None
+
+def mk_inventory(name, organization=None, persisted=True):
+    inv = Inventory(name=name)
+    inv.organization = organization
+    if persisted:
+        inv.save()
+    return inv
+
+def mk_job_template(name, project=None, inventory=None, credential=None, job_type='run', persisted=True):
+    jt = JobTemplate(name=name, job_type=job_type)
+    jt.project = project
+    jt.inventory = inventory
+    jt.credential = credential
+    if persisted:
+        jt.save()
+    return jt
+
+
+class _Mapped(object):
+    def __init__(self, d):
+        self.d = d
+        for k,v in d.items():
+            setattr(self, k, v)
+
+    def all(self):
+        return self.d.values()
+
+def create_job_template(name, *args, **kwargs):
+    Objects = namedtuple("Objects", "organization, job_template, inventory, project, credential")
+
+    org = None
+    proj = None
+    inv = None
+    cred = None
+
+    if 'organization' in kwargs:
+        org = kwargs['organization']
+        if type(org) is not Organization:
+            org = mk_organization(org, '%s-desc'.format(org))
+
+    if 'credential' in kwargs:
+        cred = kwargs['credential']
+        if type(cred) is not Credential:
+            cred = mk_credential(cred)
+
+    if 'project' in kwargs:
+        proj = kwargs['project']
+        if type(proj) is not Project:
+            proj = mk_project(proj, org)
+
+    if 'inventory' in kwargs:
+        inv = kwargs['inventory']
+        if type(inv) is not Inventory:
+            inv = mk_inventory(inv, org)
+
+    jt = mk_job_template(name, proj, inv, cred)
+
+    return Objects(job_template=jt,
+                   project=proj,
+                   inventory=inv,
+                   credential=cred)
+
+def create_organization(name, *args, **kwargs):
+    Objects = namedtuple("Objects", "organization,teams,users,superusers,projects,labels")
+
+    org = mk_organization(name, '%s-desc'.format(name))
+
+    superusers = {}
+    users = {}
+    teams = {}
+    projects = {}
+    labels = {}
+
+    if 'teams' in kwargs:
+        for t in kwargs['teams']:
+            if type(t) is Team:
+                teams[t.name] = t
+            else:
+                teams[t] = mk_team(t, org)
+
+    if 'projects' in kwargs:
+        for p in kwargs['projects']:
+            if type(p) is Project:
+                projects[p.name] = p
+            else:
+                projects[p] = mk_project(p, org)
+
+    if 'superusers' in kwargs:
+        # remove this duplication eventually
+        for u in kwargs['superusers']:
+            if type(u) is User:
+                users[u.username] = u
+            else:
+                p1, sep, p2 = u.partition(':')
+                if p2:
+                    t = teams[p1]
+                    superusers[p2] = mk_user(p2, org, t, True)
+                else:
+                    superusers[p1] = mk_user(p1, org, None, True)
+
+    if 'users' in kwargs:
+        # remove this duplication eventually
+        for u in kwargs['users']:
+            if type(u) is User:
+                users[u.username] = u
+            else:
+                p1, sep, p2 = u.partition(':')
+                if p2:
+                    t = teams[p1]
+                    users[p2] = mk_user(p2, org, t)
+                else:
+                    users[p1] = mk_user(p1, org)
+
+    if 'labels' in kwargs:
+        for l in kwargs['labels']:
+            if type(l) is Label:
+                labels[l.name] = l
+            else:
+                labels[l] = mk_label(l, org)
+
+    return Objects(organization=org,
+                   superusers=_Mapped(superusers),
+                   users=_Mapped(users),
+                   teams=_Mapped(teams),
+                   projects=_Mapped(projects),
+                   labels=_Mapped(labels))
+
+
diff --git a/awx/main/tests/functional/test_fixture_factories.py b/awx/main/tests/functional/test_fixture_factories.py
new file mode 100644
index 0000000000..ea8220f177
--- /dev/null
+++ b/awx/main/tests/functional/test_fixture_factories.py
@@ -0,0 +1,25 @@
+import pytest
+
+@pytest.mark.django_db
+def test_org_factory(organization_factory):
+    objects = organization_factory('organization1',
+                                   teams=['team1'],
+                                   superusers=['superuser'],
+                                   users=['admin', 'alice', 'team1:bob'],
+                                   projects=['proj1'])
+    assert hasattr(objects.users, 'admin')
+    assert hasattr(objects.users, 'alice')
+    assert hasattr(objects.superusers, 'superuser')
+    assert objects.users.bob in objects.teams.team1.member_role.members.all()
+    assert objects.projects.proj1.organization == objects.organization
+
+@pytest.mark.django_db
+def test_job_template_factory(job_template_factory):
+    jt_objects = job_template_factory('testJT', organization='org1',
+                                      project='proj1', inventory='inventory1',
+                                      credential='cred1')
+    assert jt_objects.job_template.name == 'testJT'
+    assert jt_objects.project.name == 'proj1'
+    assert jt_objects.inventory.name == 'inventory1'
+    assert jt_objects.credential.name == 'cred1'
+    assert jt_objects.organization.name == 'org1'

From bcef4a698c13f6561c14de646ac359056bbdfb8e Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Mon, 23 May 2016 17:14:50 -0400
Subject: [PATCH 02/16] updating some tests to use new factories

---
 awx/main/tests/functional/conftest.py        | 25 ++++++++++----------
 awx/main/tests/functional/test_rbac_label.py | 20 +++++++++-------
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py
index 59d34a2832..fbdba20044 100644
--- a/awx/main/tests/functional/conftest.py
+++ b/awx/main/tests/functional/conftest.py
@@ -38,6 +38,11 @@ from awx.main.models.organization import (
 
 from awx.main.models.notifications import NotificationTemplate
 
+from awx.main.tests.factories import (
+    create_organization,
+    create_job_template,
+)
+
 '''
 Disable all django model signals.
 '''
@@ -147,18 +152,6 @@ def instance(settings):
 def organization(instance):
     return Organization.objects.create(name="test-org", description="test-org-desc")
 
-@pytest.fixture
-def organization_factory(instance):
-    def factory(name):
-        try:
-            org = Organization.objects.get(name=name)
-        except Organization.DoesNotExist:
-            org = Organization.objects.create(name=name,
-                                              description="description for " + name,
-                                              )
-        return org
-    return factory
-
 @pytest.fixture
 def credential():
     return Credential.objects.create(kind='aws', name='test-cred')
@@ -474,3 +467,11 @@ def job_template_labels(organization, job_template):
     job_template.labels.create(name="label-2", organization=organization)
 
     return job_template
+
+@pytest.fixture
+def job_template_factory():
+    return create_job_template
+
+@pytest.fixture
+def organization_factory():
+    return create_organization
diff --git a/awx/main/tests/functional/test_rbac_label.py b/awx/main/tests/functional/test_rbac_label.py
index ec3c83f314..a5e50d95dc 100644
--- a/awx/main/tests/functional/test_rbac_label.py
+++ b/awx/main/tests/functional/test_rbac_label.py
@@ -31,20 +31,22 @@ def test_label_access_superuser(label, user):
     assert access.can_delete(label)
 
 @pytest.mark.django_db
-def test_label_access_admin(label, user, organization_factory):
+def test_label_access_admin(organization_factory):
     '''can_change because I am an admin of that org'''
-    a = user('admin', False)
-    org_no_members = organization_factory("no_members")
-    org_members = organization_factory("has_members")
+    no_members = organization_factory("no_members")
+    members = organization_factory("has_members",
+                                       users=['admin'],
+                                       labels=['test'])
 
-    label.organization.admin_role.members.add(a)
-    org_members.admin_role.members.add(a)
+    label = members.labels.test
+    admin = members.users.admin
+    members.organization.admin_role.members.add(admin)
 
-    access = LabelAccess(user('admin', False))
-    assert not access.can_change(label, {'organization': org_no_members.id})
+    access = LabelAccess(admin)
+    assert not access.can_change(label, {'organization': no_members.organization.id})
     assert access.can_read(label)
     assert access.can_change(label, None)
-    assert access.can_change(label, {'organization': org_members.id})
+    assert access.can_change(label, {'organization': members.organization.id})
     assert access.can_delete(label)
 
 @pytest.mark.django_db

From becce311febb40fb28f8ea90180763efd2318385 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Tue, 24 May 2016 11:31:23 -0400
Subject: [PATCH 03/16] cleanup make methods and update asserts

---
 awx/main/tests/factories/tower.py             | 104 +++++++++++-------
 .../functional/test_fixture_factories.py      |   2 +-
 2 files changed, 68 insertions(+), 38 deletions(-)

diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
index 5d780fb76f..43793a0d7a 100644
--- a/awx/main/tests/factories/tower.py
+++ b/awx/main/tests/factories/tower.py
@@ -14,9 +14,11 @@ from awx.main.models import (
     Label,
 )
 
+
 def instance():
     return Instance.objects.get_or_create(uuid=settings.SYSTEM_UUID, primary=True, hostname="instance.example.org")
 
+
 def mk_organization(name, desc, persisted=True):
     org = Organization(name=name, description=desc)
     if persisted:
@@ -24,62 +26,84 @@ def mk_organization(name, desc, persisted=True):
         org.save()
     return org
 
-def mk_label(name, organization=None, persisted=True):
+
+def mk_label(name, **kwargs):
     label = Label(name=name, description="%s-desc".format(name))
+    organization = kwargs.get('organization')
     if organization is not None:
         label.organization = organization
-    if persisted:
+    if kwargs.get('persisted', True):
         label.save()
     return label
 
-def mk_team(name, organization=None, persisted=True):
+
+def mk_team(name, **kwargs):
     team = Team(name=name)
+    organization = kwargs.get('organization')
     if organization is not None:
         team.organization = organization
-    if persisted:
+    if kwargs.get('persisted', True):
         instance()
         team.save()
     return team
 
 
-def mk_user(name, organization=None, team=None, is_superuser=False, persisted=True):
-    user = User(username=name)
+def mk_user(name, **kwargs):
+    user = User(username=name, is_superuser=kwargs.get('is_superuser', False))
 
-    if persisted:
+    if kwargs.get('persisted', True):
         user.save()
+        organization = kwargs.get('organization')
         if organization is not None:
             organization.member_role.members.add(user)
+        team = kwargs.get('team')
         if team is not None:
             team.member_role.members.add(user)
     return user
 
 
-def mk_project(name, organization=None, persisted=True):
+def mk_project(name, **kwargs):
     project = Project(name=name)
-
+    organization = kwargs.get('organization')
     if organization is not None:
         project.organization = organization
-    if persisted:
+    if kwargs.get('persisted', True):
         project.save()
     return project
 
 
-def mk_credential(name, *args, **kwargs):
-    return None
+def mk_credential(name, **kwargs):
+    cred = Credential(name=name)
+    cred.cloud = kwargs.get('cloud', False)
+    cred.kind = kwargs.get('kind', 'ssh')
+    if kwargs.get('persisted', True):
+        cred.save()
+    return cred
 
-def mk_inventory(name, organization=None, persisted=True):
+
+def mk_inventory(name, **kwargs):
     inv = Inventory(name=name)
-    inv.organization = organization
-    if persisted:
+    organization = kwargs.get('organization', None)
+    if organization is not None:
+        inv.organization = organization
+    if kwargs.get('persisted', True):
         inv.save()
     return inv
 
-def mk_job_template(name, project=None, inventory=None, credential=None, job_type='run', persisted=True):
-    jt = JobTemplate(name=name, job_type=job_type)
-    jt.project = project
-    jt.inventory = inventory
-    jt.credential = credential
-    if persisted:
+def mk_job_template(name, **kwargs):
+    jt = JobTemplate(name=name, job_type=kwargs.get('job_type', 'run'))
+
+    jt.inventory = kwargs.get('inventory', None)
+    if jt.inventory is None:
+        jt.ask_inventory_on_launch = True
+
+    jt.credential = kwargs.get('credential', None)
+    if jt.credential is None:
+        jt.ask_credential_on_launch = True
+
+    jt.project = kwargs.get('project', None)
+
+    if kwargs.get('persisted', True):
         jt.save()
     return jt
 
@@ -93,42 +117,47 @@ class _Mapped(object):
     def all(self):
         return self.d.values()
 
-def create_job_template(name, *args, **kwargs):
-    Objects = namedtuple("Objects", "organization, job_template, inventory, project, credential")
+def create_job_template(name, **kwargs):
+    Objects = namedtuple("Objects", "job_template, inventory, project, credential, job_type")
 
     org = None
     proj = None
     inv = None
     cred = None
+    job_type = kwargs.get('job_type', 'run')
+    persisted = kwargs.get('persisted', True)
 
     if 'organization' in kwargs:
         org = kwargs['organization']
         if type(org) is not Organization:
-            org = mk_organization(org, '%s-desc'.format(org))
+            org = mk_organization(org, '%s-desc'.format(org), persisted=persisted)
 
     if 'credential' in kwargs:
         cred = kwargs['credential']
         if type(cred) is not Credential:
-            cred = mk_credential(cred)
+            cred = mk_credential(cred, persisted=persisted)
 
     if 'project' in kwargs:
         proj = kwargs['project']
         if type(proj) is not Project:
-            proj = mk_project(proj, org)
+            proj = mk_project(proj, organization=org, persisted=persisted)
 
     if 'inventory' in kwargs:
         inv = kwargs['inventory']
         if type(inv) is not Inventory:
-            inv = mk_inventory(inv, org)
+            inv = mk_inventory(inv, organization=org, persisted=persisted)
 
-    jt = mk_job_template(name, proj, inv, cred)
+    jt = mk_job_template(name, project=proj,
+                         inventory=inv, credential=cred,
+                         job_type=job_type, persisted=persisted)
 
     return Objects(job_template=jt,
                    project=proj,
                    inventory=inv,
-                   credential=cred)
+                   credential=cred,
+                   job_type=job_type)
 
-def create_organization(name, *args, **kwargs):
+def create_organization(name, **kwargs):
     Objects = namedtuple("Objects", "organization,teams,users,superusers,projects,labels")
 
     org = mk_organization(name, '%s-desc'.format(name))
@@ -138,20 +167,21 @@ def create_organization(name, *args, **kwargs):
     teams = {}
     projects = {}
     labels = {}
+    persisted = kwargs.get('persisted', True)
 
     if 'teams' in kwargs:
         for t in kwargs['teams']:
             if type(t) is Team:
                 teams[t.name] = t
             else:
-                teams[t] = mk_team(t, org)
+                teams[t] = mk_team(t, organization=org, persisted=persisted)
 
     if 'projects' in kwargs:
         for p in kwargs['projects']:
             if type(p) is Project:
                 projects[p.name] = p
             else:
-                projects[p] = mk_project(p, org)
+                projects[p] = mk_project(p, organization=org, persisted=persisted)
 
     if 'superusers' in kwargs:
         # remove this duplication eventually
@@ -162,9 +192,9 @@ def create_organization(name, *args, **kwargs):
                 p1, sep, p2 = u.partition(':')
                 if p2:
                     t = teams[p1]
-                    superusers[p2] = mk_user(p2, org, t, True)
+                    superusers[p2] = mk_user(p2, organization=org, team=t, is_superuser=True, persisted=persisted)
                 else:
-                    superusers[p1] = mk_user(p1, org, None, True)
+                    superusers[p1] = mk_user(p1, organization=org, team=None, is_superuser=True, persisted=persisted)
 
     if 'users' in kwargs:
         # remove this duplication eventually
@@ -175,16 +205,16 @@ def create_organization(name, *args, **kwargs):
                 p1, sep, p2 = u.partition(':')
                 if p2:
                     t = teams[p1]
-                    users[p2] = mk_user(p2, org, t)
+                    users[p2] = mk_user(p2, organization=org, team=t, is_superuser=False, persisted=persisted)
                 else:
-                    users[p1] = mk_user(p1, org)
+                    users[p1] = mk_user(p1, organization=org, is_superuser=False, persisted=persisted)
 
     if 'labels' in kwargs:
         for l in kwargs['labels']:
             if type(l) is Label:
                 labels[l.name] = l
             else:
-                labels[l] = mk_label(l, org)
+                labels[l] = mk_label(l, org, persisted=persisted)
 
     return Objects(organization=org,
                    superusers=_Mapped(superusers),
diff --git a/awx/main/tests/functional/test_fixture_factories.py b/awx/main/tests/functional/test_fixture_factories.py
index ea8220f177..d1cd39f89e 100644
--- a/awx/main/tests/functional/test_fixture_factories.py
+++ b/awx/main/tests/functional/test_fixture_factories.py
@@ -22,4 +22,4 @@ def test_job_template_factory(job_template_factory):
     assert jt_objects.project.name == 'proj1'
     assert jt_objects.inventory.name == 'inventory1'
     assert jt_objects.credential.name == 'cred1'
-    assert jt_objects.organization.name == 'org1'
+    assert jt_objects.inventory.organization.name == 'org1'

From 7ab385524d05db770c0a669016ce1b7daac0ca22 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Tue, 24 May 2016 12:45:54 -0400
Subject: [PATCH 04/16] added roles support for organization_factory

---
 awx/main/tests/factories/tower.py | 48 +++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
index 43793a0d7a..9b142eaadf 100644
--- a/awx/main/tests/factories/tower.py
+++ b/awx/main/tests/factories/tower.py
@@ -12,6 +12,7 @@ from awx.main.models import (
     Credential,
     Inventory,
     Label,
+    Role,
 )
 
 
@@ -112,7 +113,7 @@ class _Mapped(object):
     def __init__(self, d):
         self.d = d
         for k,v in d.items():
-            setattr(self, k, v)
+            setattr(self, k.replace(' ','_'), v)
 
     def all(self):
         return self.d.values()
@@ -158,7 +159,7 @@ def create_job_template(name, **kwargs):
                    job_type=job_type)
 
 def create_organization(name, **kwargs):
-    Objects = namedtuple("Objects", "organization,teams,users,superusers,projects,labels")
+    Objects = namedtuple("Objects", "organization,teams,users,superusers,projects,labels,roles")
 
     org = mk_organization(name, '%s-desc'.format(name))
 
@@ -167,6 +168,7 @@ def create_organization(name, **kwargs):
     teams = {}
     projects = {}
     labels = {}
+    roles = {}
     persisted = kwargs.get('persisted', True)
 
     if 'teams' in kwargs:
@@ -216,11 +218,47 @@ def create_organization(name, **kwargs):
             else:
                 labels[l] = mk_label(l, org, persisted=persisted)
 
+    if 'roles' in kwargs:
+        # refactor this .. alot
+        if not persisted:
+            raise RuntimeError('roles can not be used when persisted=False')
+
+        all_objects = {}
+        for d in [superusers, users, teams, projects, labels]:
+            for k,v in d.iteritems():
+                if all_objects.get(k) is not None:
+                    raise KeyError('object names must be unique when using roles \
+                                   {} key already exists with value {}'.format(k,v))
+                all_objects[k] = v
+
+        for role in kwargs.get('roles'):
+            obj_role, sep, member_role = role.partition(':')
+            if not member_role:
+                raise RuntimeError('you must an assignment role, got None')
+
+            obj_str, o_role_str = obj_role.split('.')
+            member_str, m_sep, m_role_str = member_role.partition('.')
+
+            obj = all_objects[obj_str]
+            obj_role = getattr(obj, o_role_str)
+
+            member = all_objects[member_str]
+            if m_role_str:
+                if hasattr(member, m_role_str):
+                    member_role = getattr(member, m_role_str)
+                    obj_role.parents.add(member_role)
+                else:
+                    raise RuntimeError('unable to find {} role for {}'.format(m_role_str, member_str))
+            else:
+                if type(member) is User:
+                    obj_role.members.add(member)
+                else:
+                    raise RuntimeError('unable to add non-user {} for members list of {}'.format(member_str, obj_str))
+
     return Objects(organization=org,
                    superusers=_Mapped(superusers),
                    users=_Mapped(users),
                    teams=_Mapped(teams),
                    projects=_Mapped(projects),
-                   labels=_Mapped(labels))
-
-
+                   labels=_Mapped(labels),
+                   roles=_Mapped(roles))

From 38778045995d0d39afba9369adb03f54676a8ba7 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Tue, 24 May 2016 12:46:15 -0400
Subject: [PATCH 05/16] updated test to use new organization_factory

---
 awx/main/tests/functional/test_projects.py | 34 ++++++++++------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/awx/main/tests/functional/test_projects.py b/awx/main/tests/functional/test_projects.py
index d8bcd5d151..60b541f0c4 100644
--- a/awx/main/tests/functional/test_projects.py
+++ b/awx/main/tests/functional/test_projects.py
@@ -11,36 +11,32 @@ from awx.main.models import Project
 #
 
 @pytest.mark.django_db
-def test_user_project_list(get, project_factory, organization, admin, alice, bob):
+def test_user_project_list(get, organization_factory):
     'List of projects a user has access to, filtered by projects you can also see'
 
-    organization.member_role.members.add(alice, bob)
+    objects = organization_factory('org1',
+                                   projects=['alice project', 'bob project', 'shared project'],
+                                   superusers=['admin'],
+                                   users=['alice', 'bob'],
+                                   roles=['alice project.admin_role:alice',
+                                          'bob project.admin_role:bob',
+                                          'shared project.admin_role:bob',
+                                          'shared project.admin_role:alice'])
 
-    alice_project = project_factory('alice project')
-    alice_project.admin_role.members.add(alice)
-
-    bob_project = project_factory('bob project')
-    bob_project.admin_role.members.add(bob)
-
-    shared_project = project_factory('shared project')
-    shared_project.admin_role.members.add(alice)
-    shared_project.admin_role.members.add(bob)
-
-    # admins can see all projects
-    assert get(reverse('api:user_projects_list', args=(admin.pk,)), admin).data['count'] == 3
+    assert get(reverse('api:user_projects_list', args=(objects.superusers.admin.pk,)), objects.superusers.admin).data['count'] == 3
 
     # admins can see everyones projects
-    assert get(reverse('api:user_projects_list', args=(alice.pk,)), admin).data['count'] == 2
-    assert get(reverse('api:user_projects_list', args=(bob.pk,)), admin).data['count'] == 2
+    assert get(reverse('api:user_projects_list', args=(objects.users.alice.pk,)), objects.superusers.admin).data['count'] == 2
+    assert get(reverse('api:user_projects_list', args=(objects.users.bob.pk,)), objects.superusers.admin).data['count'] == 2
 
     # users can see their own projects
-    assert get(reverse('api:user_projects_list', args=(alice.pk,)), alice).data['count'] == 2
+    assert get(reverse('api:user_projects_list', args=(objects.users.alice.pk,)), objects.users.alice).data['count'] == 2
 
     # alice should only be able to see the shared project when looking at bobs projects
-    assert get(reverse('api:user_projects_list', args=(bob.pk,)), alice).data['count'] == 1
+    assert get(reverse('api:user_projects_list', args=(objects.users.bob.pk,)), objects.users.alice).data['count'] == 1
 
     # alice should see all projects they can see when viewing an admin
-    assert get(reverse('api:user_projects_list', args=(admin.pk,)), alice).data['count'] == 2
+    assert get(reverse('api:user_projects_list', args=(objects.superusers.admin.pk,)), objects.users.alice).data['count'] == 2
 
 
 def setup_test_team_project_list(project_factory, team_factory, admin, alice, bob):

From 2f09c7fee17805b4cee9e0b19d2fc21bf3753087 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Tue, 24 May 2016 12:53:42 -0400
Subject: [PATCH 06/16] added test for factory roles

---
 .../tests/functional/test_fixture_factories.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/awx/main/tests/functional/test_fixture_factories.py b/awx/main/tests/functional/test_fixture_factories.py
index d1cd39f89e..68f498bcf3 100644
--- a/awx/main/tests/functional/test_fixture_factories.py
+++ b/awx/main/tests/functional/test_fixture_factories.py
@@ -1,5 +1,22 @@
 import pytest
 
+@pytest.mark.django_db
+def test_org_factory_roles(organization_factory):
+    objects = organization_factory('org_roles_test',
+                                   teams=['team1', 'team2'],
+                                   users=['team1:foo', 'bar'],
+                                   projects=['baz', 'bang'],
+                                   roles=['team2.member_role:foo',
+                                          'team2.admin_role:bar',
+                                          'team1.admin_role:team2.admin_role',
+                                          'baz.admin_role:foo'])
+
+    assert objects.users.bar in objects.teams.team1.admin_role
+    assert objects.users.foo in objects.projects.baz.admin_role
+    assert objects.users.foo in objects.teams.team1.member_role
+    assert objects.teams.team2.admin_role in objects.teams.team1n.admin_role.parents.all()
+
+
 @pytest.mark.django_db
 def test_org_factory(organization_factory):
     objects = organization_factory('organization1',
@@ -13,6 +30,7 @@ def test_org_factory(organization_factory):
     assert objects.users.bob in objects.teams.team1.member_role.members.all()
     assert objects.projects.proj1.organization == objects.organization
 
+
 @pytest.mark.django_db
 def test_job_template_factory(job_template_factory):
     jt_objects = job_template_factory('testJT', organization='org1',

From 73e41ef6ccdb958d0fd024ab06749769cf03a0c4 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Tue, 24 May 2016 13:00:22 -0400
Subject: [PATCH 07/16] flake8 fixes

---
 awx/main/tests/factories/tower.py            | 1 -
 awx/main/tests/functional/test_rbac_label.py | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
index 9b142eaadf..b8c473dd7e 100644
--- a/awx/main/tests/factories/tower.py
+++ b/awx/main/tests/factories/tower.py
@@ -12,7 +12,6 @@ from awx.main.models import (
     Credential,
     Inventory,
     Label,
-    Role,
 )
 
 
diff --git a/awx/main/tests/functional/test_rbac_label.py b/awx/main/tests/functional/test_rbac_label.py
index a5e50d95dc..e425d50908 100644
--- a/awx/main/tests/functional/test_rbac_label.py
+++ b/awx/main/tests/functional/test_rbac_label.py
@@ -35,8 +35,8 @@ def test_label_access_admin(organization_factory):
     '''can_change because I am an admin of that org'''
     no_members = organization_factory("no_members")
     members = organization_factory("has_members",
-                                       users=['admin'],
-                                       labels=['test'])
+                                   users=['admin'],
+                                   labels=['test'])
 
     label = members.labels.test
     admin = members.users.admin

From 4453e487295f95aaa2ddc5f751a057ac101d1359 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Tue, 24 May 2016 15:29:01 -0400
Subject: [PATCH 08/16] updated factories docs

---
 awx/main/tests/factories/README.md | 48 ++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/awx/main/tests/factories/README.md b/awx/main/tests/factories/README.md
index ed8e9bb115..8b864d0708 100644
--- a/awx/main/tests/factories/README.md
+++ b/awx/main/tests/factories/README.md
@@ -1,7 +1,49 @@
-fixtures
-========
+factories
+=========
 
-This is a module for defining stand-alone fixtures. Ideally a fixture will implement a single item.
+This is a module for defining stand-alone factories and fixtures. Ideally a fixture will implement a single item.
 DO NOT decorate fixtures in this module with the @pytest.fixture. These fixtures are to be combined
 with fixture factories and composition using the `conftest.py` convention. Those composed fixtures
 will be decorated for usage and discovery.
+
+Use the fixtures directly in factory methods to build up the desired set of components and relationships.
+Each fixture should create exactly one object and should support the option for that object to be persisted
+or not.
+
+A factory should create at a minimum a single object for that factory type. The creation of any
+associated objects should be explicit. For example, the `create_organization` factory when given only
+a `name` parameter will create an Organization but it will not implicitly create any other objects.
+
+teams
+-----
+
+There is some special handling for users when adding teams. There is a short hand that allows you to
+assign a user to the member\_role of a team using the string notation of `team_name:user_name`. There is
+no shortcut for adding a user to the admin\_role of a team. See the roles section for more information
+about how to do that.
+
+roles
+-----
+
+The roles helper allows you pass in roles to a factory. These roles assignments will happen after
+the objects are created. Using the roles parameter required that persisted=True (default).
+
+You can use a string notation of `object_name.role_name:user` OR `object_name.role_name:object_name.child_role`
+
+    obj.parent_role:user  # This will make the user a member of parent_role
+    obj1.role:obj2.role   # This will make obj2 a child role of obj1
+
+    team1.admin_role:joe
+    team1.admin_role:project1.admin_role
+
+examples
+--------
+
+    objects = create_organization('test-org')
+    assert objects.organization.name == 'test-org'
+
+    objects = create_organization('test-org', projects=['test-proj'])
+    assert objects.projects.test-proj.organization == objects.organization
+
+    objects = create_organization('test-org', persisted=False)
+    assert not objects.organization.pk

From aaabc2582d27056f16df8b59ddffc31c521a2cfb Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Tue, 24 May 2016 15:29:28 -0400
Subject: [PATCH 09/16] refactored roles handling and added some more tests

---
 awx/main/tests/factories/tower.py             | 90 ++++++++++---------
 .../functional/test_fixture_factories.py      | 43 ++++++++-
 2 files changed, 89 insertions(+), 44 deletions(-)

diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
index b8c473dd7e..48f4c44c2f 100644
--- a/awx/main/tests/factories/tower.py
+++ b/awx/main/tests/factories/tower.py
@@ -107,6 +107,44 @@ def mk_job_template(name, **kwargs):
         jt.save()
     return jt
 
+def apply_roles(roles, objects, persisted):
+    if roles is None:
+        return None
+
+    if not persisted:
+        raise RuntimeError('roles can not be used when persisted=False')
+
+    all_objects = {}
+    for d in objects:
+        for k,v in d.iteritems():
+            if all_objects.get(k) is not None:
+                raise KeyError('object names must be unique when using roles {} key already exists with value {}'.format(k,v))
+            all_objects[k] = v
+
+    for role in roles:
+        obj_role, sep, member_role = role.partition(':')
+        if not member_role:
+            raise RuntimeError('you must provide an assignment role, got None')
+
+        obj_str, o_role_str = obj_role.split('.')
+        member_str, m_sep, m_role_str = member_role.partition('.')
+
+        obj = all_objects[obj_str]
+        obj_role = getattr(obj, o_role_str)
+
+        member = all_objects[member_str]
+        if m_role_str:
+            if hasattr(member, m_role_str):
+                member_role = getattr(member, m_role_str)
+                obj_role.parents.add(member_role)
+            else:
+                raise RuntimeError('unable to find {} role for {}'.format(m_role_str, member_str))
+        else:
+            if type(member) is User:
+                obj_role.members.add(member)
+            else:
+                raise RuntimeError('unable to add non-user {} for members list of {}'.format(member_str, obj_str))
+
 
 class _Mapped(object):
     def __init__(self, d):
@@ -151,6 +189,9 @@ def create_job_template(name, **kwargs):
                          inventory=inv, credential=cred,
                          job_type=job_type, persisted=persisted)
 
+    objects = [{o.name: o} for o in [org, proj, inv, cred]]
+    apply_roles(kwargs.get('roles'), objects, persisted)
+
     return Objects(job_template=jt,
                    project=proj,
                    inventory=inv,
@@ -158,18 +199,17 @@ def create_job_template(name, **kwargs):
                    job_type=job_type)
 
 def create_organization(name, **kwargs):
-    Objects = namedtuple("Objects", "organization,teams,users,superusers,projects,labels,roles")
-
-    org = mk_organization(name, '%s-desc'.format(name))
+    Objects = namedtuple("Objects", "organization,teams,users,superusers,projects,labels")
 
     superusers = {}
     users = {}
     teams = {}
     projects = {}
     labels = {}
-    roles = {}
     persisted = kwargs.get('persisted', True)
 
+    org = mk_organization(name, '%s-desc'.format(name), persisted=persisted)
+
     if 'teams' in kwargs:
         for t in kwargs['teams']:
             if type(t) is Team:
@@ -217,47 +257,11 @@ def create_organization(name, **kwargs):
             else:
                 labels[l] = mk_label(l, org, persisted=persisted)
 
-    if 'roles' in kwargs:
-        # refactor this .. alot
-        if not persisted:
-            raise RuntimeError('roles can not be used when persisted=False')
-
-        all_objects = {}
-        for d in [superusers, users, teams, projects, labels]:
-            for k,v in d.iteritems():
-                if all_objects.get(k) is not None:
-                    raise KeyError('object names must be unique when using roles \
-                                   {} key already exists with value {}'.format(k,v))
-                all_objects[k] = v
-
-        for role in kwargs.get('roles'):
-            obj_role, sep, member_role = role.partition(':')
-            if not member_role:
-                raise RuntimeError('you must an assignment role, got None')
-
-            obj_str, o_role_str = obj_role.split('.')
-            member_str, m_sep, m_role_str = member_role.partition('.')
-
-            obj = all_objects[obj_str]
-            obj_role = getattr(obj, o_role_str)
-
-            member = all_objects[member_str]
-            if m_role_str:
-                if hasattr(member, m_role_str):
-                    member_role = getattr(member, m_role_str)
-                    obj_role.parents.add(member_role)
-                else:
-                    raise RuntimeError('unable to find {} role for {}'.format(m_role_str, member_str))
-            else:
-                if type(member) is User:
-                    obj_role.members.add(member)
-                else:
-                    raise RuntimeError('unable to add non-user {} for members list of {}'.format(member_str, obj_str))
-
+    apply_roles(kwargs.get('roles'), [superusers, users, teams, projects, labels], persisted)
     return Objects(organization=org,
                    superusers=_Mapped(superusers),
                    users=_Mapped(users),
                    teams=_Mapped(teams),
                    projects=_Mapped(projects),
-                   labels=_Mapped(labels),
-                   roles=_Mapped(roles))
+                   labels=_Mapped(labels))
+
diff --git a/awx/main/tests/functional/test_fixture_factories.py b/awx/main/tests/functional/test_fixture_factories.py
index 68f498bcf3..908eb6f053 100644
--- a/awx/main/tests/functional/test_fixture_factories.py
+++ b/awx/main/tests/functional/test_fixture_factories.py
@@ -1,5 +1,46 @@
 import pytest
 
+
+def test_roles_exc_not_persisted(organization_factory):
+    with pytest.raises(RuntimeError) as exc:
+        organization_factory('test-org', roles=['test-org.admin_role:user1'], persisted=False)
+    assert 'persisted=False' in str(exc.value)
+
+
+@pytest.mark.django_db
+def test_roles_exc_bad_object(organization_factory):
+    with pytest.raises(KeyError):
+        organization_factory('test-org', roles=['test-project.admin_role:user'])
+
+
+@pytest.mark.django_db
+def test_roles_exc_not_unique(organization_factory):
+    with pytest.raises(KeyError) as exc:
+        organization_factory('test-org', projects=['foo'], teams=['foo'], roles=['foo.admin_role:user'])
+    assert 'must be unique' in str(exc.value)
+
+
+@pytest.mark.django_db
+def test_roles_exc_not_assignment(organization_factory):
+    with pytest.raises(RuntimeError) as exc:
+        organization_factory('test-org', projects=['foo'], roles=['foo.admin_role'])
+    assert 'provide an assignment' in str(exc.value)
+
+
+@pytest.mark.django_db
+def test_roles_exc_not_found(organization_factory):
+    with pytest.raises(RuntimeError) as exc:
+        organization_factory('test-org', users=['user'], projects=['foo'], roles=['foo.admin_role:user.bad_role'])
+    assert 'unable to find' in str(exc.value)
+
+
+@pytest.mark.django_db
+def test_roles_exc_not_user(organization_factory):
+    with pytest.raises(RuntimeError) as exc:
+        organization_factory('test-org', projects=['foo'], roles=['foo.admin_role:foo'])
+    assert 'unable to add non-user' in str(exc.value)
+
+
 @pytest.mark.django_db
 def test_org_factory_roles(organization_factory):
     objects = organization_factory('org_roles_test',
@@ -14,7 +55,7 @@ def test_org_factory_roles(organization_factory):
     assert objects.users.bar in objects.teams.team1.admin_role
     assert objects.users.foo in objects.projects.baz.admin_role
     assert objects.users.foo in objects.teams.team1.member_role
-    assert objects.teams.team2.admin_role in objects.teams.team1n.admin_role.parents.all()
+    assert objects.teams.team2.admin_role in objects.teams.team1.admin_role.parents.all()
 
 
 @pytest.mark.django_db

From fa010c2228f706d85241bea6186f91b7c32428df Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Tue, 24 May 2016 16:27:28 -0400
Subject: [PATCH 10/16] fix mk_label method

---
 awx/main/tests/factories/tower.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
index 48f4c44c2f..7379cf4e6c 100644
--- a/awx/main/tests/factories/tower.py
+++ b/awx/main/tests/factories/tower.py
@@ -255,7 +255,7 @@ def create_organization(name, **kwargs):
             if type(l) is Label:
                 labels[l.name] = l
             else:
-                labels[l] = mk_label(l, org, persisted=persisted)
+                labels[l] = mk_label(l, organization=org, persisted=persisted)
 
     apply_roles(kwargs.get('roles'), [superusers, users, teams, projects, labels], persisted)
     return Objects(organization=org,

From c514cd44c31982169e2c2234acd6415bf84431f7 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Wed, 25 May 2016 10:29:22 -0400
Subject: [PATCH 11/16] restructure factories/fixtures and fix tests

---
 awx/main/tests/factories/__init__.py          |   7 +
 awx/main/tests/factories/exc.py               |   5 +
 awx/main/tests/factories/fixtures.py          | 132 ++++++++
 awx/main/tests/factories/tower.py             | 294 +++++++++---------
 awx/main/tests/functional/conftest.py         |  21 +-
 .../functional/test_fixture_factories.py      |  11 +-
 6 files changed, 309 insertions(+), 161 deletions(-)
 create mode 100644 awx/main/tests/factories/exc.py
 create mode 100644 awx/main/tests/factories/fixtures.py

diff --git a/awx/main/tests/factories/__init__.py b/awx/main/tests/factories/__init__.py
index 2df095f34e..8c8eb326d1 100644
--- a/awx/main/tests/factories/__init__.py
+++ b/awx/main/tests/factories/__init__.py
@@ -1,9 +1,16 @@
 from .tower import (
     create_organization,
     create_job_template,
+    create_notification_template,
+)
+
+from .exc import (
+    NotUnique,
 )
 
 __all__ = [
     'create_organization',
     'create_job_template',
+    'create_notification_template',
+    'NotUnique',
 ]
diff --git a/awx/main/tests/factories/exc.py b/awx/main/tests/factories/exc.py
new file mode 100644
index 0000000000..aa51de5bd3
--- /dev/null
+++ b/awx/main/tests/factories/exc.py
@@ -0,0 +1,5 @@
+class NotUnique(Exception):
+    def __init__(self, name, objects):
+        msg = '{} is not a unique key, found {}={}'.format(name, name, objects[name])
+        super(Exception, self).__init__(msg)
+
diff --git a/awx/main/tests/factories/fixtures.py b/awx/main/tests/factories/fixtures.py
new file mode 100644
index 0000000000..35845bc874
--- /dev/null
+++ b/awx/main/tests/factories/fixtures.py
@@ -0,0 +1,132 @@
+from django.contrib.auth.models import User
+
+from awx.main.models import (
+    Organization,
+    Project,
+    Team,
+    Instance,
+    JobTemplate,
+    NotificationTemplate,
+    Credential,
+    Inventory,
+    Label,
+)
+
+# mk methods should create only a single object of a single type.
+# they should also have the option of being persisted or not.
+# if the object must be persisted an error should be raised when
+# persisted=False
+#
+
+def mk_instance(persisted=True):
+    if not persisted:
+        raise RuntimeError('creating an Instance requires persisted=True')
+    from django.conf import settings
+    return Instance.objects.get_or_create(uuid=settings.SYSTEM_UUID, primary=True, hostname="instance.example.org")
+
+
+def mk_organization(name, desc, persisted=True):
+    org = Organization(name=name, description=desc)
+    if persisted:
+        mk_instance(True)
+        org.save()
+    return org
+
+
+def mk_label(name, **kwargs):
+    label = Label(name=name, description="%s-desc".format(name))
+    organization = kwargs.get('organization')
+    if organization is not None:
+        label.organization = organization
+    if kwargs.get('persisted', True):
+        label.save()
+    return label
+
+
+def mk_team(name, **kwargs):
+    team = Team(name=name)
+    organization = kwargs.get('organization')
+    if organization is not None:
+        team.organization = organization
+    if kwargs.get('persisted', True):
+        mk_instance(True)
+        team.save()
+    return team
+
+
+def mk_user(name, **kwargs):
+    user = User(username=name, is_superuser=kwargs.get('is_superuser', False))
+
+    if kwargs.get('persisted', True):
+        user.save()
+        organization = kwargs.get('organization')
+        if organization is not None:
+            organization.member_role.members.add(user)
+        team = kwargs.get('team')
+        if team is not None:
+            team.member_role.members.add(user)
+    return user
+
+
+def mk_project(name, **kwargs):
+    project = Project(name=name)
+    organization = kwargs.get('organization')
+    if organization is not None:
+        project.organization = organization
+    if kwargs.get('persisted', True):
+        project.save()
+    return project
+
+
+def mk_credential(name, **kwargs):
+    cred = Credential(name=name)
+    cred.cloud = kwargs.get('cloud', False)
+    cred.kind = kwargs.get('kind', 'ssh')
+    if kwargs.get('persisted', True):
+        cred.save()
+    return cred
+
+
+def mk_notification_template(name, **kwargs):
+    nt = NotificationTemplate(name=name)
+    nt.notification_type = kwargs.get('type', 'webhook')
+
+    configuration = kwargs.get('configuration',
+                               dict(url="http://localhost", headers={"Test": "Header"}))
+    nt.notification_configuration = configuration
+
+    organization = kwargs.get('organization')
+    if organization is not None:
+        nt.organization = organization
+    if kwargs.get('persisted', True):
+        nt.save()
+    return nt
+
+
+def mk_inventory(name, **kwargs):
+    inv = Inventory(name=name)
+    organization = kwargs.get('organization', None)
+    if organization is not None:
+        inv.organization = organization
+    if kwargs.get('persisted', True):
+        inv.save()
+    return inv
+
+
+def mk_job_template(name, **kwargs):
+    jt = JobTemplate(name=name, job_type=kwargs.get('job_type', 'run'))
+
+    jt.inventory = kwargs.get('inventory', None)
+    if jt.inventory is None:
+        jt.ask_inventory_on_launch = True
+
+    jt.credential = kwargs.get('credential', None)
+    if jt.credential is None:
+        jt.ask_credential_on_launch = True
+
+    jt.project = kwargs.get('project', None)
+
+    if kwargs.get('persisted', True):
+        jt.save()
+    return jt
+
diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
index 7379cf4e6c..95dd30baf1 100644
--- a/awx/main/tests/factories/tower.py
+++ b/awx/main/tests/factories/tower.py
@@ -1,126 +1,85 @@
 from collections import namedtuple
 
 from django.contrib.auth.models import User
-from django.conf import settings
 
 from awx.main.models import (
     Organization,
     Project,
     Team,
-    Instance,
-    JobTemplate,
+    NotificationTemplate,
     Credential,
     Inventory,
     Label,
 )
 
+from .fixtures import (
+    mk_organization,
+    mk_team,
+    mk_user,
+    mk_job_template,
+    mk_credential,
+    mk_inventory,
+    mk_project,
+    mk_label,
+    mk_notification_template,
+)
 
-def instance():
-    return Instance.objects.get_or_create(uuid=settings.SYSTEM_UUID, primary=True, hostname="instance.example.org")
+from .exc import NotUnique
 
 
-def mk_organization(name, desc, persisted=True):
-    org = Organization(name=name, description=desc)
-    if persisted:
-        instance()
-        org.save()
-    return org
+def build_role_objects(objects):
+    '''build_role_objects assembles a dictionary of all possible objects by name.
+    It will raise an exception if any of the objects share a name due to the fact that
+    it is to be used with apply_roles, which expects unique object names.
 
-
-def mk_label(name, **kwargs):
-    label = Label(name=name, description="%s-desc".format(name))
-    organization = kwargs.get('organization')
-    if organization is not None:
-        label.organization = organization
-    if kwargs.get('persisted', True):
-        label.save()
-    return label
-
-
-def mk_team(name, **kwargs):
-    team = Team(name=name)
-    organization = kwargs.get('organization')
-    if organization is not None:
-        team.organization = organization
-    if kwargs.get('persisted', True):
-        instance()
-        team.save()
-    return team
-
-
-def mk_user(name, **kwargs):
-    user = User(username=name, is_superuser=kwargs.get('is_superuser', False))
-
-    if kwargs.get('persisted', True):
-        user.save()
-        organization = kwargs.get('organization')
-        if organization is not None:
-            organization.member_role.members.add(user)
-        team = kwargs.get('team')
-        if team is not None:
-            team.member_role.members.add(user)
-    return user
-
-
-def mk_project(name, **kwargs):
-    project = Project(name=name)
-    organization = kwargs.get('organization')
-    if organization is not None:
-        project.organization = organization
-    if kwargs.get('persisted', True):
-        project.save()
-    return project
-
-
-def mk_credential(name, **kwargs):
-    cred = Credential(name=name)
-    cred.cloud = kwargs.get('cloud', False)
-    cred.kind = kwargs.get('kind', 'ssh')
-    if kwargs.get('persisted', True):
-        cred.save()
-    return cred
-
-
-def mk_inventory(name, **kwargs):
-    inv = Inventory(name=name)
-    organization = kwargs.get('organization', None)
-    if organization is not None:
-        inv.organization = organization
-    if kwargs.get('persisted', True):
-        inv.save()
-    return inv
-
-def mk_job_template(name, **kwargs):
-    jt = JobTemplate(name=name, job_type=kwargs.get('job_type', 'run'))
-
-    jt.inventory = kwargs.get('inventory', None)
-    if jt.inventory is None:
-        jt.ask_inventory_on_launch = True
-
-    jt.credential = kwargs.get('credential', None)
-    if jt.credential is None:
-        jt.ask_credential_on_launch = True
-
-    jt.project = kwargs.get('project', None)
-
-    if kwargs.get('persisted', True):
-        jt.save()
-    return jt
+    roles share a common name e.g. admin_role, member_role. This ensures that the
+    roles short hand used for mapping Roles and Users in apply_roles will function as desired.
+    '''
+    combined_objects = {}
+    for o in objects:
+        if type(o) is dict:
+            for k,v in o.iteritems():
+                if combined_objects.get(k) is not None:
+                    raise NotUnique(k, combined_objects)
+                combined_objects[k] = v
+        elif hasattr(o, 'name'):
+            if combined_objects.get(o.name) is not None:
+                raise NotUnique(o.name, combined_objects)
+            combined_objects[o.name] = o
+        else:
+            raise RuntimeError('expected a list of dict or list of list, got a type {}'.format(type(o)))
+    return combined_objects
 
 def apply_roles(roles, objects, persisted):
+    '''apply_roles evaluates a list of Role relationships represented as strings.
+    The format of this string is 'role:[user|role]'. When a user is provided, they will be
+    made a member of the role on the LHS. When a role is provided that role will be added to
+    the children of the role on the LHS.
+
+    This function assumes that objects is a dictionary that contains a unique set of key to value
+    mappings for all possible "Role objects". See the example below:
+
+        Mapping Users
+        -------------
+        roles = ['org1.admin_role:user1', 'team1.admin_role:user1']
+        objects = {'org1': Organization, 'team1': Team, 'user1': User]
+
+        Mapping Roles
+        -------------
+        roles = ['org1.admin_role:team1.admin_role']
+        objects = {'org1': Organization, 'team1': Team}
+
+        Invalid Mapping
+        ---------------
+        roles = ['org1.admin_role:team1.admin_role']
+        objects = {'org1': Organization', 'user1': User} # Exception, no team1 entry
+    '''
     if roles is None:
         return None
 
     if not persisted:
         raise RuntimeError('roles can not be used when persisted=False')
 
-    all_objects = {}
-    for d in objects:
-        for k,v in d.iteritems():
-            if all_objects.get(k) is not None:
-                raise KeyError('object names must be unique when using roles {} key already exists with value {}'.format(k,v))
-            all_objects[k] = v
-
     for role in roles:
         obj_role, sep, member_role = role.partition(':')
         if not member_role:
@@ -129,14 +88,14 @@ def apply_roles(roles, objects, persisted):
         obj_str, o_role_str = obj_role.split('.')
         member_str, m_sep, m_role_str = member_role.partition('.')
 
-        obj = all_objects[obj_str]
+        obj = objects[obj_str]
         obj_role = getattr(obj, o_role_str)
 
-        member = all_objects[member_str]
+        member = objects[member_str]
         if m_role_str:
             if hasattr(member, m_role_str):
                 member_role = getattr(member, m_role_str)
-                obj_role.parents.add(member_role)
+                obj_role.children.add(member_role)
             else:
                 raise RuntimeError('unable to find {} role for {}'.format(m_role_str, member_str))
         else:
@@ -145,16 +104,68 @@ def apply_roles(roles, objects, persisted):
             else:
                 raise RuntimeError('unable to add non-user {} for members list of {}'.format(member_str, obj_str))
 
+def generate_users(organization, teams, superuser, persisted, **kwargs):
+    '''generate_users evaluates a mixed list of User objects and strings.
+    If a string is encountered a user with that username is created and added to the lookup dict.
+    If a User object is encountered the User.username is used as a key for the lookup dict.
+
+    A short hand for assigning a user to a team is available in the following format: "team_name:username".
+    If a string in that format is encounted an attempt to lookup the team by the key team_name from the teams
+    argumnent is made, a KeyError will be thrown if the team does not exist in the dict. The teams argument should
+    be a dict of {Team.name:Team}
+    '''
+    users = {}
+    key = 'superusers' if superuser else 'users'
+    if key in kwargs and kwargs.get(key) is not None:
+        for u in kwargs[key]:
+            if type(u) is User:
+                users[u.username] = u
+            else:
+                p1, sep, p2 = u.partition(':')
+                if p2:
+                    t = teams[p1]
+                    users[p2] = mk_user(p2, organization=organization, team=t, is_superuser=superuser, persisted=persisted)
+                else:
+                    users[p1] = mk_user(p1, organization=organization, team=None, is_superuser=superuser, persisted=persisted)
+    return users
+
+def generate_teams(organization, persisted, **kwargs):
+    '''generate_teams evalutes a mixed list of Team objects and strings.
+    If a string is encountered a team with that string name is created and added to the lookup dict.
+    If a Team object is encounted the Team.name is used as a key for the lookup dict.
+    '''
+    teams = {}
+    if 'teams' in kwargs and kwargs.get('teams') is not None:
+        for t in kwargs['teams']:
+            if type(t) is Team:
+                teams[t.name] = t
+            else:
+                teams[t] = mk_team(t, organization=organization, persisted=persisted)
+    return teams
+
 
 class _Mapped(object):
+    '''_Mapped is a helper class that replaces spaces and dashes
+    in the name of an object and assigns the object as an attribute
+
+         input: {'my org': Organization}
+         output: instance.my_org = Organization
+    '''
     def __init__(self, d):
         self.d = d
         for k,v in d.items():
+            k = k.replace(' ', '_')
+            k = k.replace('-', '_')
+
             setattr(self, k.replace(' ','_'), v)
 
     def all(self):
         return self.d.values()
 
+# create methods are intended to be called directly as needed
+# or encapsulated by specific factory fixtures in a conftest
+#
+
 def create_job_template(name, **kwargs):
     Objects = namedtuple("Objects", "job_template, inventory, project, credential, job_type")
 
@@ -189,8 +200,8 @@ def create_job_template(name, **kwargs):
                          inventory=inv, credential=cred,
                          job_type=job_type, persisted=persisted)
 
-    objects = [{o.name: o} for o in [org, proj, inv, cred]]
-    apply_roles(kwargs.get('roles'), objects, persisted)
+    role_objects = build_role_objects([org, proj, inv, cred])
+    apply_roles(kwargs.get('roles'), role_objects, persisted)
 
     return Objects(job_template=jt,
                    project=proj,
@@ -199,24 +210,15 @@ def create_job_template(name, **kwargs):
                    job_type=job_type)
 
 def create_organization(name, **kwargs):
-    Objects = namedtuple("Objects", "organization,teams,users,superusers,projects,labels")
+    Objects = namedtuple("Objects", "organization,teams,users,superusers,projects,labels,notification_templates")
 
-    superusers = {}
-    users = {}
-    teams = {}
     projects = {}
     labels = {}
+    notification_templates = {}
     persisted = kwargs.get('persisted', True)
 
     org = mk_organization(name, '%s-desc'.format(name), persisted=persisted)
 
-    if 'teams' in kwargs:
-        for t in kwargs['teams']:
-            if type(t) is Team:
-                teams[t.name] = t
-            else:
-                teams[t] = mk_team(t, organization=org, persisted=persisted)
-
     if 'projects' in kwargs:
         for p in kwargs['projects']:
             if type(p) is Project:
@@ -224,31 +226,9 @@ def create_organization(name, **kwargs):
             else:
                 projects[p] = mk_project(p, organization=org, persisted=persisted)
 
-    if 'superusers' in kwargs:
-        # remove this duplication eventually
-        for u in kwargs['superusers']:
-            if type(u) is User:
-                users[u.username] = u
-            else:
-                p1, sep, p2 = u.partition(':')
-                if p2:
-                    t = teams[p1]
-                    superusers[p2] = mk_user(p2, organization=org, team=t, is_superuser=True, persisted=persisted)
-                else:
-                    superusers[p1] = mk_user(p1, organization=org, team=None, is_superuser=True, persisted=persisted)
-
-    if 'users' in kwargs:
-        # remove this duplication eventually
-        for u in kwargs['users']:
-            if type(u) is User:
-                users[u.username] = u
-            else:
-                p1, sep, p2 = u.partition(':')
-                if p2:
-                    t = teams[p1]
-                    users[p2] = mk_user(p2, organization=org, team=t, is_superuser=False, persisted=persisted)
-                else:
-                    users[p1] = mk_user(p1, organization=org, is_superuser=False, persisted=persisted)
+    teams = generate_teams(org, persisted, teams=kwargs.get('teams'))
+    superusers = generate_users(org, teams, True, persisted, superusers=kwargs.get('superusers'))
+    users = generate_users(org, teams, False, persisted, users=kwargs.get('users'))
 
     if 'labels' in kwargs:
         for l in kwargs['labels']:
@@ -257,11 +237,43 @@ def create_organization(name, **kwargs):
             else:
                 labels[l] = mk_label(l, organization=org, persisted=persisted)
 
-    apply_roles(kwargs.get('roles'), [superusers, users, teams, projects, labels], persisted)
+    if 'notification_templates' in kwargs:
+        for nt in kwargs['notification_templates']:
+            if type(nt) is NotificationTemplate:
+                notification_templates[nt.name] = nt
+            else:
+                notification_templates[nt] = mk_notification_template(nt, organization=org, persisted=persisted)
+
+    role_objects = build_role_objects([superusers, users, teams, projects, labels, notification_templates])
+    apply_roles(kwargs.get('roles'), role_objects, persisted)
     return Objects(organization=org,
                    superusers=_Mapped(superusers),
                    users=_Mapped(users),
                    teams=_Mapped(teams),
                    projects=_Mapped(projects),
-                   labels=_Mapped(labels))
+                   labels=_Mapped(labels),
+                   notification_templates=_Mapped(notification_templates))
 
+def create_notification_template(name, **kwargs):
+    Objects = namedtuple("Objects", "notification_template,organization,users,superusers,teams")
+
+    organization = None
+    persisted = kwargs.get('persisted', True)
+
+    if 'organization' in kwargs:
+        org = kwargs['organization']
+        organization = mk_organization(org, persisted=persisted)
+
+    notification_template = mk_notification_template(name, organization=organization, persisted=persisted)
+
+    teams = generate_teams(organization, persisted, teams=kwargs.get('teams'))
+    superusers = generate_users(org, teams, True, persisted, superusers=kwargs.get('superusers'))
+    users = generate_users(org, teams, False, persisted, users=kwargs.get('users'))
+
+    role_objects = build_role_objects([organization, notification_template])
+    apply_roles(kwargs.get('roles'), role_objects, persisted)
+    return Objects(notification_template=notification_template,
+                   organization=organization,
+                   users=users,
+                   superusers=superusers,
+                   teams=teams)
diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py
index fbdba20044..68bd1257fd 100644
--- a/awx/main/tests/functional/conftest.py
+++ b/awx/main/tests/functional/conftest.py
@@ -41,6 +41,7 @@ from awx.main.models.notifications import NotificationTemplate
 from awx.main.tests.factories import (
     create_organization,
     create_job_template,
+    create_notification_template,
 )
 
 '''
@@ -275,21 +276,6 @@ def permissions():
                  'update':False, 'delete':False, 'scm_update':False, 'execute':False, 'use':True,},
     }
 
-@pytest.fixture
-def notification_template_factory(organization):
-    def n(name="test-notification_template"):
-        try:
-            notification_template = NotificationTemplate.objects.get(name=name)
-        except NotificationTemplate.DoesNotExist:
-            notification_template = NotificationTemplate(name=name,
-                                                         organization=organization,
-                                                         notification_type="webhook",
-                                                         notification_configuration=dict(url="http://localhost",
-                                                                                         headers={"Test": "Header"}))
-            notification_template.save()
-        return notification_template
-    return n
-
 @pytest.fixture
 def post():
     def rf(url, data, user=None, middleware=None, **kwargs):
@@ -475,3 +461,8 @@ def job_template_factory():
 @pytest.fixture
 def organization_factory():
     return create_organization
+
+@pytest.fixture
+def notification_template_factory():
+    return create_notification_template
+
diff --git a/awx/main/tests/functional/test_fixture_factories.py b/awx/main/tests/functional/test_fixture_factories.py
index 908eb6f053..286d375a1a 100644
--- a/awx/main/tests/functional/test_fixture_factories.py
+++ b/awx/main/tests/functional/test_fixture_factories.py
@@ -1,5 +1,6 @@
 import pytest
 
+from awx.main.tests.factories import NotUnique
 
 def test_roles_exc_not_persisted(organization_factory):
     with pytest.raises(RuntimeError) as exc:
@@ -15,9 +16,9 @@ def test_roles_exc_bad_object(organization_factory):
 
 @pytest.mark.django_db
 def test_roles_exc_not_unique(organization_factory):
-    with pytest.raises(KeyError) as exc:
+    with pytest.raises(NotUnique) as exc:
         organization_factory('test-org', projects=['foo'], teams=['foo'], roles=['foo.admin_role:user'])
-    assert 'must be unique' in str(exc.value)
+    assert 'not a unique key' in str(exc.value)
 
 
 @pytest.mark.django_db
@@ -48,14 +49,14 @@ def test_org_factory_roles(organization_factory):
                                    users=['team1:foo', 'bar'],
                                    projects=['baz', 'bang'],
                                    roles=['team2.member_role:foo',
-                                          'team2.admin_role:bar',
+                                          'team1.admin_role:bar',
                                           'team1.admin_role:team2.admin_role',
                                           'baz.admin_role:foo'])
 
-    assert objects.users.bar in objects.teams.team1.admin_role
+    assert objects.users.bar in objects.teams.team2.admin_role
     assert objects.users.foo in objects.projects.baz.admin_role
     assert objects.users.foo in objects.teams.team1.member_role
-    assert objects.teams.team2.admin_role in objects.teams.team1.admin_role.parents.all()
+    assert objects.teams.team2.admin_role in objects.teams.team1.admin_role.children.all()
 
 
 @pytest.mark.django_db

From 2052f93792d4512246bac70281d866f1672975f3 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Wed, 25 May 2016 10:50:20 -0400
Subject: [PATCH 12/16] fixing function calls and None handling

---
 awx/main/tests/factories/tower.py | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
index 95dd30baf1..bf0311d58b 100644
--- a/awx/main/tests/factories/tower.py
+++ b/awx/main/tests/factories/tower.py
@@ -47,7 +47,8 @@ def build_role_objects(objects):
                 raise NotUnique(o.name, combined_objects)
             combined_objects[o.name] = o
         else:
-            raise RuntimeError('expected a list of dict or list of list, got a type {}'.format(type(o)))
+            if o is not None:
+                raise RuntimeError('expected a list of dict or list of list, got a type {}'.format(type(o)))
     return combined_objects
 
 def apply_roles(roles, objects, persisted):
@@ -244,7 +245,7 @@ def create_organization(name, **kwargs):
             else:
                 notification_templates[nt] = mk_notification_template(nt, organization=org, persisted=persisted)
 
-    role_objects = build_role_objects([superusers, users, teams, projects, labels, notification_templates])
+    role_objects = build_role_objects([org, superusers, users, teams, projects, labels, notification_templates])
     apply_roles(kwargs.get('roles'), role_objects, persisted)
     return Objects(organization=org,
                    superusers=_Mapped(superusers),
@@ -262,18 +263,18 @@ def create_notification_template(name, **kwargs):
 
     if 'organization' in kwargs:
         org = kwargs['organization']
-        organization = mk_organization(org, persisted=persisted)
+        organization = mk_organization(org, '{}-desc'.format(org), persisted=persisted)
 
     notification_template = mk_notification_template(name, organization=organization, persisted=persisted)
 
     teams = generate_teams(organization, persisted, teams=kwargs.get('teams'))
-    superusers = generate_users(org, teams, True, persisted, superusers=kwargs.get('superusers'))
-    users = generate_users(org, teams, False, persisted, users=kwargs.get('users'))
+    superusers = generate_users(organization, teams, True, persisted, superusers=kwargs.get('superusers'))
+    users = generate_users(organization, teams, False, persisted, users=kwargs.get('users'))
 
     role_objects = build_role_objects([organization, notification_template])
     apply_roles(kwargs.get('roles'), role_objects, persisted)
     return Objects(notification_template=notification_template,
                    organization=organization,
-                   users=users,
-                   superusers=superusers,
+                   users=_Mapped(users),
+                   superusers=_Mapped(superusers),
                    teams=teams)

From 1189ce033aea4c899abbf1f45721507348d72947 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Wed, 25 May 2016 10:50:31 -0400
Subject: [PATCH 13/16] fixing broken test cases

---
 awx/main/tests/functional/test_projects.py    | 51 ++++++++-----------
 .../functional/test_rbac_notifications.py     | 39 ++++++++------
 2 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/awx/main/tests/functional/test_projects.py b/awx/main/tests/functional/test_projects.py
index 60b541f0c4..902ffa75d0 100644
--- a/awx/main/tests/functional/test_projects.py
+++ b/awx/main/tests/functional/test_projects.py
@@ -9,6 +9,19 @@ from awx.main.models import Project
 #
 # Project listing and visibility tests
 #
+@pytest.fixture
+def team_project_list(organization_factory):
+    objects = organization_factory('org-test',
+                                   superusers=['admin'],
+                                   users=['team1:alice', 'team2:bob'],
+                                   teams=['team1', 'team2'],
+                                   projects=['pteam1', 'pteam2', 'pshared'],
+                                   roles=['team1.member_role:pteam1.admin_role',
+                                          'team2.member_role:pteam2.admin_role',
+                                          'team1.member_role:pshared.admin_role',
+                                          'team2.member_role:pshared.admin_role'])
+    return objects
+
 
 @pytest.mark.django_db
 def test_user_project_list(get, organization_factory):
@@ -39,28 +52,12 @@ def test_user_project_list(get, organization_factory):
     assert get(reverse('api:user_projects_list', args=(objects.superusers.admin.pk,)), objects.users.alice).data['count'] == 2
 
 
-def setup_test_team_project_list(project_factory, team_factory, admin, alice, bob):
-    team1 = team_factory('team1')
-    team2 = team_factory('team2')
-
-    team1_project = project_factory('team1 project')
-    team1_project.admin_role.parents.add(team1.member_role)
-
-    team2_project = project_factory('team2 project')
-    team2_project.admin_role.parents.add(team2.member_role)
-
-    shared_project = project_factory('shared project')
-    shared_project.admin_role.parents.add(team1.member_role)
-    shared_project.admin_role.parents.add(team2.member_role)
-
-    team1.member_role.members.add(alice)
-    team2.member_role.members.add(bob)
-    return team1, team2
-
 @pytest.mark.django_db
-def test_team_project_list(get, project_factory, team_factory, admin, alice, bob):
-    'List of projects a team has access to, filtered by projects you can also see'
-    team1, team2 = setup_test_team_project_list(project_factory, team_factory, admin, alice, bob)
+def test_team_project_list(get, team_project_list):
+    objects = team_project_list
+
+    team1, team2 = objects.teams.team1, objects.teams.team2
+    alice, bob, admin = objects.users.alice, objects.users.bob, objects.superusers.admin
 
     # admins can see all projects on a team
     assert get(reverse('api:team_projects_list', args=(team1.pk,)), admin).data['count'] == 2
@@ -94,17 +91,11 @@ def test_team_project_list(get, project_factory, team_factory, admin, alice, bob
     assert get(reverse('api:user_projects_list', args=(admin.pk,)), alice).data['count'] == 2
 
 @pytest.mark.django_db
-def test_team_project_list_fail1(get, project_factory, team_factory, admin, alice, bob):
-    # alice should not be able to see team2 projects because she doesn't have access to team2
-    team1, team2 = setup_test_team_project_list(project_factory, team_factory, admin, alice, bob)
-    res = get(reverse('api:team_projects_list', args=(team2.pk,)), alice)
+def test_team_project_list_fail1(get, team_project_list):
+    objects = team_project_list
+    res = get(reverse('api:team_projects_list', args=(objects.teams.team2.pk,)), objects.users.alice)
     assert res.status_code == 403
 
-@pytest.mark.django_db
-def test_team_project_list_fail2(get, project_factory, team_factory, admin, alice, bob):
-    team1, team2 = setup_test_team_project_list(project_factory, team_factory, admin, alice, bob)
-    # alice should not be able to see bob
-
 @pytest.mark.parametrize("u,expected_status_code", [
     ('rando', 403),
     ('org_member', 403),
diff --git a/awx/main/tests/functional/test_rbac_notifications.py b/awx/main/tests/functional/test_rbac_notifications.py
index 467ae8038a..9cabec6fbf 100644
--- a/awx/main/tests/functional/test_rbac_notifications.py
+++ b/awx/main/tests/functional/test_rbac_notifications.py
@@ -25,35 +25,44 @@ def test_notification_template_get_queryset_orgadmin(notification_template, user
     assert access.get_queryset().count() == 1
 
 @pytest.mark.django_db
-def test_notification_template_access_superuser(notification_template, user, notification_template_factory):
-    access = NotificationTemplateAccess(user('admin', True))
-    assert access.can_read(notification_template)
-    assert access.can_change(notification_template, None)
-    assert access.can_delete(notification_template)
-    nf = notification_template_factory("test-orphaned")
+def test_notification_template_access_superuser(notification_template_factory):
+    nf_objects = notification_template_factory('test-orphaned', organization='test', superusers=['admin'])
+    admin = nf_objects.superusers.admin
+    nf = nf_objects.notification_template
+
+    access = NotificationTemplateAccess(admin)
+    assert access.can_read(nf)
+    assert access.can_change(nf, None)
+    assert access.can_delete(nf)
+
     nf.organization = None
     nf.save()
+
     assert access.can_read(nf)
     assert access.can_change(nf, None)
     assert access.can_delete(nf)
 
 @pytest.mark.django_db
-def test_notification_template_access_admin(notification_template, user, organization_factory, notification_template_factory):
-    adm = user('admin', False)
-    other_org = organization_factory('other')
-    present_org = organization_factory('present')
-    notification_template.organization.admin_role.members.add(adm)
-    present_org.admin_role.members.add(adm)
+def test_notification_template_access_admin(organization_factory, notification_template_factory):
+    other_objects = organization_factory('other')
+    present_objects = organization_factory('present',
+                                           users=['admin'],
+                                           notification_templates=['test-notification'],
+                                           roles=['present.admin_role:admin'])
 
-    access = NotificationTemplateAccess(user('admin', False))
+    notification_template = present_objects.notification_templates.test_notification
+    other_org = other_objects.organization
+    present_org = present_objects.organization
+    admin = present_objects.users.admin
+
+    access = NotificationTemplateAccess(admin)
     assert not access.can_change(notification_template, {'organization': other_org.id})
     assert access.can_read(notification_template)
     assert access.can_change(notification_template, None)
     assert access.can_change(notification_template, {'organization': present_org.id})
     assert access.can_delete(notification_template)
+
     nf = notification_template_factory("test-orphaned")
-    nf.organization = None
-    nf.save()
     assert not access.can_read(nf)
     assert not access.can_change(nf, None)
     assert not access.can_delete(nf)

From a6e97626ba52115299e70f3b119d261988ee5c48 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Wed, 25 May 2016 11:02:46 -0400
Subject: [PATCH 14/16] README updates

---
 awx/main/tests/factories/README.md | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/awx/main/tests/factories/README.md b/awx/main/tests/factories/README.md
index 8b864d0708..c451c02598 100644
--- a/awx/main/tests/factories/README.md
+++ b/awx/main/tests/factories/README.md
@@ -47,3 +47,19 @@ examples
 
     objects = create_organization('test-org', persisted=False)
     assert not objects.organization.pk
+
+patterns
+--------
+
+`mk` functions are single object fixtures. They should create only a single object with the minimum deps.
+They should also accept a `persited` flag, if they must be persisted to work, they raise an error if persisted=False
+
+`generate` and `apply` functions are helpers that build up the various parts of a `create` functions objects. These
+should be useful for more than one create function to use and should explicitly accept all of the values needed
+to execute. These functions should also be robust and have very speciifc error reporting about constraints and/or
+bad values.
+
+`create` functions compose many of the `mk` and `generate` functions to make different object
+factories. These functions when giving the minimum set of arguments should only produce a
+single artifact (or the minimum needed for that object). These should be wrapped by discoverable
+fixtures in various conftest.py files.

From ec505f3f6016dc6ed144b01275cf1d44312cd12f Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Wed, 25 May 2016 11:40:30 -0400
Subject: [PATCH 15/16] adjusting test and renaming helper

---
 awx/main/tests/factories/tower.py          | 10 +++++-----
 awx/main/tests/functional/test_projects.py |  7 -------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
index bf0311d58b..2a9d493efe 100644
--- a/awx/main/tests/factories/tower.py
+++ b/awx/main/tests/factories/tower.py
@@ -27,8 +27,8 @@ from .fixtures import (
 from .exc import NotUnique
 
 
-def build_role_objects(objects):
-    '''build_role_objects assembles a dictionary of all possible objects by name.
+def generate_role_objects(objects):
+    '''generate_role_objects assembles a dictionary of all possible objects by name.
     It will raise an exception if any of the objects share a name due to the fact that
     it is to be used with apply_roles, which expects unique object names.
 
@@ -201,7 +201,7 @@ def create_job_template(name, **kwargs):
                          inventory=inv, credential=cred,
                          job_type=job_type, persisted=persisted)
 
-    role_objects = build_role_objects([org, proj, inv, cred])
+    role_objects = generate_role_objects([org, proj, inv, cred])
     apply_roles(kwargs.get('roles'), role_objects, persisted)
 
     return Objects(job_template=jt,
@@ -245,7 +245,7 @@ def create_organization(name, **kwargs):
             else:
                 notification_templates[nt] = mk_notification_template(nt, organization=org, persisted=persisted)
 
-    role_objects = build_role_objects([org, superusers, users, teams, projects, labels, notification_templates])
+    role_objects = generate_role_objects([org, superusers, users, teams, projects, labels, notification_templates])
     apply_roles(kwargs.get('roles'), role_objects, persisted)
     return Objects(organization=org,
                    superusers=_Mapped(superusers),
@@ -271,7 +271,7 @@ def create_notification_template(name, **kwargs):
     superusers = generate_users(organization, teams, True, persisted, superusers=kwargs.get('superusers'))
     users = generate_users(organization, teams, False, persisted, users=kwargs.get('users'))
 
-    role_objects = build_role_objects([organization, notification_template])
+    role_objects = generate_role_objects([organization, notification_template])
     apply_roles(kwargs.get('roles'), role_objects, persisted)
     return Objects(notification_template=notification_template,
                    organization=organization,
diff --git a/awx/main/tests/functional/test_projects.py b/awx/main/tests/functional/test_projects.py
index 902ffa75d0..4c32d1dd69 100644
--- a/awx/main/tests/functional/test_projects.py
+++ b/awx/main/tests/functional/test_projects.py
@@ -1,7 +1,6 @@
 import mock # noqa
 import pytest
 
-from django.db import transaction
 from django.core.urlresolvers import reverse
 from awx.main.models import Project
 
@@ -71,12 +70,6 @@ def test_team_project_list(get, team_project_list):
     assert get(reverse('api:team_projects_list', args=(team2.pk,)), alice).data['count'] == 1
     team2.read_role.members.remove(alice)
 
-    # Test user endpoints first, very similar tests to test_user_project_list
-    # but permissions are being derived from team membership instead.
-    with transaction.atomic():
-        res = get(reverse('api:user_projects_list', args=(bob.pk,)), alice)
-        assert res.status_code == 403
-
     # admins can see all projects
     assert get(reverse('api:user_projects_list', args=(admin.pk,)), admin).data['count'] == 3
 

From b8865c3749d0865556280b4ae2ed0b7f5a5e90c6 Mon Sep 17 00:00:00 2001
From: Wayne Witzel III <wayne@riotousliving.com>
Date: Wed, 25 May 2016 12:00:46 -0400
Subject: [PATCH 16/16] make arguments to mk_ methods explicit

---
 awx/main/tests/factories/fixtures.py | 80 +++++++++++++---------------
 1 file changed, 36 insertions(+), 44 deletions(-)

diff --git a/awx/main/tests/factories/fixtures.py b/awx/main/tests/factories/fixtures.py
index 35845bc874..91ead460db 100644
--- a/awx/main/tests/factories/fixtures.py
+++ b/awx/main/tests/factories/fixtures.py
@@ -25,108 +25,100 @@ def mk_instance(persisted=True):
     return Instance.objects.get_or_create(uuid=settings.SYSTEM_UUID, primary=True, hostname="instance.example.org")
 
 
-def mk_organization(name, desc, persisted=True):
-    org = Organization(name=name, description=desc)
+def mk_organization(name, description=None, persisted=True):
+    description = description or '{}-description'.format(name)
+    org = Organization(name=name, description=description)
     if persisted:
-        mk_instance(True)
+        mk_instance(persisted)
         org.save()
     return org
 
 
-def mk_label(name, **kwargs):
-    label = Label(name=name, description="%s-desc".format(name))
-    organization = kwargs.get('organization')
+def mk_label(name, organization=None, description=None, persisted=True):
+    description = description or '{}-description'.format(name)
+    label = Label(name=name, description=description)
     if organization is not None:
         label.organization = organization
-    if kwargs.get('persisted', True):
+    if persisted:
         label.save()
     return label
 
 
-def mk_team(name, **kwargs):
+def mk_team(name, organization=None, persisted=True):
     team = Team(name=name)
-    organization = kwargs.get('organization')
     if organization is not None:
         team.organization = organization
-    if kwargs.get('persisted', True):
-        mk_instance(True)
+    if persisted:
+        mk_instance(persisted)
         team.save()
     return team
 
 
-def mk_user(name, **kwargs):
-    user = User(username=name, is_superuser=kwargs.get('is_superuser', False))
-
-    if kwargs.get('persisted', True):
+def mk_user(name, is_superuser=False, organization=None, team=None, persisted=True):
+    user = User(username=name, is_superuser=is_superuser)
+    if persisted:
         user.save()
-        organization = kwargs.get('organization')
         if organization is not None:
             organization.member_role.members.add(user)
-        team = kwargs.get('team')
         if team is not None:
             team.member_role.members.add(user)
     return user
 
 
-def mk_project(name, **kwargs):
-    project = Project(name=name)
-    organization = kwargs.get('organization')
+def mk_project(name, organization=None, description=None, persisted=True):
+    description = description or '{}-description'.format(name)
+    project = Project(name=name, description=description)
     if organization is not None:
         project.organization = organization
-    if kwargs.get('persisted', True):
+    if persisted:
         project.save()
     return project
 
 
-def mk_credential(name, **kwargs):
-    cred = Credential(name=name)
-    cred.cloud = kwargs.get('cloud', False)
-    cred.kind = kwargs.get('kind', 'ssh')
-    if kwargs.get('persisted', True):
+def mk_credential(name, cloud=False, kind='ssh', persisted=True):
+    cred = Credential(name=name, cloud=cloud, kind=kind)
+    if persisted:
         cred.save()
     return cred
 
 
-def mk_notification_template(name, **kwargs):
+def mk_notification_template(name, notification_type='webhook', configuration=None, organization=None, persisted=True):
     nt = NotificationTemplate(name=name)
-    nt.notification_type = kwargs.get('type', 'webhook')
+    nt.notification_type = notification_type
+    nt.notification_configuration = configuration or dict(url="http://localhost", headers={"Test": "Header"})
 
-    configuration = kwargs.get('configuration',
-                               dict(url="http://localhost", headers={"Test": "Header"}))
-    nt.notification_configuration = configuration
-
-    organization = kwargs.get('organization')
     if organization is not None:
         nt.organization = organization
-    if kwargs.get('persisted', True):
+    if persisted:
         nt.save()
     return nt
 
 
-def mk_inventory(name, **kwargs):
+def mk_inventory(name, organization=None, persisted=True):
     inv = Inventory(name=name)
-    organization = kwargs.get('organization', None)
     if organization is not None:
         inv.organization = organization
-    if kwargs.get('persisted', True):
+    if persisted:
         inv.save()
     return inv
 
 
-def mk_job_template(name, **kwargs):
-    jt = JobTemplate(name=name, job_type=kwargs.get('job_type', 'run'))
+def mk_job_template(name, job_type='run',
+                    organization=None, inventory=None,
+                    credential=None, persisted=True,
+                    project=None):
+    jt = JobTemplate(name=name, job_type=job_type)
 
-    jt.inventory = kwargs.get('inventory', None)
+    jt.inventory = inventory
     if jt.inventory is None:
         jt.ask_inventory_on_launch = True
 
-    jt.credential = kwargs.get('credential', None)
+    jt.credential = credential
     if jt.credential is None:
         jt.ask_credential_on_launch = True
 
-    jt.project = kwargs.get('project', None)
+    jt.project = project
 
-    if kwargs.get('persisted', True):
+    if persisted:
         jt.save()
     return jt
-