feat: supporting is_seasonal manual set to TDG, JBDA, ODPT

functions-python/operations_api/src/feeds_operations/impl/feeds_operations_impl.py

@@ -95,6 +95,20 @@
 # comparing it would report a phantom change whenever the request omits it.
 _DERIVED_SOURCE_INFO_FIELDS: Final[tuple[str, ...]] = ("license_is_spdx",)
 
+# Tri-state fields: an omitted value means "preserve the stored one", and `to_orm` skips
+# them, so the diff must not report a phantom change. The inverse case of
+# _DERIVED_SOURCE_INFO_FIELDS: those are readable but not settable, these are settable but
+# not clearable by omission. Without this, a request that never mentions one of them would
+# force the write branch of `_update_feed` -- a 200 instead of a 204, plus a needless
+# materialized view refresh and web revalidation task.
+#
+# This is what lets a feed GET response be sent straight back as an update with no change
+# detected: absence means "unchanged" for every field that has no other way to say it.
+_PRESERVE_WHEN_OMITTED_FIELDS: Final[tuple[str, ...]] = (
+    "seasonal",
+    "operational_status",
+)
+
 
 def _normalize_for_diff(value):
     """Recursively coerce "absent" representations to None so change detection mirrors
@@ -311,15 +325,15 @@ def detect_changes(
     ) -> DeepDiff:
         """Detect changes between the feed and the update request."""
         copy_feed = impl_class.from_orm(feed)
-        copy_feed.operational_status_action = (
-            update_request_feed.operational_status_action
-        )
         current_values = _strip_derived_fields(
             _normalize_for_diff(copy_feed.model_dump())
         )
         requested_values = _strip_derived_fields(
             _normalize_for_diff(update_request_feed.model_dump())
         )
+        for field in _PRESERVE_WHEN_OMITTED_FIELDS:
+            if requested_values.get(field) is None:
+                requested_values[field] = current_values.get(field)
         diff = DeepDiff(
             current_values,
             requested_values,
@@ -393,10 +407,9 @@ def _update_feed(
                 else UpdateRequestGtfsRtFeedImpl
             )
             diff = self.detect_changes(feed_from_db, update_request_feed, impl_class)
-            if len(diff.affected_paths) > 0 or (
-                update_request_feed.operational_status_action is not None
-                and update_request_feed.operational_status_action != "no_change"
-            ):
+            # Every settable field, `operational_status` included, is visible to the diff, so
+            # this is the single gate: write exactly when something actually changed.
+            if len(diff.affected_paths) > 0:
                 # Capture pre-mutation state for notification events (before to_orm mutates the object).
                 old_producer_url = getattr(feed_from_db, "producer_url", None)
                 old_redirect_target_ids = {
@@ -499,15 +512,6 @@ def _update_feed(
     @staticmethod
     def _populate_feed_values(feed, impl_class, session, update_request_feed):
         impl_class.to_orm(update_request_feed, feed, session)
-        action = update_request_feed.operational_status_action
-        # This is a temporary solution as the operational_status is not visible in the diff
-        if action is not None and not action.lower() == "no_change":
-            if action.lower() == "wip":
-                feed.operational_status = "wip"
-            elif action.lower() == "published":
-                feed.operational_status = "published"
-            elif action.lower() == "unpublished":
-                feed.operational_status = "unpublished"
         session.add(feed)
 
     @staticmethod

functions-python/operations_api/src/feeds_operations/impl/models/update_request_gtfs_feed_impl.py

@@ -75,6 +75,7 @@ def from_orm(cls, obj: Gtfsfeed | None) -> UpdateRequestGtfsFeed | None:
             ),
             official=obj.official,
             seasonal=obj.seasonal,
+            operational_status=obj.operational_status,
         )
 
     @classmethod
@@ -90,7 +91,15 @@ def to_orm(
         entity.note = update_request.note
         entity.feed_contact_email = update_request.feed_contact_email
         entity.official = update_request.official
-        entity.seasonal = update_request.seasonal
+        # Tri-state, matching the catalog CSV's empty cell (populate_db_gtfs.py): an omitted
+        # `seasonal` means "leave the stored value alone", so only an explicit true/false
+        # writes. A feed marked seasonal by hand must survive an update request from a client
+        # whose spec predates the field.
+        if update_request.seasonal is not None:
+            entity.seasonal = update_request.seasonal
+        # Tri-state as well: omitted means "leave the operational status alone".
+        if update_request.operational_status is not None:
+            entity.operational_status = update_request.operational_status
         entity.producer_url = (
             None
             if (

functions-python/operations_api/src/feeds_operations/impl/models/update_request_gtfs_rt_feed_impl.py

@@ -87,6 +87,7 @@ def from_orm(cls, obj: Gtfsrealtimefeed | None) -> UpdateRequestGtfsRtFeed | Non
             feed_references=sorted([item.stable_id for item in obj.gtfs_feeds]),
             official=obj.official,
             seasonal=obj.seasonal,
+            operational_status=obj.operational_status,
         )
 
     @classmethod
@@ -102,7 +103,15 @@ def to_orm(
         entity.note = update_request.note
         entity.feed_contact_email = update_request.feed_contact_email
         entity.official = update_request.official
-        entity.seasonal = update_request.seasonal
+        # Tri-state, matching the catalog CSV's empty cell (populate_db_gtfs.py): an omitted
+        # `seasonal` means "leave the stored value alone", so only an explicit true/false
+        # writes. A feed marked seasonal by hand must survive an update request from a client
+        # whose spec predates the field.
+        if update_request.seasonal is not None:
+            entity.seasonal = update_request.seasonal
+        # Tri-state as well: omitted means "leave the operational status alone".
+        if update_request.operational_status is not None:
+            entity.operational_status = update_request.operational_status
         entity.producer_url = (
             None
             if (

functions-python/operations_api/tests/feeds_operations/impl/models/test_update_request_gtfs_feed_impl.py

@@ -123,7 +123,10 @@ def test_to_orm_invalid_source_info():
     result = UpdateRequestGtfsFeedImpl.to_orm(update_request, entity, session)
     assert result.producer_url is None
     assert result.is_producer_url_unstable is None
-    assert result.seasonal is False
+    # The request omits `seasonal`, so to_orm leaves the attribute alone. This entity was
+    # never persisted, so the NOT NULL server default has not applied yet -- None here is
+    # the preserve path, not a stored value.
+    assert result.seasonal is None
     assert result.authentication_type is None
     assert result.authentication_info_url is None
     assert result.api_key_parameter_name is None

functions-python/operations_api/tests/feeds_operations/impl/models/test_update_request_gtfs_rt_feed_impl.py

@@ -35,6 +35,7 @@ def test_from_orm():
         license_url="http://license.url",
         redirectingids=[redirecting_id],
         externalids=[external_id],
+        seasonal=True,
     )
 
     result = UpdateRequestGtfsRtFeedImpl.from_orm(gtfs_feed)
@@ -48,6 +49,7 @@ def test_from_orm():
     assert result.source_info.authentication_type == 1
     assert result.source_info.authentication_info_url == "http://auth.info.url"
     assert result.source_info.api_key_parameter_name == "api_key"
+    assert result.seasonal is True
     assert result.source_info.license_url == "http://license.url"
     assert len(result.redirects) == 1
     assert result.redirects[0].target_id == "target_stable_id"
@@ -79,6 +81,7 @@ def test_to_orm():
         external_ids=[ExternalIdImpl(external_id="external_id")],
         entity_types=["vp"],
         feed_references=["feed_reference"],
+        seasonal=True,
     )
     entity = Gtfsrealtimefeed(id="1", stable_id="stable_id", data_type="gtfs")
     target_feed = Gtfsfeed(id=2, stable_id="target_stable_id")
@@ -98,6 +101,7 @@ def test_to_orm():
     assert result.note == "note"
     assert result.feed_contact_email == "email@example.com"
     assert result.producer_url == "http://producer.url"
+    assert result.seasonal is True
     assert result.authentication_type == "1"
     assert result.authentication_info_url == "http://auth.info.url"
     assert result.api_key_parameter_name == "api_key"

functions-python/operations_api/tests/feeds_operations/impl/test_create_feeds_operations_impl_gtfs.py

@@ -45,7 +45,6 @@ def update_request_gtfs_feed():
             license_url=feed_mdb_40.license_url,
         ),
         redirects=[],
-        operational_status_action="no_change",
         official=True,
     )
 

functions-python/operations_api/tests/feeds_operations/impl/test_detect_changes.py

@@ -32,7 +32,6 @@ def _make_request(source_info: SourceInfo, **overrides) -> UpdateRequestGtfsFeed
         source_info=source_info,
         redirects=[],
         external_ids=[],
-        operational_status_action="no_change",
         official=True,
     )
     payload.update(overrides)
@@ -110,6 +109,65 @@ def test_detect_changes_detects_cleared_list():
     assert diff.affected_paths
 
 
+def test_detect_changes_ignores_omitted_seasonal():
+    """An omitted `seasonal` means preserve, so it is not a change.
+
+    `to_orm` skips a None `seasonal`, so reporting it here would push `_update_feed` down
+    the write branch for nothing: a 200 instead of a 204, plus a materialized view refresh
+    and a web revalidation task on an update that changes nothing.
+    """
+    source_info = SourceInfo(producer_url="https://example.com/feed")
+    current = _make_request(source_info, seasonal=True)
+    requested = _make_request(source_info)  # client's spec predates the field
+
+    assert requested.seasonal is None
+    diff = _detect(current, requested)
+
+    assert not diff.affected_paths
+
+
+def test_detect_changes_detects_explicitly_cleared_seasonal():
+    """An explicit false is a real edit and must still be reported, unlike an omission."""
+    source_info = SourceInfo(producer_url="https://example.com/feed")
+    current = _make_request(source_info, seasonal=True)
+    requested = _make_request(source_info, seasonal=False)
+
+    diff = _detect(current, requested)
+
+    assert diff.affected_paths
+
+
+def test_detect_changes_ignores_omitted_operational_status():
+    """Omitting `operational_status` means preserve, so it is not a change.
+
+    Replaces the old `operational_status_action="no_change"` sentinel, which sat outside the
+    diff entirely.
+    """
+    source_info = SourceInfo(producer_url="https://example.com/feed")
+    current = _make_request(source_info, operational_status="published")
+    requested = _make_request(source_info)
+
+    assert requested.operational_status is None
+    diff = _detect(current, requested)
+
+    assert not diff.affected_paths
+
+
+def test_detect_changes_detects_operational_status_change():
+    """A real status change is reported by the diff rather than bypassing it.
+
+    It used to be invisible to change detection and applied by a special case in
+    `_populate_feed_values`; now the diff is the single gate on whether a write happens.
+    """
+    source_info = SourceInfo(producer_url="https://example.com/feed")
+    current = _make_request(source_info, operational_status="wip")
+    requested = _make_request(source_info, operational_status="published")
+
+    diff = _detect(current, requested)
+
+    assert diff.affected_paths
+
+
 def test_normalize_for_diff_coerces_empty_values():
     normalized = _normalize_for_diff(
         {

functions-python/operations_api/tests/feeds_operations/impl/test_feeds_operations_impl_gtfs.py

@@ -35,7 +35,6 @@ def update_request_gtfs_feed():
             license_is_spdx=True,
         ),
         redirects=[],
-        operational_status_action="no_change",
         official=True,
     )
 
@@ -87,7 +86,7 @@ async def test_update_gtfs_feed_field_change(
 async def test_update_gtfs_feed_set_wip(
     mock_revalidation, update_request_gtfs_feed, db_session
 ):
-    update_request_gtfs_feed.operational_status_action = "wip"
+    update_request_gtfs_feed.operational_status = "wip"
     api = OperationsApiImpl()
     response: Response = api.update_gtfs_feed(update_request_gtfs_feed)
     assert response.status_code == 200
@@ -102,8 +101,15 @@ async def test_update_gtfs_feed_set_wip(
 
 @pytest.mark.asyncio
 @pytest.mark.usefixtures("update_request_gtfs_feed", "db_session")
-async def test_update_gtfs_feed_set_wip_nochange(update_request_gtfs_feed, db_session):
-    update_request_gtfs_feed.operational_status_action = "no_change"
+async def test_update_gtfs_feed_omitted_operational_status_is_preserved(
+    update_request_gtfs_feed, db_session
+):
+    """Omitting `operational_status` leaves the stored value alone and reports no change.
+
+    This replaces the old `operational_status_action="no_change"` sentinel: absence now
+    carries that meaning, the same tri-state contract `seasonal` uses.
+    """
+    assert update_request_gtfs_feed.operational_status is None
     api = OperationsApiImpl()
     response: Response = api.update_gtfs_feed(update_request_gtfs_feed)
     assert response.status_code == 204
@@ -122,7 +128,7 @@ async def test_update_gtfs_feed_set_wip_nochange(update_request_gtfs_feed, db_se
 async def test_update_gtfs_feed_set_published(
     mock_revalidation, update_request_gtfs_feed, db_session
 ):
-    update_request_gtfs_feed.operational_status_action = "published"
+    update_request_gtfs_feed.operational_status = "published"
     api = OperationsApiImpl()
     response: Response = api.update_gtfs_feed(update_request_gtfs_feed)
     assert response.status_code == 200
@@ -141,7 +147,7 @@ async def test_update_gtfs_feed_set_published(
 async def test_update_gtfs_feed_set_unpublished(
     mock_revalidation, update_request_gtfs_feed, db_session
 ):
-    update_request_gtfs_feed.operational_status_action = "unpublished"
+    update_request_gtfs_feed.operational_status = "unpublished"
     api = OperationsApiImpl()
     response: Response = api.update_gtfs_feed(update_request_gtfs_feed)
     assert response.status_code == 200
@@ -178,3 +184,77 @@ async def test_update_gtfs_feed_official_field(update_request_gtfs_feed, db_sess
         .one()
     )
     assert db_feed.official is True
+
+
+@pytest.mark.asyncio
+@patch("feeds_operations.impl.feeds_operations_impl.create_web_revalidation_task")
+async def test_update_gtfs_feed_seasonal_field(
+    mock_revalidation, update_request_gtfs_feed, db_session
+):
+    """An explicit `seasonal` in the request is persisted."""
+    # Establish a known pre-state so toggling `seasonal` to True is a genuine change
+    # regardless of test ordering (the row is shared across this module).
+    seeded_feed = (
+        db_session.query(Gtfsfeed)
+        .filter(Gtfsfeed.stable_id == feed_mdb_40.stable_id)
+        .one()
+    )
+    seeded_feed.seasonal = False
+    db_session.commit()
+
+    update_request_gtfs_feed.seasonal = True
+    api = OperationsApiImpl()
+    response: Response = api.update_gtfs_feed(update_request_gtfs_feed)
+    assert response.status_code == 200
+
+    db_session.expire_all()
+    db_feed = (
+        db_session.query(Gtfsfeed)
+        .filter(Gtfsfeed.stable_id == feed_mdb_40.stable_id)
+        .one()
+    )
+    assert db_feed.seasonal is True
+
+
+@pytest.mark.asyncio
+@patch("feeds_operations.impl.feeds_operations_impl.create_web_revalidation_task")
+async def test_update_gtfs_feed_omitted_seasonal_is_preserved(
+    mock_revalidation, update_request_gtfs_feed, db_session
+):
+    """A request that never mentions `seasonal` must not clear it.
+
+    Clients generated from a spec predating the field send no `seasonal` at all. While the
+    property carried `default: false`, that omission reset an operator-set flag on the next
+    edit of any other field -- which would silently un-mark the TDG/ODPT/JBDA feeds this
+    issue exists to mark. The no-phantom-change half of the fix is pinned in
+    test_detect_changes.py, which does not depend on this shared row's state.
+    """
+    seeded_feed = (
+        db_session.query(Gtfsfeed)
+        .filter(Gtfsfeed.stable_id == feed_mdb_40.stable_id)
+        .one()
+    )
+    seeded_feed.seasonal = True
+    db_session.commit()
+
+    # Drive an unrelated edit by making the STORED note stale, rather than by changing the
+    # request. The write then restores `note` to its fixture value, so this test leaves the
+    # module-shared row exactly as it found it (sibling tests assert on feed_name/provider).
+    seeded_feed.note = "stale note"
+    db_session.commit()
+
+    # The fixture never sets `seasonal`; that is exactly the request shape under test.
+    assert update_request_gtfs_feed.seasonal is None
+
+    api = OperationsApiImpl()
+    response: Response = api.update_gtfs_feed(update_request_gtfs_feed)
+    assert response.status_code == 200
+
+    db_session.expire_all()
+    db_feed = (
+        db_session.query(Gtfsfeed)
+        .filter(Gtfsfeed.stable_id == feed_mdb_40.stable_id)
+        .one()
+    )
+    assert db_feed.note == feed_mdb_40.note
+    assert db_feed.seasonal is True