Skip to content

Commit

Permalink
[Issue #2387] Don't cascade delete through current_opportunity_summary (
Browse files Browse the repository at this point in the history
#2388)

## Summary
Fixes #2387

### Time to review: __5 mins__

## Changes proposed
Remove cascade-deletes from automatically happening on relationships to
the current_opportunity_summary table

## Context for reviewers
Cascade deletes are useful to automatically delete records across
several related tables, but this is definitely a place we don't want it
(deleting a current_opportunity_summary says nothing about the
opportunity or summary objects needing to be deleted).

In our set-current-opportunities script, we sometimes delete the current
opportunity summary, which at the moment was fully deleting the
opportunity - which we definitely do not want.

I adjusted the way the set-current-opportunities test works so that this
issue is caught AND THEN I fixed the cascade configuration we had to
stop doing it. Without the fix, the tests would fail complaining that
the opportunity no longer exists.

## Additional information
Found this in the staging environment as the data did not match dev
despite having the same initial data - due to some status updates being
applied at different times, some records were deleted incorrectly.

---------

Co-authored-by: nava-platform-bot <[email protected]>
  • Loading branch information
chouinar and nava-platform-bot authored Oct 7, 2024
1 parent 9db412c commit 347ae6f
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 10 deletions.
8 changes: 2 additions & 6 deletions api/src/db/models/opportunity_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,19 +362,15 @@ class CurrentOpportunitySummary(ApiSchemaTable, TimestampMixin):
opportunity_id: Mapped[int] = mapped_column(
BigInteger, ForeignKey(Opportunity.opportunity_id), primary_key=True, index=True
)
opportunity: Mapped[Opportunity] = relationship(
single_parent=True, cascade="all, delete-orphan"
)
opportunity: Mapped[Opportunity] = relationship(single_parent=True)

opportunity_summary_id: Mapped[int] = mapped_column(
BigInteger,
ForeignKey(OpportunitySummary.opportunity_summary_id),
primary_key=True,
index=True,
)
opportunity_summary: Mapped[OpportunitySummary] = relationship(
single_parent=True, cascade="all, delete-orphan"
)
opportunity_summary: Mapped[OpportunitySummary] = relationship(single_parent=True)

opportunity_status: Mapped[OpportunityStatus] = mapped_column(
"opportunity_status_id",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import pytest

from src.constants.lookup_constants import OpportunityStatus
from src.db.models.opportunity_models import CurrentOpportunitySummary, OpportunitySummary
from src.db.models.opportunity_models import Opportunity, OpportunitySummary
from src.task.opportunities.set_current_opportunities_task import SetCurrentOpportunitiesTask
from src.util.datetime_util import get_now_us_eastern_date
from tests.conftest import BaseTestClass
Expand Down Expand Up @@ -131,11 +131,16 @@ def with_summary(
def validate_current_opportunity(
db_session, container: OpportunityContainer, expected_status: OpportunityStatus | None
):
current_opportunity_summary = (
db_session.query(CurrentOpportunitySummary)
.where(CurrentOpportunitySummary.opportunity_id == container.opportunity.opportunity_id)
# Force all opportunity changes to be flushed to the DB and removed from any session cache
db_session.commit()
db_session.expunge_all()

opportunity = (
db_session.query(Opportunity)
.where(Opportunity.opportunity_id == container.opportunity.opportunity_id)
.one_or_none()
)
current_opportunity_summary = opportunity.current_opportunity_summary

is_current_none = current_opportunity_summary is None
is_none_expected = container.expected_current_summary is None
Expand Down
Binary file modified documentation/api/database/erds/api-schema.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified documentation/api/database/erds/full-schema.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 347ae6f

Please sign in to comment.