Skip to content

Commit 8b3ac9f

Browse files
navinkarkeranavinkarkera
committed
refactor: simplify and merge duplicate unit configure options
1 parent d0751c9 commit 8b3ac9f

17 files changed

Lines changed: 165 additions & 98 deletions

File tree

‎src/course-outline/data/api.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,9 @@ export async function configureCourseUnit(variables: ConfigureUnitData): Promise
276276
...(variables.type === PUBLISH_TYPES.republish ? {
277277
metadata: {
278278
visible_to_staff_only: variables.isVisibleToStaffOnly ? true : null,
279-
discussion_enabled: variables.discussionEnabled,
279+
...(variables.discussionEnabled !== undefined && {
280+
discussion_enabled: variables.discussionEnabled,
281+
}),
280282
...(variables.groupAccess != null && { group_access: variables.groupAccess }),
281283
},
282284
} : {}),

‎src/course-outline/data/apiHooks.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ import {
66
ConfigureUnitData,
77
StaticFileNotices,
88
} from '@src/course-outline/data/types';
9+
import { getNotificationMessage } from '@src/course-unit/data/utils';
910
import { createGlobalState } from '@src/data/apiHooks';
1011
import type { XBlockBase, XblockChildInfo } from '@src/data/types';
1112
import { getBlockType, getCourseKey } from '@src/generic/key-utils';
13+
import { useProcessingNotification } from '@src/generic/processing-notification/context';
1214
import { useMutationWithProcessingNotification } from '@src/generic/processing-notification/data/apiHooks';
1315
import { handleResponseErrors } from '@src/generic/saving-error-alert';
1416
import { ParentIds } from '@src/generic/types';
@@ -250,9 +252,14 @@ export const useConfigureSubsection = () => {
250252
};
251253

252254
export const useConfigureUnit = () => {
255+
const { setMessage } = useProcessingNotification();
253256
const queryClient = useQueryClient();
254257
return useMutationWithProcessingNotification({
255258
mutationFn: (variables: ConfigureUnitData & ParentIds) => configureCourseUnit(variables),
259+
onMutate: (variables) => {
260+
const msg = getNotificationMessage(variables.type, variables.isVisibleToStaffOnly, true);
261+
setMessage(msg);
262+
},
256263
onSettled: (_data, _err, variables) => {
257264
queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseDetails(getCourseKey(variables.unitId)) });
258265
invalidateParentQueries(queryClient, variables).catch((e) => handleResponseErrors(e));

‎src/course-outline/data/types.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,9 @@ export interface ConfigureSubsectionData {
120120
export interface ConfigureUnitData {
121121
unitId: string;
122122
isVisibleToStaffOnly: boolean;
123-
type: keyof typeof PUBLISH_TYPES;
123+
type: typeof PUBLISH_TYPES[keyof typeof PUBLISH_TYPES];
124124
groupAccess: Record<string, any> | null,
125-
discussionEnabled: boolean;
125+
discussionEnabled?: boolean;
126126
}
127127

128128
export type StaticFileNotices = {

‎src/course-unit/CourseUnit.test.tsx‎

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ import {
4343
import {
4444
createNewCourseXBlock,
4545
deleteUnitItemQuery,
46-
editCourseUnitVisibilityAndData,
4746
fetchCourseSectionVerticalData,
4847
fetchCourseVerticalChildrenData,
4948
getCourseOutlineInfoQuery,
@@ -373,7 +372,8 @@ describe('<CourseUnit />', () => {
373372
published_by: userName,
374373
},
375374
});
376-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
375+
const publishBtn = await screen.findByRole('button', { name: /Publish/ });
376+
await user.click(publishBtn);
377377

378378
// check if the sidebar status is Published and Live
379379
expect(await screen.findByText(
@@ -415,7 +415,7 @@ describe('<CourseUnit />', () => {
415415
axiosMock
416416
.onGet(getCourseSectionVerticalApiUrl(blockId))
417417
.reply(200, courseSectionVerticalMock);
418-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
418+
await user.click(publishBtn);
419419

420420
expect(await screen.findByTitle(
421421
xblockContainerIframeMessages.xblockIframeTitle.defaultMessage,
@@ -540,7 +540,8 @@ describe('<CourseUnit />', () => {
540540
published_by: userName,
541541
},
542542
});
543-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
543+
const publishBtn = await screen.findByRole('button', { name: /Publish/ });
544+
await user.click(publishBtn);
544545

545546
await waitFor(() => {
546547
// check if the sidebar status is Published and Live
@@ -557,7 +558,7 @@ describe('<CourseUnit />', () => {
557558
axiosMock
558559
.onGet(getCourseSectionVerticalApiUrl(blockId))
559560
.reply(200, courseSectionVerticalMock);
560-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
561+
await user.click(publishBtn);
561562

562563
const xblockIframe = await screen.findByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage);
563564
expect(xblockIframe).toHaveAttribute(
@@ -711,7 +712,8 @@ describe('<CourseUnit />', () => {
711712
},
712713
});
713714

714-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
715+
const publishBtn = await screen.findByRole('button', { name: /Publish/ });
716+
await user.click(publishBtn);
715717

716718
await waitFor(async () => {
717719
const problemButton = screen.getByRole('button', {
@@ -726,7 +728,7 @@ describe('<CourseUnit />', () => {
726728
.onGet(getCourseSectionVerticalApiUrl(blockId))
727729
.reply(200, courseSectionVerticalMock);
728730

729-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
731+
await user.click(publishBtn);
730732

731733
// after creating problem xblock, the sidebar status changes to Draft (unpublished changes)
732734
expect(screen.getByText(
@@ -895,7 +897,7 @@ describe('<CourseUnit />', () => {
895897
},
896898
});
897899

898-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
900+
await user.click(publishButton);
899901

900902
await waitFor(() => {
901903
// check if the sidebar status is Published and Live
@@ -920,7 +922,7 @@ describe('<CourseUnit />', () => {
920922
.onGet(getCourseSectionVerticalApiUrl(blockId))
921923
.reply(200, courseSectionVerticalMock);
922924

923-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
925+
await user.click(publishButton);
924926

925927
// after creating video xblock, the sidebar status changes to Draft (unpublished changes)
926928
expect(screen.getByText(
@@ -974,7 +976,8 @@ describe('<CourseUnit />', () => {
974976
},
975977
});
976978

977-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
979+
const publishButton = await screen.findByRole('button', { name: legacySidebarMessages.actionButtonPublishTitle.defaultMessage });
980+
await user.click(publishButton);
978981

979982
await waitFor(async () => {
980983
// check if the sidebar status is Published and Live
@@ -1004,7 +1007,7 @@ describe('<CourseUnit />', () => {
10041007
.onGet(getCourseSectionVerticalApiUrl(blockId))
10051008
.reply(200, courseSectionVerticalMock);
10061009

1007-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
1010+
await user.click(publishButton);
10081011

10091012
// after creating video xblock, the sidebar status changes to Draft (unpublished changes)
10101013
expect(screen.getByText(
@@ -1138,7 +1141,8 @@ describe('<CourseUnit />', () => {
11381141
},
11391142
});
11401143

1141-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.republish, true), store.dispatch);
1144+
const publishButton = await screen.findByRole('button', { name: legacySidebarMessages.actionButtonPublishTitle.defaultMessage });
1145+
await user.click(publishButton);
11421146

11431147
await waitFor(async () => {
11441148
expect(visibilityCheckbox).toBeChecked();
@@ -1174,7 +1178,7 @@ describe('<CourseUnit />', () => {
11741178
.onGet(getCourseSectionVerticalApiUrl(blockId))
11751179
.reply(200, courseSectionVerticalMock);
11761180

1177-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.republish, null), store.dispatch);
1181+
await user.click(publishButton);
11781182

11791183
expect(visibilityCheckbox).not.toBeChecked();
11801184
expect(draftUnpublishedChangesHeading).toBeInTheDocument();
@@ -1211,7 +1215,8 @@ describe('<CourseUnit />', () => {
12111215
},
12121216
});
12131217

1214-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch);
1218+
const publishButton = await screen.findByRole('button', { name: legacySidebarMessages.actionButtonPublishTitle.defaultMessage });
1219+
await user.click(publishButton);
12151220

12161221
expect(within(courseUnitSidebar)
12171222
.getByText(legacySidebarMessages.sidebarTitlePublishedAndLive.defaultMessage)).toBeInTheDocument();
@@ -1271,11 +1276,8 @@ describe('<CourseUnit />', () => {
12711276
},
12721277
});
12731278

1274-
await executeThunk(editCourseUnitVisibilityAndData(
1275-
blockId,
1276-
PUBLISH_TYPES.discardChanges,
1277-
true,
1278-
), store.dispatch);
1279+
const publishButton = await screen.findByRole('button', { name: legacySidebarMessages.actionButtonPublishTitle.defaultMessage });
1280+
await user.click(publishButton);
12791281

12801282
expect(within(courseUnitSidebar)
12811283
.getByText(legacySidebarMessages.sidebarTitlePublishedNotYetReleased.defaultMessage)).toBeInTheDocument();
@@ -2430,7 +2432,8 @@ describe('<CourseUnit />', () => {
24302432
},
24312433
});
24322434

2433-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.republish, true), store.dispatch);
2435+
const publishButton = await screen.findByRole('button', { name: legacySidebarMessages.actionButtonPublishTitle.defaultMessage });
2436+
await user.click(publishButton);
24342437
// Move to Details
24352438
const detailsTab = screen.getByRole('tab', { name: /details/i });
24362439
await user.click(detailsTab);
@@ -2461,7 +2464,7 @@ describe('<CourseUnit />', () => {
24612464
},
24622465
});
24632466

2464-
await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.republish, false), store.dispatch);
2467+
await user.click(publishButton);
24652468

24662469
// Move to Details
24672470
await user.click(detailsTab);
@@ -2538,13 +2541,8 @@ describe('<CourseUnit />', () => {
25382541
},
25392542
});
25402543

2541-
await executeThunk(editCourseUnitVisibilityAndData(
2542-
blockId,
2543-
PUBLISH_TYPES.republish,
2544-
false,
2545-
null,
2546-
false,
2547-
), store.dispatch);
2544+
const publishButton = await screen.findByRole('button', { name: legacySidebarMessages.actionButtonPublishTitle.defaultMessage });
2545+
await user.click(publishButton);
25482546

25492547
expect(discussionButton).not.toBeChecked();
25502548
});

‎src/course-unit/CourseUnit.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,6 @@ const CourseUnit = () => {
201201
handleTitleEditSubmit,
202202
headerNavigationsActions,
203203
handleTitleEdit,
204-
handleConfigureSubmit,
205204
courseVerticalChildren,
206205
canPasteComponent,
207206
isMoveModalOpen,
@@ -305,7 +304,6 @@ const CourseUnit = () => {
305304
isTitleEditFormOpen={isTitleEditFormOpen}
306305
handleTitleEdit={handleTitleEdit}
307306
handleTitleEditSubmit={handleTitleEditSubmit}
308-
handleConfigureSubmit={handleConfigureSubmit}
309307
/>
310308
)}
311309
breadcrumbs={(
@@ -360,7 +358,6 @@ const CourseUnit = () => {
360358
isUnitVerticalType={isUnitVerticalType}
361359
unitXBlockActions={unitXBlockActions}
362360
courseVerticalChildren={courseVerticalChildren.children}
363-
handleConfigureSubmit={handleConfigureSubmit}
364361
/>
365362
)}
366363
{!readOnly && showPasteXBlock && canPasteComponent && isUnitVerticalType && sharedClipboardData

‎src/course-unit/data/apiHooks.ts‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
1-
import { useMutation } from '@tanstack/react-query';
1+
import { useConfigureUnit } from '@src/course-outline/data/apiHooks';
2+
import { ConfigureUnitData } from '@src/course-outline/data/types';
3+
import { fetchCourseSectionVerticalDataSuccess, updateCourseVerticalChildren } from '@src/course-unit/data/slice';
4+
import { ParentIds } from '@src/generic/types';
5+
import { DefaultError, useMutation, UseMutationOptions } from '@tanstack/react-query';
6+
import { useDispatch } from 'react-redux';
27

38
import {
49
acceptLibraryBlockChanges,
10+
getCourseContainerChildren,
11+
getVerticalData,
512
ignoreLibraryBlockChanges,
613
} from './api';
714

@@ -20,3 +27,28 @@ export const useAcceptLibraryBlockChanges = () => useMutation({
2027
export const useIgnoreLibraryBlockChanges = () => useMutation({
2128
mutationFn: ignoreLibraryBlockChanges,
2229
});
30+
31+
/**
32+
* Wrapper around useConfigureUnit that updates unit data after processing
33+
*/
34+
export const useConfigureUnitWithPageUpdates = () => {
35+
const mutationFn = useConfigureUnit();
36+
const dispatch = useDispatch();
37+
return {
38+
...mutationFn,
39+
mutate: (mutationArgs: ConfigureUnitData & ParentIds, options?: UseMutationOptions<
40+
object,
41+
DefaultError,
42+
ConfigureUnitData & ParentIds
43+
>) => mutationFn.mutate(mutationArgs, {
44+
...options,
45+
onSuccess: async (...onMutateArgs) => {
46+
const courseSectionVerticalData = await getVerticalData(onMutateArgs[1].unitId);
47+
dispatch(fetchCourseSectionVerticalDataSuccess(courseSectionVerticalData));
48+
const courseVerticalChildrenData = await getCourseContainerChildren(onMutateArgs[1].unitId);
49+
dispatch(updateCourseVerticalChildren(courseVerticalChildrenData));
50+
options?.onSuccess?.(...onMutateArgs);
51+
},
52+
}),
53+
};
54+
}

‎src/course-unit/data/thunk.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,11 @@ import {
2828
updateLoadingCourseSectionVerticalDataStatus,
2929
updateCourseVerticalChildren,
3030
updateCourseVerticalChildrenLoadingStatus,
31-
updateQueryPendingStatus,
3231
fetchStaticFileNoticesSuccess,
3332
updateCourseOutlineInfo,
3433
updateCourseOutlineInfoLoadingStatus,
3534
updateMovedXBlockParams,
3635
} from './slice';
37-
import { getNotificationMessage } from './utils';
3836

3937
export function fetchCourseSectionVerticalData(courseId, sequenceId) {
4038
return async (dispatch) => {

‎src/course-unit/header-title/HeaderTitle.tsx‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ import { getCourseUnitData } from '../data/selectors';
1616
import { updateQueryPendingStatus } from '../data/slice';
1717
import messages from './messages';
1818
import { isUnitPageNewDesignEnabled } from '../utils';
19+
import { useIframe } from '@src/generic/hooks/context/hooks';
20+
import { messageTypes, PUBLISH_TYPES } from '@src/course-unit/constants';
21+
import { useConfigureUnitWithPageUpdates } from '@src/course-unit/data/apiHooks';
1922

2023
type HeaderTitleProps = {
2124
unitTitle: string;
2225
isTitleEditFormOpen: boolean;
2326
handleTitleEdit: () => void;
2427
handleTitleEditSubmit: (title: string) => void;
25-
handleConfigureSubmit: (variables: ConfigureUnitData & { closeModalFn?: () => void }) => void;
2628
};
2729

2830
/**
@@ -37,7 +39,6 @@ const HeaderTitle = ({
3739
isTitleEditFormOpen,
3840
handleTitleEdit,
3941
handleTitleEditSubmit,
40-
handleConfigureSubmit,
4142
}: HeaderTitleProps) => {
4243
const intl = useIntl();
4344
const dispatch = useDispatch();
@@ -51,11 +52,19 @@ const HeaderTitle = ({
5152
COURSE_BLOCK_NAMES.component.id,
5253
].includes(currentItemData.category);
5354

55+
const configureFn = useConfigureUnitWithPageUpdates();
56+
const { sendMessageToIframe } = useIframe();
5457
const onConfigureSubmit = (variables: Omit<ConfigureUnitData, 'unitId'>) => {
55-
handleConfigureSubmit({
58+
configureFn.mutate({
5659
...variables,
60+
type: PUBLISH_TYPES.republish,
5761
unitId: currentItemData.id,
58-
closeModalFn: closeConfigureModal,
62+
}, {
63+
onSuccess: () => sendMessageToIframe(
64+
messageTypes.completeManageXBlockAccess,
65+
{ locator: currentItemData.id }
66+
),
67+
onSettled: () => closeConfigureModal(),
5968
});
6069
};
6170

0 commit comments

Comments
 (0)