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

Handle categorisation when all plans have zero cost

backend/app/domain/classes/plan.py

@@ -60,6 +60,14 @@ class Plan:
             case _:
                 raise NotImplementedError
 
+    @property
+    def cost(self) -> float:
+        return (
+            self.record.cost_of_works
+            if self.record.cost_of_works is not None
+            else float("inf")
+        )
+
     def to_sqlalchemy(self) -> PlanPersistence:
         scenario_record = self.scenario.record
 

backend/categorisation/processor.py

@@ -50,10 +50,13 @@ def process_portfolio(
         if not property_plans:
             raise ValueError(f"No plans for property {property_id}")
 
-        cheapest_plan = choose_cheapest_relevant_plan(
-            property_plans, scenario_priority_order
-        )
-        logger.info(f"Successfully found cheapest plan for Property {property_id}")
+        try:
+            cheapest_plan = choose_cheapest_relevant_plan(
+                property_plans, scenario_priority_order
+            )
+        except Exception:
+            logger.error(f"Failed to find cheapest plan for property {property_id}")
+            raise
 
         updated_property_plan_models, updated_property_scenario_models = (
             _update_plan_and_scenario_objects(property_plans, cheapest_plan)
@@ -85,23 +88,11 @@ def choose_cheapest_relevant_plan(
                 f"All plans must have an ID, but found a plan with no ID: {plan}"
             )
 
-    min_cost: float = min(
-        (
-            plan.record.cost_of_works
-            if plan.record.cost_of_works is not None
-            else float("inf")
-        )
-        for plan in eligible_plans
-    )
+    min_cost: float = min(plan.cost for plan in eligible_plans)
 
-    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
-        ]
+    cheapest_plans: List[Plan] = [
+        plan for plan in eligible_plans if plan.cost == min_cost
+    ]
 
     for priority_scenario_id in scenario_priority_order:
         for plan in cheapest_plans:

backend/categorisation/tests/test_prioritised_plan_selected.py

@@ -118,6 +118,27 @@ def test_all_plans_zero_cost__highest_priority_returned(
     assert actual_default_plan.id == expected_default_plan_id
 
 
+def test_some_plans_zero_cost__cheapest_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=50.0, name="EPC C - Minor Works"
+    )
+    scenario_priority_order: List[int] = [4, 3]
+    expected_default_plan_id = 1
+
+    # 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: