diff --git a/alembic/versions/77b065750596_new_request_statuses.py b/alembic/versions/77b065750596_new_request_statuses.py new file mode 100644 index 00000000..28cce335 --- /dev/null +++ b/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 diff --git a/atst/domain/requests.py b/atst/domain/requests.py index 5c04d3c7..e7b64e5d 100644 --- a/atst/domain/requests.py +++ b/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: diff --git a/atst/models/request.py b/atst/models/request.py index e8690332..66e5bf1e 100644 --- a/atst/models/request.py +++ b/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) diff --git a/atst/models/request_status_event.py b/atst/models/request_status_event.py index 4f65a2ec..2e492ac4 100644 --- a/atst/models/request_status_event.py +++ b/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") diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 0597714a..1d696097 100644 --- a/tests/domain/test_requests.py +++ b/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 diff --git a/tests/factories.py b/tests/factories.py index 5cefc1eb..dc68eb5c 100644 --- a/tests/factories.py +++ b/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) - diff --git a/tests/models/__init__.py b/tests/models/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/models/test_requests.py b/tests/models/test_requests.py new file mode 100644 index 00000000..a1990a35 --- /dev/null +++ b/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"