[Issue #1736] Adjust current opportunity summary logic to want null r…

…evision number (#1743)

## Summary
Fixes #1736

### Time to review: __3 mins__

## Changes proposed
Modify the set-current-opportunity script to determine the latest
opportunity summary based on the revision number being null (rather than
the max value)

## Context for reviewers
My understanding of the revision number in the prior implementation was
incorrect. We want to determine the latest revision of an opportunity
summary based on the revision number, however the latest in the current
Oracle system will always be null. The way the current system works is
that the revision number is only added once it is moved to the history
table. Because we effectively want to ignore all of the historical
synopsis/forecast records, we can just check if the value is null.

Due to how the data is structured in the current system where the values
are unique in the synopsis & forecast tables for a given opportunity, we
know there will only ever be a single forecast / non-forecast that has
this value set to null in the data.

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 ###

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]

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

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,15 +241,14 @@ 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,
                 archive_date=summary_info.archive_date,
                 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,
         )
     )