feat(pages-and-resources): remove isEditable prop from forms and related components

Author: bra-i-am

src/advanced-settings/AdvancedSettings.tsx

@@ -72,7 +72,7 @@ const AdvancedSettings = () => {
 
   const isEditable = !isAuthzEnabled
     || isLoadingUserPermissions
-    || !!canManageAdvancedSettings;
+    || canManageAdvancedSettings;
 
   const updateSettingsButtonState = {
     labels: {

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

@@ -42,7 +42,7 @@ const AppConfigForm = ({
   const navigate = useNavigate();
 
   const { formRef } = useContext(AppConfigFormContext);
-  const { path: pagesAndResourcesPath, isEditable } = useContext(PagesAndResourcesContext);
+  const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext);
   const { appId: routeAppId } = useParams();
   const [isLoading, setLoading] = useState(true);
   const {
@@ -102,7 +102,6 @@ const AppConfigForm = ({
         formRef={formRef}
         onSubmit={handleSubmit}
         legacy
-        isEditable={isEditable}
       />
     );
   } else if (selectedAppId === 'openedx') {
@@ -111,15 +110,13 @@ const AppConfigForm = ({
         formRef={formRef}
         onSubmit={handleSubmit}
         legacy={false}
-        isEditable={isEditable}
       />
     );
   } else {
     form = (
       <LtiConfigForm
         formRef={formRef}
         onSubmit={handleSubmit}
-        isEditable={isEditable}
       />
     );
   }

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

@@ -18,7 +18,7 @@ import { useModel } from '../../../../../generic/model-store';
 
 ensureConfig(['SITE_NAME', 'SUPPORT_EMAIL'], 'LTI Config Form');
 
-const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
+const LtiConfigForm = ({ onSubmit, formRef }) => {
   const intl = useIntl();
   const dispatch = useDispatch();
 
@@ -113,7 +113,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
                 onChange={handleChange}
                 onBlur={handleBlur}
                 value={values.consumerKey}
-                disabled={!isEditable}
+
               />
               {isInvalidConsumerKey && (
                 <Form.Control.Feedback type="invalid" hasIcon={false}>
@@ -131,7 +131,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
                 onChange={handleChange}
                 onBlur={handleBlur}
                 value={values.consumerSecret}
-                disabled={!isEditable}
+
               />
               {isInvalidConsumerSecret && (
                 <Form.Control.Feedback type="invalid" hasIcon={false}>
@@ -145,7 +145,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
                 onChange={handleChange}
                 onBlur={handleBlur}
                 value={values.launchUrl}
-                disabled={!isEditable}
+
               />
               {isInvalidLaunchUrl && (
                 <Form.Control.Feedback type="invalid" hasIcon={false}>
@@ -166,7 +166,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
                 onBlur={handleBlur}
                 checked={values.piiShareUsername}
                 label={intl.formatMessage(messages.piiShareUsername)}
-                disabled={!isEditable}
+
               />
               <Form.Check
                 type="checkbox"
@@ -175,7 +175,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => {
                 onBlur={handleBlur}
                 checked={values.piiShareEmail}
                 label={intl.formatMessage(messages.piiShareEmail)}
-                disabled={!isEditable}
+
               />
             </Form.Group>
           </div>
@@ -194,7 +194,6 @@ LtiConfigForm.propTypes = {
   onSubmit: PropTypes.func.isRequired,
   // eslint-disable-next-line react/forbid-prop-types
   formRef: PropTypes.object.isRequired,
-  isEditable: PropTypes.bool,
 };
 
 export default LtiConfigForm;

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

@@ -25,7 +25,6 @@ const OpenedXConfigForm = ({
   onSubmit,
   formRef,
   legacy,
-  isEditable = false,
 }) => {
   const intl = useIntl();
   const {
@@ -146,7 +145,6 @@ const OpenedXConfigForm = ({
                         onBlur={handleBlur}
                         onChange={handleChange}
                         values={values}
-                        disabled={!isEditable}
                       />
                       <AppConfigFormDivider thick />
                     </>
@@ -155,15 +153,14 @@ const OpenedXConfigForm = ({
                   onBlur={handleBlur}
                   onChange={handleChange}
                   values={values}
-                  disabled={!isEditable}
                 />
                 <AppConfigFormDivider thick />
-                <DiscussionTopics disabled={!isEditable} />
+                <DiscussionTopics />
                 <AppConfigFormDivider thick />
-                <DivisionByGroupFields disabled={!isEditable} />
+                <DivisionByGroupFields />
                 <AppConfigFormDivider thick />
-                <ReportedContentEmailNotifications disabled={!isEditable} />
-                <DiscussionRestriction disabled={!isEditable} />
+                <ReportedContentEmailNotifications />
+                <DiscussionRestriction />
               </Form>
             </Card>
           </OpenedXConfigFormProvider>
@@ -178,7 +175,6 @@ OpenedXConfigForm.propTypes = {
   onSubmit: PropTypes.func.isRequired,
   // eslint-disable-next-line react/forbid-prop-types
   formRef: PropTypes.object.isRequired,
-  isEditable: PropTypes.bool,
 };
 
 export default OpenedXConfigForm;

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

@@ -77,15 +77,14 @@ describe('OpenedXConfigForm', () => {
     axiosMock.reset();
   });
 
-  const createComponent = (onSubmit = jest.fn(), formRef = createRef(), legacy = true, isEditable = false) => {
+  const createComponent = (onSubmit = jest.fn(), formRef = createRef(), legacy = true) => {
     const wrapper = render(
       <AppProvider store={store}>
         <IntlProvider locale="en">
           <OpenedXConfigForm
             onSubmit={onSubmit}
             formRef={formRef}
             legacy={legacy}
-            isEditable={isEditable}
           />
         </IntlProvider>
       </AppProvider>,
@@ -116,35 +115,6 @@ describe('OpenedXConfigForm', () => {
     expect(queryByText(container, messages.groupInContextSubsectionLabel.defaultMessage)).toBeInTheDocument();
   });
 
-  test('renders editable form when isEditable=true', async () => {
-    await mockStore(legacyApiResponse);
-    createComponent(jest.fn(), createRef(), true, true);
-    const form = container.querySelector('form');
-    expect(form).toBeInTheDocument();
-  });
-
-  test('renders form using default isEditable=false when prop is omitted', async () => {
-    await mockStore(legacyApiResponse);
-    const formRef = createRef();
-    // Render directly without isEditable to trigger the default param = false
-    const wrapper = render(
-      <AppProvider store={store}>
-        <IntlProvider locale="en">
-          <OpenedXConfigForm
-            onSubmit={jest.fn()}
-            formRef={formRef}
-            legacy
-          />
-        </IntlProvider>
-      </AppProvider>,
-    );
-    const form = wrapper.container.querySelector('form');
-    expect(form).toBeInTheDocument();
-    // isEditable defaults to false — child controls should be disabled
-    const divideByCohorts = wrapper.container.querySelector('#divideByCohorts');
-    expect(divideByCohorts).toBeDisabled();
-  });
-
   test('calls onSubmit when the formRef is submitted', async () => {
     const formRef = createRef();
     const handleSubmit = jest.fn();

src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx

@@ -11,7 +11,6 @@ const InContextDiscussionFields = ({
   onBlur,
   onChange,
   values,
-  disabled = false,
 }) => {
   const intl = useIntl();
   const {
@@ -45,13 +44,12 @@ const InContextDiscussionFields = ({
         )
         : (
           <FormSwitchGroup
-            onChange={() => !disabled && setShowPopup(true)}
+            onChange={() => setShowPopup(true)}
             onBlur={onBlur}
             id="enableGradedUnits"
             checked={values.enableGradedUnits}
             label={intl.formatMessage(messages.gradedUnitPagesLabel)}
             helpText={intl.formatMessage(messages.gradedUnitPagesHelp)}
-            disabled={disabled}
           />
         )}
       <AppConfigFormDivider />
@@ -62,7 +60,6 @@ const InContextDiscussionFields = ({
         checked={values.groupAtSubsection}
         label={intl.formatMessage(messages.groupInContextSubsectionLabel)}
         helpText={intl.formatMessage(messages.groupInContextSubsectionHelp)}
-        disabled={disabled}
       />
     </>
   );
@@ -75,7 +72,6 @@ InContextDiscussionFields.propTypes = {
     enableGradedUnits: PropTypes.bool,
     groupAtSubsection: PropTypes.bool,
   }).isRequired,
-  disabled: PropTypes.bool,
 };
 
 export default InContextDiscussionFields;

src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx

@@ -10,7 +10,6 @@ const gradedUnitLabel = messages.gradedUnitPagesLabel.defaultMessage;
 const defaultProps = {
   onBlur: jest.fn(),
   onChange: jest.fn(),
-  setFieldValue: jest.fn(),
   values: {
     enableGradedUnits: false,
     groupAtSubsection: false,
@@ -36,18 +35,6 @@ describe('InContextDiscussionFields', () => {
   });
 
   it('renders without crashing', () => {
-    renderComponent();
-    // Component renders - check for presence of expected text
-    expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument();
-  });
-
-  it('renders with disabled prop', () => {
-    renderComponent({ disabled: true });
-    expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument();
-  });
-
-  it('renders with default disabled (false)', () => {
-    // Test default param - no disabled prop passed
     renderComponent();
     expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument();
   });
@@ -59,62 +46,28 @@ describe('InContextDiscussionFields', () => {
     expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument();
   });
 
-  it('shows confirmation popup when toggling enableGradedUnits when not disabled', () => {
-    renderComponent({ disabled: false });
-    // When not disabled, clicking the switch should trigger the onChange which sets showPopup
-    // The component should still render - we test via the callback behavior
-    const switchControl = screen.getByLabelText(gradedUnitLabel);
-    expect(switchControl).not.toBeDisabled();
-  });
-
-  it('does NOT show popup when disabled', () => {
-    renderComponent({ disabled: true });
-    // When disabled, clicking should not trigger popup logic
-    // Verify the switch is disabled
-    const switchControl = screen.getByLabelText(gradedUnitLabel);
-    expect(switchControl).toBeDisabled();
-  });
-
   it('shows confirmation popup and handles confirm', () => {
-    renderComponent({ disabled: false, setFieldValue: jest.fn() });
-    // Click the switch to show popup
+    renderComponent({ setFieldValue: jest.fn() });
     const switchControl = screen.getByLabelText(gradedUnitLabel);
     fireEvent.click(switchControl);
-    // Check popup appears with confirm button
     expect(screen.getByText(/Confirm/i)).toBeInTheDocument();
-    // Click confirm
     fireEvent.click(screen.getByText(/Confirm/i));
-    // Popup should close (no longer visible)
   });
 
   it('shows popup with cancel labels when enableGradedUnits is already true', () => {
     renderComponent({
-      disabled: false,
       values: { enableGradedUnits: true, groupAtSubsection: false },
     });
     const switchControl = screen.getByLabelText(gradedUnitLabel);
     fireEvent.click(switchControl);
-    // Popup shows cancel-related labels (enableGradedUnits=true branch)
     expect(screen.getByText(/Confirm/i)).toBeInTheDocument();
   });
 
   it('shows confirmation popup and handles cancel', () => {
-    renderComponent({ disabled: false, setFieldValue: jest.fn() });
-    // Click the switch to show popup
+    renderComponent({ setFieldValue: jest.fn() });
     const switchControl = screen.getByLabelText(gradedUnitLabel);
     fireEvent.click(switchControl);
-    // Check popup appears with cancel button
     expect(screen.getByText(/Cancel/i)).toBeInTheDocument();
-    // Click cancel
     fireEvent.click(screen.getByText(/Cancel/i));
-    // Popup should close (no longer visible)
-  });
-
-  it('does not show popup when onChange fires with disabled=true', () => {
-    renderComponent({ disabled: true });
-    const switchControl = screen.getByLabelText(gradedUnitLabel);
-    // fireEvent bypasses browser disabled-input behavior, but the !disabled guard in onChange prevents the popup
-    fireEvent.click(switchControl);
-    expect(screen.queryByText(/Confirm/i)).not.toBeInTheDocument();
   });
 });