diff --git a/gi_service/contract/rest/organisation_api_contract.yaml b/gi_service/contract/rest/organisation_api_contract.yaml index 2d38284..92473f5 100644 --- a/gi_service/contract/rest/organisation_api_contract.yaml +++ b/gi_service/contract/rest/organisation_api_contract.yaml @@ -132,7 +132,16 @@ paths: tags: - Organization summary: Get cabinet flow for a president (How the organisation structure changed over time). - description: Returns a d3-force-directed graph data with nodes, links and dates (nodes correspond to the ministers and the links link two minister nodes with a value corresponding to the department count. The dates array contains the list of dates for which the cabinet flow is computed.) + description: >- + Returns d3-force-directed graph data with nodes, links, and dates. + Nodes correspond to ministers; links connect minister nodes across + consecutive included dates, with a value equal to the number of + departments that moved between them. The dates array reports the + processing status for every requested date. Only dates with + status "ok" contribute to nodes and links; dates with status + "no_data" (departmentsCount 0) or "error" are excluded from the + chart. When a middle date is excluded, links bridge directly + between the nearest included dates on either side. operationId: getCabinetFlow parameters: - name: president_id @@ -142,7 +151,7 @@ paths: type: string requestBody: required: true - description: List of dates (max 3) for which to compute the cabinet flow. + description: List of dates (max 10) for which to compute the cabinet flow. content: application/json: schema: @@ -204,30 +213,43 @@ paths: description: IDs of the departments that moved along this link between consecutive dates. dates: type: array + description: >- + One entry per requested date. Entries with status "ok" + are included in nodes and links; entries with status + "no_data" or "error" are reported here but omitted + from the chart. items: type: object required: - date - status - - departmentsCount properties: date: type: string description: The date snapshot (YYYY-MM-DD). status: type: string - description: Status of the data retrieval for this date (e.g. "ok"). + description: >- + Processing status for this date. "ok" — data + retrieved and included in nodes/links. + "no_data" — no departments on this date + (departmentsCount 0); excluded from nodes/links. + "error" — retrieval failed; excluded from + nodes/links. departmentsCount: type: integer - description: Total number of active departments on this date. + description: >- + Total number of active departments on this date. + Present and 0 when status is "no_data"; omitted + when status is "error". example: nodes: - id: "2289-43_min_1" name: "Minister of Defence" - time: "2024-09-23" + time: "2024-01-01" - id: "2289-43_min_1" name: "Minister of Defence" - time: "2024-09-24" + time: "2024-03-01" links: - source: 0 target: 1 @@ -236,10 +258,13 @@ paths: - "2289-43_dep_1" - "2289-43_dep_2" dates: - - date: "2024-09-23" + - date: "2024-01-01" status: "ok" departmentsCount: 449 - - date: "2024-09-24" + - date: "2024-02-01" + status: "no_data" + departmentsCount: 0 + - date: "2024-03-01" status: "ok" departmentsCount: 449 "400": diff --git a/src/services/organisation_service.py b/src/services/organisation_service.py index 1c5091a..35db10a 100644 --- a/src/services/organisation_service.py +++ b/src/services/organisation_service.py @@ -471,7 +471,7 @@ async def get_ministers_and_departments(self, president_id: str, selected_date: raise InternalServerError("An unexpected error occurred") from e # API: cabinet flow for the given president id and date range of the presidency - async def fetch_cabinet_flow(self, president_id: str, dates: Sequence[str], max_dates: int = 3): + async def fetch_cabinet_flow(self, president_id: str, dates: Sequence[str], max_dates: int = 10): """ Fetch Cabinet Flow @@ -495,6 +495,10 @@ async def fetch_cabinet_flow(self, president_id: str, dates: Sequence[str], max_ {"date": "", "status": "", "departmentsCount": } ] } + + Only dates with status "ok" contribute to nodes and links. Dates with + status "no_data" (departmentsCount 0) or "error" are excluded from the + chart; consecutive included dates are linked directly across skipped dates. """ if len(dates) > max_dates: @@ -510,12 +514,11 @@ async def fetch_cabinet_flow(self, president_id: str, dates: Sequence[str], max_ ] dates_gov_struct = await asyncio.gather(*tasks_for_dates, return_exceptions=True) - departments_by_ministers = {} # maps department_id -> [node_index_at_date0, node_index_at_date1, ...] to track which minister held each department across dates + departments_by_ministers = {} # maps department_id -> [node_index_at_chart_slot0, ...] across included dates only name_lookup = {} # maps entity_id -> human-readable name, populated after fetching minister names - expected_slots = len(dates) # number of dates requested, used to initialise fixed-size timeline slots per department nodes: list[dict[str, str]] = [] # list of graph nodes, each representing a minister at a specific date e.g. {"id": "minister_001", "time": "2015-01-01"} - node_indices: dict[tuple[str, int], int] = {} # maps (minister_id, date_index) -> index in `nodes`, avoids duplicate nodes for the same minister at the same date - links_departments: dict[tuple[int, int], list[str]] = {} # maps (source_node_index, target_node_index) -> department ids that moved between those two ministers across consecutive dates + node_indices: dict[tuple[str, int], int] = {} # maps (minister_id, chart_index) -> index in `nodes`, avoids duplicate nodes for the same minister at the same chart slot + links_departments: dict[tuple[int, int], list[str]] = {} # maps (source_node_index, target_node_index) -> department ids that moved between those two ministers across consecutive included dates date_status: list[dict[str, object]] = [ {"date": d, "status": "pending"} for d in dates ] # tracks processing status per date ("pending" -> "ok" / "error" / "no_data") for the response metadata @@ -551,6 +554,19 @@ async def fetch_cabinet_flow(self, president_id: str, dates: Sequence[str], max_ "departmentsCount": len(result), } + included_indices = [i for i, s in enumerate(date_status) if s["status"] == "ok"] + chart_slot_count = len(included_indices) + + for chart_index, date_index in enumerate(included_indices): + result = sorted( + dates_gov_struct[date_index], + key=lambda r: ( + (r.get("ministerId") or "", r.get("departmentId") or "") + if isinstance(r, dict) + else ("", "") + ), + ) + for relation in result: if not isinstance(relation, dict): continue @@ -565,8 +581,8 @@ async def fetch_cabinet_flow(self, president_id: str, dates: Sequence[str], max_ # eg: nodes = [{"id": "minister_001", "time": "2015-01-01"}, {"id": "minister_002", "time": "2015-01-01"}] node_index = None if minister_id: - node_key = (minister_id, date_index) # e.g. ("minister_001", 0) — unique key per minister per date - node_index = node_indices.get(node_key) # check if this minister already has a node for this date + node_key = (minister_id, chart_index) # e.g. ("minister_001", 0) — unique key per minister per chart slot + node_index = node_indices.get(node_key) # check if this minister already has a node for this chart slot if node_index is None: node_index = len(nodes) node_indices[node_key] = node_index @@ -575,17 +591,17 @@ async def fetch_cabinet_flow(self, president_id: str, dates: Sequence[str], max_ "time": dates[date_index] }) - # For each department, maintain a "timeline" across requested dates. - # departments_by_ministers[department_id] = [node_index_at_date0, node_index_at_date1, ...] - # Each node_index points into `nodes` (which is keyed by (minister_id, date_index)). - timeline = departments_by_ministers.get(department_id) # reuse the same list across dates + # For each department, maintain a "timeline" across included dates. + # departments_by_ministers[department_id] = [node_index_at_chart_slot0, ...] + # Each node_index points into `nodes` (which is keyed by (minister_id, chart_index)). + timeline = departments_by_ministers.get(department_id) # reuse the same list across chart slots if timeline is None: - timeline = [None] * expected_slots + timeline = [None] * chart_slot_count departments_by_ministers[department_id] = timeline - # Compare consecutive dates for this department to detect a move. - # previous_index is the minister-node at dates[date_index - 1]; node_index is at dates[date_index]. - previous_index = timeline[date_index - 1] if date_index > 0 else None - timeline[date_index] = node_index + # Compare consecutive included dates for this department to detect a move. + # previous_index is the minister-node at the prior chart slot; node_index is at chart_index. + previous_index = timeline[chart_index - 1] if chart_index > 0 else None + timeline[chart_index] = node_index # Aggregate movements: each department that goes from previous_index -> node_index is added to that link. if previous_index is not None and node_index is not None: diff --git a/test/test_organisation_service.py b/test/test_organisation_service.py index a6b93f9..ae33c7e 100644 --- a/test/test_organisation_service.py +++ b/test/test_organisation_service.py @@ -787,7 +787,7 @@ async def test_fetch_and_map_relations_with_errors( @pytest.mark.asyncio async def test_fetch_cabinet_flow_too_many_dates(organisation_service): president_id = "pres1" - dates = ["2024-09-23", "2024-09-24", "2024-09-25", "2024-09-26"] + dates = [f"2024-09-{day:02d}" for day in range(1, 12)] # 11 dates; max allowed is 10 with pytest.raises(BadRequestError): await organisation_service.fetch_cabinet_flow(president_id, dates) @@ -879,7 +879,9 @@ async def test_no_departments(organisation_service): assert result["nodes"] == [] assert result["links"] == [] assert result["dates"][0]["status"] == "no_data" + assert result["dates"][0]["departmentsCount"] == 0 assert result["dates"][1]["status"] == "no_data" + assert result["dates"][1]["departmentsCount"] == 0 @pytest.mark.asyncio @@ -933,12 +935,80 @@ async def test_no_departments_for_one_date(organisation_service): # date statuses should be ok and one date should be no_data assert result["dates"][0]["status"] == "ok" + assert result["dates"][0]["departmentsCount"] == 4 assert result["dates"][1]["status"] == "no_data" + assert result["dates"][1]["departmentsCount"] == 0 # dependency should be called once per date assert organisation_service.get_ministers_and_departments.call_count == 2 +@pytest.mark.asyncio +async def test_bridge_across_empty_middle_date(organisation_service): + organisation_service.get_ministers_and_departments = AsyncMock( + side_effect=[ + [ + {"ministerId": "min4", "departmentId": "dep177"}, + {"ministerId": "min4", "departmentId": "dep175"}, + {"ministerId": "min4", "departmentId": "dep182"}, + {"ministerId": "min4", "departmentId": "dep176"}, + ], + [], + [ + {"ministerId": "min9", "departmentId": "dep177"}, + {"ministerId": "min4", "departmentId": "dep175"}, + {"ministerId": "min4", "departmentId": "dep182"}, + {"ministerId": "min10", "departmentId": "dep176"}, + ], + ] + ) + + mock_entity1 = MagicMock() + mock_entity1.id = "min4" + mock_entity1.name = "Minister 4" + + mock_entity2 = MagicMock() + mock_entity2.id = "min9" + mock_entity2.name = "Minister 9" + + mock_entity3 = MagicMock() + mock_entity3.id = "min10" + mock_entity3.name = "Minister 10" + + organisation_service.opengin_service.get_entities = AsyncMock( + return_value=[mock_entity1, mock_entity2, mock_entity3] + ) + + result = await organisation_service.fetch_cabinet_flow( + president_id="pres1", + dates=["2024-01-01", "2024-02-01", "2024-03-01"], + ) + + assert result["dates"][0]["status"] == "ok" + assert result["dates"][0]["departmentsCount"] == 4 + assert result["dates"][1]["status"] == "no_data" + assert result["dates"][1]["departmentsCount"] == 0 + assert result["dates"][2]["status"] == "ok" + assert result["dates"][2]["departmentsCount"] == 4 + + # links bridge across the empty middle date (same as two consecutive ok dates) + assert len(result["links"]) == 3 + total_flow = sum(link["value"] for link in result["links"]) + assert total_flow == 4 + + all_department_ids = { + department_id + for link in result["links"] + for department_id in link["departmentIds"] + } + assert all_department_ids == {"dep175", "dep176", "dep177", "dep182"} + + node_times = {node["time"] for node in result["nodes"]} + assert node_times == {"2024-01-01", "2024-03-01"} + + assert organisation_service.get_ministers_and_departments.call_count == 3 + + @pytest.mark.asyncio async def test_one_date_failure(organisation_service): organisation_service.get_ministers_and_departments = AsyncMock(