Skip to content

Commit

Permalink
fix: [AXIMST-597, 670, 673, 672, 671, 596] Group configurations - Bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
ruzniaievdm committed Mar 15, 2024
1 parent d09a25c commit 0960dbc
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 46 deletions.
13 changes: 10 additions & 3 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 }) => (
const renderValidationMessage = ({ text, type }) => (
<span className="d-inline-flex">
<Icon src={MESSAGE_VALIDATION_TYPES.error ? ErrorIcon : WarningIcon} size="sm" className="mr-2" />
<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
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 Down Expand Up @@ -85,7 +90,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 +115,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 +164,7 @@ ExperimentCard.defaultProps = {
usage: [],
version: undefined,
},
isExpandedByDefault: false,
onCreate: null,
experimentConfigurationActions: {},
};
Expand Down Expand Up @@ -194,6 +204,7 @@ ExperimentCard.propTypes = {
}),
scheme: PropTypes.string,
}),
isExpandedByDefault: PropTypes.bool,
onCreate: PropTypes.func,
experimentConfigurationActions: PropTypes.shape({
handleCreate: PropTypes.func,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const ExperimentForm = ({
placeholder={formatMessage(
messages.experimentConfigurationDescriptionPlaceholder,
)}
as="textarea"
/>
<Form.Control.Feedback hasIcon={false} type="default">
{formatMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -24,6 +25,8 @@ const ExperimentConfigurationsSection = ({
experimentConfigurationActions.handleCreate(configuration, hideNewConfiguration);
};

const { elementWithHash } = useScrollToHashElement({ isLoading: true });

return (
<div className="mt-2.5">
<h2 className="lead text-black mb-3 configuration-section-name">
Expand All @@ -36,6 +39,7 @@ const ExperimentConfigurationsSection = ({
key={configuration.id}
configuration={configuration}
experimentConfigurationActions={experimentConfigurationActions}
isExpandedByDefault={configuration.id === +elementWithHash}
onCreate={handleCreateConfiguration}
/>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
33 changes: 17 additions & 16 deletions src/group-configurations/experiment-configurations-section/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
12 changes: 8 additions & 4 deletions src/hooks.js
Original file line number Diff line number Diff line change
@@ -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 }) => {
Expand Down

0 comments on commit 0960dbc

Please sign in to comment.