Skip to content

Commit

Permalink
Replace "public" logic with "published" workflow.
Browse files Browse the repository at this point in the history
We'll be removing the "public" field in favor of a single, linear workflow.
This first requires updating all our existing views to check the workflow
field rather than "public".
  • Loading branch information
CM Lubinski committed Feb 28, 2018
1 parent b137538 commit 15c9798
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 50 deletions.
25 changes: 14 additions & 11 deletions api/document/tests/views_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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')
Expand All @@ -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()

Expand All @@ -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()

Expand All @@ -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
Expand Down Expand Up @@ -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}')
Expand All @@ -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
6 changes: 3 additions & 3 deletions api/document/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion api/ereqs_admin/tests/revision_admin_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
17 changes: 10 additions & 7 deletions api/reqs/tests/views_policies_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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

Expand All @@ -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):
Expand Down
56 changes: 38 additions & 18 deletions api/reqs/tests/views_requirements_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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])
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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/"
Expand All @@ -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']
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions api/reqs/tests/views_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 10 additions & 5 deletions api/reqs/views/policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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():
Expand All @@ -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):
Expand Down
5 changes: 3 additions & 2 deletions api/reqs/views/requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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

0 comments on commit 15c9798

Please sign in to comment.