Merge pull request #735 from Hestia-Homes/bug/categorisation-failure

Handle categorisation when all plans have zero cost

backend/categorisation/categorisation_trigger_request.py

@@ -9,4 +9,4 @@ class CategorisationTriggerRequest(BaseModel):
     scenario_priority_order: Optional[List[int]] = None
 
 
-# {"portfolio_id": 556, "plans_to_consider": [1589319,1589320], "plan_priority_order": [1589319,1589320]}
+# {"portfolio_id": 556, "scenarios_to_consider": [1039,1041], "scenario_priority_order": [1041,1039]}

backend/categorisation/local_handler/invoke_local_lambda.py

@@ -10,8 +10,8 @@ payload = {
             "body": json.dumps(
                 {
                     "portfolio_id": 556,
-                    "scenarios_to_consider": [1039, 1041],
-                    "scenarios_priority_order": [],
+                    "scenarios_to_consider": [],
+                    "scenario_priority_order": [],
                 }
             )
         }

backend/categorisation/local_runner.py

@@ -1,10 +1,18 @@
+from typing import List
+
 from backend.categorisation.processor import process_portfolio
 
 
 def main() -> None:
     portfolio_id = 556
+    scenarios_to_consider: List[int] = []
+    scenario_priority_order: List[int] = []
 
-    process_portfolio(portfolio_id)
+    process_portfolio(
+        portfolio_id=portfolio_id,
+        scenarios_to_consider=scenarios_to_consider,
+        scenario_priority_order=scenario_priority_order,
+    )
 
 
 if __name__ == "__main__":

backend/categorisation/processor.py

@@ -37,8 +37,10 @@ def process_portfolio(
         )
 
     plans: List[Plan] = _load_plans_for_portfolio(portfolio_id, scenarios_to_consider)
+    logger.info(f"Successfully loaded {len(plans)}")
 
     plans_by_property: Dict[int, List[Plan]] = _group_plans_by_property(plans)
+    logger.info("Successfully grouped plans by property")
 
     updated_plan_models: List[PlanModel] = []
     updated_scenario_models: List[ScenarioModel] = []
@@ -51,6 +53,7 @@ def process_portfolio(
         cheapest_plan = choose_cheapest_relevant_plan(
             property_plans, scenario_priority_order
         )
+        logger.info(f"Successfully found cheapest plan for Property {property_id}")
 
         updated_property_plan_models, updated_property_scenario_models = (
             _update_plan_and_scenario_objects(property_plans, cheapest_plan)
@@ -60,6 +63,7 @@ def process_portfolio(
         updated_scenario_models.extend(updated_property_scenario_models)
 
     if len(updated_plan_models) > 0:
+        logger.info(f"Updating {len(updated_plan_models)} Plans in database")
         bulk_update_plans(updated_plan_models, updated_scenario_models)
         logger.info("Successfully updated Plan default values in database")
 
@@ -90,11 +94,14 @@ def choose_cheapest_relevant_plan(
         for plan in eligible_plans
     )
 
-    cheapest_plans: List[Plan] = [
-        plan
-        for plan in eligible_plans
-        if (plan.record.cost_of_works or float("inf")) == min_cost
-    ]
+    if all(p.record.cost_of_works == 0 for p in eligible_plans):
+        cheapest_plans = eligible_plans
+    else:
+        cheapest_plans: List[Plan] = [
+            plan
+            for plan in eligible_plans
+            if (plan.record.cost_of_works or float("inf")) == min_cost
+        ]
 
     for priority_scenario_id in scenario_priority_order:
         for plan in cheapest_plans:
@@ -116,9 +123,10 @@ def _unset_defaults_for_scenarios_not_being_considered(
         if id not in scenarios_to_consider:
             scenarios_to_unset_default.append(id)
 
-    logger.info(
-        f"Unsetting {scenarios_to_unset_default} as default scenario(s) as not included in provided list of scenarios to consider"
-    )
+    if len(scenarios_to_unset_default) > 0:
+        logger.info(
+            f"Unsetting {scenarios_to_unset_default} as default scenario(s) as not included in provided list of scenarios to consider"
+        )
 
     if len(scenarios_to_unset_default) > 0:
         plans_to_unset_default: List[int] = get_plan_ids_by_scenario_ids(
@@ -133,9 +141,9 @@ def _load_plans_for_portfolio(
 ) -> List[Plan]:
 
     if scenarios_to_consider:
-        logger.info(f"Getting {len(scenarios_to_consider)} plans")
+        logger.info(f"Getting plans for {len(scenarios_to_consider)} scenarios")
         plan_models: List[PlanModel] = get_plans_by_scenario_ids(scenarios_to_consider)
-
+        logger.info(f"Got {len(plan_models)} plan models from database")
     else:
         logger.info(
             f"No list of Plans to consider provided. Getting all Plans for portfolio {portfolio_id}"
@@ -159,11 +167,8 @@ def _load_plans_for_portfolio(
         plans.append(
             Plan.from_sqlalchemy(model, Scenario.from_sqlalchemy(scenario_model))
         )
-        logger.debug(
-            f"Successfully mapped plan {model.id} and scenario {scenario_model.id} to domain object"
-        )
 
-    logger.debug(f"Got {len(plans)} plans from database")
+    logger.info(f"Got {len(plans)} Plans")
     return plans
 
 

backend/categorisation/tests/test_prioritised_plan_selected.py

@@ -1,5 +1,5 @@
 from datetime import datetime
-from typing import List
+from typing import List, Optional
 import pytest
 
 from backend.app.domain.classes.plan import Plan
@@ -16,7 +16,7 @@ def created_at_datetime() -> datetime:
 
 
 def make_plan_record(
-    created_at: datetime, default: bool, cost_of_works: float = 500.0
+    created_at: datetime, default: bool, cost_of_works: Optional[float] = 500.0
 ) -> PlanRecord:
     return PlanRecord(
         property_id=1,
@@ -43,7 +43,10 @@ def make_scenario(name: str, created_at: datetime, is_default: bool) -> Scenario
 
 
 def make_plan(
-    created_at: datetime, default: bool, cost_of_works: float = 500.0, name: str = ""
+    created_at: datetime,
+    default: bool,
+    cost_of_works: Optional[float] = 500.0,
+    name: str = "",
 ) -> Plan:
     scenario = make_scenario(name, created_at, default)
     plan_id = 1 if default else 2
@@ -92,3 +95,45 @@ def test_cheapest_plan_returned_if_not_in_priority_list(
 
     # assert
     assert actual_default_plan.id == expected_default_plan_id
+
+
+def test_all_plans_zero_cost__highest_priority_returned(
+    created_at_datetime: datetime,
+) -> None:
+    # arrange
+    epc_c_plan = make_plan(created_at_datetime, True, cost_of_works=0.0, name="EPC C")
+    minor_works_plan = make_plan(
+        created_at_datetime, False, cost_of_works=0.0, name="EPC C - Minor Works"
+    )
+    scenario_priority_order: List[int] = [4, 3]
+    expected_default_plan_id = 2
+
+    # act
+    actual_default_plan = choose_cheapest_relevant_plan(
+        plans=[epc_c_plan, minor_works_plan],
+        scenario_priority_order=scenario_priority_order,
+    )
+
+    # assert
+    assert actual_default_plan.id == expected_default_plan_id
+
+
+def test_all_plans_null_cost__highest_priority_returned(
+    created_at_datetime: datetime,
+) -> None:
+    # arrange
+    epc_c_plan = make_plan(created_at_datetime, True, cost_of_works=None, name="EPC C")
+    minor_works_plan = make_plan(
+        created_at_datetime, False, cost_of_works=None, name="EPC C - Minor Works"
+    )
+    scenario_priority_order: List[int] = [4, 3]
+    expected_default_plan_id = 2
+
+    # act
+    actual_default_plan = choose_cheapest_relevant_plan(
+        plans=[epc_c_plan, minor_works_plan],
+        scenario_priority_order=scenario_priority_order,
+    )
+
+    # assert
+    assert actual_default_plan.id == expected_default_plan_id