Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: altering bulk enrollment action behaviour #1081

Merged
merged 6 commits into from
Nov 6, 2023
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('<DeleteHighlightSet />', () => {
expect(sendEnterpriseTrackEvent).toHaveBeenCalledTimes(1);
});

it('cancelling confirmation modal closes modal', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just trying to make sure we're using only the US spelling throughout the code

Copy link
Member

Choose a reason for hiding this comment

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

I never knew the double "l" was more used outside of the US! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See I have to catch myself because apparently I use british spelling for a lot of stuff accidentally? Like grey (which is gray in the US?)

it('canceling confirmation modal closes modal', () => {
renderWithRouter(
<DeleteHighlightSetWrapper />,
{ route: initialRouterEntry },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,62 +1,56 @@
import React from 'react';
import PropTypes from 'prop-types';
import {
Icon,
IconButton,
OverlayTrigger,
Stack,
Tooltip,
Icon, IconButton, OverlayTrigger, Stack, Tooltip,
} from '@edx/paragon';
import { Mail, DoNotDisturbOn } from '@edx/paragon/icons';

const AssignmentRowActionTableCell = ({ row }) => {
const cancelButtonMarginLeft = row.original.state === 'allocated' ? 'ml-2.5' : 'ml-auto';
const isLearnerStateWaiting = row.original.learnerState === 'waiting';
const emailAltText = row.original.learnerEmail ? `for ${row.original.learnerEmail}` : '';
return (
<div className="d-flex">
{row.original.state === 'allocated' && (
<>
<OverlayTrigger
key="Remind"
placement="top"
overlay={<Tooltip variant="light" id="tooltip-remind">Remind learner</Tooltip>}
>
<IconButton
className="ml-auto mr-0"
src={Mail}
iconAs={Icon}
alt="Remind learner"
// eslint-disable-next-line no-console
onClick={() => console.log(`Reminding ${row.original.uuid}`)}
data-testid={`remind-assignment-${row.original.uuid}`}
/>
</OverlayTrigger>
<Stack direction="horizontal" gap={1} />
</>
<Stack direction="horizontal" gap="3" className="justify-content-end">
{isLearnerStateWaiting && (
<OverlayTrigger
key="Remind"
placement="top"
overlay={<Tooltip id={`tooltip-remind-${row.original.uuid}`}>Remind learner</Tooltip>}
>
<IconButton
src={Mail}
iconAs={Icon}
alt={`Send reminder ${emailAltText}`}
// eslint-disable-next-line no-console
onClick={() => console.log(`Reminding ${row.original.uuid}`)}

Check warning on line 24 in src/components/learner-credit-management/AssignmentRowActionTableCell.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/learner-credit-management/AssignmentRowActionTableCell.jsx#L24

Added line #L24 was not covered by tests
data-testid={`remind-assignment-${row.original.uuid}`}
/>
</OverlayTrigger>
)}
<OverlayTrigger
key="Cancel"
placement="top"
overlay={<Tooltip variant="light" id="tooltip-cancel">Cancel assignment</Tooltip>}
overlay={<Tooltip id={`tooltip-cancel-${row.original.uuid}`}>Cancel assignment</Tooltip>}
>
<IconButton
className={`${cancelButtonMarginLeft} mr-1 text-danger-500`}
variant="danger"
src={DoNotDisturbOn}
iconAs={Icon}
alt="Cancel assignment"
alt={`Cancel assignment ${emailAltText}`}
// eslint-disable-next-line no-console
onClick={() => console.log(`Canceling ${row.original.uuid}`)}
data-testid={`cancel-assignment-${row.original.uuid}`}
/>
</OverlayTrigger>
</div>
</Stack>
);
};

AssignmentRowActionTableCell.propTypes = {
row: PropTypes.shape({
original: PropTypes.shape({
learnerEmail: PropTypes.string,
learnerState: PropTypes.string,
uuid: PropTypes.string.isRequired,
state: PropTypes.string.isRequired,
}).isRequired,
}).isRequired,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@
import { Mail } from '@edx/paragon/icons';

const AssignmentTableRemindAction = ({ selectedFlatRows, ...rest }) => {
const hideRemindAction = selectedFlatRows.some(
row => row.original.state !== 'allocated',
);
if (hideRemindAction) {
return null;
}
const selectedRemindableRows = selectedFlatRows.filter(row => row.original.learnerState === 'waiting').length;
return (
// eslint-disable-next-line no-console
<Button iconBefore={Mail} onClick={() => console.log('Remind', selectedFlatRows, rest)}>
{`Remind (${selectedFlatRows.length})`}
<Button
disabled={selectedRemindableRows === 0}
iconBefore={Mail}
// eslint-disable-next-line no-console
onClick={() => console.log('Remind', selectedFlatRows, rest)}

Check warning on line 13 in src/components/learner-credit-management/AssignmentTableRemind.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/learner-credit-management/AssignmentTableRemind.jsx#L13

Added line #L13 was not covered by tests
>
{`Remind (${selectedRemindableRows})`}
</Button>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,16 @@ describe('<BudgetDetailPage />', () => {
expect(screen.getByText('loading budget activity overview')).toBeInTheDocument();
});

it('displays remind row and bulk actions when allocated', async () => {
it.each([
{
learnerState: 'waiting',
shouldDisplayRemindAction: true,
},
{
learnerState: 'notifying',
shouldDisplayRemindAction: false,
},
])('displays remind and cancel row and bulk actions when appropriate (%s)', async ({ learnerState, shouldDisplayRemindAction }) => {
useParams.mockReturnValue({
budgetId: mockSubsidyAccessPolicyUUID,
activeTabKey: 'activity',
Expand Down Expand Up @@ -786,7 +795,7 @@ describe('<BudgetDetailPage />', () => {
uuid: 'test-uuid',
contentKey: mockCourseKey,
contentQuantity: -19900,
learnerState: 'active',
learnerState,
recentAction: { actionType: 'assigned', timestamp: '2023-10-27' },
actions: [],
errorReason: null,
Expand All @@ -800,73 +809,22 @@ describe('<BudgetDetailPage />', () => {
});
renderWithRouter(<BudgetDetailPageWrapper />);
const cancelRowAction = screen.getByTestId('cancel-assignment-test-uuid');
const remindRowAction = screen.getByTestId('remind-assignment-test-uuid');
expect(cancelRowAction).toBeInTheDocument();
expect(remindRowAction).toBeInTheDocument();
if (shouldDisplayRemindAction) {
const remindRowAction = screen.getByTestId('remind-assignment-test-uuid');
expect(remindRowAction).toBeInTheDocument();
}

const checkBox = screen.getByTestId('datatable-select-column-checkbox-cell');
expect(checkBox).toBeInTheDocument();
userEvent.click(checkBox);
await waitFor(() => {
expect(screen.getByText('Remind (1)')).toBeInTheDocument();
});
await waitFor(() => {
expect(screen.getByText('Cancel (1)')).toBeInTheDocument();
});
});

it('hides remind row and bulk actions when allocated', () => {
useOfferRedemptions.mockReturnValue({
isLoading: false,
offerRedemptions: mockEmptyOfferRedemptions,
fetchOfferRedemptions: jest.fn(),
});
useBudgetDetailActivityOverview.mockReturnValue({
isLoading: false,
data: {
contentAssignments: { count: 1 },
spentTransactions: { count: 0 },
},
});
useParams.mockReturnValue({
budgetId: mockSubsidyAccessPolicyUUID,
activeTabKey: 'activity',
});
useSubsidyAccessPolicy.mockReturnValue({
isInitialLoading: false,
data: mockAssignableSubsidyAccessPolicy,
});
useBudgetDetailActivityOverview.mockReturnValue({
isLoading: false,
data: {
contentAssignments: { count: 1 },
spentTransactions: { count: 0 },
},
});
useBudgetContentAssignments.mockReturnValue({
isLoading: false,
contentAssignments: {
count: 1,
results: [
{
uuid: 'test-uuid',
contentKey: mockCourseKey,
contentQuantity: -19900,
learnerState: 'accepted',
recentAction: { actionType: 'assigned', timestamp: '2023-10-27' },
actions: [],
state: 'accepted',
},
],
numPages: 1,
currentPage: 1,
},
fetchContentAssignments: jest.fn(),
});
renderWithRouter(<BudgetDetailPageWrapper />);
expect(screen.queryByTestId('remind-assignment-test-uuid')).not.toBeInTheDocument();
const checkBox = screen.getByTestId('datatable-select-column-checkbox-cell');
userEvent.click(checkBox);
expect(screen.queryByText('Remind (1)')).not.toBeInTheDocument();
expect(await screen.findByText('Cancel (1)')).toBeInTheDocument();
if (shouldDisplayRemindAction) {
expect(await screen.findByText('Remind (1)')).toBeInTheDocument();
} else {
const remindButton = await screen.findByText('Remind (0)');
expect(remindButton).toBeInTheDocument();
expect(remindButton).toBeDisabled();
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ const SSOConfigConfigureStep = ({
/>
<Form.Text>
Configurable value that represents the amount of time in seconds, no greater than 30, that the
edX system will wait for a response before cancelling the request.
edX system will wait for a response before canceling the request.
</Form.Text>
{!odataApiTimeoutIntervalValid && (
<Form.Control.Feedback type="invalid">
Expand Down
2 changes: 1 addition & 1 deletion src/data/services/EnterpriseAccessApiService.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class EnterpriseAccessApiService {
page: 1,
page_size: 25,
// Only include assignments with allocated or errored states. The table should NOT
// include assignments in the cancelled or accepted states.
// include assignments in the canceled or accepted states.
state__in: 'allocated,errored',
...snakeCaseObject(options),
});
Expand Down