Skip to content

Commit 7a512f8

Browse files
committed
squash!: update to use new provider api
1 parent 81d3e47 commit 7a512f8

14 files changed

Lines changed: 287 additions & 232 deletions

File tree

src/pages-and-resources/discussions/DiscussionsSettings.jsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { PagesAndResourcesContext } from '../PagesAndResourcesProvider';
1616

1717
import messages from './messages';
1818
import DiscussionsProvider from './DiscussionsProvider';
19-
import { fetchApps } from './data/thunks';
19+
import { fetchProviders } from './data/thunks';
2020
import AppList from './app-list';
2121
import AppConfigForm from './app-config-form';
2222
import { DENIED, FAILED } from './data/slice';
@@ -36,7 +36,7 @@ function DiscussionsSettings({ courseId, intl }) {
3636
const courseDetail = useModel('courseDetails', courseId);
3737

3838
useEffect(() => {
39-
dispatch(fetchApps(courseId));
39+
dispatch(fetchProviders(courseId));
4040
}, [courseId]);
4141

4242
const discussionsPath = `${pagesAndResourcesPath}/discussion`;

src/pages-and-resources/discussions/DiscussionsSettings.test.jsx

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
55
import { AppProvider, PageRoute } from '@edx/frontend-platform/react';
66
import {
77
act,
8+
findByRole,
89
getByRole,
910
queryByLabelText,
1011
queryByRole,
@@ -19,21 +20,22 @@ import userEvent from '@testing-library/user-event';
1920
import MockAdapter from 'axios-mock-adapter';
2021
import React from 'react';
2122
import { Switch } from 'react-router';
23+
import { fetchCourseDetail } from '../../data/thunks';
2224
import initializeStore from '../../store';
25+
import { executeThunk } from '../../utils';
2326
import PagesAndResourcesProvider from '../PagesAndResourcesProvider';
27+
import ltiMessages from './app-config-form/apps/lti/messages';
2428
import appMessages from './app-config-form/messages';
2529
import messages from './app-list/messages';
26-
import ltiMessages from './app-config-form/apps/lti/messages';
27-
import { getAppsUrl } from './data/api';
30+
import { getDiscussionsProvidersUrl, getDiscussionsSettingsUrl } from './data/api';
2831
import DiscussionsSettings from './DiscussionsSettings';
2932
import {
33+
courseDetailResponse,
3034
generatePiazzaApiResponse,
35+
generateProvidersApiResponse,
3136
legacyApiResponse,
3237
piazzaApiResponse,
33-
courseDetailResponse,
3438
} from './factories/mockApiResponses';
35-
import { executeThunk } from '../../utils';
36-
import { fetchCourseDetail } from '../../data/thunks';
3739

3840
const courseId = 'course-v1:edX+TestX+Test_Course';
3941
let axiosMock;
@@ -88,7 +90,10 @@ describe('DiscussionsSettings', () => {
8890

8991
describe('with successful network connections', () => {
9092
beforeEach(() => {
91-
axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse);
93+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId))
94+
.reply(200, generateProvidersApiResponse(false));
95+
axiosMock.onGet(getDiscussionsSettingsUrl(courseId))
96+
.reply(200, piazzaApiResponse);
9297
renderComponent();
9398
});
9499

@@ -124,14 +129,17 @@ describe('DiscussionsSettings', () => {
124129
userEvent.click(queryByLabelText(container, 'Select Piazza'));
125130
userEvent.click(queryByText(container, messages.nextButton.defaultMessage));
126131

132+
await waitForElementToBeRemoved(screen.getByRole('status'));
133+
127134
expect(queryByTestId(container, 'appList')).not.toBeInTheDocument();
128135
expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument();
129136
expect(queryByTestId(container, 'ltiConfigForm')).toBeInTheDocument();
130137
expect(queryByTestId(container, 'legacyConfigForm')).not.toBeInTheDocument();
131138
});
132139

133140
test('successfully advances to settings step for legacy', async () => {
134-
axiosMock.onGet(getAppsUrl(courseId)).reply(200, legacyApiResponse);
141+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse(false, 'legacy'));
142+
axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, legacyApiResponse);
135143
renderComponent();
136144
history.push(`/course/${courseId}/pages-and-resources/discussion`);
137145

@@ -142,6 +150,8 @@ describe('DiscussionsSettings', () => {
142150
userEvent.click(queryByLabelText(container, 'Select edX (Legacy)'));
143151
userEvent.click(queryByText(container, messages.nextButton.defaultMessage));
144152

153+
await waitForElementToBeRemoved(screen.getByRole('status'));
154+
145155
expect(queryByTestId(container, 'appList')).not.toBeInTheDocument();
146156
expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument();
147157
expect(queryByTestId(container, 'ltiConfigForm')).not.toBeInTheDocument();
@@ -183,7 +193,7 @@ describe('DiscussionsSettings', () => {
183193
test('successfully submit the modal', async () => {
184194
history.push(`/course/${courseId}/pages-and-resources/discussion`);
185195

186-
axiosMock.onPost(getAppsUrl(courseId)).reply(200, piazzaApiResponse);
196+
axiosMock.onPost(getDiscussionsSettingsUrl(courseId)).reply(200, piazzaApiResponse);
187197

188198
// This is an important line that ensures the spinner has been removed - and thus our main
189199
// content has been loaded - prior to proceeding with our expectations.
@@ -193,7 +203,7 @@ describe('DiscussionsSettings', () => {
193203

194204
userEvent.click(getByRole(container, 'button', { name: 'Next' }));
195205

196-
userEvent.click(getByRole(container, 'button', { name: 'Save' }));
206+
userEvent.click(await findByRole(container, 'button', { name: 'Save' }));
197207

198208
// This is an important line that ensures the Close button has been removed, which implies that
199209
// the full screen modal has been closed following our click of Apply. Once this has happened,
@@ -215,6 +225,7 @@ describe('DiscussionsSettings', () => {
215225
userEvent.click(getByRole(container, 'checkbox', { name: 'Select Discourse' }));
216226
userEvent.click(getByRole(container, 'button', { name: 'Next' }));
217227

228+
await findByRole(container, 'button', { name: 'Save' });
218229
userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Key' }), 'key');
219230
userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Secret' }), 'secret');
220231
userEvent.type(getByRole(container, 'textbox', { name: 'Launch URL' }), 'http://example.test');
@@ -237,6 +248,7 @@ describe('DiscussionsSettings', () => {
237248
userEvent.click(discourseBox);
238249

239250
userEvent.click(getByRole(container, 'button', { name: 'Next' }));
251+
await waitForElementToBeRemoved(screen.getByRole('status'));
240252
expect(getByRole(container, 'heading', { name: 'Discourse' })).toBeInTheDocument();
241253

242254
userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Key' }), 'a');
@@ -252,16 +264,16 @@ describe('DiscussionsSettings', () => {
252264
});
253265
});
254266

255-
describe('with network error fetchApps API requests', () => {
267+
describe('with network error fetchProviders API requests', () => {
256268
beforeEach(() => {
257269
// Expedient way of getting SUPPORT_URL into config.
258270
setConfig({
259271
...getConfig(),
260272
SUPPORT_URL: 'http://support.edx.org',
261273
});
262274

263-
axiosMock.onGet(getAppsUrl(courseId)).networkError();
264-
275+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).networkError();
276+
axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).networkError();
265277
renderComponent();
266278
});
267279

@@ -287,8 +299,11 @@ describe('DiscussionsSettings', () => {
287299
SUPPORT_URL: 'http://support.edx.org',
288300
});
289301

290-
axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse);
291-
axiosMock.onPost(getAppsUrl(courseId)).networkError();
302+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId))
303+
.reply(200, generateProvidersApiResponse());
304+
axiosMock.onGet(getDiscussionsSettingsUrl(courseId))
305+
.reply(200, piazzaApiResponse);
306+
axiosMock.onPost(getDiscussionsSettingsUrl(courseId)).networkError();
292307
renderComponent();
293308
});
294309

@@ -307,17 +322,17 @@ describe('DiscussionsSettings', () => {
307322
await waitFor(() => expect(axiosMock.history.post.length).toBe(1));
308323

309324
expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument();
310-
311-
const alert = queryByRole(container, 'alert');
325+
const alert = await findByRole(container, 'alert');
312326
expect(alert).toBeInTheDocument();
313327
expect(alert.textContent).toEqual(expect.stringContaining('We encountered a technical error when applying changes.'));
314328
expect(alert.innerHTML).toEqual(expect.stringContaining(getConfig().SUPPORT_URL));
315329
});
316330
});
317331

318-
describe('with permission denied error for fetchApps API requests', () => {
332+
describe('with permission denied error for fetchProviders API requests', () => {
319333
beforeEach(() => {
320-
axiosMock.onGet(getAppsUrl(courseId)).reply(403);
334+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(403);
335+
axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(403);
321336

322337
renderComponent();
323338
});
@@ -337,8 +352,10 @@ describe('DiscussionsSettings', () => {
337352

338353
describe('with permission denied error for postAppConfig API requests', () => {
339354
beforeEach(() => {
340-
axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse);
341-
axiosMock.onPost(getAppsUrl(courseId)).reply(403);
355+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId))
356+
.reply(200, generateProvidersApiResponse());
357+
axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, piazzaApiResponse);
358+
axiosMock.onPost(getDiscussionsSettingsUrl(courseId)).reply(403);
342359

343360
renderComponent();
344361
});
@@ -360,7 +377,7 @@ describe('DiscussionsSettings', () => {
360377
// We don't technically leave the route in this case, though the modal is hidden.
361378
expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources/discussion/configure/piazza`);
362379

363-
const alert = queryByRole(container, 'alert');
380+
const alert = await findByRole(container, 'alert');
364381
expect(alert).toBeInTheDocument();
365382
expect(alert.textContent).toEqual(expect.stringContaining('You are not authorized to view this page.'));
366383
});
@@ -394,7 +411,10 @@ describe.each([
394411

395412
// Leave the DiscussionsSettings route after the test.
396413
history.push(`/course/${courseId}/pages-and-resources`);
397-
axiosMock.onGet(getAppsUrl(courseId)).reply(200, generatePiazzaApiResponse(isAdminOnlyConfig));
414+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId))
415+
.reply(200, generateProvidersApiResponse(isAdminOnlyConfig));
416+
axiosMock.onGet(getDiscussionsSettingsUrl(courseId))
417+
.reply(200, generatePiazzaApiResponse());
398418
renderComponent();
399419
});
400420

@@ -408,6 +428,7 @@ describe.each([
408428

409429
userEvent.click(queryByLabelText(container, 'Select Piazza'));
410430
userEvent.click(queryByText(container, messages.nextButton.defaultMessage));
431+
await waitForElementToBeRemoved(screen.getByRole('status'));
411432

412433
if (showLTIConfig) {
413434
expect(queryByText(container, ltiMessages.formInstructions.defaultMessage)).toBeInTheDocument();
@@ -446,7 +467,10 @@ describe.each([
446467

447468
// Leave the DiscussionsSettings route after the test.
448469
history.push(`/course/${courseId}/pages-and-resources`);
449-
axiosMock.onGet(getAppsUrl(courseId)).reply(200, generatePiazzaApiResponse(false, piiSharingAllowed));
470+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId))
471+
.reply(200, generateProvidersApiResponse(false));
472+
axiosMock.onGet(getDiscussionsSettingsUrl(courseId))
473+
.reply(200, generatePiazzaApiResponse(piiSharingAllowed));
450474
renderComponent();
451475
});
452476

@@ -460,6 +484,7 @@ describe.each([
460484

461485
userEvent.click(queryByLabelText(container, 'Select Piazza'));
462486
userEvent.click(queryByText(container, messages.nextButton.defaultMessage));
487+
await waitForElementToBeRemoved(screen.getByRole('status'));
463488
if (enablePIISharing) {
464489
expect(queryByTestId(container, 'piiSharingFields')).toBeInTheDocument();
465490
} else {

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
DENIED,
2222
FAILED, LOADED, LOADING, selectApp,
2323
} from '../data/slice';
24-
import { saveAppConfig } from '../data/thunks';
24+
import { fetchDiscussionSettings, saveProviderConfig } from '../data/thunks';
2525
import OpenedXConfigForm from './apps/openedx';
2626
import LtiConfigForm from './apps/lti';
2727
import AppConfigFormProvider, { AppConfigFormContext } from './AppConfigFormProvider';
@@ -36,12 +36,20 @@ function AppConfigForm({
3636
const { formRef } = useContext(AppConfigFormContext);
3737
const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext);
3838
const { params: { appId: routeAppId } } = useRouteMatch();
39+
const [isLoading, setLoading] = useState(true);
3940
const {
4041
activeAppId, selectedAppId, status, saveStatus,
4142
} = useSelector(state => state.discussions);
4243

4344
const [confirmationDialogVisible, setConfirmationDialogVisible] = useState(false);
4445

46+
useEffect(() => {
47+
(async () => {
48+
await dispatch(fetchDiscussionSettings(courseId, selectedAppId));
49+
setLoading(false);
50+
})();
51+
}, [courseId, selectedAppId]);
52+
4553
useEffect(() => {
4654
if (status === LOADED) {
4755
if (routeAppId !== selectedAppId) {
@@ -58,11 +66,11 @@ function AppConfigForm({
5866
} else {
5967
setConfirmationDialogVisible(false);
6068
// Note that when this action succeeds, we redirect to pagesAndResourcesPath in the thunk.
61-
dispatch(saveAppConfig(courseId, selectedAppId, values, pagesAndResourcesPath));
69+
dispatch(saveProviderConfig(courseId, selectedAppId, values, pagesAndResourcesPath));
6270
}
6371
}, [activeAppId, confirmationDialogVisible, courseId, selectedAppId]);
6472

65-
if (!selectedAppId || status === LOADING) {
73+
if (!selectedAppId || status === LOADING || isLoading) {
6674
return (
6775
<Loading />
6876
);

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ ensureConfig(['SITE_NAME', 'SUPPORT_EMAIL'], 'LTI Config Form');
2121
function LtiConfigForm({ onSubmit, intl, formRef }) {
2222
const dispatch = useDispatch();
2323

24-
const { selectedAppId } = useSelector((state) => state.discussions);
25-
26-
const piiConfig = useModel('appConfigs', 'pii');
24+
const { selectedAppId, piiConfig } = useSelector((state) => state.discussions);
2725
const appConfig = useModel('appConfigs', selectedAppId);
2826
const app = useModel('apps', selectedAppId);
2927
const providerName = intl.formatMessage(appMessages[`appName-${app?.id}`]);

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ import { AppProvider } from '@edx/frontend-platform/react';
2121

2222
import initializeStore from '../../../../../store';
2323
import { executeThunk } from '../../../../../utils';
24-
import { getAppsUrl } from '../../../data/api';
25-
import { fetchApps } from '../../../data/thunks';
26-
import { legacyApiResponse } from '../../../factories/mockApiResponses';
24+
import { getDiscussionsProvidersUrl, getDiscussionsSettingsUrl } from '../../../data/api';
25+
import { fetchDiscussionSettings, fetchProviders } from '../../../data/thunks';
26+
import { generateProvidersApiResponse, legacyApiResponse } from '../../../factories/mockApiResponses';
2727
import messages from '../../messages';
2828
import OpenedXConfigForm from './OpenedXConfigForm';
2929
import { selectApp } from '../../../data/slice';
@@ -88,8 +88,10 @@ describe('OpenedXConfigForm', () => {
8888
};
8989

9090
const mockStore = async (mockResponse) => {
91-
axiosMock.onGet(getAppsUrl(courseId)).reply(200, mockResponse);
92-
await executeThunk(fetchApps(courseId), store.dispatch);
91+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse());
92+
axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, mockResponse);
93+
await executeThunk(fetchProviders(courseId), store.dispatch);
94+
await executeThunk(fetchDiscussionSettings(courseId), store.dispatch);
9395
store.dispatch(selectApp({ appId: 'legacy' }));
9496
};
9597

src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.test.jsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import { AppProvider } from '@edx/frontend-platform/react';
1414

1515
import initializeStore from '../../../../../../store';
1616
import { executeThunk } from '../../../../../../utils';
17-
import { getAppsUrl } from '../../../../data/api';
18-
import { fetchApps } from '../../../../data/thunks';
17+
import { getDiscussionsProvidersUrl } from '../../../../data/api';
18+
import { fetchProviders } from '../../../../data/thunks';
1919
import { legacyApiResponse } from '../../../../factories/mockApiResponses';
2020
import OpenedXConfigFormProvider from '../../openedx/OpenedXConfigFormProvider';
2121
import messages from '../../../messages';
@@ -85,8 +85,8 @@ describe('DiscussionTopics', () => {
8585
};
8686

8787
const mockStore = async (mockResponse) => {
88-
axiosMock.onGet(getAppsUrl(courseId)).reply(200, mockResponse);
89-
await executeThunk(fetchApps(courseId), store.dispatch);
88+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, mockResponse);
89+
await executeThunk(fetchProviders(courseId), store.dispatch);
9090
};
9191

9292
test('renders each discussion topic correctly', async () => {

src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/TopicItem.test.jsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import { AppProvider } from '@edx/frontend-platform/react';
2020

2121
import initializeStore from '../../../../../../store';
2222
import { executeThunk } from '../../../../../../utils';
23-
import { getAppsUrl } from '../../../../data/api';
24-
import { fetchApps } from '../../../../data/thunks';
23+
import { getDiscussionsProvidersUrl } from '../../../../data/api';
24+
import { fetchProviders } from '../../../../data/thunks';
2525
import { legacyApiResponse } from '../../../../factories/mockApiResponses';
2626
import messages from '../../../messages';
2727
import TopicItem from './TopicItem';
@@ -91,8 +91,8 @@ describe('TopicItem', () => {
9191
};
9292

9393
const mockStore = async (mockResponse) => {
94-
axiosMock.onGet(getAppsUrl(courseId)).reply(200, mockResponse);
95-
await executeThunk(fetchApps(courseId), store.dispatch);
94+
axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, mockResponse);
95+
await executeThunk(fetchProviders(courseId), store.dispatch);
9696
};
9797

9898
test('displays a collapsible card for discussion topic', async () => {

src/pages-and-resources/discussions/app-config-form/messages.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ const messages = defineMessages({
8787
},
8888
'appName-openedx': {
8989
id: 'authoring.discussions.appConfigForm.appName-openedx',
90-
defaultMessage: 'edX',
90+
defaultMessage: 'edX (NEW)',
9191
description: 'The name of the new edX Discussions app.',
9292
},
9393
divisionByGroup: {

0 commit comments

Comments
 (0)