From 487640d871a00862c4e7ce2e3784282c472c57dc Mon Sep 17 00:00:00 2001 From: shon-button Date: Wed, 29 Jan 2025 16:21:10 -0500 Subject: [PATCH] tests: add verification utils tests chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup chore: cleanup --- bc_obps/reporting/api/report_verification.py | 5 +- ...on_other_facility_coordinates_and_more.py} | 6 +- .../reporting/schema/report_verification.py | 30 ++++-- .../service/report_verification_service.py | 2 +- .../tests/test_report_version_service.py | 1 + .../verification/VerificationForm.tsx | 5 +- .../createVerificationUiSchema.ts | 13 ++- .../handleVerificationData.test.ts | 91 +++++++++++++------ .../verification/handleVerificationData.ts | 36 +++++--- .../verification/mergeVerificationData.ts | 89 +++++------------- .../jsonSchema/verification/verification.tsx | 54 +++++++---- .../verification/VerificationForm.test.tsx | 27 +----- 12 files changed, 179 insertions(+), 180 deletions(-) rename bc_obps/reporting/migrations/{0046_remove_reportverification_other_facility_coordinates_and_more.py => 0049_remove_reportverification_other_facility_coordinates_and_more.py} (96%) diff --git a/bc_obps/reporting/api/report_verification.py b/bc_obps/reporting/api/report_verification.py index 1959b85889..9df4354856 100644 --- a/bc_obps/reporting/api/report_verification.py +++ b/bc_obps/reporting/api/report_verification.py @@ -1,4 +1,5 @@ from typing import Literal +from reporting.models.report_verification import ReportVerification from common.permissions import authorize from django.http import HttpRequest from registration.decorators import handle_http_errors @@ -8,7 +9,6 @@ from .router import router from reporting.schema.report_verification import ReportVerificationIn, ReportVerificationOut from reporting.service.report_verification_service import ReportVerificationService -from reporting.models import ReportVerification @router.get( @@ -21,9 +21,10 @@ @handle_http_errors() def get_report_verification_by_version_id( request: HttpRequest, report_version_id: int -) -> tuple[Literal[200], ReportVerificationOut]: +) -> tuple[Literal[200], ReportVerification]: return 200, ReportVerificationService.get_report_verification_by_version_id(report_version_id) + @router.get( "/report-version/{report_version_id}/report-needs-verification", response={200: bool, custom_codes_4xx: Message}, diff --git a/bc_obps/reporting/migrations/0046_remove_reportverification_other_facility_coordinates_and_more.py b/bc_obps/reporting/migrations/0049_remove_reportverification_other_facility_coordinates_and_more.py similarity index 96% rename from bc_obps/reporting/migrations/0046_remove_reportverification_other_facility_coordinates_and_more.py rename to bc_obps/reporting/migrations/0049_remove_reportverification_other_facility_coordinates_and_more.py index 9092658c1b..852d850c90 100644 --- a/bc_obps/reporting/migrations/0046_remove_reportverification_other_facility_coordinates_and_more.py +++ b/bc_obps/reporting/migrations/0049_remove_reportverification_other_facility_coordinates_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 5.0.10 on 2025-01-27 15:40 +# Generated by Django 5.0.10 on 2025-01-31 17:48 import django.db.models.deletion from django.db import migrations, models @@ -7,8 +7,8 @@ class Migration(migrations.Migration): dependencies = [ - ('registration', '0069_V1_19_0'), - ('reporting', '0045_fix_incorrect_fkey_on_deletes'), + ('registration', '0070_V1_20_0'), + ('reporting', '0048_remove_activitysourcetypejsonschema_invalid_if_has_unit_and_no_fuel'), ] operations = [ diff --git a/bc_obps/reporting/schema/report_verification.py b/bc_obps/reporting/schema/report_verification.py index 0318fe734e..462e333f3a 100644 --- a/bc_obps/reporting/schema/report_verification.py +++ b/bc_obps/reporting/schema/report_verification.py @@ -1,13 +1,21 @@ -from ninja import ModelSchema, Field +from typing import Optional, List +from ninja import ModelSchema +from pydantic import Field + from reporting.models import ReportVerification, ReportVerificationVisit -from typing import List -class BaseReportVerification(ModelSchema): +class ReportVerificationBase(ModelSchema): """ Base schema for shared fields in ReportVerification schemas """ + verification_body_name: str + accredited_by: str + scope_of_verification: str + threats_to_independence: bool + verification_conclusion: str + class Meta: model = ReportVerification fields = [ @@ -24,6 +32,11 @@ class ReportVerificationVisitSchema(ModelSchema): Schema for ReportVerificationVisit model """ + visit_name: str + visit_type: Optional[str] = Field(None) + is_other_visit: bool + visit_coordinates: str + class Meta: model = ReportVerificationVisit fields = [ @@ -34,20 +47,23 @@ class Meta: ] -class ReportVerificationIn(BaseReportVerification): +class ReportVerificationIn(ReportVerificationBase): """ Schema for the input of report verification data """ report_verification_visits: List[ReportVerificationVisitSchema] = Field(default_factory=list) + class Meta(ReportVerificationBase.Meta): + fields = ReportVerificationBase.Meta.fields + -class ReportVerificationOut(BaseReportVerification): +class ReportVerificationOut(ReportVerificationBase): """ Schema for the output of report verification data """ report_verification_visits: List[ReportVerificationVisitSchema] = Field(default_factory=list) - class Meta(BaseReportVerification.Meta): - fields = BaseReportVerification.Meta.fields + ['report_version'] + class Meta(ReportVerificationBase.Meta): + fields = ReportVerificationBase.Meta.fields + ['report_version'] diff --git a/bc_obps/reporting/service/report_verification_service.py b/bc_obps/reporting/service/report_verification_service.py index 02bc7ca744..2bbc8dbd62 100644 --- a/bc_obps/reporting/service/report_verification_service.py +++ b/bc_obps/reporting/service/report_verification_service.py @@ -3,11 +3,11 @@ from reporting.models.report_verification import ReportVerification from reporting.models.report_verification_visit import ReportVerificationVisit from reporting.models import ReportVersion -from reporting.schema.report_verification import ReportVerificationIn from registration.models import Operation from reporting.service.report_additional_data import ReportAdditionalDataService from reporting.service.compliance_service import ComplianceService +from reporting.schema.report_verification import ReportVerificationIn class ReportVerificationService: diff --git a/bc_obps/service/tests/test_report_version_service.py b/bc_obps/service/tests/test_report_version_service.py index 0d9bf51211..53f49b79a7 100644 --- a/bc_obps/service/tests/test_report_version_service.py +++ b/bc_obps/service/tests/test_report_version_service.py @@ -82,5 +82,6 @@ def test_report_version_cascading_models(self): "ReportOperationRepresentative", "ReportAdditionalData", "ReportVerification", + "ReportVerificationVisit", "ReportProductEmissionAllocation", } diff --git a/bciers/apps/reporting/src/app/components/verification/VerificationForm.tsx b/bciers/apps/reporting/src/app/components/verification/VerificationForm.tsx index 77abfdec54..262544c8e7 100644 --- a/bciers/apps/reporting/src/app/components/verification/VerificationForm.tsx +++ b/bciers/apps/reporting/src/app/components/verification/VerificationForm.tsx @@ -54,7 +54,7 @@ export default function VerificationForm({ const handleSubmit = async () => { // 📷 Clone formData as payload const payload = { ...formData }; - debugger; + // ➕ Update report_verification_visits property based on visit_types and visit_others mergeVerificationData(payload); @@ -70,8 +70,7 @@ export default function VerificationForm({ const response = await actionHandler(endpoint, method, pathToRevalidate, { body: JSON.stringify(payload), }); - console.log(JSON.stringify(payload)); - console.log(response); + // 🐜 Check for errors if (response?.error) { setErrors([response.error]); diff --git a/bciers/apps/reporting/src/app/components/verification/createVerificationUiSchema.ts b/bciers/apps/reporting/src/app/components/verification/createVerificationUiSchema.ts index cdbb24dbec..a4482455f6 100644 --- a/bciers/apps/reporting/src/app/components/verification/createVerificationUiSchema.ts +++ b/bciers/apps/reporting/src/app/components/verification/createVerificationUiSchema.ts @@ -5,11 +5,10 @@ import { sfoUiSchema } from "@reporting/src/data/jsonSchema/verification/verific export const createVerificationUiSchema = ( schemaType: "SFO" | "LFO", ): RJSFSchema => { - // Retrieve a local copy of the base verification ui schema based - switch (schemaType) { - case "SFO": - return { ...sfoUiSchema }; - case "LFO": - return { ...lfoUiSchema }; - } + // Determine the schema based on the schemaType + const localUiSchema: RJSFSchema = + schemaType === "SFO" ? { ...sfoUiSchema } : { ...lfoUiSchema }; + + // Return the customized schema. + return localUiSchema; }; diff --git a/bciers/apps/reporting/src/app/utils/verification/handleVerificationData.test.ts b/bciers/apps/reporting/src/app/utils/verification/handleVerificationData.test.ts index 6e50a77e68..0fdbdabeed 100644 --- a/bciers/apps/reporting/src/app/utils/verification/handleVerificationData.test.ts +++ b/bciers/apps/reporting/src/app/utils/verification/handleVerificationData.test.ts @@ -2,59 +2,90 @@ import { describe, it, expect } from "vitest"; import { handleVerificationData } from "@reporting/src/app/utils/verification/handleVerificationData"; describe("handleVerificationData", () => { - it("should lock selection to 'None' and clear other fields if only 'None' is selected", () => { - const input = { + it("should remove 'None' if other selections are made", () => { + const data = { + visit_names: ["None", "Facility 1"], + visit_types: [], + visit_others: [], + }; + const result = handleVerificationData(data, "default"); + expect(result.visit_names).toEqual(["Facility 1"]); + }); + + it("should lock to 'None' and clear other fields if only 'None' is selected", () => { + const data = { visit_names: ["None"], - visit_types: [{ visit_name: "Test", visit_type: "A" }], - visit_others: [{ visit_name: "Other" }], + visit_types: [{ visit_name: "Old" }], + visit_others: [{ key: "value" }], }; - const result = handleVerificationData(input, "LFO"); + const result = handleVerificationData(data, "default"); expect(result.visit_names).toEqual(["None"]); expect(result.visit_types).toEqual([]); expect(result.visit_others).toEqual([{}]); }); - it("should remove 'None' if other selections are made", () => { - const input = { - visit_names: ["None", "Facility A"], + it("should enforce maxItem=1 for 'SFO' operation type", () => { + const data = { + visit_names: ["Facility 1", "Facility 2"], visit_types: [], visit_others: [], }; - const result = handleVerificationData(input, "LFO"); - expect(result.visit_names).toEqual(["Facility A"]); + const result = handleVerificationData(data, "SFO"); + expect(result.visit_names).toEqual(["Facility 2"]); // Last selected should be retained }); - it("should enforce only one selection for 'SFO', keeping the last selected value", () => { - const input = { - visit_names: ["Facility A", "Facility B"], + it("should update visit_types based on visit_names", () => { + const data = { + visit_names: ["Facility 1", "Facility 2"], visit_types: [], visit_others: [], }; - const result = handleVerificationData(input, "SFO"); - expect(result.visit_names).toEqual(["Facility B"]); + const result = handleVerificationData(data, "default"); + expect(result.visit_types).toEqual([ + { visit_name: "Facility 1", visit_type: "" }, + { visit_name: "Facility 2", visit_type: "" }, + ]); }); - it("should clear visit types and visit others when 'None' is the last selected for 'SFO'", () => { - const input = { - visit_names: ["Facility A", "None"], - visit_types: [{ visit_name: "Facility A", visit_type: "A" }], - visit_others: [{ visit_name: "Other" }], + it("should preserve existing visit_types if present", () => { + const data = { + visit_names: ["Facility 1"], + visit_types: [{ visit_name: "Facility 1", visit_type: "In-Person" }], + visit_others: [], }; - const result = handleVerificationData(input, "SFO"); - expect(result.visit_names).toEqual(["None"]); + const result = handleVerificationData(data, "default"); + expect(result.visit_types).toEqual([ + { visit_name: "Facility 1", visit_type: "In-Person" }, + ]); + }); + + it("should set visit_types to empty if only 'None' is selected", () => { + const data = { + visit_names: ["None"], + visit_types: [{ visit_name: "Facility 1" }], + visit_others: [], + }; + const result = handleVerificationData(data, "default"); expect(result.visit_types).toEqual([]); + }); + + it("should clear visit_others if 'Other' is not selected", () => { + const data = { + visit_names: ["Facility 1"], + visit_types: [], + visit_others: [{ visit_name: "Other Visit" }], + }; + const result = handleVerificationData(data, "default"); expect(result.visit_others).toEqual([{}]); }); - it("should correctly update visit_types excluding 'Other' and 'None'", () => { - const input = { - visit_names: ["Facility A", "Other", "None"], + it("should retain visit_others if 'Other' is selected", () => { + const data = { + visit_names: ["Other"], visit_types: [], - visit_others: [], + visit_others: [{ visit_name: "Other Visit" }], }; - const result = handleVerificationData(input, "LFO"); - expect(result.visit_types).toEqual([ - { visit_name: "Facility A", visit_type: "" }, - ]); + const result = handleVerificationData(data, "default"); + expect(result.visit_others).toEqual([{ visit_name: "Other Visit" }]); }); }); diff --git a/bciers/apps/reporting/src/app/utils/verification/handleVerificationData.ts b/bciers/apps/reporting/src/app/utils/verification/handleVerificationData.ts index f6c3693857..8105e92d9a 100644 --- a/bciers/apps/reporting/src/app/utils/verification/handleVerificationData.ts +++ b/bciers/apps/reporting/src/app/utils/verification/handleVerificationData.ts @@ -1,6 +1,3 @@ -// 🛠️ Function to manages interaction with the data and form -// Required(?) when JSON Schema validators struggle with conditional logic based on dynamic enums -// Required(?) when using an array, visit_names, requiring a MultiSelectWidget but with a maxItem rules export function handleVerificationData( updatedData: any, operationType: string, @@ -9,12 +6,20 @@ export function handleVerificationData( if (selectedValues.includes("None")) { if (selectedValues.length > 1) { - // Remove "None" if other selections are made - updatedData.visit_names = selectedValues.filter( - (value: string) => value !== "None", - ); + // If "None" is the last selected item, clear everything + if (selectedValues[selectedValues.length - 1] === "None") { + updatedData.visit_names = ["None"]; + updatedData.visit_types = []; + updatedData.visit_others = [{}]; + return updatedData; + } else { + // Otherwise, remove "None" + updatedData.visit_names = selectedValues.filter( + (value: string) => value !== "None", + ); + } } else { - // Lock to "None" and clear other fields + // If only "None" is selected, lock it and clear other fields updatedData.visit_names = ["None"]; updatedData.visit_types = []; updatedData.visit_others = [{}]; @@ -23,15 +28,11 @@ export function handleVerificationData( } if (operationType === "SFO" && selectedValues.length > 1) { - // Ensure "SFO" can only have one item, taking the last selected + // Ensure "SFO" visit_names maxItem=1 const lastSelected = selectedValues[selectedValues.length - 1]; - updatedData.visit_names = [lastSelected]; - if (lastSelected === "None") { - // Clear visit types and visit others only if the value is "None" - updatedData.visit_types = []; - updatedData.visit_others = [{}]; - } + // Set the last selected item as the only value for visit_names + updatedData.visit_names = [lastSelected]; } // Update `visit_types` for each facility except "Other" and "None" @@ -46,5 +47,10 @@ export function handleVerificationData( return existingVisitType || { visit_name, visit_type: "" }; }); + // Clear visit_others if "Other" is not selected + if (!updatedData.visit_names.includes("Other")) { + updatedData.visit_others = [{}]; + } + return updatedData; } diff --git a/bciers/apps/reporting/src/app/utils/verification/mergeVerificationData.ts b/bciers/apps/reporting/src/app/utils/verification/mergeVerificationData.ts index 9e68b64683..226722ac3c 100644 --- a/bciers/apps/reporting/src/app/utils/verification/mergeVerificationData.ts +++ b/bciers/apps/reporting/src/app/utils/verification/mergeVerificationData.ts @@ -1,23 +1,10 @@ -// 🛠️ Function to update the report_verification_visits property for the API schema +// 🛠️ Function to update the report_verification_visits property for POST to the API export function mergeVerificationData(formData: any): void { // Initialize the report_verification_visits array formData.report_verification_visits = []; // Check if "None" is selected in visit_names - if ( - Array.isArray(formData.visit_names) && - formData.visit_names.includes("None") - ) { - formData.report_verification_visits = [ - { - visit_name: "None", - is_other_visit: false, - visit_coordinates: "", - visit_type: "", - }, - ]; - return; // Exit early as "None" overrides all other data - } else if (formData.visit_names === "None") { + if (formData.visit_names.includes("None")) { formData.report_verification_visits = [ { visit_name: "None", @@ -29,63 +16,31 @@ export function mergeVerificationData(formData: any): void { return; // Exit early as "None" overrides all other data } - // Handle visit_types - if (Array.isArray(formData.visit_types)) { - formData.report_verification_visits.push( - ...formData.visit_types.map((type: any) => ({ - visit_name: type.visit_name || "", - visit_type: type.visit_type || "", - is_other_visit: false, - visit_coordinates: "", // Default for non-other visits - })), - ); - } else if (formData.visit_types) { - // Handle single object scenario for visit_types - formData.report_verification_visits.push({ - visit_name: formData.visit_types.visit_name || "", - visit_type: formData.visit_types.visit_type || "", + // Handle visit_types and visit_others, filtering out empty visit_names during mapping + const visits = [ + ...formData.visit_types.map((type: any) => ({ + visit_name: type.visit_name || "", + visit_type: type.visit_type || "", is_other_visit: false, visit_coordinates: "", // Default for non-other visits - }); - } - - // Handle visit_others - if (Array.isArray(formData.visit_others)) { - formData.report_verification_visits.push( - ...formData.visit_others.map((other: any) => ({ + })), + ...formData.visit_others + .map((other: any) => ({ visit_name: other.visit_name || "", visit_type: other.visit_type || "", is_other_visit: true, visit_coordinates: other.visit_coordinates || "", - })), - ); - } else if (formData.visit_others) { - // Handle single object scenario for visit_others - formData.report_verification_visits.push({ - visit_name: formData.visit_others.visit_name || "", - visit_type: formData.visit_others.visit_type || "", - is_other_visit: true, - visit_coordinates: formData.visit_others.visit_coordinates || "", - }); - } - - // If "None" is found in visit_types or visit_others, override with "None" - const noneSelectedInVisitTypes = Array.isArray(formData.visit_types) - ? formData.visit_types.some((type: any) => type.visit_name === "None") - : formData.visit_types?.visit_name === "None"; + })) + .filter((other: any) => other.visit_name !== ""), // Filter out items with empty visit_name + ]; - const noneSelectedInVisitOthers = Array.isArray(formData.visit_others) - ? formData.visit_others.some((other: any) => other.visit_name === "None") - : formData.visit_others?.visit_name === "None"; - - if (noneSelectedInVisitTypes || noneSelectedInVisitOthers) { - formData.report_verification_visits = [ - { - visit_name: "None", - is_other_visit: false, - visit_coordinates: "", - visit_type: "", - }, - ]; - } + // Assign the filtered visits to the report_verification_visits array + formData.report_verification_visits = visits.filter((visit: any) => { + return ( + visit.visit_name !== "" || // Remove empty visit_name + visit.visit_type !== "" || // Remove empty visit_type + visit.visit_coordinates !== "" || // Remove empty visit_coordinates + visit.is_other_visit !== false // Remove false is_other_visit + ); + }); } diff --git a/bciers/apps/reporting/src/data/jsonSchema/verification/verification.tsx b/bciers/apps/reporting/src/data/jsonSchema/verification/verification.tsx index 5bd2f202ed..a8ef785353 100644 --- a/bciers/apps/reporting/src/data/jsonSchema/verification/verification.tsx +++ b/bciers/apps/reporting/src/data/jsonSchema/verification/verification.tsx @@ -129,27 +129,56 @@ const getAssociatedVisitName = ( * @param {FieldTemplateProps} props - Props including id, classNames, children, and formContext * @returns {JSX.Element} - Rendered label and input field */ + const DynamicLabelVisitType: React.FC = ({ id, classNames, children, formContext, + rawErrors = [], }: FieldTemplateProps): JSX.Element => { const visitName = getAssociatedVisitName(id, formContext); + return (
-
{children}
+ +
+ {children} +
+ + {/* Error display to the side */} + {rawErrors.length > 0 && ( +
+
+ + + +
+ Required field +
+ )}
); }; - /** * SFO Verfication Form schema */ @@ -161,7 +190,7 @@ export const sfoSchema: RJSFSchema = { ...sharedSchemaProperties, visit_names: { type: "array", - title: "Sites visited", + title: "Site visited", uniqueItems: true, minItems: 1, maxItems: 1, @@ -207,13 +236,6 @@ export const sfoSchema: RJSFSchema = { type: "array", minItems: 1, maxItems: 1, - default: [ - { - visit_name: "", - visit_coordinates: "", - visit_type: "", - }, - ], items: { type: "object", required: ["visit_name", "visit_coordinates", "visit_type"], @@ -362,13 +384,6 @@ export const lfoSchema: RJSFSchema = { title: "Other Visit(s)", type: "array", minItems: 1, - default: [ - { - visit_name: "", - visit_coordinates: "", - visit_type: "", - }, - ], items: { type: "object", required: ["visit_name", "visit_coordinates", "visit_type"], @@ -390,7 +405,6 @@ export const lfoSchema: RJSFSchema = { }, }, }, - required: ["visit_others"], }, ], }, @@ -448,7 +462,7 @@ export const lfoUiSchema: UiSchema = { "ui:FieldTemplate": FieldTemplate, "ui:options": { arrayAddLabel: "Add Other Visit", - addable: true, // Ensure users can add more visits manually + addable: true, }, items: { visit_name: { diff --git a/bciers/apps/reporting/src/tests/components/verification/VerificationForm.test.tsx b/bciers/apps/reporting/src/tests/components/verification/VerificationForm.test.tsx index 38f21b94c1..8e1d561f53 100644 --- a/bciers/apps/reporting/src/tests/components/verification/VerificationForm.test.tsx +++ b/bciers/apps/reporting/src/tests/components/verification/VerificationForm.test.tsx @@ -1,6 +1,5 @@ -import { render, screen, waitFor, fireEvent } from "@testing-library/react"; -import { actionHandler, useRouter } from "@bciers/testConfig/mocks"; -import userEvent from "@testing-library/user-event"; +import { render } from "@testing-library/react"; +import { useRouter } from "@bciers/testConfig/mocks"; import { sfoUiSchema, lfoUiSchema, @@ -8,8 +7,6 @@ import { import VerificationForm from "@reporting/src/app/components/verification/VerificationForm"; import expectButton from "@bciers/testConfig/helpers/expectButton"; import expectField from "@bciers/testConfig/helpers/expectField"; -import { fillMandatoryFields } from "@bciers/testConfig/helpers/fillMandatoryFields"; - // ✨ Mocks const mockRouterPush = vi.fn(); useRouter.mockReturnValue({ @@ -62,26 +59,6 @@ const commonMandatoryFormFields = [ }, ]; -// Test data for mandatory fields -const formDataSets = { - SFO: { - verification_body_name: "SFO Test", - accredited_by: "SCC", - scope_of_verification: "Primary Report", - visit_name: "None", - threats_to_independence: "No", - verification_conclusion: "Positive", - }, - LFO: { - verification_body_name: "LFO Test", - accredited_by: "SCC", - scope_of_verification: "Detailed Report", - visit_name: "Facility A", - threats_to_independence: "No", - verification_conclusion: "Modified", - }, -}; - // ⛏️ Helper function to render the form const renderVerificationForm = (operationType: string) => { const verificationSchema = getUiSchema(operationType);