From 8f7aea554a1fb16e53f55ed308eda4132ee9ade1 Mon Sep 17 00:00:00 2001 From: diana-villalvazo-wgu Date: Tue, 3 Feb 2026 10:52:34 -0500 Subject: [PATCH 1/2] fix: links and missing tooltip --- src/cohorts/CohortsPage.scss | 4 ++++ src/cohorts/CohortsPage.tsx | 1 + src/cohorts/components/CohortCard.tsx | 4 ++-- src/cohorts/components/CohortContext.test.tsx | 6 +++--- src/cohorts/components/DisabledCohortsView.tsx | 4 ++-- src/cohorts/components/SelectedCohortInfo.tsx | 3 ++- src/cohorts/data/api.ts | 2 +- src/cohorts/messages.ts | 5 +++++ 8 files changed, 20 insertions(+), 9 deletions(-) create mode 100644 src/cohorts/CohortsPage.scss diff --git a/src/cohorts/CohortsPage.scss b/src/cohorts/CohortsPage.scss new file mode 100644 index 00000000..55f723b8 --- /dev/null +++ b/src/cohorts/CohortsPage.scss @@ -0,0 +1,4 @@ +.assignment-tooltip .tooltip-inner { + background-color: var(--pgn-color-primary-700); + max-width: none; +} \ No newline at end of file diff --git a/src/cohorts/CohortsPage.tsx b/src/cohorts/CohortsPage.tsx index 2bf9b2f6..f5b7fdc6 100644 --- a/src/cohorts/CohortsPage.tsx +++ b/src/cohorts/CohortsPage.tsx @@ -9,6 +9,7 @@ import DisabledCohortsView from '@src/cohorts/components/DisabledCohortsView'; import EnabledCohortsView from '@src/cohorts/components/EnabledCohortsView'; import { useCohortStatus, useToggleCohorts } from '@src/cohorts/data/apiHook'; import messages from '@src/cohorts/messages'; +import './CohortsPage.scss'; const CohortsPageContent = () => { const intl = useIntl(); diff --git a/src/cohorts/components/CohortCard.tsx b/src/cohorts/components/CohortCard.tsx index f1746610..19597084 100644 --- a/src/cohorts/components/CohortCard.tsx +++ b/src/cohorts/components/CohortCard.tsx @@ -1,7 +1,7 @@ import { useParams } from 'react-router-dom'; import { useRef, useState } from 'react'; import { FormattedMessage, getExternalLinkUrl, useIntl } from '@openedx/frontend-base'; -import { Card, Tab, Tabs, Toast } from '@openedx/paragon'; +import { Card, Hyperlink, Tab, Tabs, Toast } from '@openedx/paragon'; import messages from '../messages'; import CohortsForm from './CohortsForm'; import ManageLearners from './ManageLearners'; @@ -56,7 +56,7 @@ const CohortCard = () => {

{intl.formatMessage(messages.studentsOnCohort, { users: selectedCohort?.userCount ?? 0 })}

- {intl.formatMessage(messages.warningCohortLink)} + {intl.formatMessage(messages.warningCohortLink)}

{}}> diff --git a/src/cohorts/components/CohortContext.test.tsx b/src/cohorts/components/CohortContext.test.tsx index 46dfe04c..dd75490e 100644 --- a/src/cohorts/components/CohortContext.test.tsx +++ b/src/cohorts/components/CohortContext.test.tsx @@ -1,9 +1,9 @@ -import React from 'react'; +import { useEffect } from 'react'; import { render, act } from '@testing-library/react'; import { CohortProvider, useCohortContext } from './CohortContext'; import { assignmentTypes } from '../constants'; -const TestComponent: React.FC = () => { +const TestComponent = () => { const { selectedCohort, setSelectedCohort, @@ -101,7 +101,7 @@ describe('CohortContext', () => { const RenderCountComponent = () => { renderCount++; const { setSelectedCohort } = useCohortContext(); - React.useEffect(() => { + useEffect(() => { setSelectedCohort({ id: 1, name: 'Cohort 1', diff --git a/src/cohorts/components/DisabledCohortsView.tsx b/src/cohorts/components/DisabledCohortsView.tsx index 911cbf33..b8b9c6b5 100644 --- a/src/cohorts/components/DisabledCohortsView.tsx +++ b/src/cohorts/components/DisabledCohortsView.tsx @@ -1,5 +1,5 @@ import { getExternalLinkUrl, useIntl } from '@openedx/frontend-base'; -import { Button } from '@openedx/paragon'; +import { Button, Hyperlink } from '@openedx/paragon'; import messages from '../messages'; interface DisabledCohortsViewProps { @@ -12,7 +12,7 @@ const DisabledCohortsView = ({ onEnableCohorts }: DisabledCohortsViewProps) => { return (

- {intl.formatMessage(messages.noCohortsMessage)} {intl.formatMessage(messages.learnMore)} + {intl.formatMessage(messages.noCohortsMessage)} {intl.formatMessage(messages.learnMore)}

diff --git a/src/cohorts/components/SelectedCohortInfo.tsx b/src/cohorts/components/SelectedCohortInfo.tsx index 2dbbb9bf..43006727 100644 --- a/src/cohorts/components/SelectedCohortInfo.tsx +++ b/src/cohorts/components/SelectedCohortInfo.tsx @@ -3,6 +3,7 @@ import { useParams } from 'react-router-dom'; import CohortCard from './CohortCard'; import messages from '../messages'; import dataDownloadsMessages from '@src/dataDownloads/messages'; +import { Hyperlink } from '@openedx/paragon'; const SelectedCohortInfo = () => { const intl = useIntl(); @@ -12,7 +13,7 @@ const SelectedCohortInfo = () => { <>

- {intl.formatMessage(messages.cohortDisclaimer)} {intl.formatMessage(dataDownloadsMessages.pageTitle)} {intl.formatMessage(messages.page)} + {intl.formatMessage(messages.cohortDisclaimer)} {intl.formatMessage(dataDownloadsMessages.pageTitle)} {intl.formatMessage(messages.page)}

); diff --git a/src/cohorts/data/api.ts b/src/cohorts/data/api.ts index 7c00e164..5378c31d 100644 --- a/src/cohorts/data/api.ts +++ b/src/cohorts/data/api.ts @@ -28,7 +28,7 @@ export const createCohort = async (courseId: string, cohortDetails: BasicCohortD }; export const getContentGroups = async (courseId: string) => { - const url = `${getApiBaseUrl()}/api/instructor/v1/courses/${courseId}/group_configurations`; + const url = `${getApiBaseUrl()}/api/cohorts/v2/courses/${courseId}/group_configurations`; const { data } = await getAuthenticatedHttpClient().get(url); return camelCaseObject(data); }; diff --git a/src/cohorts/messages.ts b/src/cohorts/messages.ts index f8457bc0..e9595670 100644 --- a/src/cohorts/messages.ts +++ b/src/cohorts/messages.ts @@ -186,6 +186,11 @@ const messages = defineMessages({ defaultMessage: 'Add Learners', description: 'Label for the add learners button' }, + manualAssignmentDisabledTooltip: { + id: 'instruct.cohorts.manualAssignmentDisabledTooltip', + defaultMessage: 'There must be one cohort to which students can automatically be assigned.', + description: 'Tooltip message when manual assignment is disabled' + } }); export default messages; From 9a3d9f2e06e698643999b119dfa28b41c6b64eb1 Mon Sep 17 00:00:00 2001 From: diana-villalvazo-wgu Date: Tue, 3 Feb 2026 22:54:22 -0500 Subject: [PATCH 2/2] feat: add alert context --- src/cohorts/CohortsPage.tsx | 5 +- src/cohorts/components/CohortCard.tsx | 20 ++-- src/cohorts/components/CohortsForm.test.tsx | 53 +++++----- src/cohorts/components/CohortsForm.tsx | 54 ++++++---- .../components/EnabledCohortsView.test.tsx | 11 +- src/cohorts/components/EnabledCohortsView.tsx | 32 ++++-- .../components/ManageLearners.test.tsx | 15 +-- src/cohorts/components/ManageLearners.tsx | 66 +++++++++++- .../components/SelectedCohortInfo.test.tsx | 85 +++++++++++++++ src/cohorts/data/apiHook.ts | 1 + src/cohorts/messages.ts | 27 ++++- src/components/AlertContext.test.tsx | 100 ++++++++++++++++++ src/components/AlertContext.tsx | 48 +++++++++ 13 files changed, 441 insertions(+), 76 deletions(-) create mode 100644 src/cohorts/components/SelectedCohortInfo.test.tsx create mode 100644 src/components/AlertContext.test.tsx create mode 100644 src/components/AlertContext.tsx diff --git a/src/cohorts/CohortsPage.tsx b/src/cohorts/CohortsPage.tsx index f5b7fdc6..729e5634 100644 --- a/src/cohorts/CohortsPage.tsx +++ b/src/cohorts/CohortsPage.tsx @@ -7,6 +7,7 @@ import { CohortProvider, useCohortContext } from '@src/cohorts/components/Cohort import DisableCohortsModal from '@src/cohorts/components/DisableCohortsModal'; import DisabledCohortsView from '@src/cohorts/components/DisabledCohortsView'; import EnabledCohortsView from '@src/cohorts/components/EnabledCohortsView'; +import { AlertProvider } from '@src/components/AlertContext'; import { useCohortStatus, useToggleCohorts } from '@src/cohorts/data/apiHook'; import messages from '@src/cohorts/messages'; import './CohortsPage.scss'; @@ -67,7 +68,9 @@ const CohortsPageContent = () => { const CohortsPage = () => { return ( - + + + ); }; diff --git a/src/cohorts/components/CohortCard.tsx b/src/cohorts/components/CohortCard.tsx index 19597084..8a43ac0c 100644 --- a/src/cohorts/components/CohortCard.tsx +++ b/src/cohorts/components/CohortCard.tsx @@ -2,12 +2,13 @@ import { useParams } from 'react-router-dom'; import { useRef, useState } from 'react'; import { FormattedMessage, getExternalLinkUrl, useIntl } from '@openedx/frontend-base'; import { Card, Hyperlink, Tab, Tabs, Toast } from '@openedx/paragon'; -import messages from '../messages'; -import CohortsForm from './CohortsForm'; -import ManageLearners from './ManageLearners'; -import { useCohortContext } from './CohortContext'; -import { usePatchCohort } from '../data/apiHook'; -import { CohortData } from '../types'; +import { useAlert } from '@src/components/AlertContext'; +import messages from '@src/cohorts/messages'; +import { CohortData } from '@src/cohorts/types'; +import { usePatchCohort } from '@src/cohorts/data/apiHook'; +import CohortsForm from '@src/cohorts/components/CohortsForm'; +import ManageLearners from '@src/cohorts/components/ManageLearners'; +import { useCohortContext } from '@src/cohorts/components/CohortContext'; const assignmentLink = { random: 'https://docs.openedx.org/en/latest/educators/references/advanced_features/managing_cohort_assignment.html#about-auto-cohorts', @@ -26,19 +27,24 @@ const CohortCard = () => { const { mutate: editCohort } = usePatchCohort(courseId); const formRef = useRef<{ resetForm: () => void }>(null); const [showSuccessMessage, setShowSuccessMessage] = useState(false); + const { clearAlerts } = useAlert(); if (!selectedCohort) { return null; } const handleEditCohort = (updatedCohort: CohortData) => { + clearAlerts(); editCohort({ cohortId: selectedCohort.id, cohortInfo: updatedCohort }, { onSuccess: () => { setShowSuccessMessage(true); setSelectedCohort({ ...selectedCohort, ...updatedCohort }); }, - onError: (error) => console.error(error) + onError: (error) => { + // TODO: add modal error + console.error(error); + } } ); }; diff --git a/src/cohorts/components/CohortsForm.test.tsx b/src/cohorts/components/CohortsForm.test.tsx index 49e88056..9e73e63c 100644 --- a/src/cohorts/components/CohortsForm.test.tsx +++ b/src/cohorts/components/CohortsForm.test.tsx @@ -1,22 +1,23 @@ import { screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import CohortsForm from './CohortsForm'; -import messages from '../messages'; +import CohortsForm from '@src/cohorts/components/CohortsForm'; +import messages from '@src/cohorts/messages'; import { renderWithIntl } from '@src/testUtils'; -import { useContentGroupsData } from '../data/apiHook'; -import { CohortProvider } from './CohortContext'; -import * as CohortContextModule from './CohortContext'; +import { useContentGroupsData } from '@src/cohorts/data/apiHook'; +import { CohortProvider } from '@src/cohorts/components/CohortContext'; +import * as CohortContextModule from '@src/cohorts/components/CohortContext'; +import { AlertProvider } from '@src/components/AlertContext'; jest.mock('react-router-dom', () => ({ useParams: () => ({ courseId: 'course-v1:edX+DemoX+Demo_Course' }), })); const mockContentGroups = [ - { id: '1:2', displayName: 'Group 1' }, - { id: '2:3', displayName: 'Group 2' }, + { id: '2', name: 'Group 1' }, + { id: '3', name: 'Group 2' }, ]; -jest.mock('../data/apiHook', () => ({ +jest.mock('@src/cohorts/data/apiHook', () => ({ useContentGroupsData: jest.fn(), })); @@ -27,7 +28,9 @@ describe('CohortsForm', () => { const renderComponent = () => renderWithIntl( - + + + ); @@ -36,30 +39,30 @@ describe('CohortsForm', () => { }); it('renders cohort name input', () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); renderComponent(); expect(screen.getByPlaceholderText(messages.cohortName.defaultMessage)).toBeInTheDocument(); }); it('renders assignment method radios', () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); renderComponent(); expect(screen.getByLabelText(messages.automatic.defaultMessage)).toBeInTheDocument(); expect(screen.getByLabelText(messages.manual.defaultMessage)).toBeInTheDocument(); }); it('renders content group radios and select', () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); renderComponent(); expect(screen.getByLabelText(messages.noContentGroup.defaultMessage)).toBeInTheDocument(); expect(screen.getByLabelText(messages.selectAContentGroup.defaultMessage)).toBeInTheDocument(); expect(screen.getByRole('combobox')).toBeInTheDocument(); - expect(screen.getByText(mockContentGroups[0].displayName)).toBeInTheDocument(); - expect(screen.getByText(mockContentGroups[1].displayName)).toBeInTheDocument(); + expect(screen.getByText(mockContentGroups[0].name)).toBeInTheDocument(); + expect(screen.getByText(mockContentGroups[1].name)).toBeInTheDocument(); }); it('calls onCancel when Cancel button is clicked', async () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); renderComponent(); const user = userEvent.setup(); const cancelButton = screen.getByRole('button', { name: messages.cancelLabel.defaultMessage }); @@ -68,7 +71,7 @@ describe('CohortsForm', () => { }); it('calls onSubmit when Save button is enabled and clicked', async () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); renderComponent(); const user = userEvent.setup(); const input = screen.getByPlaceholderText(messages.cohortName.defaultMessage); @@ -78,7 +81,7 @@ describe('CohortsForm', () => { }); it('updates cohort name input value', async () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); renderComponent(); const input = screen.getByPlaceholderText(messages.cohortName.defaultMessage); const user = userEvent.setup(); @@ -87,7 +90,7 @@ describe('CohortsForm', () => { }); it('disables select when "Select a Content Group" is not chosen', async () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); renderComponent(); const select = screen.getByRole('combobox'); expect(select).toBeDisabled(); @@ -97,14 +100,14 @@ describe('CohortsForm', () => { }); it('renders warning and create link when no content groups', () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: [] }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: [] }] } }); renderComponent(); expect(screen.getByText(messages.noContentGroups.defaultMessage)).toBeInTheDocument(); expect(screen.getByText(messages.createContentGroup.defaultMessage)).toBeInTheDocument(); }); it('submits correct data when selecting a content group', async () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups, id: 1 }] } }); renderComponent(); const user = userEvent.setup(); @@ -121,15 +124,15 @@ describe('CohortsForm', () => { expect(onSubmit).toHaveBeenCalledWith( expect.objectContaining({ name: 'Cohort With Group', - groupId: null, - userPartitionId: null, + groupId: 3, + userPartitionId: 1, assignmentType: 'random', }) ); }); it('disables manual assignment radio when disableManualAssignment is true', () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); renderWithIntl( @@ -140,7 +143,7 @@ describe('CohortsForm', () => { }); it('shows initial values in context', async () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); jest.spyOn(CohortContextModule, 'useCohortContext').mockReturnValue({ selectedCohort: { @@ -174,6 +177,6 @@ describe('CohortsForm', () => { expect(selectGroupRadio).toBeChecked(); const groupSelect = screen.getByRole('combobox'); - expect(groupSelect).toHaveValue('2:3'); + expect(groupSelect).toHaveValue('2'); }); }); diff --git a/src/cohorts/components/CohortsForm.tsx b/src/cohorts/components/CohortsForm.tsx index 2e6da92d..1390fe18 100644 --- a/src/cohorts/components/CohortsForm.tsx +++ b/src/cohorts/components/CohortsForm.tsx @@ -1,13 +1,13 @@ import { useParams } from 'react-router-dom'; import { useState, useEffect, forwardRef, useImperativeHandle, useCallback } from 'react'; -import { ActionRow, Button, Form, FormControl, FormGroup, FormLabel, FormRadioSet, Hyperlink, Icon } from '@openedx/paragon'; +import { ActionRow, Button, Form, FormControl, FormGroup, FormLabel, FormRadioSet, Hyperlink, Icon, OverlayTrigger, Tooltip } from '@openedx/paragon'; import { useIntl } from '@openedx/frontend-base'; -import messages from '../messages'; -import { useContentGroupsData } from '../data/apiHook'; +import messages from '@src/cohorts/messages'; +import { useContentGroupsData } from '@src/cohorts/data/apiHook'; import { Warning } from '@openedx/paragon/icons'; -import { assignmentTypes } from '../constants'; -import { useCohortContext } from './CohortContext'; -import { CohortData } from '../types'; +import { assignmentTypes } from '@src/cohorts/constants'; +import { CohortData } from '@src/cohorts/types'; +import { useCohortContext } from '@src/cohorts/components/CohortContext'; interface CohortsFormProps { disableManualAssignment?: boolean, @@ -22,26 +22,25 @@ export interface CohortsFormRef { const CohortsForm = forwardRef(({ disableManualAssignment = false, onCancel, onSubmit }, ref) => { const intl = useIntl(); const { courseId = '' } = useParams<{ courseId: string }>(); - const { data = [] } = useContentGroupsData(courseId); + const { data = { allGroupConfigurations: [{ groups: [] }] } } = useContentGroupsData(courseId); const { selectedCohort } = useCohortContext(); const initialCohortName = (selectedCohort?.name) ?? ''; const initialAssignmentType = selectedCohort?.assignmentType ?? assignmentTypes.automatic; const initialContentGroupOption = selectedCohort?.groupId ? 'selectContentGroup' : 'noContentGroup'; - const initialContentGroup = selectedCohort?.groupId && selectedCohort?.userPartitionId ? `${selectedCohort.groupId}:${selectedCohort.userPartitionId}` : 'null'; + const initialContentGroup = selectedCohort?.groupId ? selectedCohort.groupId : null; - const [selectedContentGroup, setSelectedContentGroup] = useState(initialContentGroup); + const [selectedContentGroup, setSelectedContentGroup] = useState(initialContentGroup); const [selectedContentGroupOption, setSelectedContentGroupOption] = useState(initialContentGroupOption); const [selectedAssignmentType, setSelectedAssignmentType] = useState(initialAssignmentType); const [name, setName] = useState(initialCohortName); const resetToInitialValues = useCallback(() => { if (selectedCohort) { - const contentGroup = selectedCohort.groupId && selectedCohort.userPartitionId ? `${selectedCohort.groupId}:${selectedCohort.userPartitionId}` : 'null'; setName(selectedCohort.name); setSelectedAssignmentType(selectedCohort.assignmentType); setSelectedContentGroupOption(selectedCohort.groupId ? 'selectContentGroup' : 'noContentGroup'); - setSelectedContentGroup(contentGroup); + setSelectedContentGroup(selectedCohort.groupId ?? null); } }, [selectedCohort]); @@ -54,18 +53,15 @@ const CohortsForm = forwardRef(({ disableManua // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectedCohort]); - const contentGroups = [{ id: 'null', displayName: intl.formatMessage(messages.notSelected) }, ...data]; + const contentGroups = [{ id: 'null', name: intl.formatMessage(messages.notSelected) }, ...data.allGroupConfigurations[0].groups]; const handleSubmit = (event: React.FormEvent) => { event.preventDefault(); - const contentGroups = selectedContentGroupOption.split(':'); - const groupId = contentGroups.length > 1 ? Number(contentGroups[0]) : null; - const userPartitionId = contentGroups.length > 1 ? Number(contentGroups[1]) : null; onSubmit({ name, assignmentType: selectedAssignmentType, - groupId, - userPartitionId, + groupId: selectedContentGroup, + userPartitionId: data.allGroupConfigurations[0].id, }); }; @@ -79,7 +75,21 @@ const CohortsForm = forwardRef(({ disableManua {intl.formatMessage(messages.cohortAssignmentMethod)} setSelectedAssignmentType(e.target.value)}> {intl.formatMessage(messages.automatic)} - {intl.formatMessage(messages.manual)} + {disableManualAssignment ? ( + + {intl.formatMessage(messages.manualAssignmentDisabledTooltip)} + + )} + > + + {intl.formatMessage(messages.manual)} + + + ) + : {intl.formatMessage(messages.manual)}} @@ -91,8 +101,8 @@ const CohortsForm = forwardRef(({ disableManua > {intl.formatMessage(messages.noContentGroup)}
- {intl.formatMessage(messages.selectAContentGroup)} - { data.length > 0 + {intl.formatMessage(messages.selectAContentGroup)} + { data.allGroupConfigurations[0].groups.length > 0 ? ( (({ disableManua size="sm" disabled={selectedContentGroupOption !== 'selectContentGroup'} name="contentGroup" - onChange={(e) => setSelectedContentGroup(e.target.value)} + onChange={(e) => setSelectedContentGroup(Number(e.target.value))} value={selectedContentGroup} > { contentGroups.map((contentGroup) => ( )) } diff --git a/src/cohorts/components/EnabledCohortsView.test.tsx b/src/cohorts/components/EnabledCohortsView.test.tsx index 194bea1e..abb3fb54 100644 --- a/src/cohorts/components/EnabledCohortsView.test.tsx +++ b/src/cohorts/components/EnabledCohortsView.test.tsx @@ -6,6 +6,7 @@ import { CohortProvider } from '@src/cohorts/components/CohortContext'; import EnabledCohortsView from '@src/cohorts/components/EnabledCohortsView'; import { useCohorts, useContentGroupsData } from '@src/cohorts/data/apiHook'; import messages from '@src/cohorts/messages'; +import { AlertProvider } from '@src/components/AlertContext'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -25,11 +26,17 @@ const mockCohorts = [ { id: 2, name: 'Cohort 2' }, ]; +const mockContentGroups = [ + { id: '2', name: 'Group 1' }, + { id: '3', name: 'Group 2' }, +]; + describe('EnabledCohortsView', () => { - const renderWithCohortProvider = () => renderWithIntl(); + const renderWithCohortProvider = () => renderWithIntl(); beforeEach(() => { jest.clearAllMocks(); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); (useParams as jest.Mock).mockReturnValue({ courseId: 'course-v1:edX+Test+2024' }); }); @@ -46,7 +53,6 @@ describe('EnabledCohortsView', () => { it('calls handleSelectCohort on select change', async () => { (useCohorts as jest.Mock).mockReturnValue({ data: mockCohorts }); - (useContentGroupsData as jest.Mock).mockReturnValue({ data: [] }); renderWithCohortProvider(); const select = screen.getByRole('combobox'); const user = userEvent.setup(); @@ -56,7 +62,6 @@ describe('EnabledCohortsView', () => { it('calls handleAddCohort on button click', async () => { (useCohorts as jest.Mock).mockReturnValue({ data: [] }); - (useContentGroupsData as jest.Mock).mockReturnValue({ data: [] }); renderWithCohortProvider(); const user = userEvent.setup(); const button = screen.getByRole('button', { name: `+ ${messages.addCohort.defaultMessage}` }); diff --git a/src/cohorts/components/EnabledCohortsView.tsx b/src/cohorts/components/EnabledCohortsView.tsx index 97afdc4f..e894388b 100644 --- a/src/cohorts/components/EnabledCohortsView.tsx +++ b/src/cohorts/components/EnabledCohortsView.tsx @@ -2,6 +2,7 @@ import { useState } from 'react'; import { useParams } from 'react-router-dom'; import { useIntl } from '@openedx/frontend-base'; import { FormControl, Button, Card, Alert } from '@openedx/paragon'; +import { useAlert } from '@src/components/AlertContext'; import { CheckCircle } from '@openedx/paragon/icons'; import { useCohortContext } from '@src/cohorts/components/CohortContext'; import CohortsForm from '@src/cohorts/components/CohortsForm'; @@ -13,23 +14,23 @@ import { CohortData, BasicCohortData } from '@src/cohorts/types'; const EnabledCohortsView = () => { const intl = useIntl(); - const { courseId = '' } = useParams(); + const { courseId = '' } = useParams<{ courseId: string }>(); const { data = [] } = useCohorts(courseId); const { mutate: createCohort } = useCreateCohort(courseId); const { clearSelectedCohort, selectedCohort, setSelectedCohort } = useCohortContext(); const [displayAddForm, setDisplayAddForm] = useState(false); - const [showSuccessAlert, setShowSuccessAlert] = useState(false); + const { alerts, addAlert, removeAlert, clearAlerts } = useAlert(); const cohortsList = [{ id: 'null', name: intl.formatMessage(messages.selectCohortPlaceholder) }, ...data]; const handleAddCohort = () => { clearSelectedCohort(); - setShowSuccessAlert(false); + clearAlerts(); setDisplayAddForm(true); }; const handleSelectCohort = (event: React.ChangeEvent) => { - setShowSuccessAlert(false); + clearAlerts(); const selectedValue = event.target.value; const selectedCohortFromApi = cohortsList.find(cohort => cohort.id?.toString() === selectedValue); setDisplayAddForm(false); @@ -52,11 +53,16 @@ const EnabledCohortsView = () => { const handleNewCohort = (newCohort: BasicCohortData) => { createCohort(newCohort, { onSuccess: (newCohort: CohortData) => { - setShowSuccessAlert(true); + addAlert({ + type: 'success', + message: intl.formatMessage(messages.addCohortSuccessMessage, { cohortName: newCohort.name }) + }); setSelectedCohort(newCohort); hideAddForm(); }, - onError: (error) => console.error(error) + onError: (error) => { + console.error(error); + } }); }; @@ -85,7 +91,19 @@ const EnabledCohortsView = () => {
- {showSuccessAlert && {intl.formatMessage(messages.addCohortSuccessMessage, { cohortName: selectedCohort?.name })}} + {alerts.map(alert => ( + removeAlert(alert.id)} + > +

{alert.message}

+ {alert.extraContent} + + ))} {displayAddForm && ( diff --git a/src/cohorts/components/ManageLearners.test.tsx b/src/cohorts/components/ManageLearners.test.tsx index c5fb89b3..57ca087a 100644 --- a/src/cohorts/components/ManageLearners.test.tsx +++ b/src/cohorts/components/ManageLearners.test.tsx @@ -5,6 +5,7 @@ import { useCohortContext } from '@src/cohorts/components/CohortContext'; import ManageLearners from '@src/cohorts/components/ManageLearners'; import messages from '@src/cohorts/messages'; import { renderWithIntl } from '@src/testUtils'; +import { AlertProvider } from '@src/components/AlertContext'; jest.mock('react-router-dom', () => ({ useParams: jest.fn(), @@ -18,6 +19,8 @@ jest.mock('@src/cohorts/components/CohortContext', () => ({ useCohortContext: jest.fn(), })); +const renderWithAlertProvider = () => renderWithIntl(); + describe('ManageLearners', () => { const mutateMock = jest.fn(); @@ -29,7 +32,7 @@ describe('ManageLearners', () => { }); it('render all static texts', () => { - renderWithIntl(); + renderWithAlertProvider(); expect(screen.getByRole('heading', { name: messages.addLearnersTitle.defaultMessage })).toBeInTheDocument(); expect(screen.getByText(messages.addLearnersSubtitle.defaultMessage)).toBeInTheDocument(); expect(screen.getByText(messages.addLearnersInstructions.defaultMessage)).toBeInTheDocument(); @@ -39,7 +42,7 @@ describe('ManageLearners', () => { }); it('updates textarea value and calls mutate on button click', () => { - renderWithIntl(); + renderWithAlertProvider(); const textarea = screen.getByPlaceholderText(messages.learnersExample.defaultMessage); fireEvent.change(textarea, { target: { value: 'user1@example.com,user2@example.com' } }); fireEvent.click(screen.getByRole('button', { name: /\+ Add Learners/i })); @@ -53,10 +56,10 @@ describe('ManageLearners', () => { }); it('handles empty input gracefully', () => { - renderWithIntl(); + renderWithAlertProvider(); fireEvent.click(screen.getByRole('button', { name: /\+ Add Learners/i })); expect(mutateMock).toHaveBeenCalledWith( - [''], + [], expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function), @@ -65,7 +68,7 @@ describe('ManageLearners', () => { }); it('calls onError if mutate fails', () => { - renderWithIntl(); + renderWithAlertProvider(); const textarea = screen.getByPlaceholderText(messages.learnersExample.defaultMessage); fireEvent.change(textarea, { target: { value: 'user@example.com' } }); fireEvent.click(screen.getByRole('button', { name: /\+ Add Learners/i })); @@ -79,7 +82,7 @@ describe('ManageLearners', () => { it('uses default cohort id 0 if selectedCohort is missing', () => { (useCohortContext as jest.Mock).mockReturnValue({ selectedCohort: undefined }); - renderWithIntl(); + renderWithAlertProvider(); fireEvent.click(screen.getByRole('button', { name: /\+ Add Learners/i })); expect(mutateMock).toHaveBeenCalled(); }); diff --git a/src/cohorts/components/ManageLearners.tsx b/src/cohorts/components/ManageLearners.tsx index 644e5182..3e052401 100644 --- a/src/cohorts/components/ManageLearners.tsx +++ b/src/cohorts/components/ManageLearners.tsx @@ -5,6 +5,15 @@ import { Button, FormControl } from '@openedx/paragon'; import { useCohortContext } from '@src/cohorts/components/CohortContext'; import { useAddLearnersToCohort } from '@src/cohorts/data/apiHook'; import messages from '@src/cohorts/messages'; +import { useAlert } from '@src/components/AlertContext'; + +interface AddLearnersResponse { + added: string[], + changed: string[], + preassigned: string[], + present: string[], + unknown: string[], +} const ManageLearners = () => { const { courseId = '' } = useParams(); @@ -12,13 +21,62 @@ const ManageLearners = () => { const { selectedCohort } = useCohortContext(); const { mutate: addLearnersToCohort } = useAddLearnersToCohort(courseId, selectedCohort?.id ? Number(selectedCohort.id) : 0); const [users, setUsers] = useState(''); + const { addAlert, clearAlerts } = useAlert(); + + const handleAlertMessages = (response: AddLearnersResponse) => { + const { added, changed, preassigned, present, unknown } = response; + if (preassigned.length > 0) { + addAlert({ + type: 'warning', + message: intl.formatMessage(messages.addLearnersWarningMessage, { + countLearners: preassigned.length, + }), + extraContent: ( + preassigned.map((learner: string) => ( +

• {learner}

+ )) + ) + }); + } + if (present.length > 0 || added.length > 0 || changed.length > 0) { + addAlert({ + type: 'success', + message: intl.formatMessage(messages.addLearnersSuccessMessage, { + countLearners: added.length + changed.length, + }), + extraContent: ( + present.length > 0 && ( + present.map((learner: string) => ( +

• {intl.formatMessage(messages.existingLearner, { learner })}

+ )) + )) + }); + } + if (unknown.length > 0) { + addAlert({ + type: 'error', + message: intl.formatMessage(messages.addLearnersErrorMessage, { + countLearners: unknown.length, + }), + extraContent: ( + unknown.map((learner: string) => ( +

• {intl.formatMessage(messages.unknownLearner, { learner })}

+ )) + ) + }); + } + }; const handleAddLearners = () => { - addLearnersToCohort(users.split(','), { - onSuccess: () => { - // Handle success (e.g., show a success message) - }, + clearAlerts(); + const usersArray = users.split(/[\n,]+/).map(u => u.trim()).filter(Boolean); + addLearnersToCohort(usersArray, { + onSuccess: handleAlertMessages, onError: (error) => { + addAlert({ + type: 'error', + message: intl.formatMessage(messages.addLearnersErrorMessage) + }); console.error(error); } }); diff --git a/src/cohorts/components/SelectedCohortInfo.test.tsx b/src/cohorts/components/SelectedCohortInfo.test.tsx new file mode 100644 index 00000000..52463e7f --- /dev/null +++ b/src/cohorts/components/SelectedCohortInfo.test.tsx @@ -0,0 +1,85 @@ +import { screen } from '@testing-library/react'; +import SelectedCohortInfo from './SelectedCohortInfo'; +import messages from '../messages'; +import dataDownloadsMessages from '@src/dataDownloads/messages'; +import { renderWithIntl } from '@src/testUtils'; +import * as CohortContextModule from '@src/cohorts/components/CohortContext'; +import { CohortProvider } from './CohortContext'; +import { AlertProvider } from '@src/components/AlertContext'; +import { useCohorts, useContentGroupsData } from '../data/apiHook'; + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useParams: () => ({ courseId: 'course-v1:edX+DemoX+Demo_Course' }), +})); + +const mockCohorts = [ + { + id: 1, + name: 'Initial Cohort', + assignmentType: 'manual', + groupId: 2, + userPartitionId: 3, + userCount: 0 + }, + { id: 2, name: 'Cohort 2', + assignmentType: 'automatic', + groupId: null, + userPartitionId: null, + userCount: 5 + }, +]; + +const mockContentGroups = [ + { id: '2', name: 'Group 1' }, + { id: '3', name: 'Group 2' }, +]; + +jest.mock('@src/cohorts/data/apiHook', () => ({ + useCohorts: jest.fn(), + useContentGroupsData: jest.fn(), + useCreateCohort: () => ({ mutate: jest.fn() }), + usePatchCohort: () => ({ mutate: jest.fn() }), + useAddLearnersToCohort: () => ({ mutate: jest.fn() }), +})); + +function renderWithProviders() { + return renderWithIntl( + + + + + + ); +} + +describe('SelectedCohortInfo', () => { + beforeEach(() => { + (useCohorts as jest.Mock).mockReturnValue({ data: mockCohorts }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { allGroupConfigurations: [{ groups: mockContentGroups }] } }); + jest.spyOn(CohortContextModule, 'useCohortContext').mockReturnValue({ + selectedCohort: mockCohorts[0], + setSelectedCohort: jest.fn(), + clearSelectedCohort: jest.fn(), + updateCohortField: jest.fn(), + }); + }); + + it('if a cohort is selected renders CohortCard', () => { + renderWithProviders(); + expect(screen.getByRole('tablist')).toBeInTheDocument(); + expect(screen.getByRole('tab', { name: messages.manageLearners.defaultMessage })).toBeInTheDocument(); + }); + + it('renders cohort disclaimer message', () => { + renderWithProviders(); + expect(screen.getByText(new RegExp(messages.cohortDisclaimer.defaultMessage))).toBeInTheDocument(); + }); + + it('renders data downloads hyperlink with correct destination', () => { + renderWithProviders(); + const link = screen.getByRole('link', { name: dataDownloadsMessages.pageTitle.defaultMessage }); + expect(link).toBeInTheDocument(); + expect(link).toHaveAttribute('href', '/instructor/course-v1:edX+DemoX+Demo_Course/data_downloads'); + }); +}); diff --git a/src/cohorts/data/apiHook.ts b/src/cohorts/data/apiHook.ts index a061d1dc..c4bd2e79 100644 --- a/src/cohorts/data/apiHook.ts +++ b/src/cohorts/data/apiHook.ts @@ -43,6 +43,7 @@ export const useContentGroupsData = (courseId: string) => ( useQuery({ queryKey: cohortsQueryKeys.contentGroups(courseId), queryFn: () => getContentGroups(courseId), + enabled: !!courseId, }) ); diff --git a/src/cohorts/messages.ts b/src/cohorts/messages.ts index e9595670..8bfd2ee7 100644 --- a/src/cohorts/messages.ts +++ b/src/cohorts/messages.ts @@ -190,7 +190,32 @@ const messages = defineMessages({ id: 'instruct.cohorts.manualAssignmentDisabledTooltip', defaultMessage: 'There must be one cohort to which students can automatically be assigned.', description: 'Tooltip message when manual assignment is disabled' - } + }, + addLearnersSuccessMessage: { + id: 'instruct.cohorts.addLearnersSuccessMessage', + defaultMessage: '{countLearners} learners have been added to this cohort.', + description: 'Success message displayed when learners are added to a cohort' + }, + addLearnersErrorMessage: { + id: 'instruct.cohorts.addLearnersErrorMessage', + defaultMessage: '{countLearners} learners could not be added to this cohort.', + description: 'Error message displayed when there is an issue adding learners to a cohort' + }, + addLearnersWarningMessage: { + id: 'instruct.cohorts.addLearnersWarningMessage', + defaultMessage: '{countLearners} were pre-assigned for this cohort. This learner will automatically be added to the cohort when they enroll in the course.', + description: 'Warning message displayed when some learners could not be added to a cohort' + }, + existingLearner: { + id: 'instruct.cohorts.existingLearner', + defaultMessage: '{learner} learner was already in the cohort.', + description: 'Message indicating that a learner is already assigned to a cohort' + }, + unknownLearner: { + id: 'instruct.cohorts.unknownLearner', + defaultMessage: 'Unknown username or email: {learner}', + description: 'Message indicating that a learner is not recognized in the course' + }, }); export default messages; diff --git a/src/components/AlertContext.test.tsx b/src/components/AlertContext.test.tsx new file mode 100644 index 00000000..d6c10c84 --- /dev/null +++ b/src/components/AlertContext.test.tsx @@ -0,0 +1,100 @@ +import { ReactElement } from 'react'; +import { render, screen } from '@testing-library/react'; +import { AlertProvider, useAlert } from '@src/components/AlertContext'; +import userEvent from '@testing-library/user-event'; + +const TestComponent = () => { + const { alerts, addAlert, removeAlert, clearAlerts } = useAlert(); + + return ( +
+ + + + +
    + {alerts.map(alert => ( +
  • + {alert.type}: {alert.message} +
    • + ))} +
+
+ ); +}; + +const renderWithProvider = (ui: ReactElement) => + render({ui}); + +describe('AlertContext', () => { + it('throws error when used outside provider', () => { + const BrokenComponent = () => { + useAlert(); + return <>; + }; + expect(() => render()).toThrow( + 'useAlert must be used within a AlertProvider' + ); + }); + + it('adds an alert', async () => { + renderWithProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /Add Success/i })); + expect(screen.getAllByRole('listitem')).toHaveLength(1); + }); + + it('adds multiple alerts of different types', async () => { + renderWithProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /Add Success/i })); + await user.click(screen.getByRole('button', { name: /Add Error/i })); + expect(screen.getAllByRole('listitem')).toHaveLength(2); + expect(screen.getByText('success: Success!')).toBeInTheDocument(); + expect(screen.getByText('error: Error!')).toBeInTheDocument(); + }); + + it('removes an alert by id', async () => { + renderWithProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /Add Success/i })); + await user.click(screen.getByRole('button', { name: /Remove First/i })); + expect(screen.queryAllByRole('listitem')).toHaveLength(0); + }); + + it('clears all alerts', async () => { + renderWithProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /Add Success/i })); + await user.click(screen.getByRole('button', { name: /Add Error/i })); + await user.click(screen.getByRole('button', { name: /Clear All/i })); + expect(screen.queryAllByRole('listitem')).toHaveLength(0); + }); + + it('alerts have unique ids', async () => { + renderWithProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /Add Success/i })); + await user.click(screen.getByRole('button', { name: /Add Success/i })); + const alerts = screen.getAllByRole('listitem'); + expect(alerts.length).toBe(2); + expect(alerts[0].textContent).toBe('success: Success!'); + expect(alerts[1].textContent).toBe('success: Success!'); + }); +}); diff --git a/src/components/AlertContext.tsx b/src/components/AlertContext.tsx new file mode 100644 index 00000000..11ab757f --- /dev/null +++ b/src/components/AlertContext.tsx @@ -0,0 +1,48 @@ +import { createContext, useContext, useState, ReactNode } from 'react'; + +export type AlertType = 'success' | 'error' | 'info' | 'warning'; + +export interface AlertProps { + id: string, + type: AlertType, + message: string, + extraContent?: ReactNode, +} + +interface AlertContextProps { + alerts: AlertProps[], + addAlert: (alert: Omit) => void, + removeAlert: (id: string) => void, + clearAlerts: () => void, +} + +const AlertContext = createContext(undefined); + +export const useAlert = () => { + const context = useContext(AlertContext); + if (!context) { + throw new Error('useAlert must be used within a AlertProvider'); + } + return context; +}; + +export const AlertProvider = ({ children }: { children: ReactNode }) => { + const [alerts, setAlerts] = useState([]); + + const addAlert = (alert: Omit) => { + const id = `${Date.now()}-${Math.random()}`; + setAlerts(prev => [...prev, { ...alert, id }]); + }; + + const removeAlert = (id: string) => { + setAlerts(prev => prev.filter(alert => alert.id !== id)); + }; + + const clearAlerts = () => setAlerts([]); + + return ( + + {children} + + ); +};