refactor: address PR review feedback

chore: cleanup

Author: shon-button

bc_obps/common/migrations/0131_update_dashboard_data.py

@@ -0,0 +1,19 @@
+from django.db import migrations
+
+from common.utils import reset_dashboard_data
+
+
+def reset_dashboard_data_fn(_, schema_editor):
+    reset_dashboard_data()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("common", "0131_V5_15_1"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            code=reset_dashboard_data_fn,
+        ),
+    ]

bc_obps/common/migrations/0132_update_dashboard_data.py

@@ -7,7 +7,7 @@
 
 class TestOperationsPreviousReportableEndpoint(CommonTestSetup):
     @patch("service.operation_service.OperationService.list_previous_reportable_operations")
-    def test_list_previous_reportable_operations(
+    def test_calls_and_returns_the_service_method_output(
         self,
         mock_list_previous_reportable_operations,
     ):
@@ -40,11 +40,15 @@ def test_list_previous_reportable_operations(
 
         assert response.status_code == 200
         assert len(response.json()) == 1
-        assert response.json()[0].get("operation_name") == operation.name
-        assert response.json()[0].get("operation_id") == str(operation.id)
-        assert response.json()[0].get("reporting_year") == 2023
-        assert response.json()[0].get("registration_purposes") == [
-            Operation.Purposes.REPORTING_OPERATION,
+        assert response.json() == [
+            {
+                "operation_id": str(operation.id),
+                "operation_name": operation.name,
+                "reporting_year": 2023,
+                "registration_purposes": [
+                    str(Operation.Purposes.REPORTING_OPERATION),
+                ],
+            }
         ]
 
         mock_list_previous_reportable_operations.assert_called_once()

bc_obps/registration/tests/endpoints/_operations/test_reportable.py

@@ -258,4 +258,6 @@ def test_create_report_for_reporting_year(
 
         # Verify that the selected registration purpose was passed to the service
         _, kwargs = mock_create_report_for_reporting_year.call_args
+        assert kwargs["data"].operation_id == operation.id
+        assert kwargs["data"].reporting_year == 2023
         assert kwargs["data"].registration_purpose == Operation.Purposes.REPORTING_OPERATION

bc_obps/reporting/tests/api/test_reports.py

@@ -67,22 +67,21 @@ def get_operation_designated_operator_for_reporting_year(
     def get_operation_designated_operators_for_reporting_years(
         cls,
         operation_ids: set[UUID],
-        reporting_years: set[int],
+        min_year: int,
+        max_year: int,
     ) -> dict[tuple[UUID, int], OperationDesignatedOperatorTimelinePlus]:
         """
-        Bulk version of get_operation_designated_operator_for_reporting_year
+        Bulk version of get_operation_designated_operator_for_reporting_year.
 
-        It is intended for scenarios where many operation/reporting year
-        combinations need to be evaluated, such as determining the list of reportable operations
+        Returns designated operator timelines for every operation/year combination
+        between min_year and max_year, inclusive.
 
-        Returns a lookup keyed by (operation_id, reporting_year)
+        Returns a lookup keyed by (operation_id, reporting_year).
         """
-
-        if not operation_ids or not reporting_years:
+        if not operation_ids:
             return {}
 
-        min_year = min(reporting_years)
-        max_year = max(reporting_years)
+        reporting_years = range(min_year, max_year + 1)
 
         timelines = (
             OperationDesignatedOperatorTimeline.objects.select_related(
@@ -100,24 +99,24 @@ def get_operation_designated_operators_for_reporting_years(
         lookup: dict[tuple[UUID, int], OperationDesignatedOperatorTimelinePlus] = {}
 
         for timeline in timelines:
-            start_date = timeline.start_date
-            end_date = timeline.end_date
-
-            if start_date is None:
+            if timeline.start_date is None:
                 continue
 
+            start_year = timeline.start_date.year
+            end_year = timeline.end_date.year if timeline.end_date else None
+
             for reporting_year in reporting_years:
-                if start_date.year > reporting_year:
+                if start_year > reporting_year:
                     continue
 
-                if end_date is not None and end_date.year <= reporting_year:
+                if end_year is not None and end_year <= reporting_year:
                     continue
 
                 lookup[(timeline.operation_id, reporting_year)] = OperationDesignatedOperatorTimelinePlus(
                     operation=timeline.operation,
                     operator=timeline.operator,
-                    start_date=start_date,
-                    end_date=end_date,
+                    start_date=timeline.start_date,
+                    end_date=timeline.end_date,
                 )
 
         return lookup

bc_obps/service/operation_designated_operator_timeline_service.py

@@ -909,7 +909,8 @@ def list_previous_reportable_operations(
         designations_lookup = (
             OperationDesignatedOperatorTimelineService.get_operation_designated_operators_for_reporting_years(
                 operation_ids=all_operation_ids,
-                reporting_years=year_values,
+                min_year=min(year_values),
+                max_year=max(year_values),
             )
         )
 

bc_obps/service/operation_service.py

@@ -94,20 +94,20 @@ def test_get_operation_designated_operator_for_reporting_year():
         assert result_none is None
 
     @staticmethod
-    def test_get_operation_designated_operators_for_reporting_years():
-        operation = baker.make_recipe('registration.tests.utils.operation')
-        operator1 = baker.make_recipe('registration.tests.utils.operator')
-        operator2 = baker.make_recipe('registration.tests.utils.operator')
+    def test_get_operation_designated_operators_for_reporting_years_returns_matching_timelines():
+        operation = baker.make_recipe("registration.tests.utils.operation")
+        operator1 = baker.make_recipe("registration.tests.utils.operator")
+        operator2 = baker.make_recipe("registration.tests.utils.operator")
 
         timeline1 = baker.make_recipe(
-            'registration.tests.utils.operation_designated_operator_timeline',
+            "registration.tests.utils.operation_designated_operator_timeline",
             operation=operation,
             operator=operator1,
             start_date=timezone.datetime(2024, 6, 1).date(),
             end_date=timezone.datetime(2025, 5, 31).date(),
         )
         timeline2 = baker.make_recipe(
-            'registration.tests.utils.operation_designated_operator_timeline',
+            "registration.tests.utils.operation_designated_operator_timeline",
             operation=operation,
             operator=operator2,
             start_date=timezone.datetime(2025, 5, 31).date(),
@@ -119,6 +119,7 @@ def test_get_operation_designated_operators_for_reporting_years():
             reporting_years={2023, 2024, 2025},
         )
 
+        assert len(result) == 2
         assert (operation.id, 2023) not in result
 
         result_2024 = result[(operation.id, 2024)]
@@ -135,18 +136,22 @@ def test_get_operation_designated_operators_for_reporting_years():
         assert result_2025.end_date == timeline2.end_date
         assert result_2025.has_been_transferred is False
 
-        empty_result = (
-            OperationDesignatedOperatorTimelineService.get_operation_designated_operators_for_reporting_years(
-                operation_ids=set(),
-                reporting_years={2024, 2025},
-            )
+    @staticmethod
+    def test_get_operation_designated_operators_for_reporting_years_returns_empty_when_operation_ids_empty():
+        result = OperationDesignatedOperatorTimelineService.get_operation_designated_operators_for_reporting_years(
+            operation_ids=set(),
+            reporting_years={2024, 2025},
         )
-        assert empty_result == {}
 
-        empty_result = (
-            OperationDesignatedOperatorTimelineService.get_operation_designated_operators_for_reporting_years(
-                operation_ids={operation.id},
-                reporting_years=set(),
-            )
+        assert result == {}
+
+    @staticmethod
+    def test_get_operation_designated_operators_for_reporting_years_returns_empty_when_reporting_years_empty():
+        operation = baker.make_recipe("registration.tests.utils.operation")
+
+        result = OperationDesignatedOperatorTimelineService.get_operation_designated_operators_for_reporting_years(
+            operation_ids={operation.id},
+            reporting_years=set(),
         )
-        assert empty_result == {}
+
+        assert result == {}