Skip to content

feat: disallow edits to units in courses that are sourced from a library #1833

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.

Sign up for GitHub

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

Merged
bradenmacdonald merged 8 commits into from
Apr 24, 2025
Merged

feat: disallow edits to units in courses that are sourced from a library #1833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eeec683
feat: disallow edits to units in courses that are sourced from a library
rpenido Apr 18, 2025
11d457b
feat: adds isUnitReadOnly() and uses in CourseUnit
pomegranited Apr 24, 2025
d707402
feat: Disable editing readonly units shown in course outline
pomegranited Apr 24, 2025
daa2821
feat: disable Unit Settings if isUnitReadOnly
pomegranited Apr 24, 2025
b5139e0
revert: Units sourced from libraries can be published
pomegranited Apr 24, 2025
1a7454d
fix: ignore type lint
pomegranited Apr 24, 2025
ca18176
test: Publish button should be enabled for library-sourced units
pomegranited Apr 24, 2025
1ff27c5
fix: use upstreamInfo.upstreamRef instead of upstream
pomegranited Apr 24, 2025
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
30 changes: 21 additions & 9 deletions ...tags-sidebar-controls/TagsSidebarBody.jsx → ...tags-sidebar-controls/TagsSidebarBody.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// @ts-check
import React, { useState, useMemo } from 'react';
import { useState, useMemo } from 'react';
import {
Card, Stack, Button, Collapsible, Icon,
} from '@openedx/paragon';
Expand All @@ -10,10 +9,19 @@ import { ContentTagsDrawerSheet } from '..';

import messages from '../messages';
import { useContentTaxonomyTagsData } from '../data/apiHooks';
import type { ContentTaxonomyTagData, Tag } from '../data/types';
import { LoadingSpinner } from '../../generic/Loading';
import TagsTree from '../TagsTree';

const TagsSidebarBody = () => {
interface TagsSidebarBodyProps {
readOnly: boolean
}

type TagTree = {
[key: string]: { children: TagTree, canChangeObjecttag: boolean, canDeleteObjecttag: boolean }
};

const TagsSidebarBody = ({ readOnly }: TagsSidebarBodyProps) => {
const intl = useIntl();
const [showManageTags, setShowManageTags] = useState(false);
const contentId = useParams().blockId;
Expand All @@ -24,8 +32,8 @@ const TagsSidebarBody = () => {
isSuccess: isContentTaxonomyTagsLoaded,
} = useContentTaxonomyTagsData(contentId || '');

const buildTagsTree = (contentTags) => {
const resultTree = {};
const buildTagsTree = (contentTags: Tag[]) => {
const resultTree: TagTree = {};
contentTags.forEach(item => {
let currentLevel = resultTree;

Expand All @@ -46,7 +54,7 @@ const TagsSidebarBody = () => {
};

const tree = useMemo(() => {
const result = [];
const result: (Omit<ContentTaxonomyTagData, 'tags'> & { tags: TagTree })[] = [];
if (isContentTaxonomyTagsLoaded && contentTaxonomyTagsData) {
contentTaxonomyTagsData.taxonomies.forEach((taxonomy) => {
result.push({
Expand Down Expand Up @@ -88,7 +96,13 @@ const TagsSidebarBody = () => {
</div>
)}

<Button className="mt-3 ml-2" variant="outline-primary" size="sm" onClick={() => setShowManageTags(true)}>
<Button
className="mt-3 ml-2"
variant="outline-primary"
size="sm"
onClick={() => setShowManageTags(true)}
disabled={readOnly}
>
{intl.formatMessage(messages.manageTagsButton)}
</Button>
</Stack>
Expand All @@ -102,6 +116,4 @@ const TagsSidebarBody = () => {
);
};

TagsSidebarBody.propTypes = {};

export default TagsSidebarBody;
8 changes: 6 additions & 2 deletions ...gs-drawer/tags-sidebar-controls/index.jsx → ...gs-drawer/tags-sidebar-controls/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import TagsSidebarHeader from './TagsSidebarHeader';
import TagsSidebarBody from './TagsSidebarBody';

const TagsSidebarControls = () => (
interface TagsSidebarControlsProps {
readOnly: boolean,
}

const TagsSidebarControls = ({ readOnly }: TagsSidebarControlsProps) => (
<>
<TagsSidebarHeader />
<TagsSidebarBody />
<TagsSidebarBody readOnly={readOnly} />
</>
);

Expand Down
4 changes: 4 additions & 0 deletions src/course-outline/card-header/CardHeader.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ const CardHeader = ({
alt={intl.formatMessage(messages.altButtonEdit)}
iconAs={EditIcon}
onClick={onClickEdit}
// @ts-ignore
disabled={isDisabledEditField}
/>
</>
)}
Expand Down Expand Up @@ -178,13 +180,15 @@ const CardHeader = ({
</Dropdown.Item>
<Dropdown.Item
data-testid={`${namePrefix}-card-header__menu-configure-button`}
disabled={isDisabledEditField}
onClick={onClickConfigure}
>
{intl.formatMessage(messages.menuConfigure)}
</Dropdown.Item>
{getConfig().ENABLE_TAGGING_TAXONOMY_PAGES === 'true' && (
<Dropdown.Item
data-testid={`${namePrefix}-card-header__menu-manage-tags-button`}
disabled={isDisabledEditField}
onClick={openManageTagsDrawer}
>
{intl.formatMessage(messages.menuManageTags)}
Expand Down
29 changes: 29 additions & 0 deletions src/course-outline/card-header/CardHeader.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,35 @@ describe('<CardHeader />', () => {
expect(await findByTestId('subsection-edit-field')).toBeDisabled();
});

it('check editing is enabled when isDisabledEditField is false', async () => {
const { getByTestId } = renderComponent({
...cardHeaderProps,
});

expect(getByTestId('subsection-edit-button')).toBeEnabled();

// Ensure menu items related to editing are enabled
const menuButton = getByTestId('subsection-card-header__menu-button');
await act(async () => fireEvent.click(menuButton));
expect(await getByTestId('subsection-card-header__menu-configure-button')).not.toHaveAttribute('aria-disabled');
expect(await getByTestId('subsection-card-header__menu-manage-tags-button')).not.toHaveAttribute('aria-disabled');
});

it('check editing is disabled when isDisabledEditField is true', async () => {
const { getByTestId } = renderComponent({
...cardHeaderProps,
isDisabledEditField: true,
});

expect(await getByTestId('subsection-edit-button')).toBeDisabled();

// Ensure menu items related to editing are disabled
const menuButton = getByTestId('subsection-card-header__menu-button');
await act(async () => fireEvent.click(menuButton));
expect(await getByTestId('subsection-card-header__menu-configure-button')).toHaveAttribute('aria-disabled', 'true');
expect(await getByTestId('subsection-card-header__menu-manage-tags-button')).toHaveAttribute('aria-disabled', 'true');
});

it('calls onClickDelete when item is clicked', async () => {
const { findByText, findByTestId } = renderComponent();

Expand Down
5 changes: 4 additions & 1 deletion src/course-outline/unit-card/UnitCard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useSearchParams } from 'react-router-dom';
import CourseOutlineUnitCardExtraActionsSlot from '../../plugin-slots/CourseOutlineUnitCardExtraActionsSlot';
import { setCurrentItem, setCurrentSection, setCurrentSubsection } from '../data/slice';
import { RequestStatus } from '../../data/constants';
import { isUnitReadOnly } from '../../course-unit/data/utils';
import CardHeader from '../card-header/CardHeader';
import SortableItem from '../drag-helper/SortableItem';
import TitleLink from '../card-header/TitleLink';
Expand Down Expand Up @@ -57,6 +58,8 @@ const UnitCard = ({
discussionEnabled,
} = unit;

const readOnly = isUnitReadOnly(unit);

// re-create actions object for customizations
const actions = { ...unitActions };
// add actions to control display of move up & down menu buton.
Expand Down Expand Up @@ -175,7 +178,7 @@ const UnitCard = ({
isFormOpen={isFormOpen}
closeForm={closeForm}
onEditSubmit={handleEditSubmit}
isDisabledEditField={savingStatus === RequestStatus.IN_PROGRESS}
isDisabledEditField={readOnly || savingStatus === RequestStatus.IN_PROGRESS}
onClickDuplicate={onDuplicateSubmit}
titleComponent={titleComponent}
namePrefix={namePrefix}
Expand Down
22 changes: 14 additions & 8 deletions src/course-unit/CourseUnit.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const CourseUnit = ({ courseId }) => {
const { blockId } = useParams();
const intl = useIntl();
const {
courseUnit,
isLoading,
sequenceId,
unitTitle,
Expand Down Expand Up @@ -75,6 +76,8 @@ const CourseUnit = ({ courseId }) => {
} = useCourseUnit({ courseId, blockId });
const layoutGrid = useLayoutGrid(unitCategory, isUnitLibraryType);

const readOnly = !!courseUnit.readOnly;

useEffect(() => {
document.title = getPageHeadTitle('', unitTitle);
}, [unitTitle]);
Expand Down Expand Up @@ -195,14 +198,16 @@ const CourseUnit = ({ courseId }) => {
courseVerticalChildren={courseVerticalChildren.children}
handleConfigureSubmit={handleConfigureSubmit}
/>
<AddComponent
parentLocator={blockId}
isSplitTestType={isSplitTestType}
isUnitVerticalType={isUnitVerticalType}
handleCreateNewCourseXBlock={handleCreateNewCourseXBlock}
addComponentTemplateData={addComponentTemplateData}
/>
{showPasteXBlock && canPasteComponent && isUnitVerticalType && (
{!readOnly && (
<AddComponent
parentLocator={blockId}
isSplitTestType={isSplitTestType}
isUnitVerticalType={isUnitVerticalType}
handleCreateNewCourseXBlock={handleCreateNewCourseXBlock}
addComponentTemplateData={addComponentTemplateData}
/>
)}
{!readOnly && showPasteXBlock && canPasteComponent && isUnitVerticalType && (
<PasteComponent
clipboardData={sharedClipboardData}
onClick={
Expand All @@ -227,6 +232,7 @@ const CourseUnit = ({ courseId }) => {
blockId={blockId}
unitTitle={unitTitle}
xBlocks={courseVerticalChildren.children}
readOnly={readOnly}
/>
)}
{isSplitTestType && (
Expand Down
47 changes: 47 additions & 0 deletions src/course-unit/CourseUnit.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2195,4 +2195,51 @@ describe('<CourseUnit />', () => {
.toHaveBeenCalledWith(`/course/${courseId}/editor/html/${targetBlockId}`, { replace: true });
});
});

it('renders units from libraries with some components read-only', async () => {
setConfig({
...getConfig(),
ENABLE_TAGGING_TAXONOMY_PAGES: 'true',
});
render(<RootWrapper />);

axiosMock
.onGet(getCourseUnitApiUrl(courseId))
.reply(200, {
...courseUnitIndexMock,
upstreamInfo: {
upstreamRef: 'lct:org:lib:unit:unit-1',
},
});
await executeThunk(fetchCourseUnitQuery(courseId), store.dispatch);

// Disable the "Edit" button
const unitHeaderTitle = screen.getByTestId('unit-header-title');
const editButton = within(unitHeaderTitle).getByRole(
'button',
{ name: 'Edit' },
);
expect(editButton).toBeInTheDocument();
expect(editButton).toBeDisabled();

// The "Publish" button should still be enabled
const courseUnitSidebar = screen.getByTestId('course-unit-sidebar');
const publishButton = within(courseUnitSidebar).getByRole(
'button',
{ name: sidebarMessages.actionButtonPublishTitle.defaultMessage },
);
expect(publishButton).toBeInTheDocument();
expect(publishButton).toBeEnabled();

// Disable the "Manage Tags" button
const manageTagsButton = screen.getByRole(
'button',
{ name: tagsDrawerMessages.manageTagsButton.defaultMessage },
);
expect(manageTagsButton).toBeInTheDocument();
expect(manageTagsButton).toBeDisabled();

// Does not render the "Add Components" section
expect(screen.queryByText(addComponentMessages.title.defaultMessage)).not.toBeInTheDocument();
});
});
4 changes: 0 additions & 4 deletions src/course-unit/add-component/AddComponent.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,6 @@ const AddComponent = ({
return null;
};

AddComponent.defaultProps = {
addComponentTemplateData: {},
};

AddComponent.propTypes = {
isSplitTestType: PropTypes.bool.isRequired,
isUnitVerticalType: PropTypes.bool.isRequired,
Expand Down
4 changes: 3 additions & 1 deletion src/course-unit/data/thunk.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,16 @@ import {
updateCourseOutlineInfoLoadingStatus,
updateMovedXBlockParams,
} from './slice';
import { getNotificationMessage } from './utils';
import { getNotificationMessage, isUnitReadOnly } from './utils';

export function fetchCourseUnitQuery(courseId) {
return async (dispatch) => {
dispatch(updateLoadingCourseUnitStatus({ status: RequestStatus.IN_PROGRESS }));

try {
const courseUnit = await getCourseUnitData(courseId);
courseUnit.readOnly = isUnitReadOnly(courseUnit);

dispatch(fetchCourseItemSuccess(courseUnit));
dispatch(updateLoadingCourseUnitStatus({ status: RequestStatus.SUCCESSFUL }));
return true;
Expand Down
12 changes: 12 additions & 0 deletions src/course-unit/data/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,15 @@ export const updateXBlockBlockIdToId = (data) => {

return updatedData;
};

/**
* Returns whether the given Unit should be read-only.
*
* Units sourced from libraries are read-only (temporary, for Teak).
*
* @param {object} unit - uses the 'upstreamInfo' object if found.
* @returns {boolean} True if readOnly, False if editable.
*/
export const isUnitReadOnly = ({ upstreamInfo }) => (
upstreamInfo && upstreamInfo.upstreamRef && upstreamInfo.upstreamRef.startsWith('lct:')
);
8 changes: 6 additions & 2 deletions src/course-unit/header-title/HeaderTitle.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const HeaderTitle = ({
COURSE_BLOCK_NAMES.component.id,
].includes(currentItemData.category);

const readOnly = !!currentItemData.readOnly;

const onConfigureSubmit = (...arg) => {
handleConfigureSubmit(currentItemData.id, ...arg, closeConfigureModal);
};
Expand Down Expand Up @@ -80,12 +82,14 @@ const HeaderTitle = ({
className="ml-1 flex-shrink-0"
iconAs={EditIcon}
onClick={handleTitleEdit}
disabled={readOnly}
/>
<IconButton
alt={intl.formatMessage(messages.altButtonSettings)}
className="flex-shrink-0"
iconAs={SettingsIcon}
onClick={openConfigureModal}
disabled={readOnly}
/>
<ConfigureModal
isOpen={isConfigureModalOpen}
Expand All @@ -102,12 +106,12 @@ const HeaderTitle = ({
);
};

export default HeaderTitle;

HeaderTitle.propTypes = {
unitTitle: PropTypes.string.isRequired,
isTitleEditFormOpen: PropTypes.bool.isRequired,
handleTitleEdit: PropTypes.func.isRequired,
handleTitleEditSubmit: PropTypes.func.isRequired,
handleConfigureSubmit: PropTypes.func.isRequired,
};

export default HeaderTitle;
23 changes: 21 additions & 2 deletions src/course-unit/header-title/HeaderTitle.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,27 @@ describe('<HeaderTitle />', () => {

expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toBeInTheDocument();
expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toHaveValue(unitTitle);
expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeInTheDocument();
expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeInTheDocument();
expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeEnabled();
expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeEnabled();
});

it('Units sourced from upstream show a disabled edit form and config menu', async () => {
// Override mock unit with one sourced from an upstream library
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
axiosMock
.onGet(getCourseUnitApiUrl(blockId))
.reply(200, {
...courseUnitIndexMock,
upstreamInfo: {
upstreamRef: 'lct:org:lib:unit:unit-1',
},
});
await executeThunk(fetchCourseUnitQuery(blockId), store.dispatch);

const { getByRole } = renderComponent();

expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeDisabled();
expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeDisabled();
});

it('calls toggle edit title form by clicking on Edit button', () => {
Expand Down
Loading