4745 implement start past report by shon-button · Pull Request #4783 · bcgov/cas-registration

shon-button · 2026-06-10T20:37:16Z

Ticket: 4745

🚀 Impact

Implemented previous reporting year report creation based on reportable operations for each reporting year, with registration purposes determined by the selected operation type and validated for required report information during report creation
Updated dashboard and grid reporting labels

🔬 Local Testing:

Start Test Environment

Start the API server:
```
cd bc_obps
make prepare_backend
```
Start the app development server:
```
cd bciers && yarn dev-all
```

Test: Start a previous report

Log in using bc-cas-dev
Navigate to http://localhost:3000/reporting/reports
Expected Results:

Tab Current Reporting Year
Tab Previous Years Reporting

Navigate to Previous Years Reporting
Click button File Previous Years Report
Expected Results:

Start report form displays as expected

Select reporting year= 2024 (only)
Select an operation
Select registration purpose
Click Start
Expected Results:

Review Operation Information displays as expected
There is no Sync buton on Review Operation Information, Review Facilities, and Review Facility Information page

Submit the report
Expected Results:

Report submits as expected

Create supplementary report
Submit the report
Expected Results:

Report submits as expected

Test: Cannot start a previous report

Repeat Test: Start a previous report 1-8 with operation Brine LFO - Registered - No BORO and BCGHG ID - name from admin`
Expected Results:

User error message displays

Sepehr-Sobhani

Impressive work! 🤩

Sepehr-Sobhani · 2026-07-03T21:46:21Z

+vi.mock("@bciers/components/form/FormBase", () => ({
+  default: vi.fn(({ children, onChange, onSubmit }) => (
+    <form
+      onSubmit={(event) => {
+        event.preventDefault();
+        onSubmit({
+          formData: {
+            reporting_year: 2023,
+            operation_id: "operation-1",
+            registration_purpose: "Reporting Operation",
+          },
+        });
+      }}
+    >
+      <button
+        type="button"
+        onClick={() =>
+          onChange({
+            formData: {
+              reporting_year: 2023,
+              operation_id: "operation-1",
+              registration_purpose: "Reporting Operation",
+            },
+          })
+        }
+      >
+        Mock Change
+      </button>
+
+      {children}
+    </form>
+  )),
+}));


Do we need to mock FormBase? I think we could just let the test render without mocking.

Sepehr-Sobhani · 2026-07-03T21:50:51Z

Nit: We have a couple of any types in this file. It would be great if we could replace them with the appropriate types.

Sepehr-Sobhani · 2026-07-03T21:53:24Z

+            # the designated operator for the selected reporting year
+            if designated_operator_timeline.operator.id != user_operator.operator_id:
+                raise UserError(
+                    f"This operation was owned by another operation in {reporting_year}, "


owned by another ~~operation~~ operator

Sepehr-Sobhani · 2026-07-03T21:57:17Z

+    def _get_previous_reporting_years(cls) -> QuerySet[ReportingYear]:
+        """
+        Returns reporting years that are eligible for the Start Past Report workflow
+
+        Only reporting years from MIN_REPORTING_YEAR up to (but not including) the current
+        reporting year are returned.
+        """
+        current_reporting_year = ReportingYearService.get_current_reporting_year()
+
+        return ReportingYear.objects.filter(
+            reporting_year__gte=cls.MIN_REPORTING_YEAR,
+            reporting_year__lt=current_reporting_year.reporting_year,
+        ).order_by("-reporting_year")


Should we move this to ReportingYearService?

Sepehr-Sobhani · 2026-07-03T22:32:39Z

+    def create_report_for_reporting_year(
+        cls,
+        user_guid: UUID,
+        data: Any,


A dataclass instead of Any?

Sepehr-Sobhani · 2026-07-03T22:41:09Z

+        operator = operator_baker()
+        operation = operation_baker(


We have some existing operator_baker() and operation_baker() helpers from before we decided to standardize on recipes. I think it would be better to use recipes here as well.

For the existing usages, we can replace them with recipes in a separate tech debt ticket. I'll make a note to create one. 😃

Sepehr-Sobhani · 2026-07-03T22:47:10Z

-        { label: "View annual report(s)", href: "/reports/current-reports" },
-        { label: "View past reports", href: "/reports/previous-years" },
+        { label: "Current Reporting Year", href: "/reports/current-reports" },
+        { label: "Previous Reporting Years", href: "/reports/previous-years" },
      ]
    : [
        //tabs for internal users
-        { label: "View annual reports", href: "/reports/current-reports" },
-        { label: "View past reports", href: "/reports/previous-years" },
+        { label: "Current Reporting Year", href: "/reports/current-reports" },
+        { label: "Previous Reporting Years", href: "/reports/previous-years" },


Since we're changing these labels, should we also update the ones on the dashboard to keep them in sync?

Sepehr-Sobhani · 2026-07-07T20:37:01Z

+def load_dashboard_fixtures(apps, schema_editor):
+    from django.core.management import call_command
+
+    fixture_files = [
+        'common/fixtures/dashboard/bciers/external.json',
+        'common/fixtures/dashboard/bciers/internal.json',
+        'common/fixtures/dashboard/operators/internal.json',
+    ]
+
+    # Delete all existing DashboardData objects
+    DashboardData = apps.get_model('common', 'DashboardData')
+    DashboardData.objects.all().delete()
+
+    # Load the fixtures
+    for fixture in fixture_files:
+        call_command('loaddata', fixture)


We already have a util for it:

cas-registration/bc_obps/common/utils.py

Line 61 in 3c62065

def reset_dashboard_data() -> None:

pbastia

Started going through it and just left a bunch of mostly nitpicks that don't necessarily need to be addressed. Great work so far!

pbastia · 2026-07-07T20:58:40Z

+def load_dashboard_fixtures(apps, schema_editor):
+    from django.core.management import call_command
+
+    fixture_files = [
+        'common/fixtures/dashboard/bciers/external.json',
+        'common/fixtures/dashboard/bciers/internal.json',
+        'common/fixtures/dashboard/operators/internal.json',
+    ]
+
+    # Delete all existing DashboardData objects
+    DashboardData = apps.get_model('common', 'DashboardData')
+    DashboardData.objects.all().delete()
+
+    # Load the fixtures
+    for fixture in fixture_files:
+        call_command('loaddata', fixture)


This code already exists: reset_dashboard_data()in common/utils.py
The migration could just call that function in its RunPython method

pbastia · 2026-07-07T21:38:52Z

+
+class TestOperationsPreviousReportableEndpoint(CommonTestSetup):
+    @patch("service.operation_service.OperationService.list_previous_reportable_operations")
+    def test_list_previous_reportable_operations(


nit: method name

Suggested change

def test_list_previous_reportable_operations(

def test_calls_and_returns_the_service_method_output(

pbastia · 2026-07-07T21:53:31Z

+        reporting_years = ReportingYearService.get_previous_reporting_years()
+
+        # Assert
+        assert list(reporting_years.values_list("reporting_year", flat=True)) == [2024]


nit: we could use assertQuerySetEquals, pytest exposes them too if we don't want to extend TestCase

pbastia · 2026-07-07T22:03:28Z

+        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 empty_result == {}


nit: these could be individual test cases

pbastia · 2026-07-07T22:11:22Z

            ordered=False,
        )
+
+    def test_create_report_for_reporting_year_uses_selected_registration_purpose(self):


nit: maybe we can split those tests in its own testfile, for readability. This one is getting really really long

pbastia · 2026-07-07T22:17:12Z

+            "Only currently registered operations can be used to create a report for this reporting year.",
+        )
+
+    def test_create_report_for_reporting_year_rejects_existing_report(self):


nit: it seems like these tests are all setup the same way, they would be good candidates for parametrization

pbastia · 2026-07-07T22:30:22Z

+            if start_date is None:
+                continue


elements with a None start_date have been filtered out just above, we can remove those two lines

pbastia · 2026-07-07T22:32:09Z

+        min_year = min(reporting_years)
+        max_year = max(reporting_years)


This assumes a continuous set of reporting years, otherwise the method will behave weirdly for disjointed sets. I suggest having the user pass min_year and max_year as parameters instead.

pbastia

A few more things, but this is great! Approved once the non-nit comments are addressed.
Very nice work!

pbastia · 2026-07-08T16:11:02Z



 class ReportingYearService:
+    MIN_REPORTING_YEAR = 2024


Hardcoding this defeats the purpose of storing valid reporting years in the database (I'll make a separate PR to remove 2023 that shouldn't be there). Then we can just filter "All reporting years before the current one"

pbastia · 2026-07-08T16:16:20Z

nit: URL path is a bit redundant, I suggest "/reports/previous-years/start" or "/reports/previous-years/create"

@pbastia

The route path is to match the page title to the breadcrumb:

pbastia · 2026-07-08T16:18:08Z

+      try {
+        const finalReviewData = await getFinalReviewData(version_id);
+        setData(finalReviewData);
+      } catch (error) {
+        console.error("FinalReviewForm.fetchData() failed", error);
+        setError(error instanceof Error ? error : new Error(String(error)));
+      } finally {
+        setLoading(false);


We might want to raise an error instead to have our error boundary catch it and notify Sentry, otherwise this just disappears and relies on users emailing us about it

pbastia · 2026-07-08T16:20:09Z

+      <div className="flex justify-start gap-3 pt-6">
+        <Button
+          variant="outlined"
+          onClick={() => router.back()}


This should probably navigate to the past reports page, independently of how the user got here

pbastia · 2026-07-08T16:28:27Z

+      {
+        body: JSON.stringify(payload),
+      },
+    )) as CreateReportResponse | number;


The naming makes it sound like the "CreateReportResponse" is the valid, nominal case

Suggested change

)) as CreateReportResponse | number;

)) as CreateReportErrorResponse | number;

pbastia · 2026-07-08T16:56:19Z

+const mockActionHandler = vi.mocked(actionHandler);
+const mockUseRouter = vi.mocked(useRouter);
+
+const schema: RJSFSchema = {


using a test schema for the component has us testing RJSF internals, just verifying that it displays a test schema properly.
I suggest using the real generated schema, this way we'll catch regressions in functionality

pbastia · 2026-07-08T17:13:07Z

+    @classmethod
+    def _add_reportable_operation(
+        cls,
+        reportable_operations: list[dict],
+        added_operation_years: set[tuple[UUID, int]],
+        operation: Operation,
+        reporting_year: ReportingYear,
+        is_current_registered_fallback: bool,
+    ) -> None:


nit: coding style, particularly in class or static methods, we should try avoid functions that modify data structures in place and act as a side-effect.

Instead, the function should take inputs, treat them as immutable, and return new outputs

chore: revert

chore: cleanup chore: cleanup chore: add registration purpose to startReport schema chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup

chore: cleanup

chore: cleanup chore: cleanup

chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup

chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup

chore: cleanup chore: cleanup chore: cleanup chore: cleanup

shon-button force-pushed the feature/4745-past-year-reporting branch 12 times, most recently from b2a0d34 to 1fa7e41 Compare June 17, 2026 17:02

shon-button force-pushed the feature/4745-past-year-reporting branch from 42646bf to efb06c3 Compare June 19, 2026 17:31

shon-button marked this pull request as draft June 19, 2026 17:37

shon-button marked this pull request as ready for review June 22, 2026 16:39

shon-button force-pushed the feature/4745-past-year-reporting branch 4 times, most recently from 9814659 to d4322d3 Compare June 23, 2026 17:57

shon-button force-pushed the feature/4745-past-year-reporting branch 2 times, most recently from 6d92528 to 2f92d69 Compare July 2, 2026 14:50

shon-button added the run-all-ci Trigger label to force all CI to run on a PR. label Jul 2, 2026

shon-button force-pushed the feature/4745-past-year-reporting branch 6 times, most recently from 80e9f4d to e055402 Compare July 3, 2026 17:16

Sepehr-Sobhani reviewed Jul 3, 2026

View reviewed changes

shon-button force-pushed the feature/4745-past-year-reporting branch 6 times, most recently from 92dd9a5 to 0d68f4e Compare July 7, 2026 16:44

Sepehr-Sobhani reviewed Jul 7, 2026

View reviewed changes

pbastia reviewed Jul 7, 2026

View reviewed changes

shon-button force-pushed the feature/4745-past-year-reporting branch from 5e6801a to a46fc97 Compare July 8, 2026 13:37

shon-button added run-all-ci Trigger label to force all CI to run on a PR. and removed run-all-ci Trigger label to force all CI to run on a PR. labels Jul 8, 2026

shon-button force-pushed the feature/4745-past-year-reporting branch 2 times, most recently from 6ae6a30 to 308b461 Compare July 8, 2026 15:35

pbastia approved these changes Jul 8, 2026

View reviewed changes

shon-button added 9 commits July 9, 2026 08:30

chore: implement start past report flow

2e05183

chore: revert

chore: add service to start report for reporting year

6560702

chore: cleanup chore: cleanup chore: add registration purpose to startReport schema chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup

chore: update tests

df8c680

test: add create for reporting year services tests

64af30d

chore: cleanup

chore: preserve selected registration purpose for past reports

21478eb

chore: cleanup chore: cleanup

chore: update api tests

f2c9082

chore: add vitests

b04ac5d

chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup

chore: update dashboard tiles

82ca6f1

chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup

chore: add e2e test

1a632cc

shon-button force-pushed the feature/4745-past-year-reporting branch 2 times, most recently from 70dd94e to 97967b8 Compare July 9, 2026 13:54

refactor: address PR review feedback

8fe5d82

chore: cleanup chore: cleanup chore: cleanup chore: cleanup

shon-button force-pushed the feature/4745-past-year-reporting branch from db7b2ba to 8fe5d82 Compare July 9, 2026 14:52

shon-button merged commit a1f2811 into develop Jul 9, 2026
41 checks passed

shon-button deleted the feature/4745-past-year-reporting branch July 9, 2026 15:10

	def test_list_previous_reportable_operations(
	def test_calls_and_returns_the_service_method_output(

		min_year = min(reporting_years)
		max_year = max(reporting_years)

	)) as CreateReportResponse \| number;
	)) as CreateReportErrorResponse \| number;

Uh oh!

Conversation

shon-button commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Impact

🔬 Local Testing:

Start Test Environment

Test: Start a previous report

Test: Cannot start a previous report

Uh oh!

Sepehr-Sobhani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pbastia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pbastia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shon-button commented Jun 10, 2026 •

edited

Loading