Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/cohorts/CohortsPage.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.assignment-tooltip .tooltip-inner {
background-color: var(--pgn-color-primary-700);
max-width: none;
}
6 changes: 5 additions & 1 deletion src/cohorts/CohortsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ 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';

const CohortsPageContent = () => {
const intl = useIntl();
Expand Down Expand Up @@ -66,7 +68,9 @@ const CohortsPageContent = () => {
const CohortsPage = () => {
return (
<CohortProvider>
<CohortsPageContent />
<AlertProvider>
<CohortsPageContent />
</AlertProvider>
</CohortProvider>
);
};
Expand Down
24 changes: 15 additions & 9 deletions src/cohorts/components/CohortCard.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
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 { 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',
Expand All @@ -26,19 +27,24 @@ const CohortCard = () => {
const { mutate: editCohort } = usePatchCohort(courseId);
const formRef = useRef<{ resetForm: () => void }>(null);
const [showSuccessMessage, setShowSuccessMessage] = useState<boolean>(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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once we abstract toast provider that Jesse created we should handle it here 👍

console.error(error);
}
}
);
};
Expand All @@ -56,7 +62,7 @@ const CohortCard = () => {
<p className="ml-3 text-primary-700 mb-0">{intl.formatMessage(messages.studentsOnCohort, { users: selectedCohort?.userCount ?? 0 })}</p>
</div>
<p className="x-small mb-0 mt-2">
<FormattedMessage {...warningMessage[selectedCohort.assignmentType]} /> <a href={getExternalLinkUrl(assignmentLink[selectedCohort.assignmentType])}>{intl.formatMessage(messages.warningCohortLink)}</a>
<FormattedMessage {...warningMessage[selectedCohort.assignmentType]} /> <Hyperlink showLaunchIcon={false} target="_blank" destination={getExternalLinkUrl(assignmentLink[selectedCohort.assignmentType])}>{intl.formatMessage(messages.warningCohortLink)}</Hyperlink>
</p>
</div>
<Tabs id="cohort-management-tabs" className="mx-0" onSelect={() => {}}>
Expand Down
6 changes: 3 additions & 3 deletions src/cohorts/components/CohortContext.test.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -101,7 +101,7 @@ describe('CohortContext', () => {
const RenderCountComponent = () => {
renderCount++;
const { setSelectedCohort } = useCohortContext();
React.useEffect(() => {
useEffect(() => {
setSelectedCohort({
id: 1,
name: 'Cohort 1',
Expand Down
53 changes: 28 additions & 25 deletions src/cohorts/components/CohortsForm.test.tsx
Original file line number Diff line number Diff line change
@@ -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(),
}));

Expand All @@ -27,7 +28,9 @@ describe('CohortsForm', () => {
const renderComponent = () =>
renderWithIntl(
<CohortProvider>
<CohortsForm onCancel={onCancel} onSubmit={onSubmit} />
<AlertProvider>
<CohortsForm onCancel={onCancel} onSubmit={onSubmit} />
</AlertProvider>
</CohortProvider>
);

Expand All @@ -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 });
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();

Expand All @@ -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(
<CohortProvider>
<CohortsForm onCancel={onCancel} onSubmit={onSubmit} disableManualAssignment />
Expand All @@ -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: {
Expand Down Expand Up @@ -174,6 +177,6 @@ describe('CohortsForm', () => {
expect(selectGroupRadio).toBeChecked();

const groupSelect = screen.getByRole('combobox');
expect(groupSelect).toHaveValue('2:3');
expect(groupSelect).toHaveValue('2');
});
});
54 changes: 32 additions & 22 deletions src/cohorts/components/CohortsForm.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -22,26 +22,25 @@ export interface CohortsFormRef {
const CohortsForm = forwardRef<CohortsFormRef, CohortsFormProps>(({ 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<string>(initialContentGroup);
const [selectedContentGroup, setSelectedContentGroup] = useState<number | null>(initialContentGroup);
const [selectedContentGroupOption, setSelectedContentGroupOption] = useState<string>(initialContentGroupOption);
const [selectedAssignmentType, setSelectedAssignmentType] = useState<string>(initialAssignmentType);
const [name, setName] = useState<string>(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]);

Expand All @@ -54,18 +53,15 @@ const CohortsForm = forwardRef<CohortsFormRef, CohortsFormProps>(({ 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<HTMLFormElement>) => {
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,
});
};

Expand All @@ -79,7 +75,21 @@ const CohortsForm = forwardRef<CohortsFormRef, CohortsFormProps>(({ disableManua
<FormLabel className="text-primary-500">{intl.formatMessage(messages.cohortAssignmentMethod)}</FormLabel>
<FormRadioSet name="assignmentType" value={selectedAssignmentType} onChange={(e) => setSelectedAssignmentType(e.target.value)}>
<Form.Radio className="mb-2" value={assignmentTypes.automatic}>{intl.formatMessage(messages.automatic)}</Form.Radio>
<Form.Radio disabled={disableManualAssignment} value={assignmentTypes.manual}>{intl.formatMessage(messages.manual)}</Form.Radio>
{disableManualAssignment ? (
<OverlayTrigger
placement="top"
overlay={(
<Tooltip id="manual-assignment-tooltip" className="assignment-tooltip">
{intl.formatMessage(messages.manualAssignmentDisabledTooltip)}
</Tooltip>
)}
>
<span>
<Form.Radio disabled value={assignmentTypes.manual}>{intl.formatMessage(messages.manual)}</Form.Radio>
</span>
</OverlayTrigger>
)
: <Form.Radio value={assignmentTypes.manual}>{intl.formatMessage(messages.manual)}</Form.Radio>}
</FormRadioSet>
</FormGroup>
<FormGroup className="mb-3.5">
Expand All @@ -91,22 +101,22 @@ const CohortsForm = forwardRef<CohortsFormRef, CohortsFormProps>(({ disableManua
>
<Form.Radio value="noContentGroup">{intl.formatMessage(messages.noContentGroup)}</Form.Radio>
<div className="d-flex align-items-center">
<Form.Radio value="selectContentGroup" disabled={data.length === 0}>{intl.formatMessage(messages.selectAContentGroup)}</Form.Radio>
{ data.length > 0
<Form.Radio value="selectContentGroup" disabled={data.allGroupConfigurations[0].groups.length === 0}>{intl.formatMessage(messages.selectAContentGroup)}</Form.Radio>
{ data.allGroupConfigurations[0].groups.length > 0
? (
<FormControl
as="select"
className="ml-2"
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) => (
<option key={contentGroup.id} value={contentGroup.id}>
{contentGroup.displayName}
{contentGroup.name}
</option>
))
}
Expand Down
Loading