diff --git a/api/document/tests/views_test.py b/api/document/tests/views_test.py index 653933792..7ef3eeae1 100644 --- a/api/document/tests/views_test.py +++ b/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,7 +204,7 @@ 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 @@ -209,7 +212,7 @@ def test_editor_requires_admin(client, path_suffix): @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 diff --git a/api/document/views.py b/api/document/views.py index 8c6c5ed22..b0fade08f 100644 --- a/api/document/views.py +++ b/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, diff --git a/api/ereqs_admin/tests/revision_admin_tests.py b/api/ereqs_admin/tests/revision_admin_tests.py index 3c28c9162..cb98b2a13 100644 --- a/api/ereqs_admin/tests/revision_admin_tests.py +++ b/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() diff --git a/api/reqs/admin.py b/api/reqs/admin.py index da3b54559..a9297c233 100644 --- a/api/reqs/admin.py +++ b/api/reqs/admin.py @@ -30,7 +30,7 @@ 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 = [ @@ -38,7 +38,6 @@ class PolicyAdmin(EReqsVersionAdmin): 'omb_policy_id', 'issuance', 'sunset', - 'public', 'workflow_phase', ] diff --git a/api/reqs/migrations/0015_remove_policy_public.py b/api/reqs/migrations/0015_remove_policy_public.py new file mode 100644 index 000000000..a2ba08809 --- /dev/null +++ b/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', + ), + ] diff --git a/api/reqs/models.py b/api/reqs/models.py index 0a5131621..37bf5d006 100644 --- a/api/reqs/models.py +++ b/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 diff --git a/api/reqs/tests/views_policies_tests.py b/api/reqs/tests/views_policies_tests.py index 7c25c37e1..c90792380 100644 --- a/api/reqs/tests/views_policies_tests.py +++ b/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): diff --git a/api/reqs/tests/views_requirements_tests.py b/api/reqs/tests/views_requirements_tests.py index 1f3707560..0280e067c 100644 --- a/api/reqs/tests/views_requirements_tests.py +++ b/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 diff --git a/api/reqs/tests/views_tests.py b/api/reqs/tests/views_tests.py index 2b73a358e..af798c0a6 100644 --- a/api/reqs/tests/views_tests.py +++ b/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) diff --git a/api/reqs/views/policies.py b/api/reqs/views/policies.py index dd5827603..fbeb12c19 100644 --- a/api/reqs/views/policies.py +++ b/api/reqs/views/policies.py @@ -4,7 +4,8 @@ from reqs.filtersets import (AgencyFilter, AgencyGroupFilter, PolicyFilter, RequirementFilter, TopicFilter) -from reqs.models import Agency, AgencyGroup, Policy, Requirement, Topic +from reqs.models import (Agency, AgencyGroup, Policy, Requirement, Topic, + WorkflowPhases) from reqs.serializers import PolicySerializer @@ -62,10 +63,11 @@ def relevant_reqs_count(params): return Subquery(subquery, output_field=IntegerField()) -def policy_or_404(identifier, only_public=True): +def policy_or_404(identifier, only_published=True): queryset = Policy.objects.all() - if only_public: - queryset = queryset.filter(public=True) + if only_published: + queryset = queryset.filter( + workflow_phase=WorkflowPhases.published.name) test = Q(omb_policy_id=identifier) | Q(slug=identifier) if identifier.isdigit(): @@ -88,7 +90,10 @@ def get_queryset(self): queryset = super().get_queryset() \ .annotate(total_reqs=relevant_reqs_count({}), relevant_reqs=relevant_reqs_count(self.request.GET)) \ - .filter(public=True, relevant_reqs__gt=0) + .filter( + workflow_phase=WorkflowPhases.published.name, + relevant_reqs__gt=0, + ) return queryset def get_object(self): diff --git a/api/reqs/views/requirements.py b/api/reqs/views/requirements.py index b5fd74b68..473c6a1cc 100644 --- a/api/reqs/views/requirements.py +++ b/api/reqs/views/requirements.py @@ -5,7 +5,7 @@ from reqs.filtersets import (AgencyFilter, AgencyGroupFilter, PolicyFilter, RequirementFilter, TopicFilter) -from reqs.models import Agency, Requirement +from reqs.models import Agency, Requirement, WorkflowPhases from reqs.serializers import RequirementSerializer @@ -70,5 +70,6 @@ class RequirementViewSet(viewsets.ModelViewSet): def get_queryset(self): queryset = super().get_queryset() - queryset = queryset.filter(public=True, policy__public=True) + queryset = queryset.filter( + public=True, policy__workflow_phase=WorkflowPhases.published.name) return queryset