Skip to content

Commit

Permalink
fix: group configurations - resolve discussions
Browse files Browse the repository at this point in the history
fix: [AXIMST-714] icon is aligned with text (#210)
  • Loading branch information
ruzniaievdm committed Apr 2, 2024
1 parent d6750aa commit 2652b60
Show file tree
Hide file tree
Showing 19 changed files with 367 additions and 56 deletions.
24 changes: 24 additions & 0 deletions src/generic/prompt-if-dirty/PromptIfDirty.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { useEffect } from 'react';
import PropTypes from 'prop-types';

const PromptIfDirty = ({ dirty }) => {
useEffect(() => {
// eslint-disable-next-line consistent-return
const handleBeforeUnload = (event) => {
if (dirty) {
event.preventDefault();
}
};
window.addEventListener('beforeunload', handleBeforeUnload);

return () => {
window.removeEventListener('beforeunload', handleBeforeUnload);
};
}, [dirty]);

return null;
};
PromptIfDirty.propTypes = {
dirty: PropTypes.bool.isRequired,
};
export default PromptIfDirty;
72 changes: 72 additions & 0 deletions src/generic/prompt-if-dirty/PromptIfDirty.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import React from 'react';
import { render, unmountComponentAtNode } from 'react-dom';
import { act } from 'react-dom/test-utils';
import PromptIfDirty from './PromptIfDirty';

describe('PromptIfDirty', () => {
let container = null;
let mockEvent = null;

beforeEach(() => {
container = document.createElement('div');
document.body.appendChild(container);
mockEvent = new Event('beforeunload');
jest.spyOn(window, 'addEventListener');
jest.spyOn(window, 'removeEventListener');
jest.spyOn(mockEvent, 'preventDefault');
Object.defineProperty(mockEvent, 'returnValue', { writable: true });
mockEvent.returnValue = '';
});

afterEach(() => {
window.addEventListener.mockRestore();
window.removeEventListener.mockRestore();
mockEvent.preventDefault.mockRestore();
mockEvent = null;
unmountComponentAtNode(container);
container.remove();
container = null;
});

it('should add event listener on mount', () => {
act(() => {
render(<PromptIfDirty dirty />, container);
});

expect(window.addEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function));
});

it('should remove event listener on unmount', () => {
act(() => {
render(<PromptIfDirty dirty />, container);
});
act(() => {
unmountComponentAtNode(container);
});

expect(window.removeEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function));
});

it('should call preventDefault and set returnValue when dirty is true', () => {
act(() => {
render(<PromptIfDirty dirty />, container);
});
act(() => {
window.dispatchEvent(mockEvent);
});

expect(mockEvent.preventDefault).toHaveBeenCalled();
expect(mockEvent.returnValue).toBe('');
});

it('should not call preventDefault when dirty is false', () => {
act(() => {
render(<PromptIfDirty dirty={false} />, container);
});
act(() => {
window.dispatchEvent(mockEvent);
});

expect(mockEvent.preventDefault).not.toHaveBeenCalled();
});
});
15 changes: 15 additions & 0 deletions src/group-configurations/GroupConfigurations.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { AppProvider } from '@edx/frontend-platform/react';
import { initializeMockApp } from '@edx/frontend-platform';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';

import { RequestStatus } from '../data/constants';
import initializeStore from '../store';
import { executeThunk } from '../utils';
import { getContentStoreApiUrl } from './data/api';
Expand Down Expand Up @@ -88,4 +89,18 @@ describe('<GroupConfigurations />', () => {
queryByTestId('group-configurations-empty-placeholder'),
).not.toBeInTheDocument();
});

it('updates loading status if request fails', async () => {
axiosMock
.onGet(getContentStoreApiUrl(courseId))
.reply(404, groupConfigurationResponseMock);

renderComponent();

await executeThunk(fetchGroupConfigurationsQuery(courseId), store.dispatch);

expect(store.getState().groupConfigurations.loadingStatus).toBe(
RequestStatus.FAILED,
);
});
});
2 changes: 1 addition & 1 deletion src/group-configurations/__mocks__/contentGroupsMock.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = {
name: 'My Content Group 2',
usage: [
{
label: 'Unit / Drag and Drop',
label: 'Unit / Blank Problem',
url: '/container/block-v1:org+101+101+type@vertical+block@3d6d82348e2743b6ac36ac4af354de0e',
},
{
Expand Down
15 changes: 11 additions & 4 deletions src/group-configurations/common/UsageList.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import PropTypes from 'prop-types';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Hyperlink, Stack, Icon } from '@openedx/paragon';
import { Warning as WarningIcon, Error as ErrorIcon } from '@openedx/paragon/icons';
import {
Warning as WarningIcon,
Error as ErrorIcon,
} from '@openedx/paragon/icons';

import { MESSAGE_VALIDATION_TYPES } from '../constants';
import { formatUrlToUnitPage } from '../utils';
Expand All @@ -13,9 +16,13 @@ const UsageList = ({ className, itemList, isExperiment }) => {
? messages.experimentAccessTo
: messages.accessTo;

const renderValidationMessage = ({ text }) => (
<span className="d-inline-flex">
<Icon src={MESSAGE_VALIDATION_TYPES.error ? ErrorIcon : WarningIcon} size="sm" className="mr-2" />
const renderValidationMessage = ({ text, type }) => (
<span className="d-inline-flex align-items-center">
<Icon
src={MESSAGE_VALIDATION_TYPES.error === type ? ErrorIcon : WarningIcon}
size="sm"
className="mr-2"
/>
<span className="small text-gray-700">{text}</span>
</span>
);
Expand Down
34 changes: 34 additions & 0 deletions src/group-configurations/common/UsageList.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { render } from '@testing-library/react';
import { IntlProvider } from '@edx/frontend-platform/i18n';

import { contentGroupsMock } from '../__mocks__';
import { formatUrlToUnitPage } from '../utils';
import UsageList from './UsageList';
import messages from './messages';

const usages = contentGroupsMock.groups[1]?.usage;

const renderComponent = (props = {}) => render(
<IntlProvider locale="en">
<UsageList itemList={usages} {...props} />
</IntlProvider>,
);

describe('<UsageList />', () => {
it('renders component correctly', () => {
const { getByText, getAllByRole } = renderComponent();
expect(getByText(messages.accessTo.defaultMessage)).toBeInTheDocument();
expect(getAllByRole('link')).toHaveLength(2);
getAllByRole('link').forEach((el, idx) => {
expect(el.href).toMatch(formatUrlToUnitPage(usages[idx].url));
expect(getByText(usages[idx].label)).toBeVisible();
});
});

it('renders experiment component correctly', () => {
const { getByText } = renderComponent({ isExperiment: true });
expect(
getByText(messages.experimentAccessTo.defaultMessage),
).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {

import { WarningFilled as WarningFilledIcon } from '@openedx/paragon/icons';

import PromptIfDirty from '../../generic/PromptIfDirty';
import PromptIfDirty from '../../generic/prompt-if-dirty/PromptIfDirty';
import { isAlreadyExistsGroup } from './utils';
import messages from './messages';

Expand Down
97 changes: 97 additions & 0 deletions src/group-configurations/data/api.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import MockAdapter from 'axios-mock-adapter';
import { camelCaseObject, initializeMockApp } from '@edx/frontend-platform';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';

import { groupConfigurationResponseMock } from '../__mocks__';
import { initialContentGroupObject } from '../content-groups-section/utils';
import {
getContentStoreApiUrl,
getLegacyApiUrl,
getGroupConfigurations,
createContentGroup,
editContentGroup,
} from './api';

let axiosMock;
const courseId = 'course-v1:org+101+101';
const contentGroups = groupConfigurationResponseMock.allGroupConfigurations[1];

describe('group configurations API calls', () => {
beforeEach(() => {
initializeMockApp({
authenticatedUser: {
userId: 3,
username: 'abc123',
administrator: true,
roles: [],
},
});
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
});

afterEach(() => {
jest.clearAllMocks();
});

it('should fetch group configurations', async () => {
axiosMock
.onGet(getContentStoreApiUrl(courseId))
.reply(200, groupConfigurationResponseMock);

const result = await getGroupConfigurations(courseId);
const expected = camelCaseObject(groupConfigurationResponseMock);

expect(axiosMock.history.get[0].url).toEqual(
getContentStoreApiUrl(courseId),
);
expect(result).toEqual(expected);
});

it('should create content group', async () => {
const newContentGroupName = 'content-group-test';
const response = { ...groupConfigurationResponseMock };
const updatedContentGroups = {
...contentGroups,
groups: [
...contentGroups.groups,
initialContentGroupObject(newContentGroupName),
],
};

response.allGroupConfigurations[1] = updatedContentGroups;
axiosMock
.onPost(getLegacyApiUrl(courseId, contentGroups.id), updatedContentGroups)
.reply(200, response);

const result = await createContentGroup(courseId, updatedContentGroups);
const expected = camelCaseObject(response);

expect(axiosMock.history.post[0].url).toEqual(
getLegacyApiUrl(courseId, updatedContentGroups.id),
);
expect(result).toEqual(expected);
});

it('should edit content group', async () => {
const editedName = 'content-group-edited';
const groupId = contentGroups.groups[0].id;
const response = { ...groupConfigurationResponseMock };
const editedContentGroups = {
...contentGroups,
groups: contentGroups.groups.map((group) => (group.id === groupId ? { ...group, name: editedName } : group)),
};

response.allGroupConfigurations[1] = editedContentGroups;
axiosMock
.onPost(getLegacyApiUrl(courseId, contentGroups.id), editedContentGroups)
.reply(200, response);

const result = await editContentGroup(courseId, editedContentGroups);
const expected = camelCaseObject(response);

expect(axiosMock.history.post[0].url).toEqual(
getLegacyApiUrl(courseId, editedContentGroups.id),
);
expect(result).toEqual(expected);
});
});
4 changes: 2 additions & 2 deletions src/group-configurations/empty-placeholder/index.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import PropTypes from 'prop-types';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Add as IconAdd } from '@edx/paragon/icons';
import { Button } from '@edx/paragon';
import { Add as IconAdd } from '@openedx/paragon/icons';
import { Button } from '@openedx/paragon';

import messages from './messages';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState } from 'react';
import { useState, useEffect } from 'react';
import PropTypes from 'prop-types';
import { useParams } from 'react-router-dom';
import { getConfig } from '@edx/frontend-platform';
Expand Down Expand Up @@ -26,6 +26,7 @@ import { initialExperimentConfiguration } from './constants';
const ExperimentCard = ({
configuration,
experimentConfigurationActions,
isExpandedByDefault,
onCreate,
}) => {
const { formatMessage } = useIntl();
Expand All @@ -34,6 +35,10 @@ const ExperimentCard = ({
const [isEditMode, switchOnEditMode, switchOffEditMode] = useToggle(false);
const [isOpenDeleteModal, openDeleteModal, closeDeleteModal] = useToggle(false);

useEffect(() => {
setIsExpanded(isExpandedByDefault);
}, [isExpandedByDefault]);

const {
id, groups: groupsControl, description, usage,
} = configuration;
Expand All @@ -59,8 +64,14 @@ const ExperimentCard = ({
</span>
);

// We need to store actual idx as an additional field for getNextGroupName utility.
const configurationGroupsWithIndexField = {
...configuration,
groups: configuration.groups.map((group, idx) => ({ ...group, idx })),
};

const formValues = isEditMode
? configuration
? configurationGroupsWithIndexField

Check warning on line 74 in src/group-configurations/experiment-configurations-section/ExperimentCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/group-configurations/experiment-configurations-section/ExperimentCard.jsx#L74

Added line #L74 was not covered by tests
: initialExperimentConfiguration;

const handleDeleteConfiguration = () => {
Expand All @@ -85,7 +96,11 @@ const ExperimentCard = ({
onEditClick={handleEditConfiguration}
/>
) : (
<div className="configuration-card" data-testid="configuration-card">
<div
className="configuration-card"
data-testid="configuration-card"
id={id}
>
<div className="configuration-card-header">
<TitleButton
group={configuration}
Expand All @@ -106,7 +121,7 @@ const ExperimentCard = ({
className="configuration-card-header__delete-tooltip"
tooltipContent={formatMessage(
isUsedInLocation
? messages.deleteRestriction
? messages.experimentConfigurationDeleteRestriction
: messages.actionDelete,
)}
alt={formatMessage(messages.actionDelete)}
Expand Down Expand Up @@ -155,6 +170,7 @@ ExperimentCard.defaultProps = {
usage: [],
version: undefined,
},
isExpandedByDefault: false,
onCreate: null,
experimentConfigurationActions: {},
};
Expand Down Expand Up @@ -194,6 +210,7 @@ ExperimentCard.propTypes = {
}),
scheme: PropTypes.string,
}),
isExpandedByDefault: PropTypes.bool,
onCreate: PropTypes.func,
experimentConfigurationActions: PropTypes.shape({
handleCreate: PropTypes.func,
Expand Down
Loading

0 comments on commit 2652b60

Please sign in to comment.