Skip to content
Open
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
5 changes: 4 additions & 1 deletion plugins/course-apps/progress/Settings.jsx
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change disables this:

Image

Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import { useIntl } from '@edx/frontend-platform/i18n';
import PropTypes from 'prop-types';
import React from 'react';
import React, { useContext } from 'react';
import * as Yup from 'yup';
import { getConfig } from '@edx/frontend-platform';
import FormSwitchGroup from 'CourseAuthoring/generic/FormSwitchGroup';
import { useAppSetting } from 'CourseAuthoring/utils';
import AppSettingsModal from 'CourseAuthoring/pages-and-resources/app-settings-modal/AppSettingsModal';
import { PagesAndResourcesContext } from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider';
import messages from './messages';

const ProgressSettings = ({ onClose }) => {
const intl = useIntl();
const [disableProgressGraph, saveSetting] = useAppSetting('disableProgressGraph');
const showProgressGraphSetting = getConfig().ENABLE_PROGRESS_GRAPH_SETTINGS.toString().toLowerCase() === 'true';
const { isEditable = false } = useContext(PagesAndResourcesContext);

const handleSettingsSave = async (values) => {
if (showProgressGraphSetting) { await saveSetting(!values.enableProgressGraph); }
Expand Down Expand Up @@ -39,6 +41,7 @@ const ProgressSettings = ({ onClose }) => {
onChange={handleChange}
onBlur={handleBlur}
checked={values.enableProgressGraph}
disabled={!isEditable}
/>
)
)}
Expand Down
10 changes: 8 additions & 2 deletions src/CourseAuthoringPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,20 @@ describe('Course authoring page', () => {

axiosMock.onGet(
`${courseAppsApiUrl}/${courseId}`,
).reply(403);
).reply(403, { response: { status: 403 } });
await executeThunk(fetchCourseApps(courseId), store.dispatch);
};
test('renders PermissionDeniedAlert when courseAppsApiStatus is DENIED', async () => {
mockPathname = '/editor/';
await mockStoreDenied();

const wrapper = renderComponent(<CourseAuthoringPage />);
// Test PagesAndResources (which has the PermissionDeniedAlert logic),
// not CourseAuthoringPage which is just the layout wrapper
const wrapper = renderComponent(
<CourseAuthoringPage>
<PagesAndResources />
</CourseAuthoringPage>,
);
expect(await wrapper.findByTestId('permissionDeniedAlert')).toBeInTheDocument();
});
});
8 changes: 1 addition & 7 deletions src/CourseAuthoringPage.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import React, { useEffect } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { useDispatch } from 'react-redux';

import {
useLocation,
} from 'react-router-dom';
import { StudioFooterSlot } from '@edx/frontend-component-footer';
import Header from './header';
import NotFoundAlert from './generic/NotFoundAlert';
import PermissionDeniedAlert from './generic/PermissionDeniedAlert';
import { fetchOnlyStudioHomeData } from './studio-home/data/thunks';
import { getCourseAppsApiStatus } from './pages-and-resources/data/selectors';
import { RequestStatus } from './data/constants';
import Loading from './generic/Loading';
import { useCourseAuthoringContext } from './CourseAuthoringContext';
Expand All @@ -30,16 +28,12 @@ const CourseAuthoringPage = ({ children }: Props) => {
const courseOrg = courseDetails?.org;
const courseTitle = courseDetails?.name;
const inProgress = courseDetailStatus === RequestStatus.IN_PROGRESS || courseDetailStatus === RequestStatus.PENDING;
const courseAppsApiStatus = useSelector(getCourseAppsApiStatus);
const { pathname } = useLocation();
const isEditor = pathname.includes('/editor');

if (courseDetailStatus === RequestStatus.NOT_FOUND && !isEditor) {
return <NotFoundAlert />;
}
if (courseAppsApiStatus === RequestStatus.DENIED) {
return <PermissionDeniedAlert />;
}
return (
<div>
{
Expand Down
21 changes: 21 additions & 0 deletions src/advanced-settings/AdvancedSettings.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,25 @@ describe('<AdvancedSettings />', () => {
render();
expect(await screen.findByTestId('permissionDeniedAlert')).toBeInTheDocument();
});

it('should render settings in read-only mode when user has VIEW but not MANAGE permissions (auditor)', async () => {
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
jest.mocked(useUserPermissions).mockReturnValue({
isLoading: false,
data: { canViewAdvancedSettings: true, canManageAdvancedSettings: false },
} as unknown as ReturnType<typeof useUserPermissions>);
render();
const textarea = await screen.findByLabelText(/Advanced Module List/i);
expect(textarea).toBeDisabled();
});

it('should show permission denied when user has NO permissions (null data)', async () => {
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
jest.mocked(useUserPermissions).mockReturnValue({
isLoading: false,
data: null,
} as unknown as ReturnType<typeof useUserPermissions>);
render();
expect(await screen.findByTestId('permissionDeniedAlert')).toBeInTheDocument();
});
});
22 changes: 21 additions & 1 deletion src/advanced-settings/AdvancedSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ const AdvancedSettings = () => {
action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS,
scope: courseId,
},
canViewAdvancedSettings: {
action: COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS,
scope: courseId,
},
}, isAuthzEnabled);

const {
Expand All @@ -72,6 +76,18 @@ const AdvancedSettings = () => {
} = updateMutation;

const isLoading = isPendingSettingsStatus || (isAuthzEnabled && isLoadingUserPermissions);

// Determine if UI should be read-only (has VIEW but not MANAGE) — auditor
const isReadOnly = isAuthzEnabled
&& !isLoadingUserPermissions
&& !!userPermissions?.canViewAdvancedSettings
&& !userPermissions?.canManageAdvancedSettings;

useEffect(() => {
if (isReadOnly) {
setIsEditableState(false);
}
}, [isReadOnly]);
const updateSettingsButtonState = {
labels: {
default: intl.formatMessage(messages.buttonSaveText),
Expand Down Expand Up @@ -148,15 +164,18 @@ const AdvancedSettings = () => {
showSaveSettingsPrompt(true);
};

// Show permission denied alert when authz is enabled and user doesn't have permission
// Show permission denied alert when authz is enabled and user doesn't have VIEW or MANAGE
const authzIsEnabledAndNoPermission = isAuthzEnabled
&& !isLoadingUserPermissions
&& !userPermissions?.canViewAdvancedSettings
&& !userPermissions?.canManageAdvancedSettings;

if (authzIsEnabledAndNoPermission) {
return <PermissionDeniedAlert />;
}

// Show the page content (read-only or editable)

return (
<>
<Helmet>
Expand Down Expand Up @@ -255,6 +274,7 @@ const AdvancedSettings = () => {
handleBlur={handleSettingBlur}
isEditableState={isEditableState}
setIsEditableState={setIsEditableState}
readOnly={isReadOnly}
/>
);
})}
Expand Down
35 changes: 33 additions & 2 deletions src/advanced-settings/setting-card/SettingCard.test.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fireEvent, render, waitFor } from '@testing-library/react';
import { fireEvent, render, waitFor, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { IntlProvider } from '@edx/frontend-platform/i18n';

Expand All @@ -25,7 +25,7 @@ jest.mock('react-textarea-autosize', () =>
/>
)));

const RootWrapper = () => (
const RootWrapper = (props = {}) => (
<IntlProvider locale="en">
<SettingCard
isOn
Expand All @@ -37,6 +37,7 @@ const RootWrapper = () => (
handleBlur={handleBlur}
isEditableState
saveSettingsPrompt={false}
{...props}
/>
</IntlProvider>
);
Expand Down Expand Up @@ -91,4 +92,34 @@ describe('<SettingCard />', () => {
expect(handleBlur).toHaveBeenCalled();
});
});
it('renders in readOnly mode with disabled input', () => {
render(<RootWrapper readOnly />);
const input = screen.getByLabelText(/Setting Name/i);
expect(input).toBeDisabled();
});

it('renders enabled by default when readOnly is not specified (default false)', () => {
// readOnly defaults to false - input should be enabled
render(<RootWrapper />);
const input = screen.getByLabelText(/Setting Name/i);
expect(input).not.toBeDisabled();
});

it('calls setIsEditableState when value changes and isEditableState is false', async () => {
// Test for line 45: setIsEditableState(true) called when value changes
const { getByLabelText } = render(<RootWrapper isEditableState={false} />);
const input = getByLabelText(/Setting Name/i);
fireEvent.change(input, { target: { value: 'new-different-value' } });
await waitFor(() => {
expect(setIsEditableState).toHaveBeenCalledWith(true);
});
});

it('shows help popup when clicking info button', () => {
render(<RootWrapper />);
const helpButton = screen.getByRole('button', { name: /show help text/i });
fireEvent.click(helpButton);
// The help text should be visible in the popup - verify component renders help
expect(screen.queryByText(/This is a help message/i)).toBeInTheDocument();
});
});
3 changes: 3 additions & 0 deletions src/advanced-settings/setting-card/SettingCard.tsx
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change disables these inputs:

Image

Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const SettingCard = ({
saveSettingsPrompt,
isEditableState,
setIsEditableState,
readOnly = false,
}) => {
const intl = useIntl();
const { deprecated, help, displayName } = settingData;
Expand Down Expand Up @@ -99,6 +100,7 @@ const SettingCard = ({
onChange={handleSettingChange}
aria-label={displayName}
onBlur={handleCardBlur}
disabled={readOnly}
/>
</Form.Group>
</Card.Section>
Expand Down Expand Up @@ -133,6 +135,7 @@ SettingCard.propTypes = {
saveSettingsPrompt: PropTypes.bool.isRequired,
isEditableState: PropTypes.bool.isRequired,
setIsEditableState: PropTypes.func.isRequired,
readOnly: PropTypes.bool,
};

export default SettingCard;
4 changes: 4 additions & 0 deletions src/authz/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ export const CONTENT_LIBRARY_PERMISSIONS = {

export const COURSE_PERMISSIONS = {
MANAGE_ADVANCED_SETTINGS: 'courses.manage_advanced_settings',
VIEW_ADVANCED_SETTINGS: 'courses.view_advanced_settings',

VIEW_GRADING_SETTINGS: 'courses.view_grading_settings',
EDIT_GRADING_SETTINGS: 'courses.edit_grading_settings',

VIEW_PAGES_AND_RESOURCES: 'courses.view_pages_and_resources',
EDIT_PAGES_AND_RESOURCES: 'courses.manage_pages_and_resources',
};
120 changes: 120 additions & 0 deletions src/authz/permissionHelpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { getGradingPermissions, getPagesAndResourcesPermissions } from './permissionHelpers';
import { COURSE_PERMISSIONS } from './constants';

describe('permissionHelpers', () => {
const courseId = 'course-v1:org+course+run';

describe('getGradingPermissions', () => {
it('returns correct permission structure with VIEW action', () => {
const result = getGradingPermissions(courseId);

expect(result.canViewGradingSettings).toBeDefined();
expect(result.canViewGradingSettings.action).toBe(COURSE_PERMISSIONS.VIEW_GRADING_SETTINGS);
expect(result.canViewGradingSettings.scope).toBe(courseId);
});

it('returns correct permission structure with EDIT action', () => {
const result = getGradingPermissions(courseId);

expect(result.canEditGradingSettings).toBeDefined();
expect(result.canEditGradingSettings.action).toBe(COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS);
expect(result.canEditGradingSettings.scope).toBe(courseId);
});

it('returns both VIEW and EDIT permissions in single call', () => {
const result = getGradingPermissions(courseId);

const permissions = Object.keys(result);
expect(permissions).toContain('canViewGradingSettings');
expect(permissions).toContain('canEditGradingSettings');
});

it('uses correct courseId as scope for all permissions', () => {
const customCourseId = 'course-v1:custom+org+custom_run';
const result = getGradingPermissions(customCourseId);

expect(result.canViewGradingSettings.scope).toBe(customCourseId);
expect(result.canEditGradingSettings.scope).toBe(customCourseId);
});
});

describe('getPagesAndResourcesPermissions', () => {
it('returns correct permission structure with VIEW action', () => {
const result = getPagesAndResourcesPermissions(courseId);

expect(result.canViewPagesAndResources).toBeDefined();
expect(result.canViewPagesAndResources.action).toBe(COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES);
expect(result.canViewPagesAndResources.scope).toBe(courseId);
});

it('returns correct permission structure with EDIT action', () => {
const result = getPagesAndResourcesPermissions(courseId);

expect(result.canEditPagesAndResources).toBeDefined();
expect(result.canEditPagesAndResources.action).toBe(COURSE_PERMISSIONS.EDIT_PAGES_AND_RESOURCES);
expect(result.canEditPagesAndResources.scope).toBe(courseId);
});

it('returns both VIEW and EDIT permissions in single call', () => {
const result = getPagesAndResourcesPermissions(courseId);

const permissions = Object.keys(result);
expect(permissions).toContain('canViewPagesAndResources');
expect(permissions).toContain('canEditPagesAndResources');
});

it('uses correct courseId as scope for all permissions', () => {
const customCourseId = 'course-v1:another+test+course';
const result = getPagesAndResourcesPermissions(customCourseId);

expect(result.canViewPagesAndResources.scope).toBe(customCourseId);
expect(result.canEditPagesAndResources.scope).toBe(customCourseId);
});
});

describe('permission constants verification', () => {
it('uses correct VIEW_GRADING_SETTINGS constant', () => {
const result = getGradingPermissions(courseId);
expect(result.canViewGradingSettings.action).toBe('courses.view_grading_settings');
});

it('uses correct EDIT_GRADING_SETTINGS constant', () => {
const result = getGradingPermissions(courseId);
expect(result.canEditGradingSettings.action).toBe('courses.edit_grading_settings');
});

it('uses correct VIEW_PAGES_AND_RESOURCES constant', () => {
const result = getPagesAndResourcesPermissions(courseId);
expect(result.canViewPagesAndResources.action).toBe('courses.view_pages_and_resources');
});

it('uses correct EDIT_PAGES_AND_RESOURCES constant', () => {
const result = getPagesAndResourcesPermissions(courseId);
expect(result.canEditPagesAndResources.action).toBe('courses.manage_pages_and_resources');
});
});

describe('edge cases', () => {
it('handles empty courseId', () => {
const result = getGradingPermissions('');

expect(result.canViewGradingSettings.scope).toBe('');
expect(result.canEditGradingSettings.scope).toBe('');
});

it('handles special characters in courseId', () => {
const specialCourseId = 'course-v1:test+special:id';
const result = getPagesAndResourcesPermissions(specialCourseId);

expect(result.canViewPagesAndResources.scope).toBe(specialCourseId);
});

it('returns consistent structure across multiple calls', () => {
const result1 = getGradingPermissions(courseId);
const result2 = getGradingPermissions(courseId);

expect(Object.keys(result1)).toEqual(Object.keys(result2));
expect(result1.canViewGradingSettings.action).toBe(result2.canViewGradingSettings.action);
});
});
});
11 changes: 11 additions & 0 deletions src/authz/permissionHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,14 @@ export const getGradingPermissions = (courseId: string) => ({
scope: courseId,
},
});

export const getPagesAndResourcesPermissions = (courseId: string) => ({
canViewPagesAndResources: {
action: COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES,
scope: courseId,
},
canEditPagesAndResources: {
action: COURSE_PERMISSIONS.EDIT_PAGES_AND_RESOURCES,
scope: courseId,
},
});
Loading