From e99ddd491aa3519904bf3c0ccc4a69bf7e19e738 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 7 Aug 2018 21:03:27 -0400 Subject: [PATCH 01/13] Create Request.creator relationship - Rename creator_id to user_id --- .../05d6272bdb43_rename_request_creator_.py | 43 ++++++++++++++++ atst/domain/requests.py | 6 +-- atst/models/request.py | 8 +-- atst/routes/requests/index.py | 2 +- atst/routes/requests/jedi_request_flow.py | 2 +- tests/domain/test_requests.py | 4 +- tests/factories.py | 51 ++++++++++--------- tests/models/test_requests.py | 9 +++- 8 files changed, 89 insertions(+), 36 deletions(-) create mode 100644 alembic/versions/05d6272bdb43_rename_request_creator_.py diff --git a/alembic/versions/05d6272bdb43_rename_request_creator_.py b/alembic/versions/05d6272bdb43_rename_request_creator_.py new file mode 100644 index 00000000..c8a3966e --- /dev/null +++ b/alembic/versions/05d6272bdb43_rename_request_creator_.py @@ -0,0 +1,43 @@ +"""rename request creator + +Revision ID: 05d6272bdb43 +Revises: 77b065750596 +Create Date: 2018-08-07 20:21:22.559283 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '05d6272bdb43' +down_revision = '77b065750596' +branch_labels = None +depends_on = None + + +def upgrade(): + db = op.get_bind() + + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('requests', sa.Column('user_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.create_foreign_key('requests_user_id_fk', 'requests', 'users', ['user_id'], ['id']) + # ### end Alembic commands ### + + db.execute("UPDATE requests SET user_id = creator") + + op.alter_column('requests', 'user_id', nullable=False) + op.drop_column('requests', 'creator') + + + +def downgrade(): + db = op.get_bind() + + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('requests', sa.Column('creator', postgresql.UUID(), autoincrement=False, nullable=True)) + op.drop_constraint('requests_user_id_fk', 'requests', type_='foreignkey') + # ### end Alembic commands ### + + db.execute("UPDATE requests SET creator = user_id") + op.drop_column('requests', 'user_id') diff --git a/atst/domain/requests.py b/atst/domain/requests.py index e7b64e5d..f9127aa9 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -58,10 +58,10 @@ class Requests(object): return request @classmethod - def get_many(cls, creator_id=None): + def get_many(cls, creator=None): filters = [] - if creator_id: - filters.append(Request.creator == creator_id) + if creator: + filters.append(Request.creator == creator) requests = ( db.session.query(Request) diff --git a/atst/models/request.py b/atst/models/request.py index 66e5bf1e..4e7a2832 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -1,6 +1,6 @@ -from sqlalchemy import Column, func +from sqlalchemy import Column, func, ForeignKey from sqlalchemy.types import DateTime -from sqlalchemy.dialects.postgresql import JSONB, UUID +from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import relationship from atst.models import Base @@ -11,13 +11,15 @@ class Request(Base): __tablename__ = "requests" id = Id() - creator = Column(UUID(as_uuid=True)) time_created = Column(DateTime(timezone=True), server_default=func.now()) body = Column(JSONB) status_events = relationship( "RequestStatusEvent", backref="request", order_by="RequestStatusEvent.sequence" ) + user_id = Column(ForeignKey("users.id"), nullable=False) + creator = relationship("User") + @property def status(self): return self.status_events[-1].new_status diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index 7e098b47..15ab8fee 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -28,7 +28,7 @@ def requests_index(): ): requests = Requests.get_many() else: - requests = Requests.get_many(creator_id=g.current_user.id) + requests = Requests.get_many(creator=g.current_user) mapped_requests = [map_request(g.current_user, r) for r in requests] diff --git a/atst/routes/requests/jedi_request_flow.py b/atst/routes/requests/jedi_request_flow.py index e750a9e0..af08251b 100644 --- a/atst/routes/requests/jedi_request_flow.py +++ b/atst/routes/requests/jedi_request_flow.py @@ -124,5 +124,5 @@ class JEDIRequestFlow(object): if self.request_id: Requests.update(self.request_id, request_data) else: - request = Requests.create(self.current_user.id, request_data) + request = Requests.create(self.current_user, request_data) self.request_id = request.id diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 1d696097..68835138 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -5,7 +5,7 @@ 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 +from tests.factories import RequestFactory, UserFactory @pytest.fixture(scope="function") @@ -25,7 +25,7 @@ def test_nonexistent_request_raises(): def test_new_request_has_started_status(): - request = Requests.create(uuid4(), {}) + request = Requests.create(UserFactory.build(), {}) assert request.status == RequestStatus.STARTED diff --git a/tests/factories.py b/tests/factories.py index dc68eb5c..af5f6971 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -9,31 +9,6 @@ from atst.models.user import User from atst.models.role import Role -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 @@ -50,3 +25,29 @@ class UserFactory(factory.alchemy.SQLAlchemyModelFactory): first_name = "Fake" last_name = "User" atat_role = factory.SubFactory(RoleFactory) + + +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 + ) + creator = factory.SubFactory(UserFactory) + + +class PENumberFactory(factory.alchemy.SQLAlchemyModelFactory): + class Meta: + model = PENumber + + +class TaskOrderFactory(factory.alchemy.SQLAlchemyModelFactory): + class Meta: + model = TaskOrder diff --git a/tests/models/test_requests.py b/tests/models/test_requests.py index a1990a35..90038d4e 100644 --- a/tests/models/test_requests.py +++ b/tests/models/test_requests.py @@ -1,4 +1,4 @@ -from tests.factories import RequestFactory +from tests.factories import RequestFactory, UserFactory from atst.domain.requests import Requests, RequestStatus @@ -19,3 +19,10 @@ def test_pending_ccpo_approval_requires_ccpo(): request = Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) assert Requests.action_required_by(request) == "ccpo" + + +def test_request_has_creator(): + user = UserFactory.create() + request = RequestFactory.create(creator=user) + + assert request.creator == user From dcb45c64e8b434a8231bd06deac44edc9b440943 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 7 Aug 2018 21:26:05 -0400 Subject: [PATCH 02/13] Alter old migrations so that they can downgrade The Role -> User foreign key constraint was preventing roles from being deleted once there were existing users referencing the role. I realized it was best to just pass on the downgrade and allow the tables to be deleted. --- .../versions/4ea5917e7781_add_default_atat_role.py | 4 +--- .../96a9f3537996_add_roles_and_permissions.py | 13 +------------ 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/alembic/versions/4ea5917e7781_add_default_atat_role.py b/alembic/versions/4ea5917e7781_add_default_atat_role.py index 78b6ef55..21b03166 100644 --- a/alembic/versions/4ea5917e7781_add_default_atat_role.py +++ b/alembic/versions/4ea5917e7781_add_default_atat_role.py @@ -34,6 +34,4 @@ def upgrade(): def downgrade(): - db = op.get_bind() - db.execute("DELETE FROM roles WHERE name = 'default'") - + pass diff --git a/alembic/versions/96a9f3537996_add_roles_and_permissions.py b/alembic/versions/96a9f3537996_add_roles_and_permissions.py index 4380208a..0729127e 100644 --- a/alembic/versions/96a9f3537996_add_roles_and_permissions.py +++ b/alembic/versions/96a9f3537996_add_roles_and_permissions.py @@ -169,15 +169,4 @@ def upgrade(): def downgrade(): - db = op.get_bind() - db.execute(""" - DELETE FROM roles - WHERE name IN ( - 'ccpo', - 'owner', - 'admin', - 'developer', - 'billing_auditor', - 'security_auditor' - ); - """) + pass From f80668c6384299a50213e2dfd85e09d5a961afa0 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 7 Aug 2018 21:32:02 -0400 Subject: [PATCH 03/13] Add script/seed.py for convenience --- atst/routes/dev.py | 20 ++++++++++++-------- script/seed.py | 25 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 script/seed.py diff --git a/atst/routes/dev.py b/atst/routes/dev.py index cbd02cea..181ddc3d 100644 --- a/atst/routes/dev.py +++ b/atst/routes/dev.py @@ -9,37 +9,37 @@ _DEV_USERS = { "dod_id": "1234567890", "first_name": "Sam", "last_name": "Seeceepio", - "atat_role": "ccpo", + "atat_role_name": "ccpo", }, "amanda": { "dod_id": "2345678901", "first_name": "Amanda", "last_name": "Adamson", - "atat_role": "default", + "atat_role_name": "default", }, "brandon": { "dod_id": "3456789012", "first_name": "Brandon", "last_name": "Buchannan", - "atat_role": "default", + "atat_role_name": "default", }, "christina": { "dod_id": "4567890123", "first_name": "Christina", "last_name": "Collins", - "atat_role": "default", + "atat_role_name": "default", }, "dominick": { "dod_id": "5678901234", "first_name": "Dominick", "last_name": "Domingo", - "atat_role": "default", + "atat_role_name": "default", }, "erica": { "dod_id": "6789012345", "first_name": "Erica", "last_name": "Eichner", - "atat_role": "default", + "atat_role_name": "default", }, } @@ -48,8 +48,12 @@ _DEV_USERS = { def login_dev(): role = request.args.get("username", "amanda") user_data = _DEV_USERS[role] - basic_data = {k:v for k,v in user_data.items() if k not in ["dod_id", "atat_role"]} - user = _set_user_permissions(user_data["dod_id"], user_data["atat_role"], basic_data) + basic_data = { + k: v for k, v in user_data.items() if k not in ["dod_id", "atat_role"] + } + user = _set_user_permissions( + user_data["dod_id"], user_data["atat_role_name"], basic_data + ) session["user_id"] = user.id return redirect(url_for("atst.home")) diff --git a/script/seed.py b/script/seed.py new file mode 100644 index 00000000..81be936f --- /dev/null +++ b/script/seed.py @@ -0,0 +1,25 @@ +# Add root project dir to the python path +import os +import sys + +parent_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) +sys.path.append(parent_dir) + +from atst.app import make_config, make_app +from atst.domain.users import Users +from atst.domain.requests import Requests +from atst.routes.dev import _DEV_USERS as DEV_USERS + + +def seed_db(): + users = [Users.create(**dev_user) for (_, dev_user) in DEV_USERS.items()] + + for user in users: + [Requests.create(user, {}) for _ in range(5)] + + +if __name__ == "__main__": + config = make_config() + app = make_app(config) + with app.app_context(): + seed_db() From 04b9ae9f5364e4c7c02aa5159c739fa87b1511f2 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 7 Aug 2018 21:32:47 -0400 Subject: [PATCH 04/13] Display creator's name and human-readable status --- atst/routes/requests/index.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index 15ab8fee..5881c1e6 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -5,17 +5,17 @@ from . import requests_bp from atst.domain.requests import Requests -def map_request(user, request): +def map_request(request): time_created = pendulum.instance(request.time_created) is_new = time_created.add(days=1) > pendulum.now() return { "order_id": request.id, "is_new": is_new, - "status": request.status, + "status": request.status.name.capitalize(), "app_count": 1, "date": time_created.format("M/DD/YYYY"), - "full_name": user.full_name + "full_name": request.creator.full_name } @@ -30,6 +30,6 @@ def requests_index(): else: requests = Requests.get_many(creator=g.current_user) - mapped_requests = [map_request(g.current_user, r) for r in requests] + mapped_requests = [map_request(r) for r in requests] return render_template("requests.html", requests=mapped_requests) From fdb7c699eebc201bc41b45a23184f6d69d893dac Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 7 Aug 2018 21:41:50 -0400 Subject: [PATCH 05/13] Simplify dev user login --- atst/routes/dev.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/atst/routes/dev.py b/atst/routes/dev.py index 181ddc3d..947cc55f 100644 --- a/atst/routes/dev.py +++ b/atst/routes/dev.py @@ -48,15 +48,11 @@ _DEV_USERS = { def login_dev(): role = request.args.get("username", "amanda") user_data = _DEV_USERS[role] - basic_data = { - k: v for k, v in user_data.items() if k not in ["dod_id", "atat_role"] - } - user = _set_user_permissions( - user_data["dod_id"], user_data["atat_role_name"], basic_data + user = Users.get_or_create_by_dod_id( + user_data["dod_id"], + atat_role_name=user_data["atat_role_name"], + first_name=user_data["first_name"], + last_name=user_data["last_name"], ) session["user_id"] = user.id return redirect(url_for("atst.home")) - - -def _set_user_permissions(dod_id, role, user_data): - return Users.get_or_create_by_dod_id(dod_id, atat_role_name=role, **user_data) From 6dccf50e832a14336ac5fd72f982e89ace3c2020 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 7 Aug 2018 21:51:30 -0400 Subject: [PATCH 06/13] Title case request status name --- atst/routes/requests/index.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index 5881c1e6..1f26e6de 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -6,26 +6,25 @@ from atst.domain.requests import Requests def map_request(request): + + status_display_name = request.status.name.replace("_", " ").title() time_created = pendulum.instance(request.time_created) is_new = time_created.add(days=1) > pendulum.now() return { "order_id": request.id, "is_new": is_new, - "status": request.status.name.capitalize(), + "status": status_display_name, "app_count": 1, "date": time_created.format("M/DD/YYYY"), - "full_name": request.creator.full_name + "full_name": request.creator.full_name, } @requests_bp.route("/requests", methods=["GET"]) def requests_index(): requests = [] - if ( - "review_and_approve_jedi_workspace_request" - in g.current_user.atat_permissions - ): + if "review_and_approve_jedi_workspace_request" in g.current_user.atat_permissions: requests = Requests.get_many() else: requests = Requests.get_many(creator=g.current_user) From 47a4635eddd38c91d72f50db480a02d361ff76fa Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 7 Aug 2018 21:58:51 -0400 Subject: [PATCH 07/13] Display Request's "Total Apps" --- atst/routes/requests/index.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index 1f26e6de..0a6e1524 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -10,12 +10,13 @@ def map_request(request): status_display_name = request.status.name.replace("_", " ").title() time_created = pendulum.instance(request.time_created) is_new = time_created.add(days=1) > pendulum.now() + app_count = request.body.get("details_of_use", {}).get("num_software_systems", 0) return { "order_id": request.id, "is_new": is_new, "status": status_display_name, - "app_count": 1, + "app_count": app_count, "date": time_created.format("M/DD/YYYY"), "full_name": request.creator.full_name, } From 0c378ba07c45192124d449c1616ad2482bc8e1d7 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 8 Aug 2018 11:13:22 -0400 Subject: [PATCH 08/13] Declare helper properties on Request and RequestStatusEvent --- atst/models/request.py | 4 ++++ atst/models/request_status_event.py | 4 ++++ atst/routes/requests/index.py | 4 +--- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/atst/models/request.py b/atst/models/request.py index 4e7a2832..bd14b5db 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -23,3 +23,7 @@ class Request(Base): @property def status(self): return self.status_events[-1].new_status + + @property + def status_displayname(self): + return self.status_events[-1].displayname diff --git a/atst/models/request_status_event.py b/atst/models/request_status_event.py index 2e492ac4..26d042fa 100644 --- a/atst/models/request_status_event.py +++ b/atst/models/request_status_event.py @@ -29,3 +29,7 @@ class RequestStatusEvent(Base): sequence = Column( BigInteger, Sequence("request_status_events_sequence_seq"), nullable=False ) + + @property + def displayname(self): + return self.new_status.name.replace("_", " ").title() diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index 0a6e1524..ec9b566c 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -6,8 +6,6 @@ from atst.domain.requests import Requests def map_request(request): - - status_display_name = request.status.name.replace("_", " ").title() time_created = pendulum.instance(request.time_created) is_new = time_created.add(days=1) > pendulum.now() app_count = request.body.get("details_of_use", {}).get("num_software_systems", 0) @@ -15,7 +13,7 @@ def map_request(request): return { "order_id": request.id, "is_new": is_new, - "status": status_display_name, + "status": request.status_displayname, "app_count": app_count, "date": time_created.format("M/DD/YYYY"), "full_name": request.creator.full_name, From cb45db291e4fc95faf5b3a25ee97e9622788c960 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 8 Aug 2018 13:42:10 -0400 Subject: [PATCH 09/13] Rollback transaction if user creation fails --- atst/domain/users.py | 1 + 1 file changed, 1 insertion(+) diff --git a/atst/domain/users.py b/atst/domain/users.py index 54cf4ae0..bc3d972f 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -37,6 +37,7 @@ class Users(object): db.session.add(user) db.session.commit() except IntegrityError: + db.session.rollback() raise AlreadyExistsError("user") return user From 5f59b9f24a5cf23ebb7b34a6dc2cdf1aef5efa66 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 8 Aug 2018 13:42:28 -0400 Subject: [PATCH 10/13] Add emails to test users --- atst/routes/dev.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/atst/routes/dev.py b/atst/routes/dev.py index 947cc55f..f66e3f08 100644 --- a/atst/routes/dev.py +++ b/atst/routes/dev.py @@ -10,40 +10,45 @@ _DEV_USERS = { "first_name": "Sam", "last_name": "Seeceepio", "atat_role_name": "ccpo", + "email": "sam@test.com" }, "amanda": { "dod_id": "2345678901", "first_name": "Amanda", "last_name": "Adamson", "atat_role_name": "default", + "email": "amanda@test.com" }, "brandon": { "dod_id": "3456789012", "first_name": "Brandon", "last_name": "Buchannan", "atat_role_name": "default", + "email": "brandon@test.com" }, "christina": { "dod_id": "4567890123", "first_name": "Christina", "last_name": "Collins", "atat_role_name": "default", + "email": "christina@test.com" }, "dominick": { "dod_id": "5678901234", "first_name": "Dominick", "last_name": "Domingo", "atat_role_name": "default", + "email": "dominick@test.com" }, "erica": { "dod_id": "6789012345", "first_name": "Erica", "last_name": "Eichner", "atat_role_name": "default", + "email": "erica@test.com" }, } - @bp.route("/login-dev") def login_dev(): role = request.args.get("username", "amanda") @@ -53,6 +58,7 @@ def login_dev(): atat_role_name=user_data["atat_role_name"], first_name=user_data["first_name"], last_name=user_data["last_name"], + email=user_data["email"] ) session["user_id"] = user.id return redirect(url_for("atst.home")) From df93db4cd838bc0f4f4d5f51397545107184400b Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 8 Aug 2018 13:42:48 -0400 Subject: [PATCH 11/13] Create more varied requests in seed script --- Pipfile.lock | 12 ++++++------ script/seed.py | 16 ++++++++++++++-- tests/factories.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index b9b8cab6..7f8a00cb 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "2b149e0d8c23814a2c701b53f5c75b36714a2ccd4e2a2769924ef6e2a3f09e97" + "sha256": "5fc8273838354406366b401529a6f512a73ac6a8ecea6699afa4ab7b4996bf13" }, "pipfile-spec": 6, "requires": { @@ -271,6 +271,7 @@ "sha256:1d936da41ee06216d89fdc7ead1ee9a5da2811a8787515a976b646e110c3f622", "sha256:e4ef42e82b0b493c5849eed98b5ab49d6767caf982127e9a33167f1153b36cc5" ], + "markers": "python_version != '3.0.*' and python_version != '3.1.*' and python_version != '3.2.*' and python_version >= '2.7' and python_version != '3.3.*'", "version": "==2018.5" }, "redis": { @@ -501,6 +502,7 @@ "sha256:b9c40e9750f3d77e6e4d441d8b0266cf555e7cdabdcff33c4fd06366ca761ef8", "sha256:ec9ef8f4a9bc6f71eec99e1806bfa2de401650d996c59330782b89a5555c1497" ], + "markers": "python_version != '3.3.*' and python_version >= '2.7' and python_version != '3.1.*' and python_version != '3.0.*' and python_version != '3.2.*'", "version": "==4.3.4" }, "itsdangerous": { @@ -618,6 +620,7 @@ "sha256:6e3836e39f4d36ae72840833db137f7b7d35105079aee6ec4a62d9f80d594dd1", "sha256:95eb8364a4708392bae89035f45341871286a333f749c3141c20573d2b3876e1" ], + "markers": "python_version != '3.3.*' and python_version >= '2.7' and python_version != '3.1.*' and python_version != '3.0.*' and python_version != '3.2.*'", "version": "==0.7.1" }, "prompt-toolkit": { @@ -640,6 +643,7 @@ "sha256:3fd59af7435864e1a243790d322d763925431213b6b8529c6ca71081ace3bbf7", "sha256:e31fb2767eb657cbde86c454f02e99cb846d3cd9d61b318525140214fdc0e98e" ], + "markers": "python_version != '3.3.*' and python_version >= '2.7' and python_version != '3.1.*' and python_version != '3.0.*' and python_version != '3.2.*'", "version": "==1.5.4" }, "pygments": { @@ -689,15 +693,11 @@ }, "pyyaml": { "hashes": [ - "sha256:1cbc199009e78f92d9edf554be4fe40fb7b0bef71ba688602a00e97a51909110", "sha256:254bf6fda2b7c651837acb2c718e213df29d531eebf00edb54743d10bcb694eb", "sha256:3108529b78577327d15eec243f0ff348a0640b0c3478d67ad7f5648f93bac3e2", "sha256:3c17fb92c8ba2f525e4b5f7941d850e7a48c3a59b32d331e2502a3cdc6648e76", - "sha256:6f89b5c95e93945b597776163403d47af72d243f366bf4622ff08bdfd1c950b7", "sha256:8d6d96001aa7f0a6a4a95e8143225b5d06e41b1131044913fecb8f85a125714b", - "sha256:be622cc81696e24d0836ba71f6272a2b5767669b0d79fdcf0295d51ac2e156c8", - "sha256:c8a88edd93ee29ede719080b2be6cb2333dfee1dccba213b422a9c8e97f2967b", - "sha256:f39411e380e2182ad33be039e8ee5770a5d9efe01a2bfb7ae58d9ba31c4a2a9d" + "sha256:c8a88edd93ee29ede719080b2be6cb2333dfee1dccba213b422a9c8e97f2967b" ], "version": "==4.2b4" }, diff --git a/script/seed.py b/script/seed.py index 81be936f..d865b9d7 100644 --- a/script/seed.py +++ b/script/seed.py @@ -8,14 +8,26 @@ sys.path.append(parent_dir) from atst.app import make_config, make_app from atst.domain.users import Users from atst.domain.requests import Requests +from atst.domain.exceptions import AlreadyExistsError +from tests.factories import RequestFactory from atst.routes.dev import _DEV_USERS as DEV_USERS def seed_db(): - users = [Users.create(**dev_user) for (_, dev_user) in DEV_USERS.items()] + users = [] + for dev_user in DEV_USERS.values(): + try: + user = Users.create(**dev_user) + users.append(user) + except AlreadyExistsError: + pass for user in users: - [Requests.create(user, {}) for _ in range(5)] + for dollar_value in [1, 200, 3000, 40000, 500000, 1000000]: + request = Requests.create( + user, RequestFactory.build_request_body(user, dollar_value) + ) + Requests.submit(request) if __name__ == "__main__": diff --git a/tests/factories.py b/tests/factories.py index af5f6971..71dc1422 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -9,6 +9,7 @@ from atst.models.user import User from atst.models.role import Role + class RoleFactory(factory.alchemy.SQLAlchemyModelFactory): class Meta: model = Role @@ -41,6 +42,47 @@ class RequestFactory(factory.alchemy.SQLAlchemyModelFactory): RequestStatusFactory, "request", new_status=RequestStatus.STARTED ) creator = factory.SubFactory(UserFactory) + body = factory.LazyAttribute(lambda r: RequestFactory.build_request_body(r.creator)) + + @classmethod + def build_request_body(cls, user, dollar_value=1000000): + return { + "primary_poc": { + "dodid_poc": user.dod_id, + "email_poc": user.email, + "fname_poc": user.first_name, + "lname_poc": user.last_name + }, + "details_of_use": { + "jedi_usage": "adf", + "start_date": "2018-08-08", + "cloud_native": "yes", + "dollar_value": dollar_value, + "dod_component": "us_navy", + "data_transfers": "less_than_100gb", + "jedi_migration": "yes", + "num_software_systems": 1, + "number_user_sessions": 2, + "average_daily_traffic": 1, + "engineering_assessment": "yes", + "technical_support_team": "yes", + "estimated_monthly_spend": 100, + "expected_completion_date": "less_than_1_month", + "rationalization_software_systems": "yes", + "organization_providing_assistance": "in_house_staff" + }, + "information_about_you": { + "citizenship": "United States", + "designation": "military", + "phone_number": "1234567890", + "email_request": user.email, + "fname_request": user.first_name, + "lname_request": user.last_name, + "service_branch": "ads", + "date_latest_training": "2018-08-06" + } + } + class PENumberFactory(factory.alchemy.SQLAlchemyModelFactory): @@ -51,3 +93,4 @@ class PENumberFactory(factory.alchemy.SQLAlchemyModelFactory): class TaskOrderFactory(factory.alchemy.SQLAlchemyModelFactory): class Meta: model = TaskOrder + From d307a255b1e256a579534e87157ea6a52b475972 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 8 Aug 2018 15:46:16 -0400 Subject: [PATCH 12/13] Fix some issues from a tricky merge --- atst/domain/requests.py | 8 ++++---- atst/routes/requests/requests_form.py | 2 +- tests/conftest.py | 12 ++++++------ tests/domain/test_requests.py | 6 +++--- tests/factories.py | 4 ++-- tests/routes/test_request_new.py | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index fcdd78bb..85fb688a 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -31,8 +31,8 @@ class Requests(object): AUTO_APPROVE_THRESHOLD = 1000000 @classmethod - def create(cls, creator_id, body): - request = Request(creator=creator_id, body=body) + def create(cls, creator, body): + request = Request(creator=creator, body=body) request = Requests.set_status(request, RequestStatus.STARTED) db.session.add(request) @@ -41,11 +41,11 @@ class Requests(object): return request @classmethod - def exists(cls, request_id, creator_id): + def exists(cls, request_id, creator): try: return db.session.query( exists().where( - and_(Request.id == request_id, Request.creator == creator_id) + and_(Request.id == request_id, Request.creator == creator) ) ).scalar() except exc.DataError: diff --git a/atst/routes/requests/requests_form.py b/atst/routes/requests/requests_form.py index ad415775..d384abdf 100644 --- a/atst/routes/requests/requests_form.py +++ b/atst/routes/requests/requests_form.py @@ -111,7 +111,7 @@ def requests_submit(request_id=None): def _check_can_view_request(request_id): if Permissions.REVIEW_AND_APPROVE_JEDI_WORKSPACE_REQUEST in g.current_user.atat_permissions: pass - elif Requests.exists(request_id, g.current_user.id): + elif Requests.exists(request_id, g.current_user): pass else: raise UnauthorizedError(g.current_user, "view request {}".format(request_id)) diff --git a/tests/conftest.py b/tests/conftest.py index c3add296..ab912679 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,6 @@ import alembic.command from atst.app import make_app, make_config from atst.database import db as _db -from .mocks import MOCK_USER import tests.factories as factories @@ -75,7 +74,6 @@ class DummyForm(dict): class DummyField(object): - def __init__(self, data=None, errors=(), raw_data=None): self.data = data self.errors = list(errors) @@ -93,9 +91,11 @@ def dummy_field(): @pytest.fixture -def user_session(monkeypatch): - - def set_user_session(user=MOCK_USER): - monkeypatch.setattr("atst.domain.auth.get_current_user", lambda *args: user) +def user_session(monkeypatch, session): + def set_user_session(user=None): + monkeypatch.setattr( + "atst.domain.auth.get_current_user", + lambda *args: user or factories.UserFactory.build(), + ) return set_user_session diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index fad23c51..ebdc64a0 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -53,6 +53,6 @@ def test_dont_auto_approve_if_no_dollar_value_specified(new_request): def test_exists(session): user_allowed = UserFactory.create() user_denied = UserFactory.create() - request = RequestFactory.create(creator=user_allowed.id) - assert Requests.exists(request.id, user_allowed.id) - assert not Requests.exists(request.id, user_denied.id) + request = RequestFactory.create(creator=user_allowed) + assert Requests.exists(request.id, user_allowed) + assert not Requests.exists(request.id, user_denied) diff --git a/tests/factories.py b/tests/factories.py index 68209045..cf81fa71 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -20,7 +20,7 @@ class RoleFactory(factory.alchemy.SQLAlchemyModelFactory): permissions = [] - + class UserFactory(factory.alchemy.SQLAlchemyModelFactory): class Meta: model = User @@ -47,7 +47,7 @@ class RequestFactory(factory.alchemy.SQLAlchemyModelFactory): id = factory.Sequence(lambda x: uuid4()) status_events = factory.RelatedFactory( - RequestStatusFactory, "request", new_status=RequestStatus.STARTED + RequestStatusEventFactory, "request", new_status=RequestStatus.STARTED ) creator = factory.SubFactory(UserFactory) body = factory.LazyAttribute(lambda r: RequestFactory.build_request_body(r.creator)) diff --git a/tests/routes/test_request_new.py b/tests/routes/test_request_new.py index e402e622..e31aae79 100644 --- a/tests/routes/test_request_new.py +++ b/tests/routes/test_request_new.py @@ -33,7 +33,7 @@ def test_submit_valid_request_form(monkeypatch, client, user_session): def test_owner_can_view_request(client, user_session): user = UserFactory.create() user_session(user) - request = RequestFactory.create(creator=user.id) + request = RequestFactory.create(creator=user) response = client.get("/requests/new/1/{}".format(request.id), follow_redirects=True) From dd849df388e46476108b15c1947c06e10e6f7bc6 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 8 Aug 2018 16:18:12 -0400 Subject: [PATCH 13/13] Use enum value to store status displayname --- atst/models/request_status_event.py | 14 +++++----- tests/models/test_requests.py | 42 +++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/atst/models/request_status_event.py b/atst/models/request_status_event.py index 26d042fa..81505f2a 100644 --- a/atst/models/request_status_event.py +++ b/atst/models/request_status_event.py @@ -9,12 +9,12 @@ 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" + STARTED = "Started" + PENDING_FINANCIAL_VERIFICATION = "Pending Financial Verification" + PENDING_CCPO_APPROVAL = "Pending CCPO Approval" + APPROVED = "Approved" + EXPIRED = "Expired" + DELETED = "Deleted" class RequestStatusEvent(Base): @@ -32,4 +32,4 @@ class RequestStatusEvent(Base): @property def displayname(self): - return self.new_status.name.replace("_", " ").title() + return self.new_status.value diff --git a/tests/models/test_requests.py b/tests/models/test_requests.py index 90038d4e..d6592a25 100644 --- a/tests/models/test_requests.py +++ b/tests/models/test_requests.py @@ -26,3 +26,45 @@ def test_request_has_creator(): request = RequestFactory.create(creator=user) assert request.creator == user + + +def test_request_status_started_displayname(): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.STARTED) + + assert request.status_displayname == "Started" + + +def test_request_status_pending_financial_displayname(): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_FINANCIAL_VERIFICATION) + + assert request.status_displayname == "Pending Financial Verification" + + +def test_request_status_pending_ccpo_displayname(): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) + + assert request.status_displayname == "Pending CCPO Approval" + + +def test_request_status_pending_approved_displayname(): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.APPROVED) + + assert request.status_displayname == "Approved" + + +def test_request_status_pending_expired_displayname(): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.EXPIRED) + + assert request.status_displayname == "Expired" + + +def test_request_status_pending_deleted_displayname(): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.DELETED) + + assert request.status_displayname == "Deleted"