Skip to content
Open
Show file tree
Hide file tree
Changes from all 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';
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();
});
});
32 changes: 18 additions & 14 deletions src/advanced-settings/AdvancedSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ import {
import { CheckCircle, Info, Warning } from '@openedx/paragon/icons';
import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
import { useWaffleFlags } from '@src/data/apiHooks';
import { useUserPermissions } from '@src/authz/data/apiHooks';
import { COURSE_PERMISSIONS } from '@src/authz/constants';
import PermissionDeniedAlert from 'CourseAuthoring/generic/PermissionDeniedAlert';
import AlertProctoringError from '@src/generic/AlertProctoringError';
import { LoadingSpinner } from '@src/generic/Loading';
Expand All @@ -30,6 +27,8 @@ import validateAdvancedSettingsData from './utils';
import messages from './messages';
import ModalError from './modal-error/ModalError';
import { useCourseAdvancedSettings, useProctoringExamErrors, useUpdateCourseAdvancedSettings } from './data/apiHooks';
import { useCourseUserPermissions } from '@src/authz/hooks';
import { getAdvancedSettingsPermissions } from '@src/authz/permissionHelpers';

const AdvancedSettings = () => {
const intl = useIntl();
Expand All @@ -44,14 +43,12 @@ const AdvancedSettings = () => {

const { courseId, courseDetails } = useCourseAuthoringContext();

const waffleFlags = useWaffleFlags(courseId);
const isAuthzEnabled = waffleFlags.enableAuthzCourseAuthoring;
const { isLoading: isLoadingUserPermissions, data: userPermissions } = useUserPermissions({
canManageAdvancedSettings: {
action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS,
scope: courseId,
},
}, isAuthzEnabled);
const {
isLoading: isLoadingUserPermissions,
isAuthzEnabled,
canViewAdvancedSettings,
canManageAdvancedSettings,
} = useCourseUserPermissions(courseId, getAdvancedSettingsPermissions(courseId));

const {
data: advancedSettingsData = {},
Expand All @@ -72,6 +69,11 @@ const AdvancedSettings = () => {
} = updateMutation;

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

const isEditable = !isAuthzEnabled
|| isLoadingUserPermissions
|| canManageAdvancedSettings;

const updateSettingsButtonState = {
labels: {
default: intl.formatMessage(messages.buttonSaveText),
Expand Down Expand Up @@ -148,10 +150,11 @@ 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?.canManageAdvancedSettings;
&& !canViewAdvancedSettings
&& !canManageAdvancedSettings;

if (authzIsEnabledAndNoPermission) {
return <PermissionDeniedAlert />;
Expand Down Expand Up @@ -213,7 +216,7 @@ const AdvancedSettings = () => {
/>
<article>
<div>
<section className="setting-items-policies">
<section className="setting-items-policies" aria-disabled={!isEditable || undefined}>
<div className="small">
<FormattedMessage
id="course-authoring.advanced-settings.policies.description"
Expand Down Expand Up @@ -255,6 +258,7 @@ const AdvancedSettings = () => {
handleBlur={handleSettingBlur}
isEditableState={isEditableState}
setIsEditableState={setIsEditableState}
isEditable={isEditable}
/>
);
})}
Expand Down
34 changes: 32 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,33 @@ describe('<SettingCard />', () => {
expect(handleBlur).toHaveBeenCalled();
});
});
it('renders in read-only mode with disabled input', () => {
render(<RootWrapper isEditable={false} />);
const input = screen.getByLabelText(/Setting Name/i);
expect(input).toBeDisabled();
});

it('renders enabled by default when isEditable is not specified (default true)', () => {
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,
isEditable = true,
}) => {
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={!isEditable}
/>
</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,
isEditable: 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',
MANAGE_PAGES_AND_RESOURCES: 'courses.manage_pages_and_resources',
};
51 changes: 51 additions & 0 deletions src/authz/permissionHelpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import {
getGradingPermissions,
getPagesAndResourcesPermissions,
getAdvancedSettingsPermissions,
} from './permissionHelpers';
import { COURSE_PERMISSIONS } from './constants';

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

describe('getGradingPermissions', () => {
it('returns VIEW and EDIT permissions with the correct actions and scope', () => {
const result = getGradingPermissions(courseId);

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

describe('getPagesAndResourcesPermissions', () => {
it('returns VIEW and MANAGE permissions with the correct actions and scope', () => {
const result = getPagesAndResourcesPermissions(courseId);

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

describe('getAdvancedSettingsPermissions', () => {
it('returns VIEW and MANAGE permissions with the correct actions and scope', () => {
const result = getAdvancedSettingsPermissions(courseId);

expect(result.canViewAdvancedSettings.action).toBe(COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS);
expect(result.canViewAdvancedSettings.scope).toBe(courseId);
expect(result.canManageAdvancedSettings.action).toBe(COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS);
expect(result.canManageAdvancedSettings.scope).toBe(courseId);
});

it('uses the provided courseId as scope', () => {
const otherId = 'course-v1:another+test+run';
const result = getAdvancedSettingsPermissions(otherId);

expect(result.canViewAdvancedSettings.scope).toBe(otherId);
expect(result.canManageAdvancedSettings.scope).toBe(otherId);
});
});
});
22 changes: 22 additions & 0 deletions src/authz/permissionHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,25 @@ export const getGradingPermissions = (courseId: string) => ({
scope: courseId,
},
});

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

export const getAdvancedSettingsPermissions = (courseId: string) => ({
canViewAdvancedSettings: {
action: COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS,
scope: courseId,
},
canManageAdvancedSettings: {
action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS,
scope: courseId,
},
});
Loading