diff --git a/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/ProvisioningFormPolicyType.jsx b/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/ProvisioningFormPolicyType.jsx index 09d848669..208375feb 100644 --- a/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/ProvisioningFormPolicyType.jsx +++ b/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/ProvisioningFormPolicyType.jsx @@ -2,29 +2,46 @@ import { Form, } from '@edx/paragon'; import { v4 as uuidv4 } from 'uuid'; +import { useContextSelector } from 'use-context-selector'; import React, { useState } from 'react'; import PROVISIONING_PAGE_TEXT from '../../data/constants'; import useProvisioningContext from '../../data/hooks'; -import { indexOnlyPropType, selectProvisioningContext } from '../../data/utils'; import ProvisioningFormHelpText from '../ProvisioningFormHelpText'; +import { indexOnlyPropType } from '../../data/utils'; +import { ProvisioningContext } from '../../ProvisioningContext'; const ProvisioningFormPolicyType = ({ index }) => { - const { perLearnerCap, setPolicyType, setInvalidPolicyFields } = useProvisioningContext(); + const { + perLearnerCap, + setPolicyType, + setInvalidPolicyFields, + } = useProvisioningContext(); const { POLICY_TYPE } = PROVISIONING_PAGE_TEXT.FORM; - const [formData, showInvalidField] = selectProvisioningContext('formData', 'showInvalidField'); - const { policies } = showInvalidField; - const isPolicyTypeDefinedAndFalse = policies[index]?.policyType === false; + const contextData = useContextSelector(ProvisioningContext, v => v[0]); + const { + isEditMode, + formData, + showInvalidField: { policies }, + } = contextData; + const isFormFieldInvalid = policies[index]?.policyType === false; - const [value, setValue] = useState(null); + let submittedFormPolicyType; + if (isEditMode) { + submittedFormPolicyType = formData.policies[index].policyType; + } + const [value, setValue] = useState(submittedFormPolicyType || null); const handleChange = (e) => { const policyTypeValue = e.target.value; - if (policyTypeValue === POLICY_TYPE.OPTIONS.LEARNER_SELECTS.DESCRIPTION) { + if (isEditMode) { + return; // Editing policy type is not supported. + } + if (policyTypeValue === POLICY_TYPE.OPTIONS.LEARNER_SELECTS.VALUE) { setPolicyType({ policyType: POLICY_TYPE.OPTIONS.LEARNER_SELECTS.VALUE, accessMethod: POLICY_TYPE.OPTIONS.LEARNER_SELECTS.ACCESS_METHOD, }, index); - } else if (policyTypeValue === POLICY_TYPE.OPTIONS.ADMIN_SELECTS.DESCRIPTION) { + } else if (policyTypeValue === POLICY_TYPE.OPTIONS.ADMIN_SELECTS.VALUE) { setPolicyType({ policyType: POLICY_TYPE.OPTIONS.ADMIN_SELECTS.VALUE, accessMethod: POLICY_TYPE.OPTIONS.ADMIN_SELECTS.ACCESS_METHOD, @@ -51,24 +68,26 @@ const ProvisioningFormPolicyType = ({ index }) => { { - Object.values(POLICY_TYPE.OPTIONS).map(({ DESCRIPTION }) => ( + Object.values(POLICY_TYPE.OPTIONS).map(({ DESCRIPTION, VALUE }) => ( {DESCRIPTION} )) } - {isPolicyTypeDefinedAndFalse && ( + {isFormFieldInvalid && ( diff --git a/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/tests/ProvisioningFormPolicyContainer.test.jsx b/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/tests/ProvisioningFormPolicyContainer.test.jsx index 80fe2aaf9..c031e1089 100644 --- a/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/tests/ProvisioningFormPolicyContainer.test.jsx +++ b/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/tests/ProvisioningFormPolicyContainer.test.jsx @@ -83,10 +83,9 @@ describe('ProvisioningFormPolicyContainer', () => { ); expect(screen.getByText(POLICY_TYPE.TITLE)).toBeTruthy(); expect(screen.getByText(POLICY_TYPE.LABEL)).toBeTruthy(); - const learnerOption = screen.getByTestId(POLICY_TYPE.OPTIONS.ADMIN_SELECTS.DESCRIPTION); - userEvent.click(learnerOption); + userEvent.click(screen.getByTestId(POLICY_TYPE.OPTIONS.ADMIN_SELECTS.DESCRIPTION)); await waitFor(() => { - expect(learnerOption.checked).toBeTruthy(); + expect(screen.getByTestId(POLICY_TYPE.OPTIONS.ADMIN_SELECTS.DESCRIPTION).checked).toBeTruthy(); }); }); }); diff --git a/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/tests/ProvisioningFormPolicyType.test.jsx b/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/tests/ProvisioningFormPolicyType.test.jsx index decb685d0..993d53753 100644 --- a/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/tests/ProvisioningFormPolicyType.test.jsx +++ b/src/Configuration/Provisioning/ProvisioningForm/PolicyForm/tests/ProvisioningFormPolicyType.test.jsx @@ -40,16 +40,13 @@ describe('ProvisioningFormPolicyType', () => { expect(screen.getByText(POLICY_TYPE.LABEL)).toBeTruthy(); const policyTypeOptions = Object.keys(POLICY_TYPE.OPTIONS); - const policyTypeButtons = []; - // Retrieves a list of input elements based on test ids - for (let i = 0; i < policyTypeOptions.length; i++) { - policyTypeButtons.push(screen.getByTestId(POLICY_TYPE.OPTIONS[policyTypeOptions[i]].DESCRIPTION)); - } // Clicks on each input element and checks if it is checked - for (let i = 0; i < policyTypeButtons.length; i++) { - fireEvent.click(policyTypeButtons[i]); - expect(policyTypeButtons[i].checked).toBeTruthy(); + for (let i = 0; i < policyTypeOptions.length; i++) { + const buttonBeforeClick = screen.getByTestId(POLICY_TYPE.OPTIONS[policyTypeOptions[i]].DESCRIPTION); + fireEvent.click(buttonBeforeClick); + const buttonAfterClick = screen.getByTestId(POLICY_TYPE.OPTIONS[policyTypeOptions[i]].DESCRIPTION); + expect(buttonAfterClick.checked).toBeTruthy(); } expect(screen.getByText('Not editable')).toBeTruthy(); }); diff --git a/src/Configuration/Provisioning/SubsidyEditView/SubsidyEditView.jsx b/src/Configuration/Provisioning/SubsidyEditView/SubsidyEditView.jsx index 9b06a2804..8771b22af 100644 --- a/src/Configuration/Provisioning/SubsidyEditView/SubsidyEditView.jsx +++ b/src/Configuration/Provisioning/SubsidyEditView/SubsidyEditView.jsx @@ -4,6 +4,7 @@ import { useContextSelector } from 'use-context-selector'; import { logError } from '@edx/frontend-platform/logging'; +import { generateBudgetDisplayName } from '../data/utils'; import useProvisioningContext from '../data/hooks'; import PROVISIONING_PAGE_TEXT from '../data/constants'; import { ProvisioningContext } from '../ProvisioningContext'; @@ -87,13 +88,10 @@ const SubsidyEditView = () => { - {(multipleFunds !== undefined) && formData.policies?.map(({ - uuid, - policyFormTitle, - }, index) => ( + {(multipleFunds !== undefined) && formData.policies?.map((policy, index) => ( ))} diff --git a/src/Configuration/Provisioning/SubsidyEditView/tests/SubsidyEditView.test.jsx b/src/Configuration/Provisioning/SubsidyEditView/tests/SubsidyEditView.test.jsx index ece7f3ef3..0c6f352d6 100644 --- a/src/Configuration/Provisioning/SubsidyEditView/tests/SubsidyEditView.test.jsx +++ b/src/Configuration/Provisioning/SubsidyEditView/tests/SubsidyEditView.test.jsx @@ -147,6 +147,7 @@ describe('SubsidyEditView', () => { expect(screen.getByTestId('internal-only-checkbox').checked).toBeTruthy(); expect(screen.getByTestId('partner-no-rev-prepay').checked).toBeTruthy(); expect(screen.getByText('No, create one Learner Credit budget')).toBeInTheDocument(); + expect(screen.getByText('Open Courses budget')).toBeInTheDocument(); expect(screen.getByTestId('account-name').value).toBe('Paper company --- Open Courses'); expect(screen.getByText( 'The maximum USD value a single learner may redeem from the budget. This value should be less than the budget starting balance.', diff --git a/src/Configuration/Provisioning/data/hooks.js b/src/Configuration/Provisioning/data/hooks.js index d8faa38dc..14301b770 100644 --- a/src/Configuration/Provisioning/data/hooks.js +++ b/src/Configuration/Provisioning/data/hooks.js @@ -243,11 +243,11 @@ export default function useProvisioningContext() { // Old values should remain static and will help us later decide whether to skip catalog creation. oldPredefinedQueryType: predefinedQueryType, oldCustomCatalog: !predefinedQueryType, - oldCatalogUuid: catalog.uuid, + oldCatalogUuid: !predefinedQueryType ? catalog.uuid : undefined, // New values will change over time as different options are selected. predefinedQueryType, customCatalog: !predefinedQueryType, - catalogUuid: catalog.uuid, + catalogUuid: !predefinedQueryType ? catalog.uuid : undefined, // We ostensibly don't rely on the catalog title for anything critical, but in case it is a custom catalog // we can cache the title here so that we have something to display in the detail view header. catalogTitle: catalog.title, diff --git a/src/Configuration/Provisioning/data/tests/utils.test.js b/src/Configuration/Provisioning/data/tests/utils.test.js index 5cc401bc7..715a82023 100644 --- a/src/Configuration/Provisioning/data/tests/utils.test.js +++ b/src/Configuration/Provisioning/data/tests/utils.test.js @@ -342,7 +342,7 @@ const emptyDataSet = { subsidyRevReq: '', }; describe('determineInvalidFields', () => { - it('returns false for all subsidy fields', async () => { + it('returns false (invalid)for all subsidy fields', async () => { getAuthenticatedHttpClient.mockImplementation(() => ({ get: jest.fn().mockResolvedValue({ data: [{ id: uuidv4() }] }), })); @@ -358,7 +358,7 @@ describe('determineInvalidFields', () => { const output = await determineInvalidFields(emptyDataSet); expect(output).toEqual([expectedFailedSubsidyOutput]); }); - it('returns false for all policy fields', async () => { + it('returns false (invalid)for all policy fields', async () => { const expectedFailedPolicyOutput = [{ subsidyTitle: false, enterpriseUUID: false, @@ -370,7 +370,7 @@ describe('determineInvalidFields', () => { }, [{ accountName: false, accountValue: false, - catalogUuid: false, + catalogUuid: true, // This is true (i.e. valid) because catalogUuid is not required when customCatalog != true. predefinedQueryType: false, perLearnerCap: false, perLearnerCapAmount: false, @@ -410,7 +410,7 @@ describe('transformPatchPolicyData', () => { it('returns the correct data', async () => { const mockFormData = { policies: [{ - policy_type: 'PerLearnerSpendCreditAccessPolicy', + policyType: 'PerLearnerSpendCreditAccessPolicy', accountDescription: 'awesome policy description', customCatalog: true, catalogTitle: 'awesome custom catalog', @@ -428,7 +428,6 @@ describe('transformPatchPolicyData', () => { catalogUuid: '2afb0a7f-103d-43c3-8b1a-db8c5b3ba1f4', subsidyUuid: '205f11a4-0303-4407-a2e7-80261ef8fb8f', perLearnerSpendLimit: 200, - spendLimit: 12000, uuid: '12324232', }]); }); diff --git a/src/Configuration/Provisioning/data/utils.js b/src/Configuration/Provisioning/data/utils.js index cb1c1e208..1b4c7896b 100644 --- a/src/Configuration/Provisioning/data/utils.js +++ b/src/Configuration/Provisioning/data/utils.js @@ -88,9 +88,11 @@ export async function determineInvalidFields(formData) { const policyData = { accountName: !!accountName, accountValue: !!accountValue, - // Either a predefined query type must be selected, or a custom catalog is selected. - predefinedQueryType: !!predefinedQueryType && !customCatalog, - catalogUuid: !!catalogUuid && customCatalog, + // Either a predefined query type must be selected, or a catalog UUID is selected, depending on customCatalog. + // When customCatalog is false, make sure predefinedQueryType is selected: + predefinedQueryType: !customCatalog ? !!predefinedQueryType : true, + // When customCatalog is true, make sure predefinedQueryType is selected: + catalogUuid: customCatalog ? !!catalogUuid : true, perLearnerCap: perLearnerCap !== undefined || perLearnerCap === false, perLearnerCapAmount: !!perLearnerCapAmount || perLearnerCap === false, policyType: !!policyType, @@ -130,7 +132,7 @@ export function hasValidPolicyAndSubsidy(formData) { const isAccountNameValid = !!policy.accountName; const isAccountValueValid = !!policy.accountValue; - const isCatalogDefined = policy.customCatalog === true ? !!policy.catalogUuid : !!policy.predefinedQueryType; + const isCatalogDefined = policy.customCatalog ? !!policy.catalogUuid : !!policy.predefinedQueryType; // Requires learner cap to pass conditionals to be true const { perLearnerCap, perLearnerCapAmount } = policy; @@ -460,26 +462,28 @@ export async function createPolicy({ * subsidy and catalog uuid. * * @param {{ -* description: String, -* catalogUuid: String, -* subsidyUuid: String, -* perLearnerSpendLimit: Number, -* spendLimit: Number -* }} -* @returns {Promise} - Returns a promise that resolves to the response data from the API -*/ + * description: String, + * catalogUuid: String, + * subsidyUuid: String, + * perLearnerSpendLimit: Number, + * accessMethod: String, + * }} + * @returns {Promise} - Returns a promise that resolves to the response data from the API + */ export async function patchPolicy({ uuid, description, catalogUuid, perLearnerSpendLimit, + accessMethod, }) { - const data = LmsApiService.patchSubsidyAccessPolicy( + const data = LmsApiService.patchSubsidyAccessPolicy({ uuid, description, catalogUuid, perLearnerSpendLimit, - ); + accessMethod, + }); return data; } @@ -564,7 +568,13 @@ export function transformPatchPolicyPayload(formData, catalogCreationResponses) catalogUuid: catalogCreationResponses[index]?.uuid || policy.catalogUuid, subsidyUuid, perLearnerSpendLimit: policy.perLearnerCap ? policy.perLearnerCapAmount : null, - spendLimit: policy.accountValue, + + // The spendLimit is currently NOT EDITABLE so do not include it in the PATCH payload. + // spendLimit: policy.accountValue, + + // The policyType and accessMethod is currently NOT EDITABLE so do not include it in the PATCH payload. + // policyType: policy.policyType, + // accessMethod: policy.accessMethod, })); return payloads; } diff --git a/src/data/services/EnterpriseApiService.js b/src/data/services/EnterpriseApiService.js index d021378bf..aec660c00 100644 --- a/src/data/services/EnterpriseApiService.js +++ b/src/data/services/EnterpriseApiService.js @@ -89,14 +89,14 @@ class LmsApiService { }, ); - static patchSubsidyAccessPolicy = ( + static patchSubsidyAccessPolicy = ({ uuid, description, catalogUuid, perLearnerSpendLimit, - accessMethod = 'direct', + accessMethod, active = true, - ) => LmsApiService.apiClient().patch( + }) => LmsApiService.apiClient().patch( `${getConfig().ENTERPRISE_ACCESS_BASE_URL}/api/v1/subsidy-access-policies/${uuid}/`, { description, diff --git a/src/data/services/SubsidyApiService.js b/src/data/services/SubsidyApiService.js index 88197aef0..12ad43c89 100644 --- a/src/data/services/SubsidyApiService.js +++ b/src/data/services/SubsidyApiService.js @@ -25,6 +25,22 @@ class SubsidyApiService { return SubsidyApiService.apiClient().get(`${subsidiesURL}?uuid=${uuid}`); }; + /** + * postSubsidy gets or creates a learner credit Subsidy (and corresponding ledger). + * + * @param {String} financialIdentifier - A reference to the object responsible for originating this subsidy, and the + * key on which existing subsidies are retrieved. + * @param {String} title + * @param {String} enterpriseUUID + * @param {String} startDate + * @param {String} endDate + * @param {Number} startingBalance - The initial balance of the new subsidy in USD Cents (integer). + * @param {String} revenueCategory + * @param {Boolean} internalOnly + * @param {String} unit = 'usd_cents' + * + * @returns {Object} - The subsidy create endpoint response, containing a serialized subsidy. + */ static postSubsidy = ( financialIdentifier, title, @@ -37,7 +53,6 @@ class SubsidyApiService { unit = 'usd_cents', ) => { const subsidiesURL = `${getConfig().SUBSIDY_BASE_URL}/api/v1/subsidies/`; - const wholeDollarStartingBalance = startingBalance * 100; return SubsidyApiService.apiClient().post( subsidiesURL, { @@ -47,7 +62,7 @@ class SubsidyApiService { default_active_datetime: startDate, default_expiration_datetime: endDate, default_unit: unit, - default_starting_balance: wholeDollarStartingBalance, + default_starting_balance: startingBalance, default_revenue_category: revenueCategory, default_internal_only: internalOnly, },