diff --git a/src/generic/prompt-if-dirty/PromptIfDirty.jsx b/src/generic/prompt-if-dirty/PromptIfDirty.jsx new file mode 100644 index 0000000000..a686ea2e87 --- /dev/null +++ b/src/generic/prompt-if-dirty/PromptIfDirty.jsx @@ -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; diff --git a/src/generic/prompt-if-dirty/PromptIfDirty.test.jsx b/src/generic/prompt-if-dirty/PromptIfDirty.test.jsx new file mode 100644 index 0000000000..b429a7e137 --- /dev/null +++ b/src/generic/prompt-if-dirty/PromptIfDirty.test.jsx @@ -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(, container); + }); + + expect(window.addEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function)); + }); + + it('should remove event listener on unmount', () => { + act(() => { + render(, 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(, container); + }); + act(() => { + window.dispatchEvent(mockEvent); + }); + + expect(mockEvent.preventDefault).toHaveBeenCalled(); + expect(mockEvent.returnValue).toBe(''); + }); + + it('should not call preventDefault when dirty is false', () => { + act(() => { + render(, container); + }); + act(() => { + window.dispatchEvent(mockEvent); + }); + + expect(mockEvent.preventDefault).not.toHaveBeenCalled(); + }); +}); diff --git a/src/group-configurations/GroupConfigurations.test.jsx b/src/group-configurations/GroupConfigurations.test.jsx index aca98646fc..34486c368b 100644 --- a/src/group-configurations/GroupConfigurations.test.jsx +++ b/src/group-configurations/GroupConfigurations.test.jsx @@ -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'; @@ -88,4 +89,18 @@ describe('', () => { 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, + ); + }); }); diff --git a/src/group-configurations/__mocks__/contentGroupsMock.js b/src/group-configurations/__mocks__/contentGroupsMock.js index 8e97cdef27..3f2ea7be21 100644 --- a/src/group-configurations/__mocks__/contentGroupsMock.js +++ b/src/group-configurations/__mocks__/contentGroupsMock.js @@ -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', }, { diff --git a/src/group-configurations/common/UsageList.jsx b/src/group-configurations/common/UsageList.jsx index b55ad10756..5c6287a9d6 100644 --- a/src/group-configurations/common/UsageList.jsx +++ b/src/group-configurations/common/UsageList.jsx @@ -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'; @@ -13,9 +16,13 @@ const UsageList = ({ className, itemList, isExperiment }) => { ? messages.experimentAccessTo : messages.accessTo; - const renderValidationMessage = ({ text }) => ( - - + const renderValidationMessage = ({ text, type }) => ( + + {text} ); diff --git a/src/group-configurations/common/UsageList.test.jsx b/src/group-configurations/common/UsageList.test.jsx new file mode 100644 index 0000000000..e4d4681279 --- /dev/null +++ b/src/group-configurations/common/UsageList.test.jsx @@ -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( + + + , +); + +describe('', () => { + 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(); + }); +}); diff --git a/src/group-configurations/content-groups-section/ContentGroupForm.jsx b/src/group-configurations/content-groups-section/ContentGroupForm.jsx index 0144ab290d..5c4b9bf5b0 100644 --- a/src/group-configurations/content-groups-section/ContentGroupForm.jsx +++ b/src/group-configurations/content-groups-section/ContentGroupForm.jsx @@ -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'; diff --git a/src/group-configurations/data/api.test.js b/src/group-configurations/data/api.test.js new file mode 100644 index 0000000000..c39e52ec2a --- /dev/null +++ b/src/group-configurations/data/api.test.js @@ -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); + }); +}); diff --git a/src/group-configurations/empty-placeholder/index.jsx b/src/group-configurations/empty-placeholder/index.jsx index c4c4625576..80b780d1d5 100644 --- a/src/group-configurations/empty-placeholder/index.jsx +++ b/src/group-configurations/empty-placeholder/index.jsx @@ -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'; diff --git a/src/group-configurations/experiment-configurations-section/ExperimentCard.jsx b/src/group-configurations/experiment-configurations-section/ExperimentCard.jsx index 7e3d0af9a8..52a0817576 100644 --- a/src/group-configurations/experiment-configurations-section/ExperimentCard.jsx +++ b/src/group-configurations/experiment-configurations-section/ExperimentCard.jsx @@ -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'; @@ -26,6 +26,7 @@ import { initialExperimentConfiguration } from './constants'; const ExperimentCard = ({ configuration, experimentConfigurationActions, + isExpandedByDefault, onCreate, }) => { const { formatMessage } = useIntl(); @@ -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; @@ -59,8 +64,14 @@ const ExperimentCard = ({ ); + // 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 : initialExperimentConfiguration; const handleDeleteConfiguration = () => { @@ -85,7 +96,11 @@ const ExperimentCard = ({ onEditClick={handleEditConfiguration} /> ) : ( -
+
{formatMessage( diff --git a/src/group-configurations/experiment-configurations-section/constants.js b/src/group-configurations/experiment-configurations-section/constants.js index 3e04eb1f17..70ed39bc88 100644 --- a/src/group-configurations/experiment-configurations-section/constants.js +++ b/src/group-configurations/experiment-configurations-section/constants.js @@ -3,8 +3,12 @@ export const initialExperimentConfiguration = { name: '', description: '', groups: [ - { name: 'Group A', version: 1, usage: [] }, - { name: 'Group B', version: 1, usage: [] }, + { + name: 'Group A', version: 1, usage: [], idx: 0, + }, + { + name: 'Group B', version: 1, usage: [], idx: 1, + }, ], scheme: 'random', parameters: {}, diff --git a/src/group-configurations/experiment-configurations-section/index.jsx b/src/group-configurations/experiment-configurations-section/index.jsx index 18afb29231..a5ed9f6365 100644 --- a/src/group-configurations/experiment-configurations-section/index.jsx +++ b/src/group-configurations/experiment-configurations-section/index.jsx @@ -3,6 +3,7 @@ import { Button, useToggle } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Add as AddIcon } from '@openedx/paragon/icons'; +import { useScrollToHashElement } from '../../hooks'; import EmptyPlaceholder from '../empty-placeholder'; import ExperimentForm from './ExperimentForm'; import ExperimentCard from './ExperimentCard'; @@ -24,6 +25,8 @@ const ExperimentConfigurationsSection = ({ experimentConfigurationActions.handleCreate(configuration, hideNewConfiguration); }; + const { elementWithHash } = useScrollToHashElement({ isLoading: true }); + return (

@@ -36,6 +39,7 @@ const ExperimentConfigurationsSection = ({ key={configuration.id} configuration={configuration} experimentConfigurationActions={experimentConfigurationActions} + isExpandedByDefault={configuration.id === +elementWithHash} onCreate={handleCreateConfiguration} /> ))} diff --git a/src/group-configurations/experiment-configurations-section/messages.js b/src/group-configurations/experiment-configurations-section/messages.js index 9718c8fdbe..01a120fae0 100644 --- a/src/group-configurations/experiment-configurations-section/messages.js +++ b/src/group-configurations/experiment-configurations-section/messages.js @@ -108,7 +108,7 @@ const messages = defineMessages({ }, subtitleModalDelete: { id: 'course-authoring.group-configurations.experiment-card.delete-modal.subtitle', - defaultMessage: 'content group', + defaultMessage: 'group configurations', }, deleteRestriction: { id: 'course-authoring.group-configurations.experiment-card.delete-restriction', diff --git a/src/group-configurations/experiment-configurations-section/utils.js b/src/group-configurations/experiment-configurations-section/utils.js index be6db84ca6..18d070ecf6 100644 --- a/src/group-configurations/experiment-configurations-section/utils.js +++ b/src/group-configurations/experiment-configurations-section/utils.js @@ -12,27 +12,28 @@ const getNextGroupName = (groups, groupFieldName = 'name') => { const existingGroupNames = groups.map((group) => group.name); const lettersCount = ALPHABET_LETTERS.length; - let nextIndex = existingGroupNames.length + 1; + // Calculate the maximum index of existing groups + const maxIdx = groups.reduce((max, group) => Math.max(max, group.idx), -1); - let groupName = ''; - while (nextIndex > 0) { - groupName = ALPHABET_LETTERS[(nextIndex - 1) % lettersCount] + groupName; - nextIndex = Math.floor((nextIndex - 1) / lettersCount); - } + // Calculate the next index for the new group + const nextIndex = maxIdx + 1; + let groupName = ''; let counter = 0; - let newName = groupName; - while (existingGroupNames.includes(`Group ${newName}`)) { - counter++; - let newIndex = existingGroupNames.length + 1 + counter; + + do { + let tempIndex = nextIndex + counter; groupName = ''; - while (newIndex > 0) { - groupName = ALPHABET_LETTERS[(newIndex - 1) % lettersCount] + groupName; - newIndex = Math.floor((newIndex - 1) / lettersCount); + while (tempIndex >= 0) { + groupName = ALPHABET_LETTERS[tempIndex % lettersCount] + groupName; + tempIndex = Math.floor(tempIndex / lettersCount) - 1; } - newName = groupName; - } - return { [groupFieldName]: `Group ${newName}`, version: 1, usage: [] }; + counter++; + } while (existingGroupNames.includes(`Group ${groupName}`)); + + return { + [groupFieldName]: `Group ${groupName}`, version: 1, usage: [], idx: nextIndex, + }; }; /** diff --git a/src/group-configurations/experiment-configurations-section/utils.test.js b/src/group-configurations/experiment-configurations-section/utils.test.js index aa0cc4ac82..4e0e5f9272 100644 --- a/src/group-configurations/experiment-configurations-section/utils.test.js +++ b/src/group-configurations/experiment-configurations-section/utils.test.js @@ -9,110 +9,141 @@ describe('utils module', () => { it('return correct next group name test-case-1', () => { const groups = [ { - name: 'Group A', + name: 'Group A', idx: 0, }, { - name: 'Group B', + name: 'Group B', idx: 1, }, ]; const nextGroup = getNextGroupName(groups); expect(nextGroup.name).toBe('Group C'); + expect(nextGroup.idx).toBe(2); }); it('return correct next group name test-case-2', () => { const groups = []; const nextGroup = getNextGroupName(groups); expect(nextGroup.name).toBe('Group A'); + expect(nextGroup.idx).toBe(0); }); it('return correct next group name test-case-3', () => { const groups = [ { - name: 'Some group', + name: 'Some group', idx: 0, + }, + { + name: 'Group B', idx: 1, }, ]; const nextGroup = getNextGroupName(groups); - expect(nextGroup.name).toBe('Group B'); + expect(nextGroup.name).toBe('Group C'); + expect(nextGroup.idx).toBe(2); }); it('return correct next group name test-case-4', () => { const groups = [ { - name: 'Group A', + name: 'Group A', idx: 0, }, { - name: 'Group A', + name: 'Group A', idx: 1, }, ]; const nextGroup = getNextGroupName(groups); expect(nextGroup.name).toBe('Group C'); + expect(nextGroup.idx).toBe(2); }); it('return correct next group name test-case-5', () => { const groups = [ { - name: 'Group A', + name: 'Group A', idx: 0, }, { - name: 'Group C', + name: 'Group C', idx: 1, }, { - name: 'Group B', + name: 'Group B', idx: 2, }, ]; const nextGroup = getNextGroupName(groups); expect(nextGroup.name).toBe('Group D'); + expect(nextGroup.idx).toBe(3); }); it('return correct next group name test-case-6', () => { const groups = [ { - name: '', + name: '', idx: 0, }, { - name: '', + name: '', idx: 1, }, ]; const nextGroup = getNextGroupName(groups); expect(nextGroup.name).toBe('Group C'); + expect(nextGroup.idx).toBe(2); }); it('return correct next group name test-case-7', () => { const groups = [ { - name: 'Group A', + name: 'Group A', idx: 0, }, { - name: 'Group C', + name: 'Group C', idx: 1, }, ]; const nextGroup = getNextGroupName(groups); expect(nextGroup.name).toBe('Group D'); + expect(nextGroup.idx).toBe(2); }); it('return correct next group name test-case-8', () => { const groups = [ { - name: 'Group D', + name: 'Group D', idx: 0, }, ]; const nextGroup = getNextGroupName(groups); expect(nextGroup.name).toBe('Group B'); + expect(nextGroup.idx).toBe(1); }); it('return correct next group name test-case-9', () => { + const groups = [ + { + name: 'Group E', idx: 4, + }, + ]; + const nextGroup = getNextGroupName(groups); + expect(nextGroup.name).toBe('Group F'); + }); + + it('return correct next group name test-case-10', () => { + const groups = [ + { + name: 'Group E', idx: 0, + }, + ]; + const nextGroup = getNextGroupName(groups); + expect(nextGroup.name).toBe('Group B'); + }); + + it('return correct next group name test-case-11', () => { const simulatedGroupWithAlphabetLength = Array.from( { length: 26 }, - () => ({ name: 'Test name' }), + (_, idx) => ({ name: 'Test name', idx }), ); const nextGroup = getNextGroupName(simulatedGroupWithAlphabetLength); expect(nextGroup.name).toBe('Group AA'); }); - it('return correct next group name test-case-10', () => { + it('return correct next group name test-case-12', () => { const simulatedGroupWithAlphabetLength = Array.from( { length: 702 }, - () => ({ name: 'Test name' }), + (_, idx) => ({ name: 'Test name', idx }), ); const nextGroup = getNextGroupName(simulatedGroupWithAlphabetLength); expect(nextGroup.name).toBe('Group AAA'); diff --git a/src/group-configurations/group-configuration-container/UsageList.jsx b/src/group-configurations/group-configuration-container/UsageList.jsx index f4b88e9407..ba9b577519 100644 --- a/src/group-configurations/group-configuration-container/UsageList.jsx +++ b/src/group-configurations/group-configuration-container/UsageList.jsx @@ -2,8 +2,8 @@ import PropTypes from 'prop-types'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Hyperlink, Stack } from '@openedx/paragon'; -import { formatUrlToUnitPage } from './utils'; -import messages from './messages'; +import { formatUrlToUnitPage } from '../utils'; +import messages from '../messages'; const UsageList = ({ className, itemList, isExperiment }) => { const { formatMessage } = useIntl(); diff --git a/src/group-configurations/index.jsx b/src/group-configurations/index.jsx index ef1f97f04f..9c59251312 100644 --- a/src/group-configurations/index.jsx +++ b/src/group-configurations/index.jsx @@ -2,7 +2,7 @@ import PropTypes from 'prop-types'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Container, Layout, Stack, Row, -} from '@edx/paragon'; +} from '@openedx/paragon'; import { RequestStatus } from '../data/constants'; import { LoadingSpinner } from '../generic/Loading'; diff --git a/src/hooks.js b/src/hooks.js index 8c649ea06b..5967d9ace6 100644 --- a/src/hooks.js +++ b/src/hooks.js @@ -1,20 +1,24 @@ -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; import { history } from '@edx/frontend-platform'; // eslint-disable-next-line import/prefer-default-export export const useScrollToHashElement = ({ isLoading }) => { + const [elementWithHash, setElementWithHash] = useState(null); + useEffect(() => { - const currentHash = window.location.hash; + const currentHash = window.location.hash.substring(1); if (currentHash) { - const element = document.querySelector(currentHash); - + const element = document.getElementById(currentHash); if (element) { element.scrollIntoView(); history.replace({ hash: '' }); } + setElementWithHash(currentHash); } }, [isLoading]); + + return { elementWithHash }; }; export const useEscapeClick = ({ onEscape, dependency }) => {