diff --git a/api/src/db/migrations/versions/2024_04_16_make_revision_number_nullable.py b/api/src/db/migrations/versions/2024_04_16_make_revision_number_nullable.py new file mode 100644 index 000000000..1fe9b31cd --- /dev/null +++ b/api/src/db/migrations/versions/2024_04_16_make_revision_number_nullable.py @@ -0,0 +1,39 @@ +"""make revision number nullable + +Revision ID: b26ea0f40066 +Revises: f8364285630d +Create Date: 2024-04-16 13:36:35.993325 + +""" +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "b26ea0f40066" +down_revision = "f8364285630d" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column( + "opportunity_summary", + "revision_number", + existing_type=sa.INTEGER(), + nullable=True, + schema="api", + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column( + "opportunity_summary", + "revision_number", + existing_type=sa.INTEGER(), + nullable=False, + schema="api", + ) + # ### end Alembic commands ### diff --git a/api/src/db/models/opportunity_models.py b/api/src/db/models/opportunity_models.py index ecf2da86b..62a34e9dd 100644 --- a/api/src/db/models/opportunity_models.py +++ b/api/src/db/models/opportunity_models.py @@ -121,7 +121,7 @@ class OpportunitySummary(ApiSchemaTable, TimestampMixin): forecasted_project_start_date: Mapped[date | None] fiscal_year: Mapped[int | None] - revision_number: Mapped[int] + revision_number: Mapped[int | None] modification_comments: Mapped[str | None] funding_category_description: Mapped[str | None] diff --git a/api/src/task/opportunities/set_current_opportunities_task.py b/api/src/task/opportunities/set_current_opportunities_task.py index 6742eb848..d3b97aad9 100644 --- a/api/src/task/opportunities/set_current_opportunities_task.py +++ b/api/src/task/opportunities/set_current_opportunities_task.py @@ -144,19 +144,15 @@ def determine_current_and_status( latest_forecasted_summary: OpportunitySummary | None = None latest_non_forecasted_summary: OpportunitySummary | None = None - # Latest is based entirely off of the revision number, we don't - # care about update/create dates or any other fields + # Latest is based entirely off of the revision number, the latest + # will always have a null revision number, and because of how the + # data is structured that we import, we'll only ever have a single + # null value for forecast and non-forecast respectively for summary in opportunity.all_opportunity_summaries: - if summary.is_forecast: - if latest_forecasted_summary is None: - latest_forecasted_summary = summary - elif summary.revision_number > latest_forecasted_summary.revision_number: - latest_forecasted_summary = summary - else: - if latest_non_forecasted_summary is None: - latest_non_forecasted_summary = summary - elif summary.revision_number > latest_non_forecasted_summary.revision_number: - latest_non_forecasted_summary = summary + if summary.is_forecast and summary.revision_number is None: + latest_forecasted_summary = summary + elif not summary.is_forecast and summary.revision_number is None: + latest_non_forecasted_summary = summary # We need to make sure the latest can actually be publicly displayed # Note that if it cannot, we do not want to use an earlier revision diff --git a/api/tests/src/task/opportunities/test_set_current_opportunities_task.py b/api/tests/src/task/opportunities/test_set_current_opportunities_task.py index 33968de1e..ddf42f36a 100644 --- a/api/tests/src/task/opportunities/test_set_current_opportunities_task.py +++ b/api/tests/src/task/opportunities/test_set_current_opportunities_task.py @@ -101,7 +101,7 @@ def with_summary( close_date: date | None = None, archive_date: date | None = None, is_forecast: bool = False, - revision_number: int = 0, + revision_number: int | None = None, is_deleted: bool = False, is_expected_current: bool = False, is_already_current: bool = False, @@ -241,7 +241,6 @@ def test_single_summary_scenario( container = ( OpportunityContainer() .with_summary( - revision_number=1, is_forecast=summary_info.is_forecast, post_date=summary_info.post_date, close_date=summary_info.close_date, @@ -249,7 +248,7 @@ def test_single_summary_scenario( is_expected_current=True if expected_opportunity_status is not None else False, ) .with_summary( - # this summary won't ever be chosen as its an older revision + # this summary won't ever be chosen as it has a revision number set revision_number=0, is_forecast=summary_info.is_forecast, post_date=YESTERDAY, @@ -401,7 +400,6 @@ def test_two_scenarios_one_forecast_one_non( close_date=expected_summary_info.close_date, archive_date=expected_summary_info.archive_date, is_expected_current=True if expected_opportunity_status is not None else False, - revision_number=5, ) .with_summary( is_forecast=other_summary_info.is_forecast, @@ -555,7 +553,6 @@ def test_via_cli(cli_runner, db_session, enable_factory_create): post_date=CURRENT_DATE - timedelta(days=5), archive_date=CURRENT_DATE + timedelta(days=120), is_expected_current=True, - revision_number=3, ) )