Fix department name in department history response

gi_service/contract/rest/organisation_api_contract.yaml

@@ -425,6 +425,8 @@ paths:
                 items:
                   type: object
                   properties:
+                    department_name:
+                      type: string
                     ministry_id:
                       type: string
                     ministry_name:
@@ -436,7 +438,14 @@ paths:
                     period:
                       type: string
               example:
-                - ministry_id: "min_01"
+                - department_name: "Department of Treasury Operations"
+                  ministry_id: "min_01"
+                  ministry_name: "Ministry of Finance"
+                  minister_id: "pers_01"
+                  minister_name: "John Doe"
+                  period: "2021 Jan 01 - 2022 Jan 01"
+                - department_name: "Department of Treasury Operations"
+                  ministry_id: "min_01"
                   ministry_name: "Ministry of Finance"
                   minister_id: "pers_01"
                   minister_name: "John Doe"
@@ -971,4 +980,4 @@ components:
         name:
           type: string
         id:
-          type: string
+          type: string

src/services/organisation_service.py

@@ -678,20 +678,38 @@ async def department_history_timeline(self, department_id: str):
         try:
             # 1. Get Lineage and Initial Relations
             department_ids = await self._get_renamed_lineage(department_id)
+            department_id_list = list(department_ids)
 
-            ministry_department_relation_map = await self._fetch_and_map_relations(
-                list(department_ids), 
-                Relation(name="AS_DEPARTMENT", direction="INCOMING")
+            department_info_map, ministry_department_relation_map = await asyncio.gather(
+                self._fetch_and_map_entities(department_id_list),
+                self._fetch_and_map_relations(
+                    department_id_list,
+                    Relation(name="AS_DEPARTMENT", direction="INCOMING")
+                )
             )
+
+            department_name_map = {
+                entity_id: Util.decode_protobuf_attribute_name(entity.name)
+                for entity_id, entity in department_info_map.items()
+            }
 
             # Filter out same startTime and endTime min-dep relations
             all_ministry_department_relations = [
-                relation for relations in ministry_department_relation_map.values() for relation in relations 
+                {
+                    "department_id": current_department_id,
+                    "department_name": department_name_map.get(current_department_id, ""),
+                    "relation": relation,
+                }
+                for current_department_id, relations in ministry_department_relation_map.items()
+                for relation in relations
                 if relation.startTime != relation.endTime
             ]
 
             # 2. Fetch all Ministry Info and Person Appointment relations in parallel
-            ministry_ids = list(set(relation.relatedEntityId for relation in all_ministry_department_relations))
+            ministry_ids = list(set(
+                relation_item["relation"].relatedEntityId
+                for relation_item in all_ministry_department_relations
+            ))
 
             ministry_info_map, person_appointment_relation_map = await asyncio.gather(
                 self._fetch_and_map_entities(ministry_ids),
@@ -704,13 +722,16 @@ async def department_history_timeline(self, department_id: str):
 
             # 4. Filter person appointments that overlap with this specific ministry-department period
             enriched = []
-            for ministry_department_relation in all_ministry_department_relations:
+            for ministry_department_item in all_ministry_department_relations:
+                ministry_department_relation = ministry_department_item["relation"]
                 ministry_id = ministry_department_relation.relatedEntityId
                 ministry_entity = ministry_info_map.get(ministry_id)
                 if not ministry_entity:
                     continue
 
                 ministry_name = Util.decode_protobuf_attribute_name(ministry_entity.name)
+                current_department_name = ministry_department_item["department_name"]
+                current_department_id = ministry_department_item["department_id"]
 
                 relevant_persons = []
                 for person_appointment in person_appointment_relation_map.get(ministry_id, []):
@@ -723,6 +744,8 @@ async def department_history_timeline(self, department_id: str):
 
                     if overlap_start < overlap_end:
                         relevant_persons.append({
+                            "_department_source_id": current_department_id,
+                            "department_name": current_department_name,
                             "ministry_id": ministry_id,
                             "ministry_name": ministry_name,
                             "minister_id": person_entity.id,
@@ -739,15 +762,21 @@ async def department_history_timeline(self, department_id: str):
                 for person in relevant_persons:
                     if current_time < person["startTime"]:
                         enriched.append({
+                            "_department_source_id": current_department_id,
+                            "department_name": current_department_name,
                             "ministry_id": ministry_id, "ministry_name": ministry_name,
-                            "minister_id": None, "startTime": current_time, "endTime": person["startTime"]
+                            "minister_id": None, "minister_name": None,
+                            "startTime": current_time, "endTime": person["startTime"]
                         })
                     current_time = person["endTime"]
 
                 if current_time < relation_end:
                     enriched.append({
+                        "_department_source_id": current_department_id,
+                        "department_name": current_department_name,
                         "ministry_id": ministry_id, "ministry_name": ministry_name,
-                        "minister_id": None, "startTime": current_time, "endTime": relation_end
+                        "minister_id": None, "minister_name": None,
+                        "startTime": current_time, "endTime": relation_end
                     })
 
                 enriched.extend(relevant_persons)
@@ -780,6 +809,7 @@ async def department_history_timeline(self, department_id: str):
             collapsed = []
             for entry in enriched:
                 if collapsed and (collapsed[-1]["minister_id"] == entry["minister_id"] and 
+                                  collapsed[-1]["_department_source_id"] == entry["_department_source_id"] and
                                   collapsed[-1]["ministry_name"] == entry["ministry_name"] and 
                                   collapsed[-1]["endTime"] >= entry["startTime"]):
                     collapsed[-1]["endTime"] = max(collapsed[-1]["endTime"], entry["endTime"])
@@ -789,15 +819,16 @@ async def department_history_timeline(self, department_id: str):
             # 7. Final Sort and clean up
             collapsed.sort(key=lambda x: x["startTime"], reverse=True)
             for entry in collapsed:
+                # Keep raw timestamps until the last step because the gap-filling and
+                # collapse logic depends on overlap comparisons.
                 actual_end = None if entry["endTime"] == FAR_FUTURE else entry["endTime"]
                 entry["period"] = Util.term(entry["startTime"], actual_end, get_full_date=True)
 
-                for key in ["startTime", "endTime"]:
+                for key in ["startTime", "endTime", "_department_source_id"]:
                     entry.pop(key, None)
 
             return collapsed
 
         except Exception as e:
             logger.error(f"Error in enrich_department_timeline: {e}")
             raise InternalServerError("An unexpected error occurred") from e
-

test/test_document_service.py

@@ -64,8 +64,14 @@ async def test_get_gazette_data_points_empty(document_service, mock_opengin_serv
 async def test_get_gazette_data_points_internal_server_error(
     document_service, mock_opengin_service
 ):
+    mock_opengin_service.get_entities.side_effect = [
+        [MagicMock(id="org_gazette_1", created="2023-01-15T10:00:00Z")],
+        [],
+    ]
+
     with patch(
-        "src.services.document_service.asyncio.gather", side_effect=Exception("failure")
+        "src.services.document_service.Util.normalize_timestamp",
+        side_effect=Exception("failure"),
     ):
         with pytest.raises(InternalServerError):
             await document_service.get_gazette_data_points()
@@ -142,4 +148,3 @@ async def test_get_gazette_data_points_robustness(document_service, mock_opengin
     assert result["data"][1]["year"] == 2024
     assert result["data"][1]["values"][4] == 1 # May
 
-

test/test_organisation_service.py

@@ -526,6 +526,8 @@ async def fetch_relation_handler(entityId, relation):
 
     async def get_entities_handler(entity):
         mapping = {
+            "dep_01": "4465706172746d656e74204f6e65",
+            "dep_02": "4465706172746d656e742054776f",
             "min_01": "4d696e6973747279204f6e65",
             "min_02": "4d696e69737472792054776f",
             "pers_01": "4d696e69737465722041",
@@ -562,13 +564,17 @@ async def get_entities_handler(entity):
 
     assert len(result) == 6
     assert result[0]["minister_name"] == "President X"
+    assert result[0]["department_name"] == "Department Two"
     assert result[1]["minister_name"] == "Minister A"
     assert result[1]["ministry_name"] == "Ministry Two"
+    assert result[1]["department_name"] == "Department Two"
     assert result[4]["minister_name"] == "Minister A"
     assert result[4]["ministry_name"] == "Ministry One"
+    assert result[4]["department_name"] == "Department One"
     assert "period" in result[0]
     assert "startTime" not in result[0]
     assert "endTime" not in result[0]
+    assert "_department_source_id" not in result[0]
 
 
 @pytest.mark.asyncio
@@ -614,6 +620,13 @@ async def fetch_relation_handler(entityId, relation):
         return []
 
     async def get_entities_handler(entity):
+        if entity.id == "dep_01":
+            return [
+                Entity(
+                    id="dep_01",
+                    name='{"value": "4465706172746d656e74206f66204d65646961"}',
+                )
+            ]
         if entity.id in ["min_01", "min_02"]:
             # "Ministry of Media" in hex
             return [
@@ -636,9 +649,75 @@ async def get_entities_handler(entity):
     # Should collapse into ONE entry because same name and same person across min_01 and min_02
     assert len(result) == 1
     assert result[0]["minister_name"] == "Ranil"
+    assert result[0]["department_name"] == "Department of Media"
     assert result[0]["period"] == "2020-01-01 - 2022-01-01"
 
 
+@pytest.mark.asyncio
+async def test_department_history_timeline_keeps_renamed_departments_separate(
+    organisation_service, mock_opengin_service
+):
+    department_id = "dep_01"
+
+    async def fetch_relation_handler(entityId, relation):
+        if relation.name == "RENAMED_TO":
+            return [Relation(relatedEntityId="dep_02")] if entityId == "dep_01" else []
+        if relation.name == "AS_DEPARTMENT":
+            if entityId == "dep_01":
+                return [
+                    Relation(
+                        relatedEntityId="min_01",
+                        startTime="2020-01-01T00:00:00Z",
+                        endTime="2021-01-01T00:00:00Z",
+                    )
+                ]
+            if entityId == "dep_02":
+                return [
+                    Relation(
+                        relatedEntityId="min_02",
+                        startTime="2021-01-01T00:00:00Z",
+                        endTime="2022-01-01T00:00:00Z",
+                    )
+                ]
+        if relation.name == "AS_APPOINTED":
+            return [
+                Relation(
+                    relatedEntityId="pers_01",
+                    startTime="2020-01-01T00:00:00Z",
+                    endTime="2022-01-01T00:00:00Z",
+                )
+            ]
+        return []
+
+    async def get_entities_handler(entity):
+        mapping = {
+            "dep_01": "4465706172746d656e74204f6e65",
+            "dep_02": "4465706172746d656e742054776f",
+            "min_01": "4d696e6973747279206f66204d65646961",
+            "min_02": "4d696e6973747279206f66204d65646961",
+            "pers_01": "52616e696c",
+        }
+        name_hex = mapping.get(entity.id)
+        return (
+            [Entity(id=entity.id, name=f'{{"value": "{name_hex}"}}')]
+            if name_hex
+            else []
+        )
+
+    mock_opengin_service.fetch_relation.side_effect = fetch_relation_handler
+    mock_opengin_service.get_entities.side_effect = get_entities_handler
+
+    result = await organisation_service.department_history_timeline(
+        department_id=department_id
+    )
+
+    assert len(result) == 2
+    assert result[0]["department_name"] == "Department Two"
+    assert result[0]["period"] == "2021-01-01 - 2022-01-01"
+    assert result[1]["department_name"] == "Department One"
+    assert result[1]["period"] == "2020-01-01 - 2021-01-01"
+
+
 @pytest.mark.asyncio
 async def test_get_renamed_lineage_chain(organisation_service, mock_opengin_service):
     # Chain: A -> B -> C

test/test_person_service.py

@@ -160,9 +160,35 @@ async def test_fetch_person_history_internal_error(
     person_service, mock_opengin_service
 ):
     person_id = "person_123"
-    with patch(
-        "src.services.person_service.asyncio.gather",
-        side_effect=Exception("Gather fail"),
+    mock_opengin_service.fetch_relation.side_effect = [
+        [
+            Relation(
+                relatedEntityId="min_1",
+                startTime="2020-01-01T00:00:00Z",
+                endTime="2021-01-01T00:00:00Z",
+            )
+        ],
+        [],
+    ]
+
+    with (
+        patch.object(
+            person_service,
+            "enrich_history_item",
+            new_callable=AsyncMock,
+            return_value={
+                "id": "min_1",
+                "name": "Ministry One",
+                "term": "2020-01-01 - 2021-01-01",
+                "is_president": False,
+                "start_time": "2020-01-01T00:00:00Z",
+                "end_time": "2021-01-01T00:00:00Z",
+            },
+        ),
+        patch(
+            "src.services.person_service.Util.history_sort_key",
+            side_effect=Exception("Sort fail"),
+        ),
     ):
         with pytest.raises(InternalServerError):
             await person_service.fetch_person_history(person_id)
@@ -699,4 +725,3 @@ async def test_fetch_all_presidents_internal_error(person_service, mock_opengin_
     with pytest.raises(InternalServerError):
         await person_service.fetch_all_presidents()
 
-