diff --git a/alembic/versions/fd0cf917f682_add_soft_delete_to_applications_and_.py b/alembic/versions/fd0cf917f682_add_soft_delete_to_applications_and_.py new file mode 100644 index 00000000..86949bd9 --- /dev/null +++ b/alembic/versions/fd0cf917f682_add_soft_delete_to_applications_and_.py @@ -0,0 +1,31 @@ +"""add soft delete to applications and environments + +Revision ID: fd0cf917f682 +Revises: 32438a35cfb5 +Create Date: 2019-04-09 06:16:15.445951 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.sql import expression + + +# revision identifiers, used by Alembic. +revision = 'fd0cf917f682' +down_revision = '32438a35cfb5' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('applications', sa.Column('deleted', sa.Boolean(), nullable=False, server_default=expression.false())) + op.add_column('environments', sa.Column('deleted', sa.Boolean(), nullable=False, server_default=expression.false())) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('environments', 'deleted') + op.drop_column('applications', 'deleted') + # ### end Alembic commands ### diff --git a/atst/domain/applications.py b/atst/domain/applications.py index 30884c59..1639e7e2 100644 --- a/atst/domain/applications.py +++ b/atst/domain/applications.py @@ -1,3 +1,5 @@ +from sqlalchemy.orm.exc import NoResultFound + from atst.database import db from atst.domain.environments import Environments from atst.domain.exceptions import NotFoundError @@ -23,7 +25,9 @@ class Applications(object): def get(cls, application_id): try: application = ( - db.session.query(Application).filter_by(id=application_id).one() + db.session.query(Application) + .filter_by(id=application_id, deleted=False) + .one() ) except NoResultFound: raise NotFoundError("application") diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 9b0fa9b5..4682adcd 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -51,7 +51,11 @@ class Environments(object): @classmethod def get(cls, environment_id): try: - env = db.session.query(Environment).filter_by(id=environment_id).one() + env = ( + db.session.query(Environment) + .filter_by(id=environment_id, deleted=False) + .one() + ) except NoResultFound: raise NotFoundError("environment") diff --git a/atst/models/application.py b/atst/models/application.py index d10bd773..d29998f7 100644 --- a/atst/models/application.py +++ b/atst/models/application.py @@ -1,4 +1,4 @@ -from sqlalchemy import Column, ForeignKey, String +from sqlalchemy import Column, ForeignKey, String, Boolean from sqlalchemy.orm import relationship from atst.models import Base @@ -15,9 +15,15 @@ class Application(Base, mixins.TimestampsMixin, mixins.AuditableMixin): portfolio_id = Column(ForeignKey("portfolios.id"), nullable=False) portfolio = relationship("Portfolio") - environments = relationship("Environment", back_populates="application") + environments = relationship( + "Environment", + back_populates="application", + primaryjoin="and_(Environment.application_id==Application.id, Environment.deleted==False)", + ) roles = relationship("ApplicationRole") + deleted = Column(Boolean, default=False) + @property def users(self): return set(role.user for role in self.roles) diff --git a/atst/models/environment.py b/atst/models/environment.py index a520e787..5f1e19d2 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -1,4 +1,4 @@ -from sqlalchemy import Column, ForeignKey, String +from sqlalchemy import Column, ForeignKey, String, Boolean from sqlalchemy.orm import relationship from atst.models import Base @@ -17,6 +17,8 @@ class Environment(Base, mixins.TimestampsMixin, mixins.AuditableMixin): cloud_id = Column(String) + deleted = Column(Boolean, default=False) + @property def users(self): return [r.user for r in self.roles] diff --git a/atst/models/portfolio.py b/atst/models/portfolio.py index 599de3e6..23c19482 100644 --- a/atst/models/portfolio.py +++ b/atst/models/portfolio.py @@ -16,7 +16,11 @@ class Portfolio(Base, mixins.TimestampsMixin, mixins.AuditableMixin): name = Column(String) defense_component = Column(String) # Department of Defense Component - applications = relationship("Application", back_populates="portfolio") + applications = relationship( + "Application", + back_populates="portfolio", + primaryjoin="and_(Application.portfolio_id==Portfolio.id, Application.deleted==False)", + ) roles = relationship("PortfolioRole") task_orders = relationship("TaskOrder") diff --git a/tests/domain/test_applications.py b/tests/domain/test_applications.py index 98dde0e2..8756bb7c 100644 --- a/tests/domain/test_applications.py +++ b/tests/domain/test_applications.py @@ -1,6 +1,9 @@ +import pytest + from atst.domain.applications import Applications -from tests.factories import UserFactory, PortfolioFactory -from atst.domain.portfolios import Portfolios +from atst.domain.exceptions import NotFoundError + +from tests.factories import ApplicationFactory, UserFactory, PortfolioFactory def test_create_application_with_multiple_environments(): @@ -53,3 +56,9 @@ def test_can_only_update_name_and_description(): assert application.description == "a new application" assert len(application.environments) == 1 assert application.environments[0].name == env_name + + +def test_get_excludes_deleted(): + app = ApplicationFactory.create(deleted=True) + with pytest.raises(NotFoundError): + Applications.get(app.id) diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index dee03fb6..c51b878c 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -1,8 +1,16 @@ +import pytest + from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles from atst.domain.portfolio_roles import PortfolioRoles +from atst.domain.exceptions import NotFoundError -from tests.factories import ApplicationFactory, UserFactory, PortfolioFactory +from tests.factories import ( + ApplicationFactory, + UserFactory, + PortfolioFactory, + EnvironmentFactory, +) def test_create_environments(): @@ -186,3 +194,11 @@ def test_get_scoped_environments(db): application2_envs = Environments.for_user(developer, portfolio.applications[1]) assert [env.name for env in application2_envs] == ["application2 staging"] + + +def test_get_excludes_deleted(): + env = EnvironmentFactory.create( + deleted=True, application=ApplicationFactory.create() + ) + with pytest.raises(NotFoundError): + Environments.get(env.id) diff --git a/tests/factories.py b/tests/factories.py index f9b5af17..29dc7fa0 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -105,7 +105,7 @@ class PortfolioFactory(Base): class Meta: model = Portfolio - name = factory.Faker("name") + name = factory.Faker("domain_word") defense_component = factory.LazyFunction(random_service_branch) @classmethod @@ -157,7 +157,7 @@ class ApplicationFactory(Base): model = Application portfolio = factory.SubFactory(PortfolioFactory) - name = factory.Faker("name") + name = factory.Faker("domain_word") description = "A test application" @classmethod @@ -192,6 +192,8 @@ class EnvironmentFactory(Base): class Meta: model = Environment + name = factory.Faker("domain_word") + @classmethod def _create(cls, model_class, *args, **kwargs): with_members = kwargs.pop("members", []) diff --git a/tests/models/test_application.py b/tests/models/test_application.py index d7bf564b..1aded029 100644 --- a/tests/models/test_application.py +++ b/tests/models/test_application.py @@ -1,4 +1,8 @@ -from tests.factories import ApplicationFactory, ApplicationRoleFactory +from tests.factories import ( + ApplicationFactory, + ApplicationRoleFactory, + EnvironmentFactory, +) def test_application_num_users(): @@ -9,3 +13,11 @@ def test_application_num_users(): ApplicationRoleFactory.create(application=application) assert application.num_users == 1 + + +def test_application_environments_excludes_deleted(): + app = ApplicationFactory.create() + env = EnvironmentFactory.create(application=app) + EnvironmentFactory.create(application=app, deleted=True) + assert len(app.environments) == 1 + assert app.environments[0].id == env.id diff --git a/tests/models/test_portfolio.py b/tests/models/test_portfolio.py new file mode 100644 index 00000000..613b6435 --- /dev/null +++ b/tests/models/test_portfolio.py @@ -0,0 +1,9 @@ +from tests.factories import ApplicationFactory, PortfolioFactory + + +def test_portfolio_applications_excludes_deleted(): + portfolio = PortfolioFactory.create() + app = ApplicationFactory.create(portfolio=portfolio) + ApplicationFactory.create(portfolio=portfolio, deleted=True) + assert len(portfolio.applications) == 1 + assert portfolio.applications[0].id == app.id