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.test.tsx b/src/cohorts/CohortsPage.test.tsx index cf52e3ed..7cbfac4d 100644 --- a/src/cohorts/CohortsPage.test.tsx +++ b/src/cohorts/CohortsPage.test.tsx @@ -2,7 +2,7 @@ import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import CohortsPage from './CohortsPage'; import { useCohorts, useCohortStatus, useToggleCohorts } from './data/apiHook'; -import { renderWithIntl } from '@src/testUtils'; +import { renderWithAlertAndIntl } from '@src/testUtils'; import messages from './messages'; import { CohortProvider } from './components/CohortContext'; @@ -19,7 +19,7 @@ jest.mock('./data/apiHook', () => ({ })); describe('CohortsPage', () => { - const renderWithCohortsProvider = () => renderWithIntl(); + const renderWithCohortsProvider = () => renderWithAlertAndIntl(); beforeEach(() => { jest.clearAllMocks(); }); 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.test.tsx b/src/cohorts/components/CohortCard.test.tsx new file mode 100644 index 00000000..b1dc210a --- /dev/null +++ b/src/cohorts/components/CohortCard.test.tsx @@ -0,0 +1,215 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { useParams } from 'react-router-dom'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { IntlProvider } from '@openedx/frontend-base'; +import CohortCard, { assignmentLink } from './CohortCard'; +import { useCohortContext } from './CohortContext'; +import { AlertProvider, useAlert } from '@src/providers/AlertProvider'; +import { useAddLearnersToCohort, usePatchCohort } from '@src/cohorts/data/apiHook'; +import { assignmentTypes } from '@src/cohorts/constants'; +import { CohortData } from '@src/cohorts/types'; +import messages from '../messages'; + +jest.mock('./CohortContext', () => ({ + useCohortContext: jest.fn(), +})); + +jest.mock('react-router-dom', () => ({ + useParams: jest.fn(), +})); + +jest.mock('@src/cohorts/data/apiHook', () => ({ + usePatchCohort: jest.fn(), + useAddLearnersToCohort: jest.fn(), + useContentGroupsData: jest.fn().mockReturnValue({ data: { groups: [{ id: '2', name: 'Group 1' }], id: 1 } }), +})); + +jest.mock('@src/providers/AlertProvider', () => ({ + useAlert: jest.fn(), + AlertProvider: ({ children }: any) => children, +})); + +const mockUsePatchCohort = usePatchCohort as jest.Mock; +const mockUseParams = useParams as jest.Mock; +const mockUseCohortContext = useCohortContext as jest.Mock; +const mockUseAlert = useAlert as jest.Mock; +const mockUseAddLearnersToCohort = useAddLearnersToCohort as jest.Mock; +const mockCohort = { + id: 1, + name: 'Test Cohort', + assignmentType: assignmentTypes.manual, + groupId: null, + userPartitionId: null, + userCount: 15, +}; + +const createMockCohort = (overrides?: Partial): CohortData => ({ + ...mockCohort, + ...overrides, +}); + +const renderCohortCard = (selectedCohort: CohortData | null = createMockCohort()) => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + const mockContextValue = { + selectedCohort, + setSelectedCohort: jest.fn(), + clearSelectedCohort: jest.fn(), + updateCohortField: jest.fn(), + }; + + mockUseCohortContext.mockReturnValue(mockContextValue); + + return render( + + + + + + + + ); +}; + +describe('CohortCard', () => { + const mockMutate = jest.fn(); + const mockClearAlerts = jest.fn(); + const mockAddLearnersMutate = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + mockUseParams.mockReturnValue({ courseId: 'course-123' }); + mockUsePatchCohort.mockReturnValue({ mutate: mockMutate }); + mockUseAlert.mockReturnValue({ clearAlerts: mockClearAlerts }); + mockUseAddLearnersToCohort.mockReturnValue({ mutate: mockAddLearnersMutate }); + }); + + describe('Rendering', () => { + it('should render null when no cohort is selected', () => { + const { container } = renderCohortCard(null); + expect(container.firstChild).toBeNull(); + }); + + it('should render cohort card with basic information', () => { + const cohort = createMockCohort(); + renderCohortCard(cohort); + expect(screen.getByText(mockCohort.name)).toBeInTheDocument(); + expect(screen.getByText(new RegExp(`\\(contains ${mockCohort.userCount} students\\)`))).toBeInTheDocument(); + }); + + it('should render warning message for manual assignment type', () => { + const cohort = createMockCohort({ assignmentType: assignmentTypes.manual }); + renderCohortCard(cohort); + expect(screen.getByText(messages.manualCohortWarning.defaultMessage)).toBeInTheDocument(); + }); + + it('should render warning message for automatic assignment type', () => { + const cohort = createMockCohort({ assignmentType: assignmentTypes.automatic }); + renderCohortCard(cohort); + expect(screen.getByText(messages.automaticCohortWarning.defaultMessage)).toBeInTheDocument(); + }); + + it('should render external link with correct URL for manual cohort', () => { + const cohort = createMockCohort({ assignmentType: assignmentTypes.manual }); + renderCohortCard(cohort); + const link = screen.getByRole('link'); + expect(link).toHaveAttribute('href', assignmentLink.manual); + expect(link).toHaveAttribute('target', '_blank'); + }); + + it('should render external link with correct URL for automatic cohort', () => { + const cohort = createMockCohort({ assignmentType: assignmentTypes.automatic }); + renderCohortCard(cohort); + const link = screen.getByRole('link'); + expect(link).toHaveAttribute('href', assignmentLink.random); + }); + }); + + describe('Tabs Navigation', () => { + it('should render both tabs with correct titles', () => { + renderCohortCard(); + expect(screen.getByText(messages.manageLearners.defaultMessage)).toBeInTheDocument(); + expect(screen.getByText(messages.settings.defaultMessage)).toBeInTheDocument(); + }); + + it('should show manage learners content by default', () => { + renderCohortCard(); + expect(screen.getByPlaceholderText(messages.learnersExample.defaultMessage)).toBeInTheDocument(); + }); + + it('should switch to settings tab and show form', async () => { + const user = userEvent.setup(); + renderCohortCard(); + const settingsTab = screen.getByText(messages.settings.defaultMessage); + await user.click(settingsTab); + await waitFor(() => { + expect(screen.getByText(messages.cohortAssignmentMethod.defaultMessage)).toBeInTheDocument(); + }); + }); + }); + + describe('Success Toast', () => { + it('should show success toast after successful form submission', async () => { + const user = userEvent.setup(); + renderCohortCard(); + const settingsTab = screen.getByText(messages.settings.defaultMessage); + await user.click(settingsTab); + await waitFor(() => { + expect(screen.getByText(messages.cohortAssignmentMethod.defaultMessage)).toBeInTheDocument(); + }); + + mockMutate.mockImplementation((_options: any, callbacks: { onSuccess: () => void }) => { + callbacks.onSuccess(); + }); + + const submitBtn = screen.getByRole('button', { name: messages.saveLabel.defaultMessage }); + await user.click(submitBtn); + await waitFor(() => { + expect(screen.getByText(messages.cohortUpdateSuccessMessage.defaultMessage)).toBeInTheDocument(); + }); + }); + + it('should close success toast when close button is clicked', async () => { + const user = userEvent.setup(); + renderCohortCard(); + const settingsTab = screen.getByText(messages.settings.defaultMessage); + await user.click(settingsTab); + await waitFor(() => { + expect(screen.getByText(messages.cohortAssignmentMethod.defaultMessage)).toBeInTheDocument(); + }); + + mockMutate.mockImplementation((_options: any, callbacks: { onSuccess: () => void }) => { + callbacks.onSuccess(); + }); + + const submitBtn = screen.getByRole('button', { name: messages.saveLabel.defaultMessage }); + await user.click(submitBtn); + await waitFor(() => { + expect(screen.getByText(messages.cohortUpdateSuccessMessage.defaultMessage)).toBeInTheDocument(); + }); + + const closeButton = screen.getByRole('button', { name: /close/i }); + await user.click(closeButton); + await waitFor(() => { + expect(screen.queryByText(messages.cohortUpdateSuccessMessage.defaultMessage)).not.toBeInTheDocument(); + }); + }); + }); + + describe('Edge Cases', () => { + it('should handle cohort with zero users', () => { + const cohort = createMockCohort({ userCount: 0 }); + renderCohortCard(cohort); + expect(screen.getByText(/0 students/)).toBeInTheDocument(); + }); + + it('should handle cohort with undefined userCount', () => { + const cohort = createMockCohort({ userCount: undefined as any }); + renderCohortCard(cohort); + expect(screen.getByText(/0 students/)).toBeInTheDocument(); + }); + }); +}); diff --git a/src/cohorts/components/CohortCard.tsx b/src/cohorts/components/CohortCard.tsx index f1746610..1f0da784 100644 --- a/src/cohorts/components/CohortCard.tsx +++ b/src/cohorts/components/CohortCard.tsx @@ -1,15 +1,16 @@ 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 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 { Card, Hyperlink, Tab, Tabs, Toast } from '@openedx/paragon'; +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'; +import { useAlert } from '@src/providers/AlertProvider'; -const assignmentLink = { +export const assignmentLink = { random: 'https://docs.openedx.org/en/latest/educators/references/advanced_features/managing_cohort_assignment.html#about-auto-cohorts', manual: 'https://docs.openedx.org/en/latest/educators/how-tos/advanced_features/manage_cohorts.html#assign-learners-to-cohorts-manually', }; @@ -26,19 +27,25 @@ const CohortCard = () => { const { mutate: editCohort } = usePatchCohort(courseId); const formRef = useRef<{ resetForm: () => void }>(null); const [showSuccessMessage, setShowSuccessMessage] = useState(false); + const { clearAlerts, showModal } = 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: Error) => { + showModal({ + message: error.message, + }); + } } ); }; @@ -56,7 +63,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/CohortsForm.test.tsx b/src/cohorts/components/CohortsForm.test.tsx index 49e88056..f932f1ec 100644 --- a/src/cohorts/components/CohortsForm.test.tsx +++ b/src/cohorts/components/CohortsForm.test.tsx @@ -1,22 +1,22 @@ import { screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import CohortsForm from './CohortsForm'; -import messages from '../messages'; -import { renderWithIntl } from '@src/testUtils'; -import { useContentGroupsData } from '../data/apiHook'; -import { CohortProvider } from './CohortContext'; -import * as CohortContextModule from './CohortContext'; +import CohortsForm from '@src/cohorts/components/CohortsForm'; +import messages from '@src/cohorts/messages'; +import { renderWithAlertAndIntl, renderWithIntl } from '@src/testUtils'; +import { useContentGroupsData } from '@src/cohorts/data/apiHook'; +import { CohortProvider } from '@src/cohorts/components/CohortContext'; +import * as CohortContextModule from '@src/cohorts/components/CohortContext'; 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(), })); @@ -25,7 +25,7 @@ describe('CohortsForm', () => { const onSubmit = jest.fn(); const renderComponent = () => - renderWithIntl( + renderWithAlertAndIntl( @@ -36,30 +36,30 @@ describe('CohortsForm', () => { }); it('renders cohort name input', () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { 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: { 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: { 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: { groups: mockContentGroups } }); renderComponent(); const user = userEvent.setup(); const cancelButton = screen.getByRole('button', { name: messages.cancelLabel.defaultMessage }); @@ -68,7 +68,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: { groups: mockContentGroups } }); renderComponent(); const user = userEvent.setup(); const input = screen.getByPlaceholderText(messages.cohortName.defaultMessage); @@ -78,7 +78,7 @@ describe('CohortsForm', () => { }); it('updates cohort name input value', async () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { groups: mockContentGroups } }); renderComponent(); const input = screen.getByPlaceholderText(messages.cohortName.defaultMessage); const user = userEvent.setup(); @@ -87,7 +87,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: { groups: mockContentGroups } }); renderComponent(); const select = screen.getByRole('combobox'); expect(select).toBeDisabled(); @@ -97,14 +97,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: { 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: { groups: mockContentGroups, id: 1 } }); renderComponent(); const user = userEvent.setup(); @@ -121,15 +121,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: { groups: mockContentGroups } }); renderWithIntl( @@ -140,7 +140,7 @@ describe('CohortsForm', () => { }); it('shows initial values in context', async () => { - (useContentGroupsData as jest.Mock).mockReturnValue({ data: mockContentGroups }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { groups: mockContentGroups } }); jest.spyOn(CohortContextModule, 'useCohortContext').mockReturnValue({ selectedCohort: { @@ -174,6 +174,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..4edb3801 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 = { groups: [], id: null } } = 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.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.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.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/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/EnabledCohortsView.test.tsx b/src/cohorts/components/EnabledCohortsView.test.tsx index 194bea1e..61342e22 100644 --- a/src/cohorts/components/EnabledCohortsView.test.tsx +++ b/src/cohorts/components/EnabledCohortsView.test.tsx @@ -1,11 +1,12 @@ import { useParams } from 'react-router-dom'; import { screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { renderWithIntl } from '@src/testUtils'; +import { renderWithAlertAndIntl } from '@src/testUtils'; import { CohortProvider } from '@src/cohorts/components/CohortContext'; import EnabledCohortsView from '@src/cohorts/components/EnabledCohortsView'; -import { useCohorts, useContentGroupsData } from '@src/cohorts/data/apiHook'; +import { useCohorts, useContentGroupsData, useCreateCohort } from '@src/cohorts/data/apiHook'; import messages from '@src/cohorts/messages'; +import * as AlertProvider from '@src/providers/AlertProvider'; +import userEvent from '@testing-library/user-event'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -15,22 +16,69 @@ jest.mock('react-router-dom', () => ({ jest.mock('@src/cohorts/data/apiHook', () => ({ useCohorts: jest.fn(), useContentGroupsData: jest.fn(), - useCreateCohort: () => ({ mutate: jest.fn() }), + useCreateCohort: jest.fn(), usePatchCohort: () => ({ mutate: jest.fn() }), useAddLearnersToCohort: () => ({ mutate: jest.fn() }), })); const mockCohorts = [ - { id: 1, name: 'Cohort 1' }, - { id: 2, name: 'Cohort 2' }, + { + id: 1, + name: 'Cohort 1', + assignmentType: 'automatic', + groupId: 1, + userPartitionId: 1, + userCount: 5 + }, + { + id: 2, + name: 'Cohort 2', + assignmentType: 'manual', + groupId: 2, + userPartitionId: 2, + userCount: 10 + }, +]; + +const mockContentGroups = [ + { id: '2', name: 'Group 1' }, + { id: '3', name: 'Group 2' }, ]; describe('EnabledCohortsView', () => { - const renderWithCohortProvider = () => renderWithIntl(); + const renderWithCohortProvider = () => renderWithAlertAndIntl(); + const createCohortMock = jest.fn(); + const addAlertMock = jest.fn(); + const removeAlertMock = jest.fn(); + const clearAlertsMock = jest.fn(); + let useAlertSpy: jest.SpyInstance; beforeEach(() => { jest.clearAllMocks(); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { groups: mockContentGroups, id: 1 } }); (useParams as jest.Mock).mockReturnValue({ courseId: 'course-v1:edX+Test+2024' }); + (useCreateCohort as jest.Mock).mockReturnValue({ mutate: createCohortMock }); + + useAlertSpy = jest.spyOn(AlertProvider, 'useAlert').mockReturnValue({ + alerts: [], + addAlert: addAlertMock, + removeAlert: removeAlertMock, + clearAlerts: clearAlertsMock, + showToast: jest.fn(), + showModal: jest.fn(), + showInlineAlert: jest.fn(), + dismissInlineAlert: jest.fn(), + inlineAlerts: [] + }); + + createCohortMock.mockReset(); + addAlertMock.mockReset(); + removeAlertMock.mockReset(); + clearAlertsMock.mockReset(); + }); + + afterEach(() => { + useAlertSpy.mockRestore(); }); it('renders select with placeholder and cohorts', () => { @@ -46,23 +94,24 @@ 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(); - await user.selectOptions(select, '1'); - expect((select as HTMLSelectElement).value).toBe('1'); + const select = screen.getByPlaceholderText(messages.selectCohortPlaceholder.defaultMessage); + await user.click(select); + await user.selectOptions(select, mockCohorts[1].id.toString()); + expect(clearAlertsMock).toHaveBeenCalled(); + expect((select as HTMLSelectElement).value).toBe(mockCohorts[1].id.toString()); }); 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}` }); await user.click(button); expect(screen.getByPlaceholderText(messages.cohortName.defaultMessage)).toBeInTheDocument(); expect(screen.getByRole('button', { name: messages.saveLabel.defaultMessage })).toBeInTheDocument(); + expect(clearAlertsMock).toHaveBeenCalled(); }); it('renders correctly when no cohorts are returned', () => { @@ -79,4 +128,191 @@ describe('EnabledCohortsView', () => { renderWithCohortProvider(); expect(useCohorts).toHaveBeenCalledWith(''); }); + + it('disables select and button when form is displayed', async () => { + (useCohorts as jest.Mock).mockReturnValue({ data: mockCohorts }); + renderWithCohortProvider(); + const user = userEvent.setup(); + const button = screen.getByRole('button', { name: `+ ${messages.addCohort.defaultMessage}` }); + await user.click(button); + + const select = screen.getByPlaceholderText(messages.selectCohortPlaceholder.defaultMessage); + const addButton = screen.getByRole('button', { name: `+ ${messages.addCohort.defaultMessage}` }); + + // Check if the select and button have the disabled attribute or aria-disabled + expect(select).toHaveAttribute('disabled'); + expect(addButton).toHaveAttribute('disabled'); + }); + + it('disables select when no cohorts are available', () => { + (useCohorts as jest.Mock).mockReturnValue({ data: [] }); + renderWithCohortProvider(); + const select = screen.getByPlaceholderText(messages.selectCohortPlaceholder.defaultMessage); + // When empty data array, length is 0, so it should be disabled + expect(select).toHaveAttribute('disabled'); + }); + + it('handles cohort creation error', async () => { + (useCohorts as jest.Mock).mockReturnValue({ data: [] }); + renderWithCohortProvider(); + const user = userEvent.setup(); + + const button = screen.getByRole('button', { name: `+ ${messages.addCohort.defaultMessage}` }); + await user.click(button); + + const nameInput = screen.getByPlaceholderText(messages.cohortName.defaultMessage); + await user.type(nameInput, 'New Cohort'); + + const saveButton = screen.getByRole('button', { name: messages.saveLabel.defaultMessage }); + await user.click(saveButton); + + // Simulate error callback + const createArgs = createCohortMock.mock.calls[0][1]; + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + createArgs.onError('Creation failed'); + + expect(consoleErrorSpy).toHaveBeenCalledWith('Creation failed'); + consoleErrorSpy.mockRestore(); + }); + + it('handles successful cohort creation', async () => { + (useCohorts as jest.Mock).mockReturnValue({ data: [] }); + renderWithCohortProvider(); + const user = userEvent.setup(); + + const button = screen.getByRole('button', { name: `+ ${messages.addCohort.defaultMessage}` }); + await user.click(button); + + const nameInput = screen.getByPlaceholderText(messages.cohortName.defaultMessage); + await user.type(nameInput, 'New Cohort'); + + const saveButton = screen.getByRole('button', { name: messages.saveLabel.defaultMessage }); + await user.click(saveButton); + + // Simulate successful creation + const createArgs = createCohortMock.mock.calls[0][1]; + const newCohort = { id: 3, name: 'New Cohort', assignmentType: 'automatic' }; + createArgs.onSuccess(newCohort); + + expect(addAlertMock).toHaveBeenCalledWith({ + type: 'success', + message: expect.stringContaining('New Cohort') + }); + }); + + describe('alert rendering', () => { + it('renders alerts when they exist', () => { + const mockAlerts = [ + { id: '1', type: 'success' as const, message: 'Success message' }, + { id: '2', type: 'error' as const, message: 'Error message' } + ]; + + useAlertSpy.mockReturnValue({ + alerts: mockAlerts, + addAlert: addAlertMock, + removeAlert: removeAlertMock, + clearAlerts: clearAlertsMock + }); + + (useCohorts as jest.Mock).mockReturnValue({ data: [] }); + renderWithCohortProvider(); + + expect(screen.getByText('Success message')).toBeInTheDocument(); + expect(screen.getByText('Error message')).toBeInTheDocument(); + }); + + it('renders alert with extra content', () => { + const mockAlerts = [ + { + id: '1', + type: 'warning' as const, + message: 'Warning message', + extraContent:
Extra content
+ } + ]; + + useAlertSpy.mockReturnValue({ + alerts: mockAlerts, + addAlert: addAlertMock, + removeAlert: removeAlertMock, + clearAlerts: clearAlertsMock + }); + + (useCohorts as jest.Mock).mockReturnValue({ data: [] }); + renderWithCohortProvider(); + + expect(screen.getByText('Warning message')).toBeInTheDocument(); + expect(screen.getByText('Extra content')).toBeInTheDocument(); + }); + + it('calls removeAlert when alert is dismissed', async () => { + const mockAlerts = [ + { id: '1', type: 'success' as const, message: 'Success message' } + ]; + + useAlertSpy.mockReturnValue({ + alerts: mockAlerts, + addAlert: addAlertMock, + removeAlert: removeAlertMock, + clearAlerts: clearAlertsMock + }); + + (useCohorts as jest.Mock).mockReturnValue({ data: [] }); + renderWithCohortProvider(); + const user = userEvent.setup(); + + // Look for the dismiss button - it might have different aria-label or text + const alert = screen.getByRole('alert'); + const dismissButton = alert.querySelector('button'); + if (dismissButton) { + await user.click(dismissButton); + expect(removeAlertMock).toHaveBeenCalledWith('1'); + } else { + // If no dismiss button found, skip this specific test assertion + expect(screen.getByText('Success message')).toBeInTheDocument(); + } + }); + + it('uses danger variant for error alerts', () => { + const mockAlerts = [ + { id: '1', type: 'error' as const, message: 'Error message' } + ]; + + useAlertSpy.mockReturnValue({ + alerts: mockAlerts, + addAlert: addAlertMock, + removeAlert: removeAlertMock, + clearAlerts: clearAlertsMock + }); + + (useCohorts as jest.Mock).mockReturnValue({ data: [] }); + renderWithCohortProvider(); + + const alert = screen.getByRole('alert'); + expect(alert).toHaveClass('alert-danger'); + }); + }); + + it('clears selected cohort when selecting null value', async () => { + (useCohorts as jest.Mock).mockReturnValue({ data: mockCohorts }); + renderWithCohortProvider(); + const user = userEvent.setup(); + const select = screen.getByRole('combobox'); + + // Select null/placeholder option + await user.selectOptions(select, 'null'); + expect(clearAlertsMock).toHaveBeenCalled(); + }); + + it('passes disableManualAssignment prop correctly to CohortsForm', async () => { + // When no cohorts exist, disableManualAssignment should be true + (useCohorts as jest.Mock).mockReturnValue({ data: [] }); + renderWithCohortProvider(); + const user = userEvent.setup(); + + const button = screen.getByRole('button', { name: `+ ${messages.addCohort.defaultMessage}` }); + await user.click(button); + + expect(screen.getByRole('button', { name: messages.saveLabel.defaultMessage })).toBeInTheDocument(); + }); }); diff --git a/src/cohorts/components/EnabledCohortsView.tsx b/src/cohorts/components/EnabledCohortsView.tsx index 97afdc4f..a92212e7 100644 --- a/src/cohorts/components/EnabledCohortsView.tsx +++ b/src/cohorts/components/EnabledCohortsView.tsx @@ -2,7 +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 { CheckCircle } from '@openedx/paragon/icons'; +import { CheckCircle, Error, WarningFilled } from '@openedx/paragon/icons'; import { useCohortContext } from '@src/cohorts/components/CohortContext'; import CohortsForm from '@src/cohorts/components/CohortsForm'; import SelectedCohortInfo from '@src/cohorts/components/SelectedCohortInfo'; @@ -10,26 +10,33 @@ import { useCohorts, useCreateCohort } from '@src/cohorts/data/apiHook'; import { assignmentTypes } from '@src/cohorts/constants'; import messages from '@src/cohorts/messages'; import { CohortData, BasicCohortData } from '@src/cohorts/types'; +import { useAlert } from '@src/providers/AlertProvider'; + +const alertIcons = { + success: CheckCircle, + error: Error, + warning: WarningFilled, +}; 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 +59,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); + } }); }; @@ -69,7 +81,7 @@ 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..f3e7f606 100644 --- a/src/cohorts/components/ManageLearners.test.tsx +++ b/src/cohorts/components/ManageLearners.test.tsx @@ -1,10 +1,12 @@ -import { screen, fireEvent } from '@testing-library/react'; +import { screen } from '@testing-library/react'; import { useParams } from 'react-router-dom'; import { useAddLearnersToCohort } from '@src/cohorts/data/apiHook'; 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 { renderWithAlertAndIntl } from '@src/testUtils'; +import * as AlertProvider from '@src/providers/AlertProvider'; +import userEvent from '@testing-library/user-event'; jest.mock('react-router-dom', () => ({ useParams: jest.fn(), @@ -18,18 +20,42 @@ jest.mock('@src/cohorts/components/CohortContext', () => ({ useCohortContext: jest.fn(), })); +const renderWithAlertProvider = () => renderWithAlertAndIntl(); + describe('ManageLearners', () => { const mutateMock = jest.fn(); + const addAlertMock = jest.fn(); + const clearAlertsMock = jest.fn(); + let useAlertSpy: jest.SpyInstance; beforeEach(() => { (useParams as jest.Mock).mockReturnValue({ courseId: 'course-v1:edX+Test+2024' }); (useCohortContext as jest.Mock).mockReturnValue({ selectedCohort: { id: 123 } }); (useAddLearnersToCohort as jest.Mock).mockReturnValue({ mutate: mutateMock }); + + useAlertSpy = jest.spyOn(AlertProvider, 'useAlert').mockReturnValue({ + addAlert: addAlertMock, + clearAlerts: clearAlertsMock, + showToast: jest.fn(), + showModal: jest.fn(), + showInlineAlert: jest.fn(), + dismissInlineAlert: jest.fn(), + inlineAlerts: [], + alerts: [], + removeAlert: jest.fn(), + }); + mutateMock.mockReset(); + addAlertMock.mockReset(); + clearAlertsMock.mockReset(); + }); + + afterEach(() => { + useAlertSpy.mockRestore(); }); 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(); @@ -38,11 +64,12 @@ describe('ManageLearners', () => { expect(screen.getByRole('button', { name: /\+ Add Learners/i })).toBeInTheDocument(); }); - it('updates textarea value and calls mutate on button click', () => { - renderWithIntl(); + it('updates textarea value and calls mutate on button click', async () => { + 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 })); + const user = userEvent.setup(); + await user.type(textarea, 'user1@example.com,user2@example.com'); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); expect(mutateMock).toHaveBeenCalledWith( ['user1@example.com', 'user2@example.com'], expect.objectContaining({ @@ -52,11 +79,12 @@ describe('ManageLearners', () => { ); }); - it('handles empty input gracefully', () => { - renderWithIntl(); - fireEvent.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + it('handles empty input gracefully', async () => { + renderWithAlertProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); expect(mutateMock).toHaveBeenCalledWith( - [''], + [], expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function), @@ -64,23 +92,205 @@ describe('ManageLearners', () => { ); }); - it('calls onError if mutate fails', () => { - renderWithIntl(); + it('calls onError if mutate fails and shows error alert', async () => { + 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 })); + const user = userEvent.setup(); + await user.type(textarea, 'user@example.com'); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); const callArgs = mutateMock.mock.calls[0][1]; const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); callArgs.onError('error!'); + expect(consoleErrorSpy).toHaveBeenCalledWith('error!'); + expect(addAlertMock).toHaveBeenCalledWith({ + type: 'error', + message: messages.addLearnersErrorMessage.defaultMessage + }); consoleErrorSpy.mockRestore(); }); - it('uses default cohort id 0 if selectedCohort is missing', () => { + it('uses default cohort id 0 if selectedCohort is missing', async () => { (useCohortContext as jest.Mock).mockReturnValue({ selectedCohort: undefined }); - renderWithIntl(); - fireEvent.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + renderWithAlertProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); expect(mutateMock).toHaveBeenCalled(); }); + + it('clears alerts before adding learners', async () => { + renderWithAlertProvider(); + const textarea = screen.getByPlaceholderText(messages.learnersExample.defaultMessage); + const user = userEvent.setup(); + await user.type(textarea, 'user@example.com'); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + + expect(clearAlertsMock).toHaveBeenCalled(); + }); + + it('handles different input formats (newlines and commas)', async () => { + renderWithAlertProvider(); + const user = userEvent.setup(); + const textarea = screen.getByPlaceholderText(messages.learnersExample.defaultMessage); + + // Test with newlines + await user.type(textarea, 'user1@example.com\nuser2@example.com\n\nuser3@example.com'); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + + expect(mutateMock).toHaveBeenCalledWith( + ['user1@example.com', 'user2@example.com', 'user3@example.com'], + expect.objectContaining({ + onSuccess: expect.any(Function), + onError: expect.any(Function), + }) + ); + }); + + it('handles mixed separators and extra spaces', async () => { + renderWithAlertProvider(); + const textarea = screen.getByPlaceholderText(messages.learnersExample.defaultMessage); + const user = userEvent.setup(); + await user.type(textarea, ' user1@example.com, user2@example.com \n user3@example.com , \n\n '); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + + expect(mutateMock).toHaveBeenCalledWith( + ['user1@example.com', 'user2@example.com', 'user3@example.com'], + expect.objectContaining({ + onSuccess: expect.any(Function), + onError: expect.any(Function), + }) + ); + }); + + describe('onSuccess callback with handleAlertMessages', () => { + it('shows warning alert for preassigned learners', async () => { + renderWithAlertProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + + const callArgs = mutateMock.mock.calls[0][1]; + const response = { + added: [], + changed: [], + preassigned: ['user1@example.com', 'user2@example.com'], + present: [], + unknown: [] + }; + + callArgs.onSuccess(response); + + expect(addAlertMock).toHaveBeenCalledWith({ + type: 'warning', + message: expect.stringContaining('2'), + extraContent: expect.any(Array) + }); + }); + + it('shows success alert for added and changed learners', async () => { + renderWithAlertProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + + const callArgs = mutateMock.mock.calls[0][1]; + const response = { + added: ['user1@example.com'], + changed: ['user2@example.com'], + preassigned: [], + present: ['user3@example.com'], + unknown: [] + }; + + callArgs.onSuccess(response); + + expect(addAlertMock).toHaveBeenCalledWith({ + type: 'success', + message: expect.stringContaining('2'), + extraContent: expect.any(Object) + }); + }); + + it('shows error alert for unknown learners', async () => { + renderWithAlertProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + + const callArgs = mutateMock.mock.calls[0][1]; + const response = { + added: [], + changed: [], + preassigned: [], + present: [], + unknown: ['invaliduser', 'anotherbaduser'] + }; + + callArgs.onSuccess(response); + + expect(addAlertMock).toHaveBeenCalledWith({ + type: 'error', + message: expect.stringContaining('2'), + extraContent: expect.any(Array) + }); + }); + + it('shows multiple alerts for mixed response types', async () => { + renderWithAlertProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + + const callArgs = mutateMock.mock.calls[0][1]; + const response = { + added: ['user1@example.com'], + changed: [], + preassigned: ['user2@example.com'], + present: ['user3@example.com'], + unknown: ['baduser'] + }; + + callArgs.onSuccess(response); + + // Should call addAlert 3 times (warning, success, error) + expect(addAlertMock).toHaveBeenCalledTimes(3); + + // Check for warning alert (preassigned) + expect(addAlertMock).toHaveBeenCalledWith({ + type: 'warning', + message: expect.stringContaining('1'), + extraContent: expect.any(Array) + }); + + // Check for success alert (added + changed) + expect(addAlertMock).toHaveBeenCalledWith({ + type: 'success', + message: expect.stringContaining('1'), + extraContent: expect.any(Object) + }); + + // Check for error alert (unknown) + expect(addAlertMock).toHaveBeenCalledWith({ + type: 'error', + message: expect.stringContaining('1'), + extraContent: expect.any(Array) + }); + }); + + it('does not show alerts when response arrays are empty', async () => { + renderWithAlertProvider(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /\+ Add Learners/i })); + + const callArgs = mutateMock.mock.calls[0][1]; + const response = { + added: [], + changed: [], + preassigned: [], + present: [], + unknown: [] + }; + + callArgs.onSuccess(response); + + expect(addAlertMock).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/cohorts/components/ManageLearners.tsx b/src/cohorts/components/ManageLearners.tsx index 644e5182..bd568542 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/providers/AlertProvider'; + +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..ab2e1593 --- /dev/null +++ b/src/cohorts/components/SelectedCohortInfo.test.tsx @@ -0,0 +1,157 @@ +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import SelectedCohortInfo from './SelectedCohortInfo'; +import messages from '../messages'; +import dataDownloadsMessages from '@src/dataDownloads/messages'; +import { renderWithAlertAndIntl } from '@src/testUtils'; +import * as CohortContextModule from '@src/cohorts/components/CohortContext'; +import { CohortProvider } from './CohortContext'; +import { useCohorts, useContentGroupsData } from '../data/apiHook'; +import { assignmentTypes } from '../constants'; + +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: assignmentTypes.manual, + groupId: 2, + userPartitionId: 3, + userCount: 0 + }, + { + id: 2, + name: 'Cohort 2', + assignmentType: assignmentTypes.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 renderWithAlertAndIntl( + + + + ); +} + +describe('SelectedCohortInfo', () => { + const mockSetSelectedCohort = jest.fn(); + const mockClearSelectedCohort = jest.fn(); + const mockUpdateCohortField = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + (useCohorts as jest.Mock).mockReturnValue({ data: mockCohorts }); + (useContentGroupsData as jest.Mock).mockReturnValue({ data: { groups: mockContentGroups, id: 1 } }); + jest.spyOn(CohortContextModule, 'useCohortContext').mockReturnValue({ + selectedCohort: mockCohorts[0], + setSelectedCohort: mockSetSelectedCohort, + clearSelectedCohort: mockClearSelectedCohort, + updateCohortField: mockUpdateCohortField, + }); + }); + + describe('Basic Rendering with Selected Cohort', () => { + it('renders CohortCard when a cohort is selected', () => { + renderWithProviders(); + expect(screen.getByRole('tablist')).toBeInTheDocument(); + expect(screen.getByRole('tab', { name: messages.manageLearners.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'); + }); + + it('renders complete disclaimer paragraph with correct structure', () => { + renderWithProviders(); + const paragraph = screen.getByText(new RegExp(messages.cohortDisclaimer.defaultMessage)).closest('p'); + expect(paragraph).toBeInTheDocument(); + expect(paragraph).toHaveTextContent(messages.cohortDisclaimer.defaultMessage); + expect(paragraph).toHaveTextContent(dataDownloadsMessages.pageTitle.defaultMessage); + expect(paragraph).toHaveTextContent(messages.page.defaultMessage); + }); + }); + + describe('No Selected Cohort', () => { + it('still renders disclaimer when no cohort is selected', () => { + jest.spyOn(CohortContextModule, 'useCohortContext').mockReturnValue({ + selectedCohort: null, + setSelectedCohort: mockSetSelectedCohort, + clearSelectedCohort: mockClearSelectedCohort, + updateCohortField: mockUpdateCohortField, + }); + + renderWithProviders(); + expect(screen.getByText(new RegExp(messages.cohortDisclaimer.defaultMessage))).toBeInTheDocument(); + expect(screen.getByRole('link', { name: dataDownloadsMessages.pageTitle.defaultMessage })).toBeInTheDocument(); + }); + + it('do not render CohortCard component even when no cohort selected', () => { + jest.spyOn(CohortContextModule, 'useCohortContext').mockReturnValue({ + selectedCohort: null, + setSelectedCohort: mockSetSelectedCohort, + clearSelectedCohort: mockClearSelectedCohort, + updateCohortField: mockUpdateCohortField, + }); + + renderWithProviders(); + expect(screen.queryByRole('tablist')).not.toBeInTheDocument(); + }); + }); + + describe('Integration with CohortCard', () => { + it('renders with manual cohort type and content group', async () => { + jest.spyOn(CohortContextModule, 'useCohortContext').mockReturnValue({ + selectedCohort: mockCohorts[0], + setSelectedCohort: mockSetSelectedCohort, + clearSelectedCohort: mockClearSelectedCohort, + updateCohortField: mockUpdateCohortField, + }); + + renderWithProviders(); + expect(screen.getByRole('tablist')).toBeInTheDocument(); + expect(screen.getByText(messages.manualCohortWarning.defaultMessage)).toBeInTheDocument(); + expect(screen.getByText(new RegExp(messages.cohortDisclaimer.defaultMessage))).toBeInTheDocument(); + const settingsTab = screen.getByRole('tab', { name: 'Settings' }); + await userEvent.click(settingsTab); + const selectElement = screen.getByRole('combobox'); + expect(selectElement).toHaveValue('2'); + }); + + it('renders with automatic cohort type and without content group', () => { + jest.spyOn(CohortContextModule, 'useCohortContext').mockReturnValue({ + selectedCohort: mockCohorts[1], + setSelectedCohort: mockSetSelectedCohort, + clearSelectedCohort: mockClearSelectedCohort, + updateCohortField: mockUpdateCohortField, + }); + + renderWithProviders(); + expect(screen.getByRole('tablist')).toBeInTheDocument(); + expect(screen.getByText(messages.automaticCohortWarning.defaultMessage)).toBeInTheDocument(); + }); + }); +}); 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/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 f8457bc0..8bfd2ee7 100644 --- a/src/cohorts/messages.ts +++ b/src/cohorts/messages.ts @@ -186,6 +186,36 @@ 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' + }, + 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;