Replace connect with useSelector() and useDispatch() 4/5 #2317 #2346

ahtesham-quraish · 2025-07-31T11:24:53Z

Description

Replace connect with useSelector() and useDispatch() 4/5 #2317

Supporting information

Link to other information about the change, such as GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.

Testing instructions

Please provide detailed step-by-step instructions for manually testing this change.

Other information

Include anything else that will help reviewers and consumers understand the change.

Does this change depend on other changes elsewhere?
Any special concerns or limitations? For example: deprecations, migrations, security, or accessibility.

Best Practices Checklist

We're trying to move away from some deprecated patterns in this codebase. Please
check if your PR meets these recommendations before asking for a review:

Any new files are using TypeScript (.ts, .tsx).
Deprecated propTypes, defaultProps, and injectIntl patterns are not used in any new or modified code.
Tests should use the helpers in src/testUtils.tsx (specifically initializeMocks)
Do not add new fields to the Redux state/store. Use React Context to share state among multiple components.
Use React Query to load data from REST APIs. See any apiHooks.ts in this repo for examples.
All new i18n messages in messages.ts files have a description for translators to use.
Imports avoid using ../. To import from parent folders, use @src, e.g. import { initializeMocks } from '@src/testUtils'; instead of from '../../../../testUtils'

…enedx#231

…enedx#2316

…enedx#2317

codecov · 2025-07-31T11:34:47Z

Codecov Report

❌ Patch coverage is 95.93023% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.44%. Comparing base (8326257) to head (9a5743b).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...sModal/components/LicenseWidget/LicenseDetails.jsx	84.61%	2 Missing ⚠️
...ingsWidget/settingsComponents/SwitchEditorCard.jsx	91.66%	1 Missing ⚠️
src/editors/containers/TextEditor/index.jsx	94.73%	1 Missing ⚠️
...eoSettingsModal/components/HandoutWidget/index.jsx	92.30%	1 Missing ⚠️
...eoSettingsModal/components/LicenseWidget/index.jsx	88.88%	1 Missing ⚠️
...ttingsModal/components/SocialShareWidget/index.jsx	93.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2346      +/-   ##
==========================================
+ Coverage   94.40%   94.44%   +0.03%     
==========================================
  Files        1167     1168       +1     
  Lines       24957    24987      +30     
  Branches     5303     5422     +119     
==========================================
+ Hits        23561    23598      +37     
+ Misses       1329     1316      -13     
- Partials       67       73       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bradenmacdonald

Thanks for those updates! The changes to the main app code mostly look good, but the test code still needs a bunch of revisions to match the other changes we've been making. Please see the things I've flagged below but then check them in all the modified tests, because I didn't point out every occurrence of each issue.

bradenmacdonald · 2025-08-22T21:36:44Z

...editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.test.tsx

+import editorRender from '../../../../../modifiedEditorTestRender';
+import ExplanationWidget from './index';
+import { initializeStore } from '../../../../../data/redux';


Suggested change

import editorRender from '../../../../../modifiedEditorTestRender';

import ExplanationWidget from './index';

import { initializeStore } from '../../../../../data/redux';

import { editorRender } from '@src/editors/editorTestRender';

import ExplanationWidget from './index';

What is this modifiedEditorTestRender ? It doesn't seem to be necessary. The existing editorRender works fine.

bradenmacdonald · 2025-08-22T21:37:29Z

...editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.test.tsx

+    initializeMocks({
+      initializeStore,
+      initialState,
+    });


Suggested change

initializeMocks({

initializeStore,

initialState,

});

initializeMocks();

Since you are passing initialState into editorRender, you don't have to do anything with the editor store here when initializing the global app mocks. There are two different redux stores. initializeMocks initializes the global app store, which the editors basically ignore. editorRender will initialize the editor redux store, which is what's relevant here.

bradenmacdonald · 2025-08-22T21:55:56Z

src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.test.tsx

+import editorRender from '../../../../../modifiedEditorTestRender';
+import { initializeStore } from '../../../../../data/redux';


Suggested change

import editorRender from '../../../../../modifiedEditorTestRender';

import { initializeStore } from '../../../../../data/redux';

import { editorRender } from '@src/editors/editorTestRender';

bradenmacdonald · 2025-08-22T21:57:11Z

src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.test.tsx

+  beforeEach(() => {
+    initializeMocks({ initialState, initializeStore });
+  });


Suggested change

beforeEach(() => {

initializeMocks({ initialState, initializeStore });

});

initializeMocks is already called below. We shouldn't call it twice. And you don't need to pass in any redux state - the global redux store that it sets up is not used for editor tests.

bradenmacdonald · 2025-08-22T21:59:17Z

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.jsx

-export const SettingsWidgetInternal = SettingsWidget; // For testing only
-export default connect(mapStateToProps, mapDispatchToProps)(SettingsWidget);
+export const SettingsWidgetInternal = SettingsWidget; // For testing
+export default injectIntl(SettingsWidget);


Please don't add injectIntl - use useIntl instead. But I don't think you need either here - it seems to be unused. Maybe this is a merge conflict error?

bradenmacdonald · 2025-08-22T22:05:23Z

...blemEditor/components/EditProblemView/SettingsWidget/settingsComponents/ScoringCard.test.jsx

+import { selectors, initializeStore } from '../../../../../../data/redux';
+import editorRender from '../../../../../../modifiedEditorTestRender';


Suggested change

import { selectors, initializeStore } from '../../../../../../data/redux';

import editorRender from '../../../../../../modifiedEditorTestRender';

import { selectors } from '@src/editors/data/redux';

import { editorRender } from '@src/editors/editorTestRender';

Please remove modifiedEditorTestRender and you don't need to import initializeStore

bradenmacdonald · 2025-08-22T22:05:34Z

...blemEditor/components/EditProblemView/SettingsWidget/settingsComponents/ScoringCard.test.jsx

-    jest.spyOn(app, 'learningContextId').mockReturnValue('learningContextId');
-    jest.spyOn(app, 'isLibrary').mockReturnValue(false);
-    initializeMocks();
+    initializeMocks({ initialState, initializeStore });


Suggested change

initializeMocks({ initialState, initializeStore });

initializeMocks();

bradenmacdonald · 2025-08-22T22:07:07Z

...roblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/ShowAnswerCard.jsx

 }) => {
-  const intl = useIntl();


This was correct - please don't replace useIntl with injectIntl; the latter is deprecated.

It should show this if you're using an IDE:

bradenmacdonald · 2025-08-22T22:08:45Z

src/editors/containers/TextEditor/index.test.tsx

+import { screen } from '@testing-library/react';
+import { initializeMocks } from '@src/testUtils';
+import { RequestKeys } from '@src/editors/data/constants/requests';
+import editorRender from '../../editorTestRender';


Suggested change

import editorRender from '../../editorTestRender';

import { editorRender } from '@src/editors/editorTestRender';

bradenmacdonald · 2025-08-22T22:09:54Z

src/editors/modifiedEditorTestRender.jsx

+import React from 'react';
+import { Provider } from 'react-redux';
+import { render as baseRender } from '../testUtils';
+import { EditorContextProvider } from './EditorContext';
+import { initializeStore } from './data/redux'; // adjust path if needed
+
+/**
+ * Custom render function for testing React components with the editor context and Redux store.
+ *
+ * Wraps the provided UI in both the EditorContextProvider and Redux Provider,
+ * ensuring that components under test have access to the necessary context and store.
+ *
+ * @param {React.ReactElement} ui - The React element to render.
+ * @param {object} [options] - Optional parameters.
+ * @param {object} [options.initialState] - Optional initial state for the store.
+ * @param {string} [options.learningContextId] - Optional learning context ID.
+ * @returns {RenderResult} The result of the render, as returned by RTL render.
+ */
+const editorRender = (
+  ui,
+  {
+    initialState = {},
+    learningContextId = 'course-v1:Org+COURSE+RUN',
+  } = {},
+) => {
+  const store = initializeStore(initialState);
+
+  return baseRender(ui, {
+    extraWrapper: ({ children }) => (
+      <EditorContextProvider learningContextId={learningContextId}>
+        <Provider store={store}>
+          {children}
+        </Provider>
+      </EditorContextProvider>
+    ),
+  });
+};
+
+export default editorRender;


Please delete this whole file. I'm not sure what it's purpose was but we don't need it, because editorTestRender works fine.

Ahtesham Quraish and others added 5 commits July 24, 2025 22:08

refactor: Replace connect with useSelector() and useDispatch() 1/5

2bb2b76

refactor: Replace connect with useSelector() and useDispatch() 2/5 op…

cced750

…enedx#231

fix: merge with main branch

f973345

refactor: Replace connect with useSelector() and useDispatch() 3/5 op…

2dfe074

…enedx#2316

refactor: Replace connect with useSelector() and useDispatch() 4/5 op…

9a5743b

…enedx#2317

fix: merge with master

a191dc4

bradenmacdonald requested changes Aug 22, 2025

View reviewed changes

bradenmacdonald mentioned this pull request Aug 22, 2025

Replace connect with useSelector() and useDispatch() 3/5 #2316 #2341

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace connect with useSelector() and useDispatch() 4/5 #2317 #2346

Replace connect with useSelector() and useDispatch() 4/5 #2317 #2346

Uh oh!

ahtesham-quraish commented Jul 31, 2025

Uh oh!

codecov bot commented Jul 31, 2025

Uh oh!

bradenmacdonald left a comment

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

bradenmacdonald Aug 22, 2025

Uh oh!

Uh oh!

		import editorRender from '../../../../../modifiedEditorTestRender';
		import { initializeStore } from '../../../../../data/redux';

	import editorRender from '../../../../../modifiedEditorTestRender';
	import { initializeStore } from '../../../../../data/redux';
	import { editorRender } from '@src/editors/editorTestRender';

	beforeEach(() => {
	initializeMocks({ initialState, initializeStore });
	});

		import { selectors, initializeStore } from '../../../../../../data/redux';
		import editorRender from '../../../../../../modifiedEditorTestRender';

	initializeMocks({ initialState, initializeStore });
	initializeMocks();

	import editorRender from '../../editorTestRender';
	import { editorRender } from '@src/editors/editorTestRender';

Replace connect with useSelector() and useDispatch() 4/5 #2317 #2346

Are you sure you want to change the base?

Replace connect with useSelector() and useDispatch() 4/5 #2317 #2346

Uh oh!

Conversation

ahtesham-quraish commented Jul 31, 2025

Description

Supporting information

Testing instructions

Other information

Best Practices Checklist

Uh oh!

codecov bot commented Jul 31, 2025

Codecov Report

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!