From 724ce1e46d09c23de9b352d99e018df7cbd844e6 Mon Sep 17 00:00:00 2001 From: Slava Ivanov Date: Sat, 4 Jan 2025 01:12:05 -0800 Subject: [PATCH] fix: update site visited logic for report verification page (#2629) ### Changes - Card: https://github.com/bcgov/cas-reporting/issues/199#issuecomment-2524359485 - Updated `ReportVerification` model to include new fields and modify existing ones - Modified `BaseReportVerificationSchema` to align with new model changes - Updated `verificationSchema` and `verificationUiSchema` in frontend to reflect new requirements - Adjusted `VerificationForm` component to handle new form structure ### Acceptance Criteria - "Site visited" options (Facility X, Other, None) include required fields for threats to independence and verification conclusion - Facility X and Other options include "Type of site visit" field - Other option shows additional fields for site name and geographic location - Backend model and schema align with frontend changes - Form validation works correctly for all new fields and dependencies ### Test Plan 1. Run migrations and start the application 2. Navigate to the Verification form 3. Test each "Site visited" option: - Verify required fields appear for each option - Ensure conditional fields show/hide appropriately 4. Submit the form with various combinations of inputs 5. Verify data is correctly saved to the database 6. Check API responses include all new fields https://www.loom.com/share/8d7c765181ea42daad40b1b9dac8669a?sid=2b18080d-aff5-4fc5-a73c-f392d83265fb --- ...ion_other_facility_coordinates_and_more.py | 67 +++++++++++++++++++ .../reporting/models/report_verification.py | 19 +++--- .../reporting/schema/report_verification.py | 4 +- .../src/data/verification/verification.ts | 53 +++++++++++---- .../verification/VerificationForm.test.tsx | 57 ++++++++-------- .../testConfig/src/helpers/expectField.ts | 24 +++++-- .../src/helpers/fillMandatoryFields.ts | 2 +- docs/developer-environment-setup.md | 2 +- 8 files changed, 167 insertions(+), 61 deletions(-) create mode 100644 bc_obps/reporting/migrations/0042_alter_reportverification_other_facility_coordinates_and_more.py diff --git a/bc_obps/reporting/migrations/0042_alter_reportverification_other_facility_coordinates_and_more.py b/bc_obps/reporting/migrations/0042_alter_reportverification_other_facility_coordinates_and_more.py new file mode 100644 index 0000000000..99e597cfd7 --- /dev/null +++ b/bc_obps/reporting/migrations/0042_alter_reportverification_other_facility_coordinates_and_more.py @@ -0,0 +1,67 @@ +# Generated by Django 5.0.10 on 2024-12-28 07:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('reporting', '0041_reportnewentrant_reportnewentrantemission_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='reportverification', + name='other_facility_coordinates', + field=models.CharField( + blank=True, db_comment='Geographic location of the other facility visited', max_length=100, null=True + ), + ), + migrations.AlterField( + model_name='reportverification', + name='other_facility_name', + field=models.CharField( + blank=True, + db_comment="Name of the other facility visited if 'Other' is selected", + max_length=100, + null=True, + ), + ), + migrations.AlterField( + model_name='reportverification', + name='threats_to_independence', + field=models.BooleanField( + db_comment='Indicates whether there were any threats to independence noted', default=False + ), + preserve_default=False, + ), + migrations.AlterField( + model_name='reportverification', + name='verification_conclusion', + field=models.CharField( + choices=[('Positive', 'Positive'), ('Modified', 'Modified'), ('Negative', 'Negative')], + db_comment='The conclusion of the verification', + default='Positive', + max_length=8, + ), + preserve_default=False, + ), + migrations.AlterField( + model_name='reportverification', + name='visit_name', + field=models.CharField( + db_comment='The name of the site visited (Facility X, Other, or None)', max_length=100 + ), + ), + migrations.AlterField( + model_name='reportverification', + name='visit_type', + field=models.CharField( + blank=True, + choices=[('In person', 'In Person'), ('Virtual', 'Virtual')], + db_comment='The type of visit conducted (Virtual or In Person)', + max_length=10, + null=True, + ), + ), + ] diff --git a/bc_obps/reporting/models/report_verification.py b/bc_obps/reporting/models/report_verification.py index c14b71516d..b0df5542d1 100644 --- a/bc_obps/reporting/models/report_verification.py +++ b/bc_obps/reporting/models/report_verification.py @@ -38,10 +38,7 @@ class ScopeOfVerification(models.TextChoices): ) threats_to_independence = models.BooleanField( - null=True, - blank=True, - db_comment="Optional field to store whether or not there is an indication of threats to independence of an other facility visited", - default=False, + db_comment="Indicates whether there were any threats to independence noted", ) class VerificationConclusion(models.TextChoices): @@ -50,14 +47,14 @@ class VerificationConclusion(models.TextChoices): NEGATIVE = "Negative" verification_conclusion = models.CharField( - null=True, - blank=True, max_length=8, choices=VerificationConclusion.choices, db_comment="The conclusion of the verification", ) - visit_name = models.CharField(max_length=100, db_comment="The name of the site visited") + visit_name = models.CharField( + max_length=100, db_comment="The name of the site visited (Facility X, Other, or None)" + ) class VisitType(models.TextChoices): IN_PERSON = "In person" @@ -68,21 +65,21 @@ class VisitType(models.TextChoices): choices=VisitType.choices, null=True, blank=True, - db_comment="Optional field to store the type of visit conducted", + db_comment="The type of visit conducted (Virtual or In Person)", ) other_facility_name = models.CharField( max_length=100, null=True, blank=True, - db_comment="Optional field to store the name of an other facility visited", + db_comment="Name of the other facility visited if 'Other' is selected", ) other_facility_coordinates = models.CharField( - max_length=50, + max_length=100, null=True, blank=True, - db_comment="Optional field to store geographic coordinates of an other facility visited", + db_comment="Geographic location of the other facility visited", ) class Meta: diff --git a/bc_obps/reporting/schema/report_verification.py b/bc_obps/reporting/schema/report_verification.py index f45350bfd4..0fc29f2b66 100644 --- a/bc_obps/reporting/schema/report_verification.py +++ b/bc_obps/reporting/schema/report_verification.py @@ -16,8 +16,8 @@ class BaseReportVerificationSchema(ModelSchema): visit_type: Optional[str] = Field(None) other_facility_name: Optional[str] = Field(None) other_facility_coordinates: Optional[str] = Field(None) - threats_to_independence: Optional[bool] = Field(None) - verification_conclusion: Optional[str] = Field(None) + threats_to_independence: bool + verification_conclusion: str class Meta: model = ReportVerification diff --git a/bciers/apps/reporting/src/data/verification/verification.ts b/bciers/apps/reporting/src/data/verification/verification.ts index 23753f06a1..ea7b3269ad 100644 --- a/bciers/apps/reporting/src/data/verification/verification.ts +++ b/bciers/apps/reporting/src/data/verification/verification.ts @@ -13,6 +13,8 @@ export const verificationSchema: RJSFSchema = { "accredited_by", "scope_of_verification", "visit_name", + "threats_to_independence", + "verification_conclusion", ], properties: { verification_body_name: { @@ -36,7 +38,16 @@ export const verificationSchema: RJSFSchema = { visit_name: { title: "Sites visited", type: "string", - enum: ["Other", "None"], // modified in components/verification/createVerificationSchema.ts + enum: ["Facility X", "Other", "None"], // modified in components/verification/createVerificationSchema.ts + }, + threats_to_independence: { + title: "Were there any threats to independence noted", + type: "boolean", + }, + verification_conclusion: { + title: "Verification conclusion", + type: "string", + enum: ["Positive", "Modified", "Negative"], }, verification_note: { //Not an actual field in the db - this is just to make the form look like the wireframes @@ -47,9 +58,6 @@ export const verificationSchema: RJSFSchema = { dependencies: { visit_name: { oneOf: [ - // type_of_facility_visit field display conditon: - // visit_name has one item - // visit_name is not "Other" or "None" { properties: { visit_name: { @@ -64,17 +72,23 @@ export const verificationSchema: RJSFSchema = { title: "Type of site visit", enum: ["Virtual", "In person"], }, + threats_to_independence: { + type: "boolean", + }, + verification_conclusion: { + type: "string", + enum: ["Positive", "Modified", "Negative"], + }, }, - required: ["visit_type"], + required: [ + "visit_type", + "threats_to_independence", + "verification_conclusion", + ], }, { - // other_site_details field display condition - // visit_name has one item - // visit_name is "Other" properties: { visit_name: { - type: "string", - minItems: 1, enum: ["Other"], }, other_facility_name: { @@ -83,7 +97,7 @@ export const verificationSchema: RJSFSchema = { }, other_facility_coordinates: { type: "string", - title: "Geographic coordinates", + title: "Geographic coordinates of site", }, visit_type: { type: "string", @@ -91,11 +105,9 @@ export const verificationSchema: RJSFSchema = { enum: ["Virtual", "In person"], }, threats_to_independence: { - title: "Were there any threats to independence noted", type: "boolean", }, verification_conclusion: { - title: "Verification conclusion", type: "string", enum: ["Positive", "Modified", "Negative"], }, @@ -108,6 +120,21 @@ export const verificationSchema: RJSFSchema = { "verification_conclusion", ], }, + { + properties: { + visit_name: { + enum: ["None"], + }, + threats_to_independence: { + type: "boolean", + }, + verification_conclusion: { + type: "string", + enum: ["Positive", "Modified", "Negative"], + }, + }, + required: ["threats_to_independence", "verification_conclusion"], + }, ], }, }, diff --git a/bciers/apps/reporting/src/tests/verification/VerificationForm.test.tsx b/bciers/apps/reporting/src/tests/verification/VerificationForm.test.tsx index e0b3c4c450..af9276f629 100644 --- a/bciers/apps/reporting/src/tests/verification/VerificationForm.test.tsx +++ b/bciers/apps/reporting/src/tests/verification/VerificationForm.test.tsx @@ -44,11 +44,21 @@ const commonMandatoryFormFields = [ key: "scope_of_verification", }, { label: "Sites visited", type: "combobox", key: "visit_name" }, + { + label: "Were there any threats to independence noted", + type: "radio", + key: "threats_to_independence", + }, + { + label: "Verification conclusion", + type: "combobox", + key: "verification_conclusion", + }, ]; const specificMandatoryFields = { facility: [{ label: "Type of site visit", type: "radio", key: "visit_type" }], - conditional: [ + other: [ { label: "Type of site visit", type: "radio", key: "visit_type" }, { label: "Please indicate the site visited", @@ -56,20 +66,10 @@ const specificMandatoryFields = { key: "other_facility_name", }, { - label: "Geographic coordinates", + label: "Geographic coordinates of site", type: "text", key: "other_facility_coordinates", }, - { - label: "Were there any threats to independence noted", - type: "radio", - key: "threats_to_independence", - }, - { - label: "Verification conclusion", - type: "combobox", - key: "verification_conclusion", - }, ], }; @@ -79,15 +79,19 @@ const formDataSets = { accredited_by: "SCC", scope_of_verification: "Supplementary Report", visit_name: "None", + threats_to_independence: "No", + verification_conclusion: "Positive", }, facility: { verification_body_name: "Test", accredited_by: "SCC", scope_of_verification: "Supplementary Report", - visit_name: "Facility A", + visit_name: "Facility X", visit_type: "Virtual", + threats_to_independence: "No", + verification_conclusion: "Positive", }, - conditional: { + other: { verification_body_name: "Test", accredited_by: "SCC", scope_of_verification: "Supplementary Report", @@ -116,7 +120,7 @@ const renderVerificationForm = () => { // ⛏️ Helper function to simulate form POST submission and assert the result const submitFormAndAssert = async ( fields: { label: string; type: string; key: string }[], - data: Record, + data: Record, ) => { actionHandler.mockReturnValueOnce({ success: true, @@ -159,12 +163,12 @@ describe("VerificationForm component", () => { ); await waitFor(() => { - expect(screen.queryAllByText(/Required field/i)).toHaveLength(4); + expect(screen.queryAllByText(/Required field/i)).toHaveLength(6); }); }); it( - "fills mandatory fields and submits successfully", + "fills mandatory fields for 'None' option and submits successfully", { timeout: 10000, }, @@ -186,6 +190,8 @@ describe("VerificationForm component", () => { accredited_by: "SCC", scope_of_verification: "Supplementary Report", visit_name: "None", + threats_to_independence: false, + verification_conclusion: "Positive", }), }, ); @@ -193,16 +199,11 @@ describe("VerificationForm component", () => { ); it( - "fills facility mandatory fields and submits successfully", + "fills mandatory fields for 'Facility X' option and submits successfully", { timeout: 10000, }, async () => { - (verificationSchema.properties?.visit_name as any).enum = [ - ...(verificationSchema.properties?.visit_name as any).enum, - "Facility A", - ]; - renderVerificationForm(); const fields = [ ...commonMandatoryFormFields, @@ -212,7 +213,7 @@ describe("VerificationForm component", () => { }, ); it( - "fills other conditionally mandatory fields and submits successfully", + "fills mandatory fields for 'Other' option and submits successfully", { timeout: 10000, }, @@ -220,10 +221,10 @@ describe("VerificationForm component", () => { renderVerificationForm(); const fields = [ ...commonMandatoryFormFields, - ...specificMandatoryFields.conditional, + ...specificMandatoryFields.other, ]; // POST submit and assert the result - await submitFormAndAssert(fields, formDataSets.conditional); + await submitFormAndAssert(fields, formDataSets.other); // Assertion if actionHandler was called correctly expect(actionHandler).toHaveBeenCalledWith( config.actionPost.endPoint, @@ -235,11 +236,11 @@ describe("VerificationForm component", () => { accredited_by: "SCC", scope_of_verification: "Supplementary Report", visit_name: "Other", + threats_to_independence: false, + verification_conclusion: "Modified", visit_type: "Virtual", other_facility_name: "Other Facility", other_facility_coordinates: "Lat 41; Long 35", - threats_to_independence: false, - verification_conclusion: "Modified", }), }, ); diff --git a/bciers/libs/testConfig/src/helpers/expectField.ts b/bciers/libs/testConfig/src/helpers/expectField.ts index 8b26890ff8..f5d84c506b 100644 --- a/bciers/libs/testConfig/src/helpers/expectField.ts +++ b/bciers/libs/testConfig/src/helpers/expectField.ts @@ -1,13 +1,27 @@ /* eslint-disable import/no-extraneous-dependencies */ import { screen } from "@testing-library/react"; -// Helper function to verify that a field with a specific name is visible and has no value - function expectField(fields: string[], expectedValue: string | null = "") { fields.forEach((fieldLabel) => { - const input = screen.getByLabelText(new RegExp(fieldLabel, "i")); - expect(input).toBeInTheDocument(); - expect(input).toHaveValue(expectedValue); + let element; + try { + element = screen.getByLabelText(new RegExp(fieldLabel, "i")); + } catch (error) { + // If getByLabelText fails, try to find the element by text + element = screen.getByText(new RegExp(fieldLabel, "i")); + } + + expect(element).toBeInTheDocument(); + + if ( + element instanceof HTMLInputElement || + element instanceof HTMLSelectElement + ) { + expect(element).toHaveValue(expectedValue); + } else { + // For non-input elements, just check if they're visible + expect(element).toBeVisible(); + } }); } diff --git a/bciers/libs/testConfig/src/helpers/fillMandatoryFields.ts b/bciers/libs/testConfig/src/helpers/fillMandatoryFields.ts index 4756a94dca..81b65a59e2 100644 --- a/bciers/libs/testConfig/src/helpers/fillMandatoryFields.ts +++ b/bciers/libs/testConfig/src/helpers/fillMandatoryFields.ts @@ -6,7 +6,7 @@ import { selectComboboxOption } from "@bciers/testConfig/helpers/selectComboboxO // Helper function to fill mandatory fields export const fillMandatoryFields = async ( formFields: { label: string; type: string; key: string }[], - formData: Record, + formData: Record, ) => { for (const { label, type, key } of formFields) { const value = formData[key as keyof typeof formData]; diff --git a/docs/developer-environment-setup.md b/docs/developer-environment-setup.md index 2e40490c27..3612550996 100644 --- a/docs/developer-environment-setup.md +++ b/docs/developer-environment-setup.md @@ -106,7 +106,7 @@ In the `bciers` directory: - To run Vitest unit tests on a specific project: `yarn nx run {project}:test`. - To run Vitest unit tests on all projects: `yarn nx run-many -t test`. -- To run playwright end-to-end tests: `nx run {project}:e2e` (For the first time, you may need to run `yarn playwright install ` to install the browsers) +- To run playwright end-to-end tests: `nx run {project}:e2e` (For the first time, you may need to run `yarn playwright install --with-deps` to install the browsers) **Or**, you can use the scripts available in `bciers/package.json`