From 4a20a8f2e974cbe49279a1a729a6bd2f39598b9d Mon Sep 17 00:00:00 2001 From: "Frank Pigeon Jr." Date: Tue, 24 Sep 2024 16:59:11 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=9B=A0=EF=B8=8F=20adds=20missing?= =?UTF-8?q?=20validation=20on=20Agreement=20form=20(#2832)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: fixes validation issues --- frontend/cypress/e2e/createAgreement.cy.js | 12 ++++---- .../e2e/createAgreementWithValidations.cy.js | 28 ++++++++++++++++--- .../AgreementEditor/AgreementEditForm.jsx | 8 ++---- .../AgreementEditor/AgreementEditFormSuite.js | 7 ++--- .../CreateBLIsAndSCs/CreateBLIsAndSCs.jsx | 5 +--- .../BudgetLineItems/CreateBLIsAndSCs/suite.js | 4 +-- .../ContractTypeSelect/ContractTypeSelect.jsx | 2 +- .../ServiceReqTypeSelect.jsx | 2 +- .../agreements/approve/ApproveAgreement.jsx | 6 ++++ frontend/src/pages/agreements/review/suite.js | 3 +- 10 files changed, 48 insertions(+), 29 deletions(-) diff --git a/frontend/cypress/e2e/createAgreement.cy.js b/frontend/cypress/e2e/createAgreement.cy.js index 6974a184e5..2cfa8b3598 100644 --- a/frontend/cypress/e2e/createAgreement.cy.js +++ b/frontend/cypress/e2e/createAgreement.cy.js @@ -62,13 +62,11 @@ it("can create an SEVERABLE agreement", () => { // complete contract type and service req type cy.get("#contract-type").select("FIRM_FIXED_PRICE"); // test default should be NON-SEVERABLE - cy.get("#serviceReqType").should("contain", "Non-Severable"); - cy.get("#serviceReqType").select("-Select Service Requirement Type-"); - cy.get(".usa-error-message").should("contain", "This is required information"); - cy.get("[data-cy='continue-btn']").should("be.disabled"); - cy.get("[data-cy='save-draft-btn']").should("be.disabled"); + cy.get("#service_requirement_type").should("contain", "Non-Severable"); + cy.get("#service_requirement_type").select("-Select Service Requirement Type-"); + cy.get(".usa-error-message").should("exist"); // change to SEVERABLE - cy.get("#serviceReqType").select("SEVERABLE"); + cy.get("#service_requirement_type").select("SEVERABLE"); cy.get(".usa-error-message").should("not.exist"); cy.get("[data-cy='continue-btn']").should("not.be.disabled"); cy.get("[data-cy='save-draft-btn']").should("not.be.disabled"); @@ -201,7 +199,7 @@ it("can create an NON-SEVERABLE agreement", () => { // complete contract type and service req type cy.get("#contract-type").select("FIRM_FIXED_PRICE"); // test default should be NON-SEVERABLE - cy.get("#serviceReqType").should("contain", "Non-Severable"); + cy.get("#service_requirement_type").should("contain", "Non-Severable"); cy.get(".usa-error-message").should("not.exist"); cy.get("[data-cy='continue-btn']").should("not.be.disabled"); cy.get("[data-cy='save-draft-btn']").should("not.be.disabled"); diff --git a/frontend/cypress/e2e/createAgreementWithValidations.cy.js b/frontend/cypress/e2e/createAgreementWithValidations.cy.js index 9e07b1d9c9..76419555a2 100644 --- a/frontend/cypress/e2e/createAgreementWithValidations.cy.js +++ b/frontend/cypress/e2e/createAgreementWithValidations.cy.js @@ -52,7 +52,6 @@ describe("create agreement and test validations", () => { cy.intercept("PATCH", "**/agreements/**").as("patchAgreement"); cy.visit(`/agreements/review/${agreementId}?mode=review`).wait(1000); cy.get("h1").should("have.text", "Please resolve the errors outlined below"); - cy.get('[data-cy="error-list"]').should("exist"); cy.get('[data-cy="error-item"]').should("have.length", 9); //send-to-approval button should be disabled @@ -61,12 +60,33 @@ describe("create agreement and test validations", () => { //fix errors cy.get('[data-cy="edit-agreement-btn"]').click(); cy.get("#continue").click(); - // get all errors on page, should be 5 - cy.get(".usa-form-group--error").should("have.length", 5); + // get all errors on page, should be 6 + cy.get(".usa-form-group--error").should("have.length", 6); + // test description + cy.get("#description").type("Test Description"); + cy.get("#description").clear(); + cy.get("#description").blur(); + cy.get(".usa-error-message").should("exist"); cy.get("#description").type("Test Description"); + // test contract type + cy.get("#contract-type").select("Firm Fixed Price (FFP)"); + cy.get("#contract-type").select("-Select an option-"); + cy.get(".usa-error-message").should("exist"); cy.get("#contract-type").select("Firm Fixed Price (FFP)"); + // test service requirement select + cy.get("#service_requirement_type").select("Severable"); + cy.get("#service_requirement_type").select("-Select Service Requirement Type-"); + cy.get(".usa-error-message").should("exist"); + cy.get("#service_requirement_type").select("Severable"); + // test product service code + cy.get("#product_service_code_id").select(1); + cy.get("#product_service_code_id").select(0); + cy.get(".usa-error-message").should("exist"); cy.get("#product_service_code_id").select(1); - cy.get("#serviceReqType").select("Severable"); + // test agreement type + cy.get("#agreement_reason").select("NEW_REQ"); + cy.get("#agreement_reason").select(0); + cy.get(".usa-error-message").should("exist"); cy.get("#agreement_reason").select("NEW_REQ"); cy.get("#project-officer-combobox-input").type("Chris Fortunato{enter}"); cy.get("#team-member-combobox-input").type("Admin Demo{enter}"); diff --git a/frontend/src/components/Agreements/AgreementEditor/AgreementEditForm.jsx b/frontend/src/components/Agreements/AgreementEditor/AgreementEditForm.jsx index 4be4683832..edb125da4b 100644 --- a/frontend/src/components/Agreements/AgreementEditor/AgreementEditForm.jsx +++ b/frontend/src/components/Agreements/AgreementEditor/AgreementEditForm.jsx @@ -328,8 +328,6 @@ export const AgreementEditForm = ({ handleConfirm={modalProps.handleConfirm} /> )} -

Agreement Type

-

Select the agreement type to get started.

{ setContractType(value); }} /> { diff --git a/frontend/src/components/Agreements/AgreementEditor/AgreementEditFormSuite.js b/frontend/src/components/Agreements/AgreementEditor/AgreementEditFormSuite.js index 45068623a3..c09a9e5b92 100644 --- a/frontend/src/components/Agreements/AgreementEditor/AgreementEditFormSuite.js +++ b/frontend/src/components/Agreements/AgreementEditor/AgreementEditFormSuite.js @@ -9,8 +9,8 @@ const suite = create((data = {}, fieldName) => { test("name", "This is required information", () => { enforce(data.name).isNotBlank(); }); - test("serviceReqType", "This is required information", () => { - enforce(data.serviceReqType).notEquals("-Select Service Requirement Type-"); + test("service_requirement_type", "This is required information", () => { + enforce(data.service_requirement_type).notEquals("-Select Service Requirement Type-"); }); test("description", "This is required information", () => { enforce(data.description).isNotBlank(); @@ -20,8 +20,6 @@ const suite = create((data = {}, fieldName) => { }); test("agreement_reason", "This is required information", () => { enforce(data.agreement_reason).isNotBlank(); - }); - test("agreement_reason", "This is required information", () => { enforce(data.agreement_reason).notEquals("0"); }); test("incumbent", "This is required information", () => { @@ -37,6 +35,7 @@ const suite = create((data = {}, fieldName) => { }); test("contract-type", "This is required information", () => { enforce(data.contract_type).notEquals("-Select an option-"); + enforce(data.contract_type).isNotEmpty(); }); test("team-members", "This is required information", () => { enforce(data.team_members).lengthNotEquals(0); diff --git a/frontend/src/components/BudgetLineItems/CreateBLIsAndSCs/CreateBLIsAndSCs.jsx b/frontend/src/components/BudgetLineItems/CreateBLIsAndSCs/CreateBLIsAndSCs.jsx index e68b66f7c9..274c8a7d98 100644 --- a/frontend/src/components/BudgetLineItems/CreateBLIsAndSCs/CreateBLIsAndSCs.jsx +++ b/frontend/src/components/BudgetLineItems/CreateBLIsAndSCs/CreateBLIsAndSCs.jsx @@ -1,6 +1,5 @@ import PropTypes from "prop-types"; import React from "react"; -import { convertCodeForDisplay } from "../../../helpers/utils"; import EditModeTitle from "../../../pages/agreements/EditModeTitle"; import AgreementBudgetLinesHeader from "../../Agreements/AgreementBudgetLinesHeader"; import AgreementTotalCard from "../../Agreements/AgreementDetailsCards/AgreementTotalCard"; @@ -61,7 +60,6 @@ export const CreateBLIsAndSCs = ({ setIncludeDrafts }) => { const { - budgetLinePageErrorsExist, handleDeleteBudgetLine, handleDuplicateBudgetLine, handleEditBLI, @@ -210,7 +208,7 @@ export const CreateBLIsAndSCs = ({ /> )} - {budgetLinePageErrorsExist && ( + {pageErrors && (
    - {convertCodeForDisplay("validation", key)}: { {value.map((message, index) => ( diff --git a/frontend/src/components/BudgetLineItems/CreateBLIsAndSCs/suite.js b/frontend/src/components/BudgetLineItems/CreateBLIsAndSCs/suite.js index a8cfb7a72b..904bc9851d 100644 --- a/frontend/src/components/BudgetLineItems/CreateBLIsAndSCs/suite.js +++ b/frontend/src/components/BudgetLineItems/CreateBLIsAndSCs/suite.js @@ -4,8 +4,8 @@ const suite = create((data) => { only(data); // test to ensure at least one budget line item exists - test("data", "Must have at least one budget line item", () => { - enforce(data.budgetLines).longerThan(0); + test("data", "Must have at least one budget line", () => { + enforce(data.budgetLines.length).greaterThan(0); }); // test budget_line_items array each(data.budgetLines, (item) => { diff --git a/frontend/src/components/ServicesComponents/ContractTypeSelect/ContractTypeSelect.jsx b/frontend/src/components/ServicesComponents/ContractTypeSelect/ContractTypeSelect.jsx index 357b09de93..c7bd7d5f4e 100644 --- a/frontend/src/components/ServicesComponents/ContractTypeSelect/ContractTypeSelect.jsx +++ b/frontend/src/components/ServicesComponents/ContractTypeSelect/ContractTypeSelect.jsx @@ -18,10 +18,10 @@ function ContractTypeSelect({ value, onChange, ...rest }) { { if (!agreement) { return
    No agreement data available.
    ; } + + /* + TODO: Add FE validation to ensure that the user is authorized to approve the agreement + and if not, display an error page + to prevent unauthorized users from accessing this page + */ const BeforeApprovalContent = React.memo( ({ groupedBudgetLinesByServicesComponent, servicesComponents, changeRequestTitle, urlChangeToStatus }) => ( <> diff --git a/frontend/src/pages/agreements/review/suite.js b/frontend/src/pages/agreements/review/suite.js index dd3eee1e04..1266832ed3 100644 --- a/frontend/src/pages/agreements/review/suite.js +++ b/frontend/src/pages/agreements/review/suite.js @@ -32,7 +32,8 @@ const suite = create((fieldName) => { enforce(fieldName.project_officer_id).isNotBlank(); }); test("contract-type", "This is required information", () => { - enforce(fieldName.contract_type).isNotBlank(); + enforce(fieldName.contract_type).notEquals("-Select an option-"); + enforce(fieldName.contract_type).isNotEmpty(); }); test("team-members", "This is required information", () => { enforce(fieldName.team_members).longerThan(0);