Merge pull request #4327 from hove-io/fix_disruptions_on_all_pois_in_journey

jormungandr: Update poi objects in journeys if active by configuration

Author: kadhikari

source/jormungandr/jormungandr/instance.py

@@ -614,6 +614,12 @@ def car_park_provider(self):
         instance_db = self.get_models()
         return get_value_or_default('car_park_provider', instance_db, self.name)
 
+    @property
+    def disruptions_on_poi(self):
+        # type: () -> bool
+        instance_db = self.get_models()
+        return get_value_or_default('disruptions_on_poi', instance_db, self.name)
+
     @property
     def max_additional_connections(self):
         # type: () -> int

source/jormungandr/jormungandr/interfaces/v1/Journeys.py

@@ -377,6 +377,14 @@ def get_disruption_uris(object):
 
                 return uris
 
+            def impact_on_poi():
+                for d in objects[0].get('disruptions', []):
+                    for io in d.get('impacted_objects', []):
+                        if io.get('pt_object', {}).get('embedded_type') == "poi":
+                            return True
+
+                return False
+
             def update_for_poi(object):
                 # Add links in poi object
                 object_copy = deepcopy(object)
@@ -388,22 +396,20 @@ def update_for_poi(object):
                             create_internal_link(_type="disruption", rel="disruptions", id=disruption_uri)
                         )
 
-            # We should only update 'from' object of the first section as well as 'to' object of the last one
-            # since object poi can only be present in those two cases
-            # If object is absent in first_section['from'] as well as last_section['to'] for the first journey
-            # then no need to verify for the remaining journeys
+            # If no disruption on poi exist, no action to do
+            if not impact_on_poi():
+                return objects
+
+            # We should update 'from' and 'to' object of all the sections if object is POI
             for j in objects[0].get('journeys', []):
                 if "sections" not in j:
                     continue
 
-                first_sec = j['sections'][0]
-                last_sec = j['sections'][-1]
-                if first_sec['from']['embedded_type'] != "poi" and last_sec['to']['embedded_type'] != "poi":
-                    break
-                if first_sec['from']['embedded_type'] == "poi":
-                    update_for_poi(first_sec['from']['poi'])
-                if last_sec['to']['embedded_type'] == "poi":
-                    update_for_poi(last_sec['to']['poi'])
+                for s in j.get('sections', []):
+                    if s.get('from', {}).get('embedded_type') == "poi":
+                        update_for_poi(s['from']['poi'])
+                    if s.get('to', {}).get('embedded_type') == "poi":
+                        update_for_poi(s['to']['poi'])
 
             return objects
 
@@ -906,6 +912,9 @@ def _set_specific_params(mod):
             if args.get('_use_predicted_traffic') is None:
                 args['_use_predicted_traffic'] = mod.use_predicted_traffic
 
+            if args.get('_disruptions_on_poi') is None:
+                args['_disruptions_on_poi'] = mod.disruptions_on_poi
+
         # When computing 'same_journey_schedules'(is_journey_schedules=True), some parameters need to be overridden
         # because they are contradictory to the request
         if args.get("is_journey_schedules"):

source/jormungandr/jormungandr/interfaces/v1/journey_common.py

@@ -395,6 +395,12 @@ def __init__(self, output_type_serializer):
             'and thus avoid disrupted public transport.\n'
             'Nota: `disruption_active=true` <=> `data_freshness=realtime`',
         )
+        parser_get.add_argument(
+            "_disruptions_on_poi",
+            type=BooleanType(),
+            hidden=True,
+            help="Fetch and display disruptions on poi in the journey response",
+        )
         # no default value for data_freshness because we need to maintain retrocomp with disruption_active
         parser_get.add_argument(
             "data_freshness",

source/jormungandr/jormungandr/interfaces/v1/test/journey_tests.py

@@ -245,7 +245,7 @@ def handle_poi_disruptions_test():
             resp = get_response_with_poi_and_disruptions()
             assert len(resp[0].get("journeys", 0)) == 2
 
-            # Journey 1: a disruption exist for poi_uri_a: the poi in journey should have links and
+            # Journey 1: a disruption exist for poi_uri_a (origin): the poi in journey should have links and
             # impacted_object should have object poi
             origin = resp[0].get("journeys", 0)[0]['sections'][0]['from']['poi']
             assert origin['id'] == "poi_uri_a"
@@ -254,23 +254,36 @@ def handle_poi_disruptions_test():
             assert impacted_object['id'] == "poi_uri_a"
             assert "links" not in impacted_object
 
-            # No disruption exist for poi_uri_b: the poi in journey doesn't have links and object poi is absent
+            # No disruption exist for poi_uri_b (destination): the poi in journey doesn't have links and object poi is absent
             # in impacted_object
             destination = resp[0].get("journeys", 0)[0]['sections'][2]['to']['poi']
             assert destination['id'] == "poi_uri_b"
             assert len(destination["links"]) == 0
 
-            # Journey 2: a disruption exist for poi_uri_a: the poi in journey should have links and
-            # impacted_object should have object poi
+            # Journey 2: a disruption exist for poi_uri_a (origin) and another exist for poi_uri_from:
+            # both pois in journey should have links and  impacted_object should have object poi
             origin = resp[0].get("journeys", 0)[1]['sections'][0]['from']['poi']
             assert origin['id'] == "poi_uri_a"
             assert len(origin['links']) == 1
             impacted_object = resp[0]['disruptions'][0]['impacted_objects'][0]['pt_object']['poi']
             assert impacted_object['id'] == "poi_uri_a"
             assert "links" not in impacted_object
 
+            sections = resp[0].get("journeys", 0)[1]['sections']
+            assert len(sections) == 3
+            poi = sections[0]['to']['poi']
+            assert poi['id'] == "poi_uri_from"
+            assert len(poi['links']) == 1
+            impacted_object = resp[0]['disruptions'][1]['impacted_objects'][0]['pt_object']['poi']
+            assert impacted_object['id'] == "poi_uri_from"
+            assert "links" not in impacted_object
+
+            poi = sections[1]['to']['poi']
+            assert poi['id'] == "poi_uri_to"
+            assert len(poi['links']) == 0
+
             # No disruption exist for poi_uri_b: the poi in journey doesn't have links and object poi is absent
             # in impacted_object
-            destination = resp[0].get("journeys", 0)[1]['sections'][0]['to']['poi']
+            destination = sections[2]['to']['poi']
             assert destination['id'] == "poi_uri_b"
             assert len(destination["links"]) == 0

source/jormungandr/jormungandr/scenarios/new_default.py

@@ -503,16 +503,21 @@ def update_total_co2_emission(pb_resp):
         j.co2_emission.unit = 'gEC'
 
 
-def update_disruptions_on_pois(instance, pb_resp):
+def update_disruptions_on_pois(instance, api_request, pb_resp):
     """
-    Maintain a set of uri from g.origin_detail and g.destination_detail of type Poi,
-    call loki with api api_disruptions&pois[]...
+    Maintain a set of uri from g.origin_detail and g.destination_detail of type Poi
+    Add uri from all journey.section.origin and journey.section.destination of type Poi
+    Call loki with api api_disruptions&pois[]...
     For each disruption on poi, add disruption id in the attribute links and add disruptions in the response
     """
+    required_mode_list = {'bss', 'car'}
+    if not api_request.get('_disruptions_on_poi'):
+        return
     if not pb_resp.journeys:
         return
     # Add uri of all the pois in a set
     poi_uris = set()
+    poi_objets = []
     since_datetime = date_to_timestamp(datetime.utcnow())
     until_datetime = date_to_timestamp(datetime.utcnow())
 
@@ -523,6 +528,23 @@ def update_disruptions_on_pois(instance, pb_resp):
     if g.destination_detail and g.destination_detail.get('embedded_type') == "poi":
         poi_uris.add(g.destination_detail.get('id'))
 
+    # Add pois present in all journeys if any of modes={'bss', 'car'} is present in
+    # origin_mode or destination_mode or direct_path_mode
+    mode_list = api_request.get('origin_mode', [])
+    mode_list.extend(api_request.get('destination_mode', []))
+    if set(mode_list).intersection(required_mode_list):
+        for j in pb_resp.journeys:
+            for s in j.sections:
+                if s.origin.embedded_type == type_pb2.POI:
+                    poi_uris.add(s.origin.uri)
+                    poi_objets.append(s.origin.poi)
+                    since_datetime = min(since_datetime, s.begin_date_time)
+
+                if s.destination.embedded_type == type_pb2.POI:
+                    poi_uris.add(s.destination.uri)
+                    poi_objets.append(s.destination.poi)
+                    until_datetime = max(until_datetime, s.end_date_time)
+
     if since_datetime >= until_datetime:
         since_datetime = until_datetime - 1
 
@@ -531,6 +553,14 @@ def update_disruptions_on_pois(instance, pb_resp):
     if poi_disruptions is None:
         return
 
+    # For each poi in pt_objects:
+    # add impact_uris from resp_poi and
+    # copy object poi in impact.impacted_objects
+    for pt_object in poi_objets:
+        impact_uris = get_impact_uris_for_poi(poi_disruptions, pt_object)
+        for impact_uri in impact_uris:
+            pt_object.impact_uris.append(impact_uri)
+
     # Add all impacts from resp_poi to the response
     add_disruptions(pb_resp, poi_disruptions)
 
@@ -1507,7 +1537,7 @@ def fill_journeys(self, request_type, api_request, instance):
         journey_filter.remove_excess_terminus(pb_resp)
 
         # Update disruptions on pois
-        update_disruptions_on_pois(instance, pb_resp)
+        update_disruptions_on_pois(instance, api_request, pb_resp)
 
         # Update booking_url in booking_rule for all sections of type ON_DEMAND_TRANSPORT
         update_booking_rule_url_in_response(pb_resp)

source/jormungandr/jormungandr/scenarios/tests/helpers_tests.py

@@ -529,8 +529,6 @@ def get_response_with_a_disruption_on_poi(uri="poi_uri", name="poi_name_from_lok
     impact.disruption_uri = "test_disruption_uri"
     impacted_object = impact.impacted_objects.add()
 
-    # poi = make_pt_object(type_pb2.POI, lon=1, lat=2, uri='poi:test_uri')
-    # impacted_object.pt_object.CopyFrom(poi)
     impacted_object.pt_object.name = name
     impacted_object.pt_object.uri = uri
     impacted_object.pt_object.embedded_type = type_pb2.POI
@@ -667,19 +665,46 @@ def get_pb_response_with_journeys_and_disruptions():
     section.destination.uri = 'address_b'
     section.destination.embedded_type = type_pb2.ADDRESS
 
-    # Add a journey : walking address to address
+    # Add a journey : walking address to poi + poi to poi + poi to address
     journey = response.journeys.add()
     section = journey.sections.add()
     section.type = response_pb2.STREET_NETWORK
     section.street_network.mode = response_pb2.Walking
     section.origin.uri = 'address_a'
     section.origin.embedded_type = type_pb2.ADDRESS
+    section.destination.uri = 'poi_uri_from'
+    section.destination.embedded_type = type_pb2.POI
+    section.destination.poi.uri = 'poi_uri_from'
+    section.destination.poi.name = 'poi_name_from'
+
+    section = journey.sections.add()
+    section.type = response_pb2.STREET_NETWORK
+    section.origin.uri = 'poi_uri_from'
+    section.origin.embedded_type = type_pb2.POI
+    section.origin.poi.uri = 'poi_uri_from'
+    section.origin.poi.name = 'poi_name_from'
+    section.street_network.mode = response_pb2.Bss
+    section.destination.uri = 'poi_uri_to'
+    section.destination.embedded_type = type_pb2.POI
+    section.destination.poi.uri = 'poi_uri_to'
+    section.destination.poi.name = 'poi_name_to'
+
+    section = journey.sections.add()
+    section.type = response_pb2.STREET_NETWORK
+    section.origin.uri = 'poi_uri_to'
+    section.origin.embedded_type = type_pb2.POI
+    section.origin.poi.uri = 'poi_uri_to'
+    section.origin.poi.name = 'poi_name_to'
+    section.street_network.mode = response_pb2.Walking
     section.destination.uri = 'address_b'
     section.destination.embedded_type = type_pb2.ADDRESS
 
-    # Add disruption on poi 'poi_uri_a':
+    # Add disruption on poi 'poi_uri_a' (poi of origin):
     pb_disruptions = get_response_with_a_disruption_on_poi(uri="poi_uri_a", name="poi_name_a")
     response.impacts.extend(pb_disruptions.impacts)
+    # Add disruption on poi 'poi_uri_from':
+    pb_disruptions = get_response_with_a_disruption_on_poi(uri="poi_uri_from", name="poi_name_from")
+    response.impacts.extend(pb_disruptions.impacts)
     response.status_code = 200
     return response
 

source/jormungandr/jormungandr/scenarios/tests/new_default_tests.py

@@ -50,6 +50,7 @@
 import pytest
 from pytest_mock import mocker
 from collections import defaultdict
+import copy
 
 """
  sections       0   1   2   3   4   5   6   7   8   9   10
@@ -816,6 +817,8 @@ def journey_with_disruptions_on_poi_test(mocker):
         journey = response_journey_with_pois.journeys[0]
         assert len(journey.sections) == 3
 
+        original_response = copy.deepcopy(response_journey_with_pois)
+
         # Prepare disruptions on poi as response of end point poi_disruptions of loki
         # pt_object poi as impacted object is absent in the response of poi_disruptions
         disruptions_with_poi = helpers_tests.get_response_with_a_disruption_on_poi()
@@ -828,7 +831,8 @@ def journey_with_disruptions_on_poi_test(mocker):
         mock = mocker.patch(
             'jormungandr.scenarios.new_default.get_disruptions_on_poi', return_value=disruptions_with_poi
         )
-        update_disruptions_on_pois(instance, response_journey_with_pois)
+        mocked_request = {'origin_mode': [], 'destination_mode': [], '_disruptions_on_poi': True}
+        update_disruptions_on_pois(instance, mocked_request, response_journey_with_pois)
 
         assert len(response_journey_with_pois.impacts) == 1
         impact = response_journey_with_pois.impacts[0]
@@ -837,8 +841,11 @@ def journey_with_disruptions_on_poi_test(mocker):
 
         # In this state we haven't yet managed the final response so poi object is empty
         helpers_tests.verify_poi_in_impacted_objects(object=object, poi_empty=True)
+        mocked_request = {'origin_mode': [], 'destination_mode': [], '_disruptions_on_poi': True}
+        update_disruptions_on_pois(instance, mocked_request, original_response)
+        assert len(original_response.impacts) == 1
 
-        mock.assert_called_once()
+        mock.assert_called()
         return
 
 

source/navitiacommon/navitiacommon/default_values.py

@@ -100,12 +100,15 @@
 # will be chosen.
 priority = 0
 
-# activate / desactivate call to bss provider
+# activate / deactivate call to bss provider
 bss_provider = True
 
-# activate / desactivate call to car parking provider
+# activate / deactivate call to car parking provider
 car_park_provider = True
 
+# activate / deactivate adding disruptions on poi in journeys
+disruptions_on_poi = False
+
 # Maximum number of connections allowed in journeys is calculated as
 # max_additional_connections + minimum connections among the journeys
 max_additional_connections = 2

source/navitiacommon/navitiacommon/models/__init__.py

@@ -464,6 +464,10 @@ class Instance(db.Model):  # type: ignore
         db.Boolean, default=default_values.car_park_provider, nullable=False, server_default=true()
     )
 
+    disruptions_on_poi = db.Column(
+        db.Boolean, default=default_values.disruptions_on_poi, nullable=False, server_default=false()
+    )
+
     max_additional_connections = db.Column(
         db.Integer, default=default_values.max_additional_connections, nullable=False, server_default='2'
     )

source/tyr/migrations/versions/ca5a0a70a9ba_add_attribute_disruptions_on_poi.py

@@ -0,0 +1,26 @@
+"""
+Add attribute disruptions_on_poi in the table instance with default value False
+
+Revision ID: ca5a0a70a9ba
+Revises: c3ba234a3b9c
+Create Date: 2024-12-04 11:47:52.807680
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'ca5a0a70a9ba'
+down_revision = 'c3ba234a3b9c'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+
+def upgrade():
+    op.add_column(
+        'instance', sa.Column('disruptions_on_poi', sa.Boolean(), server_default='false', nullable=False)
+    )
+
+
+def downgrade():
+    op.drop_column('instance', 'disruptions_on_poi')

source/tyr/tests/integration/instance_test.py

@@ -553,27 +553,31 @@ def test_update_forgotten_attributs_in_backend(create_instance):
     assert resp[0]['car_park_provider'] is True
     assert resp[0]['filter_odt_journeys'] is False
     assert resp[0]['additional_parameters'] is False
+    assert resp[0]['disruptions_on_poi'] is False
 
     params = {
         'max_additional_connections': 3,
         'successive_physical_mode_to_limit_id': 'physical_mode:Train',
         'car_park_provider': False,
         'filter_odt_journeys': True,
         'additional_parameters': True,
+        'disruptions_on_poi': True,
     }
     resp = api_put('/v0/instances/fr', data=json.dumps(params), content_type='application/json')
     assert resp['max_additional_connections'] == 3
     assert resp['successive_physical_mode_to_limit_id'] == 'physical_mode:Train'
     assert resp['car_park_provider'] is False
     assert resp['filter_odt_journeys'] is True
     assert resp['additional_parameters'] is True
+    assert resp['disruptions_on_poi'] is True
 
     resp = api_get('/v0/instances/fr')
     assert resp[0]['max_additional_connections'] == 3
     assert resp[0]['successive_physical_mode_to_limit_id'] == 'physical_mode:Train'
     assert resp[0]['car_park_provider'] is False
     assert resp[0]['filter_odt_journeys'] is True
     assert resp[0]['additional_parameters'] is True
+    assert resp[0]['disruptions_on_poi'] is True
 
 
 def test_update_taxi_speed(create_instance):