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 2 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,45 +1,40 @@
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 learnerStateBool = row.original.learnerState === 'waiting';
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
const cancelButtonMarginLeft = learnerStateBool ? 'ml-2.5' : 'ml-auto';
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
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"
<Stack direction="horizontal" gap="1">
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
{learnerStateBool && (
<OverlayTrigger
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
key="Remind"
placement="top"
overlay={<Tooltip variant="light" id="tooltip-remind">Remind learner</Tooltip>}
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
>
<IconButton
className="ml-auto mr-0"
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
src={Mail}
iconAs={Icon}
alt="Remind learner"
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
// 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} />
</>
onClick={() => console.log(`Reminding ${row.original.uuid}`)}
data-testid={`remind-assignment-${row.original.uuid}`}
/>
</OverlayTrigger>
)}
<OverlayTrigger
key="Cancel"
placement="top"
overlay={<Tooltip variant="light" id="tooltip-cancel">Cancel assignment</Tooltip>}
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
>
<IconButton
className={`${cancelButtonMarginLeft} mr-1 text-danger-500`}
className={`${cancelButtonMarginLeft} mr-1`}
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
variant="danger"
src={DoNotDisturbOn}
iconAs={Icon}
alt="Cancel assignment"
Expand All @@ -48,15 +43,15 @@ const AssignmentRowActionTableCell = ({ row }) => {
data-testid={`cancel-assignment-${row.original.uuid}`}
/>
</OverlayTrigger>
</div>
</Stack>
);
};

AssignmentRowActionTableCell.propTypes = {
row: PropTypes.shape({
original: PropTypes.shape({
learnerState: PropTypes.string.isRequired,
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 { Button } from '@edx/paragon';
import { Mail } from '@edx/paragon/icons';

const AssignmentTableRemindAction = ({ selectedFlatRows, ...rest }) => {
const hideRemindAction = selectedFlatRows.some(
row => row.original.state !== 'allocated',
);
if (hideRemindAction) {
return null;
}
const selectedEnrollableRows = selectedFlatRows.filter(row => row.original.learnerState === 'waiting').length;
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
return (
// eslint-disable-next-line no-console
<Button iconBefore={Mail} onClick={() => console.log('Remind', selectedFlatRows, rest)}>
{`Remind (${selectedFlatRows.length})`}
<Button
disabled={selectedEnrollableRows === 0}
iconBefore={Mail}
// eslint-disable-next-line no-console
onClick={() => console.log('Remind', selectedFlatRows, rest)}
>
{`Remind (${selectedEnrollableRows})`}
</Button>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ describe('<BudgetDetailPage />', () => {
expect(screen.getByText('loading budget activity overview')).toBeInTheDocument();
});

it('displays remind row and bulk actions when allocated', async () => {
it('displays remind and cancel row and bulk actions when allocated', async () => {
useParams.mockReturnValue({
budgetId: mockSubsidyAccessPolicyUUID,
activeTabKey: 'activity',
Expand All @@ -756,7 +756,7 @@ describe('<BudgetDetailPage />', () => {
useBudgetDetailActivityOverview.mockReturnValue({
isLoading: false,
data: {
contentAssignments: { count: 1 },
contentAssignments: { count: 2 },
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
spentTransactions: { count: 0 },
},
});
Expand All @@ -769,7 +769,7 @@ describe('<BudgetDetailPage />', () => {
uuid: 'test-uuid',
contentKey: mockCourseKey,
contentQuantity: -19900,
learnerState: 'active',
learnerState: 'waiting',
recentAction: { actionType: 'assigned', timestamp: '2023-10-27' },
actions: [],
errorReason: null,
Expand All @@ -790,12 +790,58 @@ describe('<BudgetDetailPage />', () => {
const checkBox = screen.getByTestId('datatable-select-column-checkbox-cell');
expect(checkBox).toBeInTheDocument();
userEvent.click(checkBox);
await waitFor(() => {
expect(screen.getByText('Remind (1)')).toBeInTheDocument();
expect(await screen.findByText('Remind (1)')).toBeInTheDocument();
expect(await screen.findByText('Cancel (1)')).toBeInTheDocument();
});

it('only allows cancel and not remind in certain states', async () => {
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
useParams.mockReturnValue({
budgetId: mockSubsidyAccessPolicyUUID,
activeTabKey: 'activity',
});
await waitFor(() => {
expect(screen.getByText('Cancel (1)')).toBeInTheDocument();
useOfferRedemptions.mockReturnValue({
isLoading: false,
offerRedemptions: mockEmptyOfferRedemptions,
fetchOfferRedemptions: jest.fn(),
});
useSubsidyAccessPolicy.mockReturnValue({
isInitialLoading: false,
data: mockAssignableSubsidyAccessPolicy,
});
useBudgetDetailActivityOverview.mockReturnValue({
isLoading: false,
data: {
contentAssignments: { count: 2 },
spentTransactions: { count: 0 },
},
});
useBudgetContentAssignments.mockReturnValue({
isLoading: false,
contentAssignments: {
count: 1,
results: [
{
uuid: 'test-uuid',
contentKey: mockCourseKey,
contentQuantity: -19900,
learnerState: 'notifying',
recentAction: { actionType: 'reminded', timestamp: '2023-10-27' },
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
actions: [],
state: 'allocated',
},
],
numPages: 1,
currentPage: 1,
},
fetchContentAssignments: jest.fn(),
});
renderWithRouter(<BudgetDetailPageWrapper />);
const checkBox = screen.getByTestId('datatable-select-column-checkbox-cell');
expect(checkBox).toBeInTheDocument();
userEvent.click(checkBox);
expect(await screen.findByText('Remind (0)')).toBeInTheDocument();
expect(await screen.findByText('Remind (0)')).toBeDisabled();
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
expect(await screen.findByText('Cancel (1)')).toBeInTheDocument();
});

it('hides remind row and bulk actions when allocated', () => {
Expand Down
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