Merge pull request #1048 from 18F/simplify-public

Simplify workflow/public logic

api/document/tests/views_test.py

@@ -14,7 +14,7 @@
 @pytest.mark.django_db
 @pytest.mark.urls('document.urls')
 def test_put_403s_for_anon_users(client):
-    policy = mommy.make(Policy)
+    policy = mommy.make(Policy, workflow_phase='published')
     root = DocCursor.new_tree('root', '0', policy=policy)
     root.nested_set_renumber()
 
@@ -34,7 +34,7 @@ def test_put_fails_with_identifier(admin_client):
 @pytest.mark.django_db
 @pytest.mark.urls('document.urls')
 def test_json_put_works_for_admin_users(admin_client):
-    policy = mommy.make(Policy)
+    policy = mommy.make(Policy, workflow_phase='published')
     root = DocCursor.new_tree('root', '0', policy=policy)
     root.add_child('sec', text='blah')
     root.nested_set_renumber()
@@ -63,7 +63,7 @@ def test_json_put_works_for_admin_users(admin_client):
 @pytest.mark.django_db
 @pytest.mark.urls('document.urls')
 def test_akn_put_works_for_admin_users(admin_client):
-    policy = mommy.make(Policy)
+    policy = mommy.make(Policy, workflow_phase='published')
     root = DocCursor.new_tree('root', '0', policy=policy)
     root.add_child('sec', text='blah')
     root.nested_set_renumber()
@@ -90,7 +90,7 @@ def test_akn_put_works_for_admin_users(admin_client):
 @pytest.mark.django_db
 @pytest.mark.urls('document.urls')
 def test_404s(client):
-    policy = mommy.make(Policy)
+    policy = mommy.make(Policy, workflow_phase='published')
     root = DocCursor.new_tree('root', '0', policy=policy)
     root.add_child('sec')
     root.nested_set_renumber()
@@ -106,7 +106,7 @@ def test_404s(client):
 @pytest.mark.django_db
 @pytest.mark.urls('document.urls')
 def test_correct_data(client):
-    policy = mommy.make(Policy)
+    policy = mommy.make(Policy, workflow_phase='published')
     root = DocCursor.new_tree('root', '0', policy=policy)
     sec1 = root.add_child('sec')
     root.add_child('sec')
@@ -130,7 +130,8 @@ def serialize(node):
 @pytest.mark.django_db
 @pytest.mark.urls('document.urls')
 def test_by_pretty_url(client):
-    policy = mommy.make(Policy, omb_policy_id='M-Something-18')
+    policy = mommy.make(Policy, omb_policy_id='M-Something-18',
+                        workflow_phase='published')
     root = DocCursor.new_tree('root', '0', policy=policy)
     root.nested_set_renumber()
 
@@ -143,7 +144,8 @@ def test_by_pretty_url(client):
 @pytest.mark.django_db
 @pytest.mark.urls('document.urls')
 def test_nonpublic(client, admin_client):
-    policy = mommy.make(Policy, omb_policy_id='M-Something-18', public=False)
+    policy = mommy.make(Policy, omb_policy_id='M-Something-18',
+                        workflow_phase='edit')
     root = DocCursor.new_tree('root', '0', policy=policy)
     root.nested_set_renumber()
 
@@ -157,7 +159,8 @@ def test_nonpublic(client, admin_client):
 @pytest.mark.django_db
 @pytest.mark.urls('document.urls')
 def test_query_count(client):
-    policy = mommy.make(Policy, omb_policy_id='M-O-A-R')
+    policy = mommy.make(Policy, omb_policy_id='M-O-A-R',
+                        workflow_phase='published')
     root = random_doc(20, save=True, policy=policy, text='placeholder')
 
     # select 3 nodes to have external links
@@ -201,15 +204,15 @@ def test_query_count(client):
 @pytest.mark.parametrize('path_suffix', ['', '/akn'])
 @pytest.mark.django_db
 def test_editor_requires_admin(client, path_suffix):
-    mommy.make(Policy, omb_policy_id='M-11-22')
+    mommy.make(Policy, omb_policy_id='M-11-22', workflow_phase='published')
     result = client.get(f'/admin/document-editor/M-11-22{path_suffix}')
     assert result.status_code == 302
 
 
 @pytest.mark.parametrize('path_suffix', ['', '/akn'])
 @pytest.mark.django_db
 def test_editor_checks_policy(admin_client, path_suffix):
-    mommy.make(Policy, omb_policy_id='M-11-22')
+    mommy.make(Policy, omb_policy_id='M-11-22', workflow_phase='published')
     result = admin_client.get(f'/admin/document-editor/M-99-88{path_suffix}')
     assert result.status_code == 404
     result = admin_client.get(f'/admin/document-editor/M-11-22{path_suffix}')
@@ -219,6 +222,6 @@ def test_editor_checks_policy(admin_client, path_suffix):
 @pytest.mark.parametrize('path_suffix', ['', '/akn'])
 @pytest.mark.django_db
 def test_editor_policy_can_be_private(admin_client, path_suffix):
-    mommy.make(Policy, omb_policy_id='M-11-22', public=False)
+    mommy.make(Policy, omb_policy_id='M-11-22', workflow_phase='edit')
     result = admin_client.get(f'/admin/document-editor/M-11-22{path_suffix}')
     assert result.status_code == 200

api/document/views.py

@@ -23,8 +23,8 @@ class TreeView(GenericAPIView):
     queryset = DocNode.objects.none()   # Used to determine permissions
 
     def get_object(self, prefetch_related=True):
-        only_public = not self.request.user.is_authenticated
-        policy = policy_or_404(self.kwargs['policy_id'], only_public)
+        only_published = not self.request.user.is_authenticated
+        policy = policy_or_404(self.kwargs['policy_id'], only_published)
         # we'll pass this policy down when we serialize
         self.policy = policy
         query_args = {'policy_id': policy.pk}
@@ -72,7 +72,7 @@ def put(self, request, *args, **kwargs):
 def render_editor(request, policy_id, filename, title):
     # Verify that the policy is valid; 404 when not. We don't actually load
     # the document content as they'll be retrieved from the API
-    policy_or_404(policy_id, only_public=False)
+    policy_or_404(policy_id, only_published=False)
     return render(request, filename, {
         'document_url': reverse('document', kwargs={'policy_id': policy_id}),
         'title': title,

api/ereqs_admin/tests/revision_admin_tests.py

@@ -26,7 +26,7 @@ def test_schema_matches(admin_client):
 def test_field_deleted(admin_client):
     policy, version = policy_with_version()
     data = json.loads(version.serialized_data)
-    del data[0]['fields']['public']
+    del data[0]['fields']['workflow_phase']
     version.serialized_data = json.dumps(data)
     version.save()
 

api/reqs/admin.py

@@ -30,15 +30,14 @@ class PolicyAdmin(EReqsVersionAdmin):
     actions = None
     list_display = ('workflow_phase', '__str__', 'issuance')
     list_display_links = ('__str__',)
-    list_filter = ['public', 'workflow_phase']
+    list_filter = ['workflow_phase']
     ordering = ['-issuance', '-pk']
     search_fields = ['title', 'omb_policy_id']
     fields = [
         'title',
         'omb_policy_id',
         'issuance',
         'sunset',
-        'public',
         'workflow_phase',
     ]
 

api/reqs/migrations/0015_remove_policy_public.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.5 on 2018-02-28 15:39
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('reqs', '0014_auto_20180131_2223'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='policy',
+            name='public',
+        ),
+    ]

api/reqs/models.py

@@ -120,10 +120,9 @@ class Meta:
     slug = models.SlugField(max_length=title.max_length)
     issuance = models.DateField()
     sunset = models.DateField(blank=True, null=True)
-    public = models.BooleanField(default=True)
     workflow_phase = models.CharField(
         max_length=32, choices=[(e.name, e.value) for e in WorkflowPhases],
-        default=WorkflowPhases.no_doc.name
+        default=WorkflowPhases.no_doc.name,
     )
 
     # Legacy data fields

api/reqs/tests/views_policies_tests.py

@@ -13,8 +13,10 @@
 
 @pytest.fixture
 def policy_setup():
-    policies = [mommy.make(Policy, policy_number='0'),
-                mommy.make(Policy, policy_number='1')]
+    policies = [mommy.make(Policy, policy_number='0',
+                           workflow_phase='published'),
+                mommy.make(Policy, policy_number='1',
+                           workflow_phase='published')]
     reqs = [mommy.make(Requirement, policy=policies[0], _quantity=3),
             mommy.make(Requirement, policy=policies[1], _quantity=4)]
     yield PolicySetup(policies, reqs)
@@ -147,7 +149,7 @@ def test_agencies_indirect(policy_setup):
 @pytest.mark.django_db
 def test_nonpublic_reqs():
     client = APIClient()
-    policy = mommy.make(Policy)
+    policy = mommy.make(Policy, workflow_phase='published')
     mommy.make(Requirement, policy=policy, public=False)
 
     assert client.get('/policies/').json()['count'] == 0
@@ -167,7 +169,7 @@ def test_omb_policy_id():
     path = "/policies/{0}".format(omb_policy_id)
     response = client.get(path)
     assert response.status_code == 301
-    mommy.make(Policy, omb_policy_id=omb_policy_id)
+    mommy.make(Policy, omb_policy_id=omb_policy_id, workflow_phase='published')
     response = client.get(path + '.json').json()
     assert response['omb_policy_id'] == omb_policy_id
 
@@ -179,7 +181,7 @@ def test_pk_id():
     path = "/policies/{0}".format(pk_id)
     response = client.get(path)
     assert response.status_code == 301
-    mommy.make(Policy, pk=pk_id)
+    mommy.make(Policy, pk=pk_id, workflow_phase='published')
     response = client.get(path + '.json').json()
     assert response['id'] == pk_id
 
@@ -191,14 +193,15 @@ def test_slug():
     path = f"/policies/{slug}.json"
     response = client.get(path)
     assert response.status_code == 404
-    mommy.make(Policy, slug=slug, pk=456)
+    mommy.make(Policy, slug=slug, pk=456, workflow_phase='published')
     response = client.get(path).json()
     assert response['id'] == 456
 
 
 @pytest.mark.django_db
 def test_policy_or_404():
-    policy = mommy.make(Policy, omb_policy_id='AAA-BBB-CCC')
+    policy = mommy.make(Policy, omb_policy_id='AAA-BBB-CCC',
+                        workflow_phase='published')
     assert policies_views.policy_or_404(f"{policy.pk}") == policy
     assert policies_views.policy_or_404("AAA-BBB-CCC") == policy
     with pytest.raises(Http404):

api/reqs/tests/views_requirements_tests.py

@@ -32,6 +32,9 @@ def test_requirement_filtering_topic(path, num_results):
     client = APIClient()
     topics = [mommy.make(Topic, name=str(i)*4) for i in range(4)]
     req1, req2, req3 = mommy.make(Requirement, _quantity=3)
+    for req in (req1, req2, req3):
+        req.policy.workflow_phase = 'published'
+        req.policy.save()
     req1.topics.add(topics[0], topics[1])
     req2.topics.add(topics[1], topics[2])
     req3.topics.add(topics[3])
@@ -46,6 +49,9 @@ def test_requirements_queryset_order():
     topics = mommy.make(Topic, _quantity=6)
     req1, req2, req3 = [mommy.make(Requirement, req_id=str(i + 1))
                         for i in range(3)]
+    for req in (req1, req2, req3):
+        req.policy.workflow_phase = 'published'
+        req.policy.save()
     req1.topics.add(topics[0], topics[1])
     req2.topics.add(topics[1], topics[2], topics[3])
     req3.topics.add(topics[0], topics[4], topics[5])
@@ -73,7 +79,8 @@ def test_requirements_ordered_by_key(params, req_ids, omb_policy_ids, result):
     """
     client = APIClient()
     for req_id, omb_policy_id in zip(req_ids, omb_policy_ids):
-        policy = mommy.make(Policy, omb_policy_id=str(omb_policy_id))
+        policy = mommy.make(Policy, omb_policy_id=str(omb_policy_id),
+                            workflow_phase='published')
         mommy.make(Requirement, req_id=str(req_id), policy=policy)
     path = "/requirements/?{0}".format(params)
     response = client.get(path)
@@ -93,8 +100,10 @@ def test_requirements_ordered_by_multiple_keys(params, result):
     """
     We should be able to pass in arbitrary sort fields.
     """
-    policy1 = mommy.make(Policy, omb_policy_id="23")
-    policy2 = mommy.make(Policy, omb_policy_id="17")
+    policy1 = mommy.make(Policy, omb_policy_id="23",
+                         workflow_phase='published')
+    policy2 = mommy.make(Policy, omb_policy_id="17",
+                         workflow_phase='published')
     mommy.make(Requirement, req_id=1, verb="zoot", policy=policy1)
     mommy.make(Requirement, req_id=2, verb="yo", policy=policy1)
     mommy.make(Requirement, req_id=3, verb="xi", policy=policy2)
@@ -120,7 +129,8 @@ def test_requirements_ordered_by_bad_key(params):
     """
     client = APIClient()
     for i in range(3):
-        policy = mommy.make(Policy, omb_policy_id=str(10 - i))
+        policy = mommy.make(Policy, omb_policy_id=str(10 - i),
+                            workflow_phase='published')
         mommy.make(Requirement, req_id=str(i), policy=policy)
     path = "/requirements/?ordering={0}".format(params)
     response = client.get(path)
@@ -131,9 +141,11 @@ def test_requirements_ordered_by_bad_key(params):
 @pytest.mark.django_db
 def test_requirements_agencies_filter():
     client = APIClient()
-    req1 = mommy.make(Requirement)
+    req1 = mommy.make(Requirement,
+                      policy=mommy.make(Policy, workflow_phase='published'))
     req1.agencies.add(*mommy.make(Agency, _quantity=3))
-    req2 = mommy.make(Requirement)
+    req2 = mommy.make(Requirement,
+                      policy=mommy.make(Policy, workflow_phase='published'))
     req2.agencies.add(*mommy.make(Agency, _quantity=4))
 
     path = "/requirements/"
@@ -147,14 +159,14 @@ def test_requirements_agencies_filter():
 
 
 @pytest.mark.django_db
-@pytest.mark.parametrize('policy_public, req_public, visible', [
-    (False, False, False),
-    (False, True, False),
-    (True, False, False),
-    (True, True, True),
+@pytest.mark.parametrize('workflow, req_public, visible', [
+    ('edit', False, False),
+    ('edit', True, False),
+    ('published', False, False),
+    ('published', True, True),
 ])
-def test_requirements_nonpublic(policy_public, req_public, visible):
-    policy = mommy.make(Policy, public=policy_public)
+def test_requirements_nonpublic(workflow, req_public, visible):
+    policy = mommy.make(Policy, workflow_phase=workflow)
     mommy.make(Requirement, policy=policy, public=req_public)
 
     num_visible = APIClient().get('/requirements/').json()['count']
@@ -166,7 +178,8 @@ def test_requirements_agencies_nonpublic():
     client = APIClient()
     agencies = mommy.make(Agency, _quantity=3)
     agencies.append(mommy.make(Agency, public=False))
-    req = mommy.make(Requirement)
+    req = mommy.make(Requirement,
+                     policy=mommy.make(Policy, workflow_phase='published'))
     req.agencies.add(*agencies)
 
     path = "/requirements/?id={0}".format(req.pk)
@@ -184,9 +197,15 @@ def test_requirements_agencies_nonpublic():
 ])
 def test_requirements_fulltext_search(term, icontains_count, search_count):
     client = APIClient()
-    mommy.make(Requirement, req_text='Full text stems words')
-    mommy.make(Requirement, req_text='Stemmed textual input')
-    mommy.make(Requirement, req_text='Fullerton place')
+    mommy.make(Requirement,
+               policy=mommy.make(Policy, workflow_phase='published'),
+               req_text='Full text stems words')
+    mommy.make(Requirement,
+               policy=mommy.make(Policy, workflow_phase='published'),
+               req_text='Stemmed textual input')
+    mommy.make(Requirement,
+               policy=mommy.make(Policy, workflow_phase='published'),
+               req_text='Fullerton place')
 
     path = "/requirements/?req_text__icontains=" + term
     response = client.get(path).json()
@@ -203,7 +222,8 @@ def applied_agencies():
     g_match, g_nonmatch = mommy.make(AgencyGroup, _quantity=2)
     g_match.agencies.add(a_group_match)
     g_nonmatch.agencies.add(a_group_nonmatch)
-    req = mommy.make(Requirement)
+    req = mommy.make(Requirement,
+                     policy=mommy.make(Policy, workflow_phase='published'))
     req.agencies.add(a_req)
     req.agency_groups.add(g_match)
     yield a_req, a_group_match, a_group_nonmatch

api/reqs/tests/views_tests.py

@@ -8,9 +8,10 @@
 @pytest.mark.django_db
 def test_excludes_nonpublic():
     """The API endpoint should not include policies flagged as nonpublic"""
-    public_policy = mommy.make(Policy, policy_number='0')
-    nonpublic_policy = mommy.make(Policy, policy_number='1', public=False)
-    assert public_policy.public  # The default should be True
+    public_policy = mommy.make(Policy, policy_number='0',
+                               workflow_phase='published')
+    nonpublic_policy = mommy.make(Policy, policy_number='1',
+                                  workflow_phase='edit')
     public_reqs = mommy.make(Requirement, policy=public_policy, _quantity=3)
     nonpublic_reqs = mommy.make(Requirement,
                                 policy=nonpublic_policy, _quantity=4)