Skip to content

Commit 39f0298

Browse files
bra-i-ambra-i-am
committed
feat(pages-and-resources): implement advanced settings permissions and refactor related components
1 parent 699af15 commit 39f0298

15 files changed

Lines changed: 82 additions & 175 deletions

‎src/advanced-settings/AdvancedSettings.tsx‎

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@ import {
1010
import { CheckCircle, Info, Warning } from '@openedx/paragon/icons';
1111
import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
1212
import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
13-
import { useWaffleFlags } from '@src/data/apiHooks';
14-
import { useUserPermissions } from '@src/authz/data/apiHooks';
15-
import { COURSE_PERMISSIONS } from '@src/authz/constants';
1613
import PermissionDeniedAlert from 'CourseAuthoring/generic/PermissionDeniedAlert';
1714
import AlertProctoringError from '@src/generic/AlertProctoringError';
1815
import { LoadingSpinner } from '@src/generic/Loading';
@@ -30,6 +27,8 @@ import validateAdvancedSettingsData from './utils';
3027
import messages from './messages';
3128
import ModalError from './modal-error/ModalError';
3229
import { useCourseAdvancedSettings, useProctoringExamErrors, useUpdateCourseAdvancedSettings } from './data/apiHooks';
30+
import { useCourseUserPermissions } from '@src/authz/hooks';
31+
import { getAdvancedSettingsPermissions } from '@src/authz/permissionHelpers';
3332

3433
const AdvancedSettings = () => {
3534
const intl = useIntl();
@@ -44,18 +43,12 @@ const AdvancedSettings = () => {
4443

4544
const { courseId, courseDetails } = useCourseAuthoringContext();
4645

47-
const waffleFlags = useWaffleFlags(courseId);
48-
const isAuthzEnabled = waffleFlags.enableAuthzCourseAuthoring;
49-
const { isLoading: isLoadingUserPermissions, data: userPermissions } = useUserPermissions({
50-
canManageAdvancedSettings: {
51-
action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS,
52-
scope: courseId,
53-
},
54-
canViewAdvancedSettings: {
55-
action: COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS,
56-
scope: courseId,
57-
},
58-
}, isAuthzEnabled);
46+
const {
47+
isLoading: isLoadingUserPermissions,
48+
isAuthzEnabled,
49+
canViewAdvancedSettings,
50+
canManageAdvancedSettings,
51+
} = useCourseUserPermissions(courseId, getAdvancedSettingsPermissions(courseId));
5952

6053
const {
6154
data: advancedSettingsData = {},
@@ -77,17 +70,12 @@ const AdvancedSettings = () => {
7770

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

80-
// Determine if UI should be read-only (has VIEW but not MANAGE) — auditor
73+
// Determine if UI should be read-only (has VIEW but not MANAGE)
8174
const isReadOnly = isAuthzEnabled
8275
&& !isLoadingUserPermissions
83-
&& !!userPermissions?.canViewAdvancedSettings
84-
&& !userPermissions?.canManageAdvancedSettings;
76+
&& !!canViewAdvancedSettings
77+
&& !canManageAdvancedSettings;
8578

86-
useEffect(() => {
87-
if (isReadOnly) {
88-
setIsEditableState(false);
89-
}
90-
}, [isReadOnly]);
9179
const updateSettingsButtonState = {
9280
labels: {
9381
default: intl.formatMessage(messages.buttonSaveText),
@@ -167,15 +155,13 @@ const AdvancedSettings = () => {
167155
// Show permission denied alert when authz is enabled and user doesn't have VIEW or MANAGE
168156
const authzIsEnabledAndNoPermission = isAuthzEnabled
169157
&& !isLoadingUserPermissions
170-
&& !userPermissions?.canViewAdvancedSettings
171-
&& !userPermissions?.canManageAdvancedSettings;
158+
&& !canViewAdvancedSettings
159+
&& !canManageAdvancedSettings;
172160

173161
if (authzIsEnabledAndNoPermission) {
174162
return <PermissionDeniedAlert />;
175163
}
176164

177-
// Show the page content (read-only or editable)
178-
179165
return (
180166
<>
181167
<Helmet>

‎src/authz/permissionHelpers.test.ts‎

Lines changed: 19 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1,120 +1,51 @@
1-
import { getGradingPermissions, getPagesAndResourcesPermissions } from './permissionHelpers';
1+
import {
2+
getGradingPermissions,
3+
getPagesAndResourcesPermissions,
4+
getAdvancedSettingsPermissions,
5+
} from './permissionHelpers';
26
import { COURSE_PERMISSIONS } from './constants';
37

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

711
describe('getGradingPermissions', () => {
8-
it('returns correct permission structure with VIEW action', () => {
12+
it('returns VIEW and EDIT permissions with the correct actions and scope', () => {
913
const result = getGradingPermissions(courseId);
1014

11-
expect(result.canViewGradingSettings).toBeDefined();
1215
expect(result.canViewGradingSettings.action).toBe(COURSE_PERMISSIONS.VIEW_GRADING_SETTINGS);
1316
expect(result.canViewGradingSettings.scope).toBe(courseId);
14-
});
15-
16-
it('returns correct permission structure with EDIT action', () => {
17-
const result = getGradingPermissions(courseId);
18-
19-
expect(result.canEditGradingSettings).toBeDefined();
2017
expect(result.canEditGradingSettings.action).toBe(COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS);
2118
expect(result.canEditGradingSettings.scope).toBe(courseId);
2219
});
23-
24-
it('returns both VIEW and EDIT permissions in single call', () => {
25-
const result = getGradingPermissions(courseId);
26-
27-
const permissions = Object.keys(result);
28-
expect(permissions).toContain('canViewGradingSettings');
29-
expect(permissions).toContain('canEditGradingSettings');
30-
});
31-
32-
it('uses correct courseId as scope for all permissions', () => {
33-
const customCourseId = 'course-v1:custom+org+custom_run';
34-
const result = getGradingPermissions(customCourseId);
35-
36-
expect(result.canViewGradingSettings.scope).toBe(customCourseId);
37-
expect(result.canEditGradingSettings.scope).toBe(customCourseId);
38-
});
3920
});
4021

4122
describe('getPagesAndResourcesPermissions', () => {
42-
it('returns correct permission structure with VIEW action', () => {
23+
it('returns VIEW and EDIT permissions with the correct actions and scope', () => {
4324
const result = getPagesAndResourcesPermissions(courseId);
4425

45-
expect(result.canViewPagesAndResources).toBeDefined();
4626
expect(result.canViewPagesAndResources.action).toBe(COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES);
4727
expect(result.canViewPagesAndResources.scope).toBe(courseId);
48-
});
49-
50-
it('returns correct permission structure with EDIT action', () => {
51-
const result = getPagesAndResourcesPermissions(courseId);
52-
53-
expect(result.canEditPagesAndResources).toBeDefined();
5428
expect(result.canEditPagesAndResources.action).toBe(COURSE_PERMISSIONS.EDIT_PAGES_AND_RESOURCES);
5529
expect(result.canEditPagesAndResources.scope).toBe(courseId);
5630
});
57-
58-
it('returns both VIEW and EDIT permissions in single call', () => {
59-
const result = getPagesAndResourcesPermissions(courseId);
60-
61-
const permissions = Object.keys(result);
62-
expect(permissions).toContain('canViewPagesAndResources');
63-
expect(permissions).toContain('canEditPagesAndResources');
64-
});
65-
66-
it('uses correct courseId as scope for all permissions', () => {
67-
const customCourseId = 'course-v1:another+test+course';
68-
const result = getPagesAndResourcesPermissions(customCourseId);
69-
70-
expect(result.canViewPagesAndResources.scope).toBe(customCourseId);
71-
expect(result.canEditPagesAndResources.scope).toBe(customCourseId);
72-
});
7331
});
7432

75-
describe('permission constants verification', () => {
76-
it('uses correct VIEW_GRADING_SETTINGS constant', () => {
77-
const result = getGradingPermissions(courseId);
78-
expect(result.canViewGradingSettings.action).toBe('courses.view_grading_settings');
79-
});
80-
81-
it('uses correct EDIT_GRADING_SETTINGS constant', () => {
82-
const result = getGradingPermissions(courseId);
83-
expect(result.canEditGradingSettings.action).toBe('courses.edit_grading_settings');
84-
});
85-
86-
it('uses correct VIEW_PAGES_AND_RESOURCES constant', () => {
87-
const result = getPagesAndResourcesPermissions(courseId);
88-
expect(result.canViewPagesAndResources.action).toBe('courses.view_pages_and_resources');
89-
});
90-
91-
it('uses correct EDIT_PAGES_AND_RESOURCES constant', () => {
92-
const result = getPagesAndResourcesPermissions(courseId);
93-
expect(result.canEditPagesAndResources.action).toBe('courses.manage_pages_and_resources');
94-
});
95-
});
96-
97-
describe('edge cases', () => {
98-
it('handles empty courseId', () => {
99-
const result = getGradingPermissions('');
100-
101-
expect(result.canViewGradingSettings.scope).toBe('');
102-
expect(result.canEditGradingSettings.scope).toBe('');
103-
});
104-
105-
it('handles special characters in courseId', () => {
106-
const specialCourseId = 'course-v1:test+special:id';
107-
const result = getPagesAndResourcesPermissions(specialCourseId);
33+
describe('getAdvancedSettingsPermissions', () => {
34+
it('returns VIEW and MANAGE permissions with the correct actions and scope', () => {
35+
const result = getAdvancedSettingsPermissions(courseId);
10836

109-
expect(result.canViewPagesAndResources.scope).toBe(specialCourseId);
37+
expect(result.canViewAdvancedSettings.action).toBe(COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS);
38+
expect(result.canViewAdvancedSettings.scope).toBe(courseId);
39+
expect(result.canManageAdvancedSettings.action).toBe(COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS);
40+
expect(result.canManageAdvancedSettings.scope).toBe(courseId);
11041
});
11142

112-
it('returns consistent structure across multiple calls', () => {
113-
const result1 = getGradingPermissions(courseId);
114-
const result2 = getGradingPermissions(courseId);
43+
it('uses the provided courseId as scope', () => {
44+
const otherId = 'course-v1:another+test+run';
45+
const result = getAdvancedSettingsPermissions(otherId);
11546

116-
expect(Object.keys(result1)).toEqual(Object.keys(result2));
117-
expect(result1.canViewGradingSettings.action).toBe(result2.canViewGradingSettings.action);
47+
expect(result.canViewAdvancedSettings.scope).toBe(otherId);
48+
expect(result.canManageAdvancedSettings.scope).toBe(otherId);
11849
});
11950
});
12051
});

‎src/authz/permissionHelpers.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,14 @@ export const getPagesAndResourcesPermissions = (courseId: string) => ({
2121
scope: courseId,
2222
},
2323
});
24+
25+
export const getAdvancedSettingsPermissions = (courseId: string) => ({
26+
canViewAdvancedSettings: {
27+
action: COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS,
28+
scope: courseId,
29+
},
30+
canManageAdvancedSettings: {
31+
action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS,
32+
scope: courseId,
33+
},
34+
});

‎src/pages-and-resources/PagesAndResources.tsx‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,8 @@ const PagesAndResources = () => {
8181
const isEditable = !isLoadingUserPermissions && canEditPagesAndResources;
8282
const hasAdditionalCoursePlugin = getConfig()?.pluginSlots?.additional_course_plugin != null;
8383

84-
// Read-only mode: has VIEW permission but not MANAGE (auditor)
85-
const isReadOnly = isAuthzEnabled
86-
&& !isLoadingUserPermissions
87-
&& canViewPagesAndResources
88-
&& !canEditPagesAndResources;
89-
90-
// For the modal: if readOnly, isEditable should be false (show disabled fields)
91-
const isEditableForModal = isReadOnly ? false : isEditable;
92-
9384
return (
94-
<PagesAndResourcesProvider courseId={courseId} isEditable={isEditableForModal}>
85+
<PagesAndResourcesProvider courseId={courseId} isEditable={isEditable}>
9586
<main className="container container-mw-md px-3">
9687
<div className="d-flex justify-content-between my-4 my-md-5 align-items-center">
9788
<h3 className="m-0">{intl.formatMessage(messages.heading)}</h3>
@@ -143,7 +134,6 @@ const PagesAndResources = () => {
143134
pages={pages}
144135
pluginSlotComponent={<AdditionalCoursePluginSlot />}
145136
courseId={courseId}
146-
readOnly={isReadOnly}
147137
/>
148138
{(contentPermissionsPages.length > 0 || hasAdditionalCoursePlugin)
149139
&& (
@@ -154,7 +144,6 @@ const PagesAndResources = () => {
154144
<PageGrid
155145
pages={contentPermissionsPages}
156146
pluginSlotComponent={<AdditionalCourseContentPluginSlot />}
157-
readOnly={isReadOnly}
158147
/>
159148
</>
160149
)}

‎src/pages-and-resources/PagesAndResourcesProvider.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ interface PagesAndResourcesProviderProps {
1515
children: React.ReactNode;
1616
}
1717

18-
const PagesAndResourcesProvider = ({ courseId, isEditable = true, children }: PagesAndResourcesProviderProps) => {
18+
const PagesAndResourcesProvider = ({ courseId, isEditable = false, children }: PagesAndResourcesProviderProps) => {
1919
const contextValue = useMemo(() => ({
2020
courseId,
2121
path: `/course/${courseId}/pages-and-resources`,

‎src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ const AppConfigForm = ({
4242
const navigate = useNavigate();
4343

4444
const { formRef } = useContext(AppConfigFormContext);
45-
const { path: pagesAndResourcesPath, isEditable: contextIsEditable = false } = useContext(PagesAndResourcesContext);
45+
const { path: pagesAndResourcesPath, isEditable = false } = useContext(PagesAndResourcesContext);
4646
const { appId: routeAppId } = useParams();
4747
const [isLoading, setLoading] = useState(true);
4848
const {
@@ -102,7 +102,7 @@ const AppConfigForm = ({
102102
formRef={formRef}
103103
onSubmit={handleSubmit}
104104
legacy
105-
isEditable={contextIsEditable}
105+
isEditable={isEditable}
106106
/>
107107
);
108108
} else if (selectedAppId === 'openedx') {
@@ -111,15 +111,15 @@ const AppConfigForm = ({
111111
formRef={formRef}
112112
onSubmit={handleSubmit}
113113
legacy={false}
114-
isEditable={contextIsEditable}
114+
isEditable={isEditable}
115115
/>
116116
);
117117
} else {
118118
form = (
119119
<LtiConfigForm
120120
formRef={formRef}
121121
onSubmit={handleSubmit}
122-
isEditable={contextIsEditable}
122+
isEditable={isEditable}
123123
/>
124124
);
125125
}

‎src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const AppConfigFormSaveButton = ({ labelText }) => {
1818
const { isEditable = false } = useContext(PagesAndResourcesContext);
1919

2020
const app = useModel('apps', selectedAppId);
21-
const canSubmit = (getAuthenticatedUser().administrator || !app?.adminOnlyConfig) && isEditable;
21+
const canSubmit = getAuthenticatedUser().administrator || !app?.adminOnlyConfig;
2222

2323
const { formRef } = useContext(AppConfigFormContext);
2424

@@ -47,6 +47,7 @@ const AppConfigFormSaveButton = ({ labelText }) => {
4747
}}
4848
state={submitButtonState}
4949
onClick={handleSave}
50+
disabled={!isEditable}
5051
style={{ minWidth: '88px' }}
5152
/>
5253
)

‎src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
7171

7272
return (
7373
<Card className="mb-5 p-5" data-testid="ltiConfigForm">
74-
<Form ref={formRef} onSubmit={handleSubmit} disabled={!isEditable}>
74+
<Form ref={formRef} onSubmit={handleSubmit}>
7575
<h3 className="mb-3">{providerName}</h3>
7676
<p>
7777
<FormattedMessage
@@ -113,6 +113,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
113113
onChange={handleChange}
114114
onBlur={handleBlur}
115115
value={values.consumerKey}
116+
disabled={!isEditable}
116117
/>
117118
{isInvalidConsumerKey && (
118119
<Form.Control.Feedback type="invalid" hasIcon={false}>
@@ -130,6 +131,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
130131
onChange={handleChange}
131132
onBlur={handleBlur}
132133
value={values.consumerSecret}
134+
disabled={!isEditable}
133135
/>
134136
{isInvalidConsumerSecret && (
135137
<Form.Control.Feedback type="invalid" hasIcon={false}>
@@ -143,6 +145,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
143145
onChange={handleChange}
144146
onBlur={handleBlur}
145147
value={values.launchUrl}
148+
disabled={!isEditable}
146149
/>
147150
{isInvalidLaunchUrl && (
148151
<Form.Control.Feedback type="invalid" hasIcon={false}>
@@ -163,6 +166,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
163166
onBlur={handleBlur}
164167
checked={values.piiShareUsername}
165168
label={intl.formatMessage(messages.piiShareUsername)}
169+
disabled={!isEditable}
166170
/>
167171
<Form.Check
168172
type="checkbox"
@@ -171,6 +175,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
171175
onBlur={handleBlur}
172176
checked={values.piiShareEmail}
173177
label={intl.formatMessage(messages.piiShareEmail)}
178+
disabled={!isEditable}
174179
/>
175180
</Form.Group>
176181
</div>

‎src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ const OpenedXConfigForm = ({
136136
return (
137137
<OpenedXConfigFormProvider value={contextValue}>
138138
<Card className="mb-5 px-4 px-sm-5 pb-4" data-testid="legacyConfigForm">
139-
<Form ref={formRef} onSubmit={handleSubmit} disabled={!isEditable}>
139+
<Form ref={formRef} onSubmit={handleSubmit}>
140140
<h3 className="text-primary-500 my-3">{intl.formatMessage(messages[`appName-${selectedAppId}`])}</h3>
141141
<AppConfigFormDivider thick />
142142
{!legacy

0 commit comments

Comments
 (0)