Merge pull request #139 from dod-ccpo/request-status-names

Update request status names to reflect document

alembic/versions/77b065750596_new_request_statuses.py

@@ -0,0 +1,49 @@
+"""new request statuses
+
+Revision ID: 77b065750596
+Revises: 1f57f784ed5b
+Create Date: 2018-08-07 16:42:11.502361
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.orm.session import sessionmaker
+
+from atst.models.request_status_event import RequestStatus
+
+
+# revision identifiers, used by Alembic.
+revision = '77b065750596'
+down_revision = '1f57f784ed5b'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    """
+    Update all existing request statuses so that the state of the
+    table reflects the statuses listed in RequestStatus.
+
+    This involves fixing the casing on existing statuses, and
+    deleting statuses that have no match.
+    """
+
+    db = op.get_bind()
+
+    status_events = db.execute("SELECT * FROM request_status_events").fetchall()
+    for status_event in status_events:
+        try:
+            status = RequestStatus[status_event["new_status"].upper()]
+            query = sa.text("""
+                UPDATE request_status_events
+                SET new_status = :status
+                WHERE id = :id"""
+            )
+            db.execute(query, id=status_event["id"], status=status.name)
+        except ValueError:
+            query = sa.text("DELETE FROM request_status_events WHERE id = :id")
+            db.execute(query, id=status_event["id"])
+
+
+def downgrade():
+    pass

atst/domain/requests.py

@@ -2,7 +2,8 @@ from sqlalchemy import exists, and_
 from sqlalchemy.orm.exc import NoResultFound
 from sqlalchemy.orm.attributes import flag_modified
 
-from atst.models import Request, RequestStatusEvent
+from atst.models.request import Request
+from atst.models.request_status_event import RequestStatusEvent, RequestStatus
 from atst.database import db
 
 from .exceptions import NotFoundError
@@ -32,9 +33,7 @@ class Requests(object):
     @classmethod
     def create(cls, creator_id, body):
         request = Request(creator=creator_id, body=body)
-
-        status_event = RequestStatusEvent(new_status="incomplete")
-        request.status_events.append(status_event)
+        request = Requests.set_status(request, RequestStatus.STARTED)
 
         db.session.add(request)
         db.session.commit()
@@ -74,10 +73,13 @@ class Requests(object):
 
     @classmethod
     def submit(cls, request):
-        request.status_events.append(RequestStatusEvent(new_status="submitted"))
-
+        new_status = None
         if Requests.should_auto_approve(request):
-            request.status_events.append(RequestStatusEvent(new_status="approved"))
+            new_status = RequestStatus.PENDING_FINANCIAL_VERIFICATION
+        else:
+            new_status = RequestStatus.PENDING_CCPO_APPROVAL
+
+        request = Requests.set_status(request, new_status)
 
         db.session.add(request)
         db.session.commit()
@@ -100,11 +102,6 @@ class Requests(object):
 
         request.body = deep_merge(request_delta, request.body)
 
-        if Requests.should_allow_submission(request):
-            request.status_events.append(
-                RequestStatusEvent(new_status="pending_submission")
-            )
-
         # Without this, sqlalchemy won't notice the change to request.body,
         # since it doesn't track dictionary mutations by default.
         flag_modified(request, "body")
@@ -112,6 +109,20 @@ class Requests(object):
         db.session.add(request)
         db.session.commit()
 
+    @classmethod
+    def set_status(cls, request: Request, status: RequestStatus):
+        status_event = RequestStatusEvent(new_status=status)
+        request.status_events.append(status_event)
+        return request
+
+    @classmethod
+    def action_required_by(cls, request):
+        return {
+            RequestStatus.STARTED: "mission_owner",
+            RequestStatus.PENDING_FINANCIAL_VERIFICATION: "mission_owner",
+            RequestStatus.PENDING_CCPO_APPROVAL: "ccpo"
+        }.get(request.status)
+
     @classmethod
     def should_auto_approve(cls, request):
         try:

atst/models/request.py

@@ -21,12 +21,3 @@ class Request(Base):
     @property
     def status(self):
         return self.status_events[-1].new_status
-
-    @property
-    def action_required_by(self):
-        return {
-            "incomplete": "mission_owner",
-            "pending_submission": "mission_owner",
-            "submitted": "ccpo",
-            "approved": "mission_owner",
-        }.get(self.status)

atst/models/request_status_event.py

@@ -1,5 +1,6 @@
-from sqlalchemy import Column, func, ForeignKey
-from sqlalchemy.types import DateTime, String, BigInteger
+from enum import Enum
+from sqlalchemy import Column, func, ForeignKey, Enum as SQLAEnum
+from sqlalchemy.types import DateTime, BigInteger
 from sqlalchemy.schema import Sequence
 from sqlalchemy.dialects.postgresql import UUID
 
@@ -7,11 +8,20 @@ from atst.models import Base
 from atst.models.types import Id
 
 
+class RequestStatus(Enum):
+    STARTED = "started"
+    PENDING_FINANCIAL_VERIFICATION = "pending_financial_verification"
+    PENDING_CCPO_APPROVAL = "pending_ccpo_approval"
+    APPROVED = "approved"
+    EXPIRED = "expired"
+    DELETED = "deleted"
+
+
 class RequestStatusEvent(Base):
     __tablename__ = "request_status_events"
 
     id = Id()
-    new_status = Column(String())
+    new_status = Column(SQLAEnum(RequestStatus))
     time_created = Column(DateTime(timezone=True), server_default=func.now())
     request_id = Column(
         UUID(as_uuid=True), ForeignKey("requests.id", ondelete="CASCADE")

tests/domain/test_requests.py

@@ -3,6 +3,7 @@ from uuid import uuid4
 
 from atst.domain.exceptions import NotFoundError
 from atst.domain.requests import Requests
+from atst.models.request_status_event import RequestStatus
 
 from tests.factories import RequestFactory
 
@@ -23,22 +24,27 @@ def test_nonexistent_request_raises():
         Requests.get(uuid4())
 
 
+def test_new_request_has_started_status():
+    request = Requests.create(uuid4(), {})
+    assert request.status == RequestStatus.STARTED
+
+
 def test_auto_approve_less_than_1m(new_request):
     new_request.body = {"details_of_use": {"dollar_value": 999999}}
     request = Requests.submit(new_request)
 
-    assert request.status == 'approved'
+    assert request.status == RequestStatus.PENDING_FINANCIAL_VERIFICATION
 
 
 def test_dont_auto_approve_if_dollar_value_is_1m_or_above(new_request):
     new_request.body = {"details_of_use": {"dollar_value": 1000000}}
     request = Requests.submit(new_request)
 
-    assert request.status == 'submitted'
+    assert request.status == RequestStatus.PENDING_CCPO_APPROVAL
 
 
 def test_dont_auto_approve_if_no_dollar_value_specified(new_request):
     new_request.body = {"details_of_use": {}}
     request = Requests.submit(new_request)
 
-    assert request.status == 'submitted'
+    assert request.status == RequestStatus.PENDING_CCPO_APPROVAL

tests/factories.py

@@ -1,35 +1,40 @@
 import factory
 from uuid import uuid4
 
-from atst.models import Request
+from atst.models.request import Request
+from atst.models.request_status_event import RequestStatusEvent, RequestStatus
 from atst.models.pe_number import PENumber
 from atst.models.task_order import TaskOrder
 from atst.models.user import User
 from atst.models.role import Role
 
 
-class RequestFactory(factory.alchemy.SQLAlchemyModelFactory):
+class RequestStatusFactory(factory.alchemy.SQLAlchemyModelFactory):
+    class Meta:
+        model = RequestStatusEvent
 
+
+class RequestFactory(factory.alchemy.SQLAlchemyModelFactory):
     class Meta:
         model = Request
 
     id = factory.Sequence(lambda x: uuid4())
+    status_events = factory.RelatedFactory(
+        RequestStatusFactory, "request", new_status=RequestStatus.STARTED
+    )
 
 
 class PENumberFactory(factory.alchemy.SQLAlchemyModelFactory):
-
     class Meta:
         model = PENumber
 
 
 class TaskOrderFactory(factory.alchemy.SQLAlchemyModelFactory):
-
     class Meta:
         model = TaskOrder
 
 
 class RoleFactory(factory.alchemy.SQLAlchemyModelFactory):
-
     class Meta:
         model = Role
 
@@ -37,7 +42,6 @@ class RoleFactory(factory.alchemy.SQLAlchemyModelFactory):
 
 
 class UserFactory(factory.alchemy.SQLAlchemyModelFactory):
-
     class Meta:
         model = User
 
@@ -46,4 +50,3 @@ class UserFactory(factory.alchemy.SQLAlchemyModelFactory):
     first_name = "Fake"
     last_name = "User"
     atat_role = factory.SubFactory(RoleFactory)
-

tests/models/test_requests.py

@@ -0,0 +1,21 @@
+from tests.factories import RequestFactory
+from atst.domain.requests import Requests, RequestStatus
+
+
+def test_started_request_requires_mo_action():
+    request = RequestFactory.create()
+    assert Requests.action_required_by(request) == "mission_owner"
+
+
+def test_pending_financial_requires_mo_action():
+    request = RequestFactory.create()
+    request = Requests.set_status(request, RequestStatus.PENDING_FINANCIAL_VERIFICATION)
+
+    assert Requests.action_required_by(request) == "mission_owner"
+
+
+def test_pending_ccpo_approval_requires_ccpo():
+    request = RequestFactory.create()
+    request = Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL)
+
+    assert Requests.action_required_by(request) == "ccpo"