diff --git a/backend/ops_api/ops/validation/rules/procurement_tracker_step.py b/backend/ops_api/ops/validation/rules/procurement_tracker_step.py index 08ab303282..dd564bd6ed 100644 --- a/backend/ops_api/ops/validation/rules/procurement_tracker_step.py +++ b/backend/ops_api/ops/validation/rules/procurement_tracker_step.py @@ -116,16 +116,27 @@ def validate(self, procurement_tracker_step: ProcurementTrackerStep, context: Va class NoUpdatingCompletedProcurementStepRule(ValidationRule): """ - Validates that completed procurement tracker steps cannot be updated. + Validates that completed procurement tracker steps cannot be updated, + except when the only field being updated is the notes. """ + # Fields that are still editable after a step has been completed. + EDITABLE_AFTER_COMPLETION = frozenset({"notes"}) + @property def name(self) -> str: return "No Updating Completed Procurement Step" def validate(self, procurement_tracker_step: ProcurementTrackerStep, context: ValidationContext) -> None: - if procurement_tracker_step.status == ProcurementTrackerStepStatus.COMPLETED: - raise ValidationError({"status": "Cannot update a procurement tracker step that is already completed."}) + if procurement_tracker_step.status != ProcurementTrackerStepStatus.COMPLETED: + return + + # Allow notes-only edits on a completed step; block any other field change. + updated_field_names = set(context.updated_fields.keys()) + if updated_field_names.issubset(self.EDITABLE_AFTER_COMPLETION): + return + + raise ValidationError({"status": "Cannot update a procurement tracker step that is already completed."}) class AcquisitionPlanningRequiredFieldsRule(ValidationRule): @@ -140,20 +151,26 @@ def name(self) -> str: return "Required Fields Check" def validate(self, procurement_tracker_step: ProcurementTrackerStep, context: ValidationContext) -> None: + if not is_procurement_tracker_step_updated_to_complete(context): + return updated_fields = context.updated_fields - acquisition_planning = ["notes", "task_completed_by", "date_completed"] - required_acquisition_planning_fields = ["task_completed_by", "date_completed"] - acquisition_planning_field_found = [field for field in acquisition_planning if field in updated_fields] - if acquisition_planning_field_found: - missing_fields = [field for field in required_acquisition_planning_fields if field not in updated_fields] - if missing_fields: - raise ValidationError( - { - field: f"{field} is required when updating procurement tracker step with acquisition planning package provided." - for field in missing_fields - } - ) + mapping = { + "task_completed_by": "acquisition_planning_task_completed_by", + "date_completed": "acquisition_planning_date_completed", + } + required_fields = ["task_completed_by", "date_completed"] + missing_fields = [field for field in required_fields if field not in updated_fields] + final_missing_fields = [ + field for field in missing_fields if getattr(procurement_tracker_step, mapping[field], None) is None + ] + if final_missing_fields: + raise ValidationError( + { + field: f"{field} is required when completing acquisition planning step." + for field in final_missing_fields + } + ) class NoFutureCompletionDateUpdateValidationRule(ValidationRule): diff --git a/backend/ops_api/tests/ops/features/test_validate_procurement_tracker_steps.py b/backend/ops_api/tests/ops/features/test_validate_procurement_tracker_steps.py index dcd204b9a9..4977ed9e20 100644 --- a/backend/ops_api/tests/ops/features/test_validate_procurement_tracker_steps.py +++ b/backend/ops_api/tests/ops/features/test_validate_procurement_tracker_steps.py @@ -101,7 +101,7 @@ def test_validate_updating_procurement_tracker_step_with_valid_status(): ... @scenario( "validate_procurement_tracker_steps.feature", - "When no presolicitation package is sent to proc shop, the request is valid with unfilled request", + "Cannot complete acquisition planning step without required fields", ) def test_validate_updating_procurement_tracker_step_without_presolicitation_package(): ... @@ -113,6 +113,13 @@ def test_validate_updating_procurement_tracker_step_without_presolicitation_pack def test_cannot_update_completed_procurement_tracker_step(): ... +@scenario( + "validate_procurement_tracker_steps.feature", + "Can update only the notes on a completed procurement tracker step", +) +def test_can_update_notes_only_on_completed_procurement_tracker_step(): ... + + @scenario( "validate_procurement_tracker_steps.feature", "Validate Procurement Tracker Step exists", @@ -590,6 +597,11 @@ def have_valid_completed_procurement_step(context): context["request_body"] = data +@when("I have a notes-only update for the completed step") +def have_notes_only_update_for_completed_step(context): + context["request_body"] = {"notes": "Adding a note after completion."} + + @when("I have a procurement step with a non-existent user in the task_completed_by step") def have_procurement_step_with_nonexistent_user(context): data = { @@ -936,6 +948,17 @@ def check_invalid_status_error_message(context, setup_and_teardown): assert response.status_code == 400 +@then("I should get a message that the notes were updated successfully") +def check_notes_only_update_success(context, loaded_db, setup_and_teardown): + response = context["response_patch"] + assert response.status_code == 200, response.get_json() + + json_data = response.get_json() + assert json_data["notes"] == "Adding a note after completion." + # The step should remain completed after a notes-only update. + assert json_data["status"] == ProcurementTrackerStepStatus.COMPLETED.name + + @then("I should get a resource not found error") def check_resource_not_found_error_message(context, setup_and_teardown): response = context["response_patch"] diff --git a/backend/ops_api/tests/ops/features/validate_procurement_tracker_steps.feature b/backend/ops_api/tests/ops/features/validate_procurement_tracker_steps.feature index e556e679b8..ef55922792 100644 --- a/backend/ops_api/tests/ops/features/validate_procurement_tracker_steps.feature +++ b/backend/ops_api/tests/ops/features/validate_procurement_tracker_steps.feature @@ -68,7 +68,7 @@ Feature: Validate Procurement Tracker Steps Then I should get a validation error -Scenario: When no presolicitation package is sent to proc shop, the request is valid with unfilled request +Scenario: Cannot complete acquisition planning step without required fields Given I am logged in as an OPS user And I have a Contract Agreement with OPS user as a team member And I have a procurement tracker @@ -77,7 +77,7 @@ Scenario: When no presolicitation package is sent to proc shop, the request is v When I have a procurement step with no presolicitation package sent to procurement shop And I submit a procurement step update - Then I should get a message that it was successful and my procurement tracker has moved onto the next step + Then I should get a validation error Scenario: Cannot update completed procurement tracker step Given I am logged in as an OPS user @@ -90,6 +90,17 @@ Scenario: Cannot update completed procurement tracker step Then I should get a validation error +Scenario: Can update only the notes on a completed procurement tracker step + Given I am logged in as an OPS user + And I have a Contract Agreement with OPS user as a team member + And I have a procurement tracker with a completed step 1 + And I am working with acquisition planning procurement tracker step + + When I have a notes-only update for the completed step + And I submit a procurement step update + + Then I should get a message that the notes were updated successfully + Scenario: Validate Procurement Tracker Step exists Given I am logged in as an OPS user And I have a Contract Agreement with OPS user as a team member diff --git a/backend/ops_api/tests/ops/procurement_tracker/test_procurement_tracker_steps_api.py b/backend/ops_api/tests/ops/procurement_tracker/test_procurement_tracker_steps_api.py index dc6d89cb42..e0d50df288 100644 --- a/backend/ops_api/tests/ops/procurement_tracker/test_procurement_tracker_steps_api.py +++ b/backend/ops_api/tests/ops/procurement_tracker/test_procurement_tracker_steps_api.py @@ -448,8 +448,8 @@ def test_update_procurement_tracker_step_creates_update_tracker_event(auth_clien select(func.count()).select_from(OpsEvent).where(OpsEvent.event_type == OpsEventType.UPDATE_PROCUREMENT_TRACKER) ) - # Complete the first step - update_data = {"status": "COMPLETED"} + # Complete the first step (acquisition planning step requires date_completed and task_completed_by) + update_data = {"status": "COMPLETED", "date_completed": date.today().isoformat(), "task_completed_by": 503} response = auth_client.patch(f"/api/v1/procurement-tracker-steps/{first_step.id}", json=update_data) assert response.status_code == 200 diff --git a/frontend/cypress/e2e/procurementTracker.cy.js b/frontend/cypress/e2e/procurementTracker.cy.js index 2d5f92a8f1..354b4e3cbf 100644 --- a/frontend/cypress/e2e/procurementTracker.cy.js +++ b/frontend/cypress/e2e/procurementTracker.cy.js @@ -288,7 +288,7 @@ describe("Procurement Tracker Step 2", () => { if (isPending) { cy.get("#users-combobox-input").should("be.disabled"); cy.get("#step-2-date-completed").should("be.disabled"); - cy.get("#notes").should("be.disabled"); + cy.get("#notes").should("not.be.disabled"); cy.get("#step-2-draft-solicitation-date").should("be.disabled"); cy.get('[data-cy="cancel-button"]').should("be.disabled"); cy.get('[data-cy="continue-btn"]').should("be.disabled"); @@ -635,7 +635,7 @@ describe("Procurement Tracker Step 3: Solicitation", () => { if (isPending) { cy.get("#users-combobox-input").should("be.disabled"); cy.get("#step-3-date-completed").should("be.disabled"); - cy.get("#notes").should("be.disabled"); + cy.get("#notes").should("not.be.disabled"); cy.get('[data-cy="cancel-button"]').should("be.disabled"); cy.get('[data-cy="continue-btn"]').should("be.disabled"); } @@ -932,7 +932,7 @@ describe("Procurement Tracker Step 5: Pre-Award", () => { if (isPending) { cy.get("#users-combobox-input").should("be.disabled"); cy.get("#step-5-date-completed").should("be.disabled"); - cy.get("#notes").should("be.disabled"); + cy.get("#notes").should("not.be.disabled"); cy.get('[data-cy="cancel-button"]').should("be.disabled"); cy.get('[data-cy="continue-btn"]').should("be.disabled"); } diff --git a/frontend/src/components/Agreements/ProcurementTracker/ProcurementTracker.constants.js b/frontend/src/components/Agreements/ProcurementTracker/ProcurementTracker.constants.js index d40d7a135d..7382f112fd 100644 --- a/frontend/src/components/Agreements/ProcurementTracker/ProcurementTracker.constants.js +++ b/frontend/src/components/Agreements/ProcurementTracker/ProcurementTracker.constants.js @@ -46,3 +46,15 @@ export const ProcurementTrackerPreAwardApprovalStatus = { DECLINED: "DECLINED", CANCELLED: "CANCELLED" }; + +/** + * Maximum number of characters allowed in a procurement tracker step's notes field. + * @type {number} + */ +export const STEP_NOTES_MAX_LENGTH = 750; + +/** + * Shared inline style for the notes TextArea rendered in each procurement tracker step. + * @type {{ height: string, minWidth: string }} + */ +export const STEP_NOTES_TEXTAREA_STYLE = { height: "8.5rem", minWidth: "30rem" }; diff --git a/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.hooks.js b/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.hooks.js index 6c51badae9..284366a44c 100644 --- a/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.hooks.js +++ b/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.hooks.js @@ -5,6 +5,7 @@ import DatePicker from "../../../UI/USWDS/DatePicker"; import suite from "./suite"; import { useUpdateProcurementTrackerStepMutation } from "../../../../api/opsAPI"; import useAlert from "../../../../hooks/use-alert.hooks"; +import useSaveNotes from "../useSaveNotes"; /** * @typedef {import("../../../../types/ProcurementTrackerTypes").ProcurementTrackerPreAwardStep} ProcurementTrackerPreAwardStep @@ -21,7 +22,6 @@ export default function useProcurementTrackerStepFive(stepFiveData, handleSetCom const [selectedUser, setSelectedUser] = React.useState(/** @type {SafeUser | undefined} */ (undefined)); const [targetCompletionDate, setTargetCompletionDate] = React.useState(""); const [step5DateCompleted, setStep5DateCompleted] = React.useState(""); - const [step5Notes, setStep5Notes] = React.useState(""); const [showModal, setShowModal] = React.useState(false); const [modalProps, setModalProps] = React.useState({ heading: "", @@ -49,6 +49,13 @@ export default function useProcurementTrackerStepFive(stepFiveData, handleSetCom let validatorRes = suite.get(); + const { + notes: step5Notes, + setNotes: setStep5Notes, + resetNotes: resetStep5Notes, + handleSaveNotes + } = useSaveNotes(patchStepFive, stepFiveData?.notes, setAlert); + /** * Handles the submission of the target completion date for step five, updating the procurement tracker step with the new date. * @param {number} stepId - The ID of the procurement tracker step being updated. @@ -119,7 +126,7 @@ export default function useProcurementTrackerStepFive(stepFiveData, handleSetCom setSelectedUser(undefined); setTargetCompletionDate(""); setStep5DateCompleted(""); - setStep5Notes(""); + resetStep5Notes(stepFiveData?.notes ?? ""); }; const cancelModalStep5 = () => { @@ -136,6 +143,7 @@ export default function useProcurementTrackerStepFive(stepFiveData, handleSetCom return { cancelStepFive, + handleSaveNotes, isPreAwardComplete, setIsPreAwardComplete, selectedUser, @@ -149,6 +157,7 @@ export default function useProcurementTrackerStepFive(stepFiveData, handleSetCom step5TargetCompletionDateLabel, step5Notes, setStep5Notes, + resetStep5Notes, step5NotesLabel, runValidate, validatorRes, diff --git a/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.hooks.test.js b/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.hooks.test.js index a5f4e48aa1..65ce9092cf 100644 --- a/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.hooks.test.js +++ b/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.hooks.test.js @@ -67,7 +67,7 @@ describe("useProcurementTrackerStepFive", () => { expect(result.current.selectedUser).toBeUndefined(); expect(result.current.targetCompletionDate).toBe(""); expect(result.current.step5DateCompleted).toBe(""); - expect(result.current.step5Notes).toBe(""); + expect(result.current.step5Notes).toBe("Pre-award approval received"); // Notes initialize from existing stepData.notes }); it("returns all setter functions", () => { @@ -554,7 +554,7 @@ describe("useProcurementTrackerStepFive", () => { expect(result.current.selectedUser).toBeUndefined(); expect(result.current.targetCompletionDate).toBe(""); expect(result.current.step5DateCompleted).toBe(""); - expect(result.current.step5Notes).toBe(""); + expect(result.current.step5Notes).toBe(mockStepFiveData.notes); }); }); @@ -606,7 +606,7 @@ describe("useProcurementTrackerStepFive", () => { }); expect(result.current.isPreAwardComplete).toBe(false); - expect(result.current.step5Notes).toBe(""); + expect(result.current.step5Notes).toBe(mockStepFiveData.notes); }); }); diff --git a/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.jsx b/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.jsx index db94c3dae4..bd1e16db9f 100644 --- a/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.jsx +++ b/frontend/src/components/Agreements/ProcurementTracker/ProcurementTrackerStepFive/ProcurementTrackerStepFive.jsx @@ -1,11 +1,12 @@ import { useNavigate } from "react-router-dom"; import { getLocalISODate } from "../../../../helpers/utils"; -import TextArea from "../../../UI/Form/TextArea"; import ConfirmationModal from "../../../UI/Modals/ConfirmationModal"; import TermTag from "../../../UI/Term/TermTag"; import Tooltip from "../../../UI/USWDS/Tooltip/Tooltip"; import UsersComboBox from "../../UsersComboBox"; import useProcurementTrackerStepFive from "./ProcurementTrackerStepFive.hooks"; +import StepNotesEditor from "../StepNotesEditor/StepNotesEditor"; +import StepNotesForm from "../StepNotesForm/StepNotesForm"; import { faCircleCheck } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { PROCUREMENT_STEP_STATUS, ProcurementTrackerPreAwardApprovalStatus } from "../ProcurementTracker.constants"; @@ -57,6 +58,7 @@ const ProcurementTrackerStepFive = ({ setStep5DateCompleted, step5Notes, setStep5Notes, + resetStep5Notes, step5NotesLabel, runValidate, validatorRes, @@ -68,6 +70,7 @@ const ProcurementTrackerStepFive = ({ setShowModal, modalProps, cancelModalStep5, + handleSaveNotes, handleStepFiveComplete } = useProcurementTrackerStepFive(stepFiveData, handleSetCompletedStepNumber); @@ -352,14 +355,11 @@ const ProcurementTrackerStepFive = ({ isDisabled={isPreAwardFieldsDisabled} /> -