From 61887bca238cf9bd696559bb9d0ee64bc8974ad5 Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Mon, 23 Sep 2024 15:04:20 -0500 Subject: [PATCH 01/10] wip: permission check --- .../agreements/approve/ApproveAgreement.jsx | 122 ++++++++++-------- 1 file changed, 70 insertions(+), 52 deletions(-) diff --git a/frontend/src/pages/agreements/approve/ApproveAgreement.jsx b/frontend/src/pages/agreements/approve/ApproveAgreement.jsx index cbc145e05b..2d641bf0c2 100644 --- a/frontend/src/pages/agreements/approve/ApproveAgreement.jsx +++ b/frontend/src/pages/agreements/approve/ApproveAgreement.jsx @@ -17,6 +17,8 @@ import { findDescription, findPeriodEnd, findPeriodStart } from "../../../helper import { convertCodeForDisplay } from "../../../helpers/utils"; import { document } from "../../../tests/data"; import useApproveAgreement from "./ApproveAgreement.hooks"; +import ErrorPage from "../../ErrorPage"; +import DebugCode from "../../../components/DebugCode"; const ApproveAgreement = () => { const { @@ -49,6 +51,20 @@ const ApproveAgreement = () => { approvedBudgetLinesPreview } = useApproveAgreement(); + const isLoggedInUserDivisionDirector = false; + const doesAgreementHaveChangeRequests = false; + const doesAgreementBelongToDivisionDirector = false; + + const hasPermissionToViewPage = + isLoggedInUserDivisionDirector && + isUserATeamMember && + doesAgreementHaveChangeRequests && + doesAgreementBelongToDivisionDirector; + + // if (!hasPermissionToViewPage) { + // return ; + // } + if (isLoadingAgreement) { return
Loading...
; } @@ -58,58 +74,6 @@ const ApproveAgreement = () => { if (!agreement) { return
No agreement data available.
; } - const BeforeApprovalContent = React.memo( - ({ groupedBudgetLinesByServicesComponent, servicesComponents, changeRequestTitle, urlChangeToStatus }) => ( - <> - {groupedBudgetLinesByServicesComponent.map((group) => ( - - - - ))} - - ) - ); - BeforeApprovalContent.displayName = "BeforeApprovalContent"; - - const AfterApprovalContent = React.memo( - ({ - groupedUpdatedBudgetLinesByServicesComponent, - servicesComponents, - changeRequestTitle, - urlChangeToStatus - }) => ( - <> - {groupedUpdatedBudgetLinesByServicesComponent.map((group) => ( - - - - ))} - - ) - ); - AfterApprovalContent.displayName = "AfterApprovalContent"; return ( @@ -126,6 +90,12 @@ const ApproveAgreement = () => { title={title} subTitle={agreement.name} /> + + + { ); }; +const BeforeApprovalContent = React.memo( + ({ groupedBudgetLinesByServicesComponent, servicesComponents, changeRequestTitle, urlChangeToStatus }) => ( + <> + {groupedBudgetLinesByServicesComponent.map((group) => ( + + + + ))} + + ) +); +BeforeApprovalContent.displayName = "BeforeApprovalContent"; + +const AfterApprovalContent = React.memo( + ({ groupedUpdatedBudgetLinesByServicesComponent, servicesComponents, changeRequestTitle, urlChangeToStatus }) => ( + <> + {groupedUpdatedBudgetLinesByServicesComponent.map((group) => ( + + + + ))} + + ) +); +AfterApprovalContent.displayName = "AfterApprovalContent"; + export default ApproveAgreement; From 1974573e067f3e545986af746d712ae24c38a31f Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Mon, 23 Sep 2024 16:57:07 -0500 Subject: [PATCH 02/10] feat: adds division_id to CAN prop --- .gitignore | 5 --- .vscode/settings.json | 8 +++++ backend/openapi.yml | 34 +++++++++++++++++++ .../ops_api/ops/schemas/budget_line_items.py | 4 +++ 4 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 .vscode/settings.json diff --git a/.gitignore b/.gitignore index ccb949a30a..f5387fb2de 100644 --- a/.gitignore +++ b/.gitignore @@ -185,11 +185,6 @@ yarn-error.log* # MacOS stuff .DS_Store -# IDE -.vscode/* -# allow vscode launch/tasks to be shared -!.vscode/launch.json -!.vscode/tasks.json .idea # HTTP-Client diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000000..c56423eb20 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,8 @@ +{ + "python.testing.pytestArgs": [ + "tests" + ], + "python.testing.cwd": "./backend/ops_api", + "python.testing.unittestEnabled": false, + "python.testing.pytestEnabled": true +} \ No newline at end of file diff --git a/backend/openapi.yml b/backend/openapi.yml index 4db9087697..784079219e 100644 --- a/backend/openapi.yml +++ b/backend/openapi.yml @@ -2740,6 +2740,9 @@ components: can_id: type: integer example: 1 + can: + type: object + $ref: "#/components/schemas/BudgetLineItemCAN" comments: type: string date_needed: @@ -2805,6 +2808,37 @@ components: description: optional notes added to a Change Request when a PATCH is made that creates a CR type: string writeOnly: true + BudgetLineItemCAN: + type: object + properties: + id: + type: integer + example: 1 + portfolio: + type: object + $ref: "#/components/schemas/PortfolioBLISchema" + portfolio_id: + type: integer + example: 1 + display_name: + type: string + nick_name: + type: string + number: + type: string + description: + type: string + active_period: + type: integer + expiration_date: + type: integer + appropriation_date: + type: integer + PortfolioBLISchema: + type: object + properties: + division_id: + type: integer BudgetLineItemRequest: type: object properties: diff --git a/backend/ops_api/ops/schemas/budget_line_items.py b/backend/ops_api/ops/schemas/budget_line_items.py index 5788ceeab5..459e9caba8 100644 --- a/backend/ops_api/ops/schemas/budget_line_items.py +++ b/backend/ops_api/ops/schemas/budget_line_items.py @@ -251,8 +251,12 @@ class Meta: email = fields.Str(default=None, allow_none=True) +class PortfolioBLISchema(Schema): + division_id = fields.Int(required=True) + class BudgetLineItemCANSchema(Schema): id = fields.Int(required=True) + portfolio = fields.Nested(PortfolioBLISchema()) display_name = fields.Str(required=True) number = fields.Str(required=True) description = fields.Str(required=True) From b3a881f476836108ed871a428e2acc3482dc2180 Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Tue, 24 Sep 2024 10:05:22 -0500 Subject: [PATCH 03/10] docs: fixes openapi issues --- backend/openapi.yml | 59 +++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 34 deletions(-) diff --git a/backend/openapi.yml b/backend/openapi.yml index 784079219e..7513d1fd9c 100644 --- a/backend/openapi.yml +++ b/backend/openapi.yml @@ -227,11 +227,11 @@ paths: required: true content: application/json: - schema: - $ref: "#/components/schemas/CreateCANRequestSchema" - examples: - "0": - $ref: "#/components/examples/CreateCANRequestSchema" + schema: + $ref: "#/components/schemas/CreateUpdateCANRequestSchema" + examples: + "0": + $ref: "#/components/examples/CreateUpdateCANRequestSchema" responses: "201": description: Created @@ -440,7 +440,7 @@ paths: application/json: schema: type: array - properties: { } + properties: {} items: type: string examples: @@ -619,7 +619,7 @@ paths: type: integer first_name: type: string - updated: { } + updated: {} email: type: string examples: @@ -2058,7 +2058,11 @@ components: type: string id: type: integer - CreateCANRequestSchema: + required: + - number + - portfolio_id + - id + CreateUpdateCANRequestSchema: description: The request object for creating a new Common Accounting Number (CAN) object. properties: nick_name: @@ -2069,6 +2073,8 @@ components: type: string portfolio_id: type: integer + funding_details_id: + type: integer required: - number - portfolio_id @@ -2097,7 +2103,7 @@ components: funding_received: type: array items: - $ref: "#/components/schemas/FundingReceived" + $ref: "#/components/schemas/FundingReceived" number: type: string portfolio: @@ -2559,16 +2565,8 @@ components: type: object example: [ - { - "id": 1, - "full_name": "Chris Fortunato", - "email": "chris.fortunato@example.com", - }, - { - "id": 2, - "full_name": "Amy Madigan", - "email": "Amy.Madigan@example.com", - }, + { "id": 1, "full_name": "Chris Fortunato", "email": "chris.fortunato@example.com" }, + { "id": 2, "full_name": "Amy Madigan", "email": "Amy.Madigan@example.com" }, { "id": 3, "full_name": "Ivelisse Martinez-Beck", @@ -2740,7 +2738,7 @@ components: can_id: type: integer example: 1 - can: + can: type: object $ref: "#/components/schemas/BudgetLineItemCAN" comments: @@ -2768,16 +2766,8 @@ components: type: object example: [ - { - "id": 1, - "full_name": "Chris Fortunato", - "email": "chris.fortunato@example.com", - }, - { - "id": 2, - "full_name": "Amy Madigan", - "email": "Amy.Madigan@example.com", - }, + { "id": 1, "full_name": "Chris Fortunato", "email": "chris.fortunato@example.com" }, + { "id": 2, "full_name": "Amy Madigan", "email": "Amy.Madigan@example.com" }, { "id": 3, "full_name": "Ivelisse Martinez-Beck", @@ -2963,7 +2953,7 @@ components: type: array items: type: object - example: [ { "id": 1 }, { "id": 2 }, { "id": 3 } ] + example: [{ "id": 1 }, { "id": 2 }, { "id": 3 }] required: - title @@ -3282,7 +3272,8 @@ components: document_name: type: string document_size: - type: number(10, 2) + type: number + description: precision number with 10 digits and 2 decimal places status: type: string created_on: @@ -3657,7 +3648,7 @@ components: "updated_by": 1 } ] - CreateCanRequestSchema: + CreateUpdateCANRequestSchema: value: | { nick_name: "Very Good CAN", @@ -3670,4 +3661,4 @@ components: [ ] security: - - bearerAuth: [ ] + - bearerAuth: [] From a4dab5daa326a610ceed7c34e3270e791c24909f Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Tue, 24 Sep 2024 14:06:31 -0500 Subject: [PATCH 04/10] wip --- .../agreements/approve/ApproveAgreement.jsx | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/frontend/src/pages/agreements/approve/ApproveAgreement.jsx b/frontend/src/pages/agreements/approve/ApproveAgreement.jsx index 2d641bf0c2..440717cfe6 100644 --- a/frontend/src/pages/agreements/approve/ApproveAgreement.jsx +++ b/frontend/src/pages/agreements/approve/ApproveAgreement.jsx @@ -1,4 +1,5 @@ import React from "react"; +import { useSelector } from "react-redux"; import App from "../../../App"; import AgreementBLIAccordion from "../../../components/Agreements/AgreementBLIAccordion"; import AgreementCANReviewAccordion from "../../../components/Agreements/AgreementCANReviewAccordion"; @@ -7,6 +8,7 @@ import DocumentCollectionView from "../../../components/Agreements/Documents/Doc import BLIDiffTable from "../../../components/BudgetLineItems/BLIDiffTable"; import { CHANGE_REQUEST_ACTION } from "../../../components/ChangeRequests/ChangeRequests.constants"; import ReviewChangeRequestAccordion from "../../../components/ChangeRequests/ReviewChangeRequestAccordion"; +import DebugCode from "../../../components/DebugCode"; import ServicesComponentAccordion from "../../../components/ServicesComponents/ServicesComponentAccordion"; import Accordion from "../../../components/UI/Accordion"; import TextArea from "../../../components/UI/Form/TextArea"; @@ -16,9 +18,8 @@ import { BLI_STATUS } from "../../../helpers/budgetLines.helpers"; import { findDescription, findPeriodEnd, findPeriodStart } from "../../../helpers/servicesComponent.helpers"; import { convertCodeForDisplay } from "../../../helpers/utils"; import { document } from "../../../tests/data"; -import useApproveAgreement from "./ApproveAgreement.hooks"; import ErrorPage from "../../ErrorPage"; -import DebugCode from "../../../components/DebugCode"; +import useApproveAgreement from "./ApproveAgreement.hooks"; const ApproveAgreement = () => { const { @@ -51,15 +52,18 @@ const ApproveAgreement = () => { approvedBudgetLinesPreview } = useApproveAgreement(); - const isLoggedInUserDivisionDirector = false; - const doesAgreementHaveChangeRequests = false; - const doesAgreementBelongToDivisionDirector = false; + const userRoles = useSelector((state) => state.auth?.activeUser?.roles) ?? []; + const userIsDivisionDirector = userRoles.includes("division-director") ?? false; + const userDivisionId = useSelector((state) => state.auth?.activeUser?.division) ?? null; + // NOTE: May be able to get from agreement.change_requests_in_review.managing_division_id + const allCanDivisionIDs = agreement?.budget_line_items?.map((bli) => bli.can.portfolio.division_id); + const uniqueCanDivisionIDs = [...new Set(allCanDivisionIDs)]; + const doesAgreementBelongToDivisionDirector = uniqueCanDivisionIDs.includes(userDivisionId) ?? false; + console.log({ uniqueCanDivisionIDs, userDivisionId, doesAgreementBelongToDivisionDirector }); + const agreementIsInReview = agreement?.in_review ?? false; const hasPermissionToViewPage = - isLoggedInUserDivisionDirector && - isUserATeamMember && - doesAgreementHaveChangeRequests && - doesAgreementBelongToDivisionDirector; + userIsDivisionDirector && agreementIsInReview && doesAgreementBelongToDivisionDirector; // if (!hasPermissionToViewPage) { // return ; From af5b981329ad9385d7e90e6f5bcc53a927c4b99b Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Tue, 24 Sep 2024 18:29:40 -0500 Subject: [PATCH 05/10] feat: adds feature-flag --- .../agreements/approve/ApproveAgreement.jsx | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/frontend/src/pages/agreements/approve/ApproveAgreement.jsx b/frontend/src/pages/agreements/approve/ApproveAgreement.jsx index be97abe13a..4b41ceb8c6 100644 --- a/frontend/src/pages/agreements/approve/ApproveAgreement.jsx +++ b/frontend/src/pages/agreements/approve/ApproveAgreement.jsx @@ -8,7 +8,7 @@ import DocumentCollectionView from "../../../components/Agreements/Documents/Doc import BLIDiffTable from "../../../components/BudgetLineItems/BLIDiffTable"; import { CHANGE_REQUEST_ACTION } from "../../../components/ChangeRequests/ChangeRequests.constants"; import ReviewChangeRequestAccordion from "../../../components/ChangeRequests/ReviewChangeRequestAccordion"; -import DebugCode from "../../../components/DebugCode"; +// import DebugCode from "../../../components/DebugCode"; import ServicesComponentAccordion from "../../../components/ServicesComponents/ServicesComponentAccordion"; import Accordion from "../../../components/UI/Accordion"; import TextArea from "../../../components/UI/Form/TextArea"; @@ -52,20 +52,26 @@ const ApproveAgreement = () => { approvedBudgetLinesPreview } = useApproveAgreement(); + const is2849Ready = false; // feature flag for 2849 readiness const userRoles = useSelector((state) => state.auth?.activeUser?.roles) ?? []; const userIsDivisionDirector = userRoles.includes("division-director") ?? false; const userDivisionId = useSelector((state) => state.auth?.activeUser?.division) ?? null; + // NOTE: May be able to get from agreement.change_requests_in_review.managing_division_id const allCanDivisionIDs = agreement?.budget_line_items?.map((bli) => bli.can.portfolio.division_id); const uniqueCanDivisionIDs = [...new Set(allCanDivisionIDs)]; const doesAgreementBelongToDivisionDirector = uniqueCanDivisionIDs.includes(userDivisionId) ?? false; - console.log({ uniqueCanDivisionIDs, userDivisionId, doesAgreementBelongToDivisionDirector }); - const agreementIsInReview = agreement?.in_review ?? false; + const agreementHasBLIsUnderReview = agreement?.budget_line_items?.some((bli) => bli.in_review) ?? false; const hasPermissionToViewPage = - userIsDivisionDirector && agreementIsInReview && doesAgreementBelongToDivisionDirector; + userIsDivisionDirector && agreementHasBLIsUnderReview && doesAgreementBelongToDivisionDirector; + // NOTE: This test is good enough for now until 2849 is ready + const isApproverAndAgreementInReview = userIsDivisionDirector && agreementHasBLIsUnderReview; - if (!hasPermissionToViewPage) { + if (!hasPermissionToViewPage && is2849Ready) { + return ; + } + if (!isApproverAndAgreementInReview) { return ; } @@ -95,10 +101,12 @@ const ApproveAgreement = () => { subTitle={agreement.name} /> - + /> + */} Date: Wed, 25 Sep 2024 11:43:59 -0500 Subject: [PATCH 06/10] chore: cleanup --- frontend/src/hooks/useToggle.js | 4 +- .../approve/ApproveAgreement.hooks.js | 51 ++++++++++++++----- .../agreements/approve/ApproveAgreement.jsx | 33 ++---------- 3 files changed, 44 insertions(+), 44 deletions(-) diff --git a/frontend/src/hooks/useToggle.js b/frontend/src/hooks/useToggle.js index b2ac8a2e2d..448824e8b2 100644 --- a/frontend/src/hooks/useToggle.js +++ b/frontend/src/hooks/useToggle.js @@ -4,8 +4,8 @@ import React from "react"; /** * A hook that returns a boolean value and a function to toggle it. * - * @param {boolean} initialValue - The initial value of the boolean state. - * @returns {[boolean, function]} - A tuple containing the boolean state and a function to toggle it. + * @param {boolean | (() => boolean)} initialValue - The initial value of the boolean state. + * @returns {[boolean, () => void]} - A tuple containing the boolean state and a function to toggle it. */ function useToggle(initialValue = false) { if (typeof initialValue !== "boolean" && typeof initialValue !== "function") { diff --git a/frontend/src/pages/agreements/approve/ApproveAgreement.hooks.js b/frontend/src/pages/agreements/approve/ApproveAgreement.hooks.js index e89694c076..8a8a592f64 100644 --- a/frontend/src/pages/agreements/approve/ApproveAgreement.hooks.js +++ b/frontend/src/pages/agreements/approve/ApproveAgreement.hooks.js @@ -18,11 +18,12 @@ import { useChangeRequestsForBudgetLines } from "../../../hooks/useChangeRequest import useGetUserFullNameFromId from "../../../hooks/user.hooks"; import useToggle from "../../../hooks/useToggle"; import { getTotalByCans } from "../review/ReviewAgreement.helpers"; +import { useSelector } from "react-redux"; /** - * @typedef {import('../../../components/ChangeRequests/ChangeRequests').ChangeRequest} ChangeRequest - * @type {ChangeRequest[]} + * @typedef {import('../../../components/ChangeRequests/ChangeRequestsTypes').ChangeRequest} ChangeRequest */ + /** * Custom hook for managing the approval process of an agreement * @typedef {Object} ApproveAgreementHookResult @@ -32,32 +33,35 @@ import { getTotalByCans } from "../review/ReviewAgreement.helpers"; * @property {Object[]} groupedBudgetLinesByServicesComponent - The budget lines grouped by services component * @property {Object[]} groupedUpdatedBudgetLinesByServicesComponent - The updated budget lines grouped by services component * @property {Object[]} budgetLinesInReview - The budget lines in review - * @property {ChangeRequest[]} changeRequestsInReview - The change requests in review + * @property {Object[]} changeRequestsInReview - The change requests in review * @property {Object} changeInCans - The change in CANs * @property {string} notes - The reviewer notes - * @property {Function} setNotes - The setter for reviewer notes + * @property {React.Dispatch>} setNotes - The setter for reviewer notes * @property {boolean} confirmation - The confirmation state - * @property {function(boolean): void} setConfirmation - The setter for confirmation state + * @property {React.Dispatch>} setConfirmation - The setter for confirmation state * @property {boolean} showModal - The modal visibility state - * @property {function(boolean): void} setShowModal - The setter for modal visibility + * @property {React.Dispatch>} setShowModal - The setter for modal visibility * @property {Object} modalProps - The modal properties * @property {string} checkBoxText - The text for the confirmation checkbox - * @property {function (): void} handleCancel - Function to handle cancellation of the approval process - * @property {Function} handleApproveChangeRequests - Function to handle approval of change requests + * @property {() => void} handleCancel - Function to handle cancellation of the approval process + * @property {(action: 'APPROVE' | 'REJECT') => void} handleApproveChangeRequests - Function to handle approval of change requests * @property {string} title - The title of the approval page * @property {boolean} afterApproval - The after approval state - * @property {Function} setAfterApproval - The setter for after approval state + * @property {() => void} setAfterApproval - The setter for after approval state * @property {string} requestorNoters - The submitter's notes * @property {string} urlChangeToStatus - The status change to from the URL * @property {string} statusForTitle - The status for the title - * @property {string} changeRequestTitle - The title of the change request, - * @property {typeof CHANGE_REQUEST_SLUG_TYPES.BUDGET | typeof CHANGE_REQUEST_SLUG_TYPES.STATUS} statusChangeTo - The type of change request - * @property { import("@reduxjs/toolkit/query").FetchBaseQueryError | import("@reduxjs/toolkit").SerializedError | undefined} errorAgreement - The error state for the agreement + * @property {string} changeRequestTitle - The title of the change request + * @property {typeof import('../../../components/ChangeRequests/ChangeRequests.constants').CHANGE_REQUEST_SLUG_TYPES.BUDGET | typeof import('../../../components/ChangeRequests/ChangeRequests.constants').CHANGE_REQUEST_SLUG_TYPES.STATUS} statusChangeTo - The type of change request + * @property {import("@reduxjs/toolkit/query").FetchBaseQueryError | import("@reduxjs/toolkit").SerializedError | undefined} errorAgreement - The error state for the agreement * @property {boolean} isLoadingAgreement - The loading state for the agreement * @property {Object[]} approvedBudgetLinesPreview - The updated budget lines - * + * @property {boolean} is2849Ready - The readiness state for the 2849 form + * @property {boolean} hasPermissionToViewPage - The permission to view the page. Dependant on 2849 + * @property {boolean} isApproverAndAgreementInReview - If logged in user has role of division director and agreement is in review state * @returns {ApproveAgreementHookResult} The data and functions for the approval process */ + const useApproveAgreement = () => { const { setAlert } = useAlert(); const urlPathParams = useParams(); @@ -169,6 +173,22 @@ const useApproveAgreement = () => { const budgetLinesToPlannedMessages = useChangeRequestsForBudgetLines(budgetLinesInReview, BLI_STATUS.PLANNED); const budgetLinesToExecutingMessages = useChangeRequestsForBudgetLines(budgetLinesInReview, BLI_STATUS.EXECUTING); + // NOTE: Permission checks + const is2849Ready = false; // feature flag for 2849 readiness + const userRoles = useSelector((state) => state.auth?.activeUser?.roles) ?? []; + const userIsDivisionDirector = userRoles.includes("division-director") ?? false; + const userDivisionId = useSelector((state) => state.auth?.activeUser?.division) ?? null; + const managingDivisionIds = + agreement?.budget_line_items?.flatMap((bli) => + bli.change_requests_in_review.map((cr) => cr.managing_division_id) + ) ?? []; + const doesAgreementBelongToDivisionDirector = managingDivisionIds.includes(userDivisionId) ?? false; + const agreementHasBLIsUnderReview = agreement?.budget_line_items?.some((bli) => bli.in_review) ?? false; + const hasPermissionToViewPage = + userIsDivisionDirector && agreementHasBLIsUnderReview && doesAgreementBelongToDivisionDirector; + // NOTE: This test is good enough for now until 2849 is ready + const isApproverAndAgreementInReview = userIsDivisionDirector && agreementHasBLIsUnderReview; + const relevantMessages = React.useMemo(() => { if (changeRequestType === CHANGE_REQUEST_SLUG_TYPES.BUDGET) { return budgetChangeMessages; @@ -434,7 +454,10 @@ const useApproveAgreement = () => { statusChangeTo, errorAgreement, isLoadingAgreement, - approvedBudgetLinesPreview + approvedBudgetLinesPreview, + is2849Ready, + hasPermissionToViewPage, + isApproverAndAgreementInReview }; }; diff --git a/frontend/src/pages/agreements/approve/ApproveAgreement.jsx b/frontend/src/pages/agreements/approve/ApproveAgreement.jsx index 4b41ceb8c6..e8f067bafd 100644 --- a/frontend/src/pages/agreements/approve/ApproveAgreement.jsx +++ b/frontend/src/pages/agreements/approve/ApproveAgreement.jsx @@ -1,5 +1,4 @@ import React from "react"; -import { useSelector } from "react-redux"; import App from "../../../App"; import AgreementBLIAccordion from "../../../components/Agreements/AgreementBLIAccordion"; import AgreementCANReviewAccordion from "../../../components/Agreements/AgreementCANReviewAccordion"; @@ -8,7 +7,6 @@ import DocumentCollectionView from "../../../components/Agreements/Documents/Doc import BLIDiffTable from "../../../components/BudgetLineItems/BLIDiffTable"; import { CHANGE_REQUEST_ACTION } from "../../../components/ChangeRequests/ChangeRequests.constants"; import ReviewChangeRequestAccordion from "../../../components/ChangeRequests/ReviewChangeRequestAccordion"; -// import DebugCode from "../../../components/DebugCode"; import ServicesComponentAccordion from "../../../components/ServicesComponents/ServicesComponentAccordion"; import Accordion from "../../../components/UI/Accordion"; import TextArea from "../../../components/UI/Form/TextArea"; @@ -49,32 +47,18 @@ const ApproveAgreement = () => { statusChangeTo, errorAgreement, isLoadingAgreement, - approvedBudgetLinesPreview + approvedBudgetLinesPreview, + is2849Ready, + hasPermissionToViewPage, + isApproverAndAgreementInReview } = useApproveAgreement(); - const is2849Ready = false; // feature flag for 2849 readiness - const userRoles = useSelector((state) => state.auth?.activeUser?.roles) ?? []; - const userIsDivisionDirector = userRoles.includes("division-director") ?? false; - const userDivisionId = useSelector((state) => state.auth?.activeUser?.division) ?? null; - - // NOTE: May be able to get from agreement.change_requests_in_review.managing_division_id - const allCanDivisionIDs = agreement?.budget_line_items?.map((bli) => bli.can.portfolio.division_id); - const uniqueCanDivisionIDs = [...new Set(allCanDivisionIDs)]; - const doesAgreementBelongToDivisionDirector = uniqueCanDivisionIDs.includes(userDivisionId) ?? false; - const agreementHasBLIsUnderReview = agreement?.budget_line_items?.some((bli) => bli.in_review) ?? false; - - const hasPermissionToViewPage = - userIsDivisionDirector && agreementHasBLIsUnderReview && doesAgreementBelongToDivisionDirector; - // NOTE: This test is good enough for now until 2849 is ready - const isApproverAndAgreementInReview = userIsDivisionDirector && agreementHasBLIsUnderReview; - if (!hasPermissionToViewPage && is2849Ready) { return ; } if (!isApproverAndAgreementInReview) { return ; } - if (isLoadingAgreement) { return
Loading...
; } @@ -96,18 +80,11 @@ const ApproveAgreement = () => { secondaryButtonText={modalProps.secondaryButtonText} /> )} + - - {/* - - */} - Date: Wed, 25 Sep 2024 14:10:30 -0500 Subject: [PATCH 07/10] chore: run linter --- backend/ops_api/ops/schemas/budget_line_items.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/ops_api/ops/schemas/budget_line_items.py b/backend/ops_api/ops/schemas/budget_line_items.py index 459e9caba8..85bf88528b 100644 --- a/backend/ops_api/ops/schemas/budget_line_items.py +++ b/backend/ops_api/ops/schemas/budget_line_items.py @@ -254,6 +254,7 @@ class Meta: class PortfolioBLISchema(Schema): division_id = fields.Int(required=True) + class BudgetLineItemCANSchema(Schema): id = fields.Int(required=True) portfolio = fields.Nested(PortfolioBLISchema()) From 2bbeeb327d19599d0b03ec39f3baa92d86f9b151 Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Wed, 25 Sep 2024 15:42:49 -0500 Subject: [PATCH 08/10] refactor: Improve division management in useApproveAgreement hook --- .../agreements/approve/ApproveAgreement.hooks.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/frontend/src/pages/agreements/approve/ApproveAgreement.hooks.js b/frontend/src/pages/agreements/approve/ApproveAgreement.hooks.js index 8a8a592f64..082685aacb 100644 --- a/frontend/src/pages/agreements/approve/ApproveAgreement.hooks.js +++ b/frontend/src/pages/agreements/approve/ApproveAgreement.hooks.js @@ -178,10 +178,13 @@ const useApproveAgreement = () => { const userRoles = useSelector((state) => state.auth?.activeUser?.roles) ?? []; const userIsDivisionDirector = userRoles.includes("division-director") ?? false; const userDivisionId = useSelector((state) => state.auth?.activeUser?.division) ?? null; - const managingDivisionIds = - agreement?.budget_line_items?.flatMap((bli) => - bli.change_requests_in_review.map((cr) => cr.managing_division_id) - ) ?? []; + + const managingDivisionIds = agreement?.budget_line_items + ? agreement.budget_line_items.flatMap((bli) => + bli.change_requests_in_review?.map((cr) => cr.managing_division_id) ?? [] + ) + : []; + const doesAgreementBelongToDivisionDirector = managingDivisionIds.includes(userDivisionId) ?? false; const agreementHasBLIsUnderReview = agreement?.budget_line_items?.some((bli) => bli.in_review) ?? false; const hasPermissionToViewPage = From 33b398d0c194944f44d34484180422975007b69e Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Wed, 25 Sep 2024 16:20:32 -0500 Subject: [PATCH 09/10] test: adds roles for division director --- ...pproveChangeRequestsAtAgreementLevel.cy.js | 12 +++++------ frontend/cypress/support/e2e.js | 21 +++++++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/frontend/cypress/e2e/approveChangeRequestsAtAgreementLevel.cy.js b/frontend/cypress/e2e/approveChangeRequestsAtAgreementLevel.cy.js index cc40f6a332..82a4224fd0 100644 --- a/frontend/cypress/e2e/approveChangeRequestsAtAgreementLevel.cy.js +++ b/frontend/cypress/e2e/approveChangeRequestsAtAgreementLevel.cy.js @@ -35,7 +35,7 @@ const testBli = { }; beforeEach(() => { - testLogin("admin"); + testLogin("division-director"); cy.visit(`/`); }); @@ -158,7 +158,7 @@ describe("Approve Change Requests at the Agreement Level", () => { cy.get('[data-cy="agreement-history-list"] > :nth-child(1) > [data-cy="log-item-children"]') .should( "have.text", - `Admin Demo approved the status change on BL ${bliId} from Draft to Planned as requested by Admin Demo.` + `Dave Director approved the status change on BL ${bliId} from Draft to Planned as requested by Dave Director.` ) // TODO: add more tests .then(() => { @@ -303,7 +303,7 @@ describe("Approve Change Requests at the Agreement Level", () => { cy.get('[data-cy="agreement-history-list"] > :nth-child(1) > [data-cy="log-item-children"]') .should( "have.text", - `Admin Demo approved the status change on BL ${bliId} from Planned to Executing as requested by Admin Demo.` + `Dave Director approved the status change on BL ${bliId} from Planned to Executing as requested by Dave Director.` ) // TODO: add more tests .then(() => { @@ -463,18 +463,18 @@ describe("Approve Change Requests at the Agreement Level", () => { checkHistoryItem( /Budget Change to Amount Approved/, - `Admin Demo approved the budget change on BL ${bliId} from $1,000,000.00 to $2,000,000.00 as requested by Admin Demo.` + `Dave Director approved the budget change on BL ${bliId} from $1,000,000.00 to $2,000,000.00 as requested by Dave Director.` ) .then(() => { return checkHistoryItem( /Budget Change to CAN Approved/, - `Admin Demo approved the budget change on BL ${bliId} from G99IA14 to G99PHS9 as requested by Admin Demo.` + `Dave Director approved the budget change on BL ${bliId} from G99IA14 to G99PHS9 as requested by Dave Director.` ); }) .then(() => { return checkHistoryItem( /Budget Change to Obligate Date/, - `Admin Demo approved the budget change on BL ${bliId} from 1/1/2025 to 9/15/2025 as requested by Admin Demo.` + `Dave Director approved the budget change on BL ${bliId} from 1/1/2025 to 9/15/2025 as requested by Dave Director.` ); }) .then(() => { diff --git a/frontend/cypress/support/e2e.js b/frontend/cypress/support/e2e.js index 61d86e75f2..41ac6cdffe 100644 --- a/frontend/cypress/support/e2e.js +++ b/frontend/cypress/support/e2e.js @@ -26,10 +26,23 @@ Cypress.Commands.add("FakeAuth", (user) => { cy.session([user], async () => { cy.visit("/login"); cy.contains("Sign in with FakeAuth").click(); - if (user === "admin") { - cy.contains("Admin User").click(); - } else if (user === "basic") { - cy.contains("Basic User").click(); + + switch (user) { + case "admin": + cy.contains("Admin User").click(); + break; + case "basic": + cy.contains("Basic User").click(); + break; + case "division-director": + cy.contains("Division Director").click(); + break; + case "budget-team": + cy.contains("Budget Team Member").click(); + break; + default: + // Handle any unspecified user types if necessary + break; } cy.wait(100); From 388c48afc96c2cfaa05de0e7523db1221d0b4c32 Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Wed, 25 Sep 2024 16:28:32 -0500 Subject: [PATCH 10/10] refactor: login role in declineChangeRequestsAtAgreementLevel.cy.js --- .../cypress/e2e/declineChangeRequestsAtAgreementLevel.cy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/cypress/e2e/declineChangeRequestsAtAgreementLevel.cy.js b/frontend/cypress/e2e/declineChangeRequestsAtAgreementLevel.cy.js index d9a7497c21..048d716369 100644 --- a/frontend/cypress/e2e/declineChangeRequestsAtAgreementLevel.cy.js +++ b/frontend/cypress/e2e/declineChangeRequestsAtAgreementLevel.cy.js @@ -35,7 +35,7 @@ const testBli = { }; beforeEach(() => { - testLogin("admin"); + testLogin("division-director"); cy.visit(`/`); });