Skip to content

Commit ee6006e

Browse files
authored
refactor(course-outline): improve query cache handling and remove redux thunks (#2884)
- Centralizes and reuses query cache handling logic: Introduces a `ParentIds` type (src/generic/types.ts) and standardizes its use across data/API hooks for updating or invalidating parent/child query caches. - Ensures cache coherence using `cancelQueries` before updating query data: Before calling `setQueryData` for any block, any inflight queries are cancelled to prevent race conditions and stale UI. - Simplifies post-sync/invalidation flows: Removes Redux thunk usages in favor of direct query invalidations using React Query APIs within course outline cards, sidebars, publish modal, and `unlinkmodal`. - Refactors data types for clarity: Splits XBlock into `XBlockBase` and derived interfaces so the presence of `childInfo` is explicit. - Cleans up redundant code and props: Removes unnecessary `memoization`, `useDispatch` imports, and duplicate logic in React components.
1 parent 8f8c6d8 commit ee6006e

16 files changed

Lines changed: 141 additions & 105 deletions

File tree

src/course-outline/data/apiHooks.ts

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { containerComparisonQueryKeys } from '@src/container-comparison/data/apiHooks';
2-
import type { XBlock } from '@src/data/types';
2+
import type { XBlockBase, XblockChildInfo } from '@src/data/types';
33
import { getCourseKey } from '@src/generic/key-utils';
44
import { handleResponseErrors } from '@src/generic/saving-error-alert';
5+
import { ParentIds } from '@src/generic/types';
56
import {
67
QueryClient,
78
skipToken, useMutation, useQuery, useQueryClient,
@@ -41,22 +42,22 @@ export const courseOutlineQueryKeys = {
4142
],
4243
};
4344

44-
type ParentIds = {
45-
/** This id will be used to invalidate data of parent subsection */
46-
subsectionId?: string;
47-
/** This id will be used to invalidate data of parent section */
48-
sectionId?: string;
49-
};
50-
5145
/**
5246
* Invalidate parent Subsection and Section data.
47+
*
48+
* This function ensures that cached data for parent subsection and section is invalidated
49+
* when child items are created, updated, or deleted.
50+
*
51+
* Priority:
52+
* 1. If sectionId exists, invalidate section data which also updates all children block data
53+
* 2. Else If subsectionId exists, invalidate subsection data
5354
*/
5455
const invalidateParentQueries = async (queryClient: QueryClient, variables: ParentIds) => {
55-
if (variables.subsectionId) {
56-
await queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseItemId(variables.subsectionId) });
57-
}
5856
if (variables.sectionId) {
5957
await queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseItemId(variables.sectionId) });
58+
} else if (variables.subsectionId) {
59+
// istanbul ignore next
60+
await queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseItemId(variables.subsectionId) });
6061
}
6162
};
6263

@@ -66,6 +67,9 @@ type CreateCourseXBlockMutationProps = CreateCourseXBlockType & ParentIds;
6667
* Hook to create an XBLOCK in a course .
6768
* The `locator` is the ID of the parent block where this new XBLOCK should be created.
6869
* Can also be used to import block from library by passing `libraryContentKey` in request body
70+
*
71+
* @param callback - Optional function called after successful creation to handle additional logic
72+
* @returns Mutation object for creating course blocks
6973
*/
7074
export const useCreateCourseBlock = (
7175
callback?: ((locator: string, parentLocator: string) => Promise<void>),
@@ -75,7 +79,6 @@ export const useCreateCourseBlock = (
7579
mutationFn: (variables: CreateCourseXBlockMutationProps) => createCourseXblock(variables),
7680
onSettled: async (data: { locator: string; }, _err, variables) => {
7781
await callback?.(data.locator, variables.parentLocator);
78-
queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseItemId(variables.parentLocator) });
7982
queryClient.invalidateQueries({
8083
queryKey: courseOutlineQueryKeys.courseDetails(getCourseKey(data.locator)),
8184
});
@@ -84,13 +87,34 @@ export const useCreateCourseBlock = (
8487
});
8588
};
8689

87-
export const useCourseItemData = <T = XBlock>(itemId?: string, initialData?: T, enabled: boolean = true) => (
88-
useQuery({
90+
export const useCourseItemData = <T extends XBlockBase>(itemId?: string, initialData?: T, enabled: boolean = true) => {
91+
const queryClient = useQueryClient();
92+
return useQuery<T>({
8993
initialData,
9094
queryKey: courseOutlineQueryKeys.courseItemId(itemId),
91-
queryFn: enabled && itemId ? () => getCourseItem<T>(itemId!) : skipToken,
92-
})
93-
);
95+
queryFn: enabled && itemId ? async () => {
96+
const data = await getCourseItem<T>(itemId!);
97+
// If the container has children blocks, update children react-query cache
98+
// data without hitting the API as each xblock call returns its children information as well.
99+
if ('childInfo' in data) {
100+
// This could mean that data is of a section or subsection
101+
(data.childInfo as XblockChildInfo).children.forEach(async (child) => {
102+
await queryClient.cancelQueries({ queryKey: courseOutlineQueryKeys.courseItemId(child.id) });
103+
queryClient.setQueryData(courseOutlineQueryKeys.courseItemId(child.id), child);
104+
if ('childInfo' in child) {
105+
// This means that the data is of section and so its children subsections also
106+
// have children i.e. units
107+
(child.childInfo as XblockChildInfo).children.forEach(async (grandChild) => {
108+
await queryClient.cancelQueries({ queryKey: courseOutlineQueryKeys.courseItemId(grandChild.id) });
109+
queryClient.setQueryData(courseOutlineQueryKeys.courseItemId(grandChild.id), grandChild);
110+
});
111+
}
112+
});
113+
}
114+
return data;
115+
} : skipToken,
116+
});
117+
};
94118

95119
export const useCourseDetails = (courseId?: string, enabled: boolean = true) => (
96120
useQuery({
@@ -99,6 +123,15 @@ export const useCourseDetails = (courseId?: string, enabled: boolean = true) =>
99123
})
100124
);
101125

126+
/**
127+
* Hook to update the display name of a course block.
128+
*
129+
* This mutation updates the display name of a course item and invalidates relevant cache queries
130+
* to ensure the UI reflects the changes.
131+
*
132+
* @param courseId - The ID of the course containing the item
133+
* @returns Mutation object for updating course block names
134+
*/
102135
export const useUpdateCourseBlockName = (courseId: string) => {
103136
const queryClient = useQueryClient();
104137
return useMutation({
@@ -107,10 +140,9 @@ export const useUpdateCourseBlockName = (courseId: string) => {
107140
displayName: string;
108141
} & ParentIds) => editItemDisplayName({ itemId: variables.itemId, displayName: variables.displayName }),
109142
onSuccess: async (_data, variables) => {
110-
await queryClient.invalidateQueries({ queryKey: containerComparisonQueryKeys.course(courseId) });
111-
await queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseDetails(courseId) });
112-
await queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseItemId(variables.itemId) });
113143
await invalidateParentQueries(queryClient, variables);
144+
queryClient.invalidateQueries({ queryKey: containerComparisonQueryKeys.course(courseId) });
145+
queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseDetails(courseId) });
114146
},
115147
});
116148
};
@@ -122,9 +154,8 @@ export const usePublishCourseItem = () => {
122154
itemId: string;
123155
} & ParentIds) => publishCourseItem(variables.itemId),
124156
onSettled: (_data, _err, variables) => {
125-
queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseItemId(variables.itemId) });
126-
queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseDetails(getCourseKey(variables.itemId)) });
127157
invalidateParentQueries(queryClient, variables).catch((e) => handleResponseErrors(e));
158+
queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseDetails(getCourseKey(variables.itemId)) });
128159
},
129160
});
130161
};

src/course-outline/hooks.jsx

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -169,24 +169,13 @@ const useCourseOutline = ({ courseId }) => {
169169
return;
170170
}
171171

172-
await unlinkDownstream(currentUnlinkModalData.value.id, {
172+
await unlinkDownstream({
173+
downstreamBlockId: currentUnlinkModalData.value.id,
174+
sectionId: currentUnlinkModalData.sectionId,
175+
subsectionId: currentUnlinkModalData.subsectionId,
176+
}, {
173177
onSuccess: () => {
174178
closeUnlinkModal();
175-
// istanbul ignore next
176-
// refresh child block data
177-
currentUnlinkModalData.value.childInfo?.children.forEach((block) => {
178-
queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseItemId(block.id) });
179-
block.childInfo?.children.forEach(({ id: blockId }) => {
180-
queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseItemId(blockId) });
181-
});
182-
});
183-
// refresh parent blocks data
184-
queryClient.invalidateQueries({
185-
queryKey: courseOutlineQueryKeys.courseItemId(currentUnlinkModalData?.sectionId),
186-
});
187-
queryClient.invalidateQueries({
188-
queryKey: courseOutlineQueryKeys.courseItemId(currentUnlinkModalData?.subsectionId),
189-
});
190179
},
191180
});
192181
}, [currentUnlinkModalData, unlinkDownstream, closeUnlinkModal]);

src/course-outline/outline-sidebar/OutlineSidebarContext.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export const OutlineSidebarProvider = ({ children }: { children?: React.ReactNod
144144
setCurrentFlow(flow);
145145
}, [setCurrentFlow, setCurrentPageKey]);
146146

147-
const { data: currentItemData } = useCourseItemData(selectedContainerState?.currentId);
147+
const { data: currentItemData } = useCourseItemData<XBlock>(selectedContainerState?.currentId);
148148
const sectionsList = useSelector(getSectionsList);
149149

150150
/** Stores last section that allows adding subsections inside it. */

src/course-outline/outline-sidebar/info-sidebar/InfoSection.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { SchoolOutline, Tag } from '@openedx/paragon/icons';
44
import { ContentTagsDrawerSheet, ContentTagsSnippet } from '@src/content-tags-drawer';
55
import { invalidateLinksQuery } from '@src/course-libraries/data/apiHooks';
66
import { courseOutlineQueryKeys, useCourseItemData } from '@src/course-outline/data/apiHooks';
7-
import { fetchCourseSectionQuery } from '@src/course-outline/data/thunk';
87
import { useOutlineSidebarContext } from '@src/course-outline/outline-sidebar/OutlineSidebarContext';
98
import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
109
import { ComponentCountSnippet, getItemIcon } from '@src/generic/block-type-utils';
@@ -44,14 +43,14 @@ export const InfoSection = ({ itemId }: Props) => {
4443
*/
4544
// istanbul ignore next
4645
const handleOnPostChangeSync = useCallback(() => {
46+
// invalidating section data will update all children blocks as well.
4747
if (selectedContainerState?.sectionId) {
48-
dispatch(fetchCourseSectionQuery([selectedContainerState.sectionId]));
48+
queryClient.invalidateQueries({
49+
queryKey: courseOutlineQueryKeys.courseItemId(selectedContainerState?.sectionId),
50+
});
4951
}
5052
if (courseId) {
5153
invalidateLinksQuery(queryClient, courseId);
52-
queryClient.invalidateQueries({
53-
queryKey: courseOutlineQueryKeys.course(courseId),
54-
});
5554
}
5655
}, [dispatch, selectedContainerState, queryClient, courseId]);
5756

src/course-outline/publish-modal/PublishModal.tsx

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
/* eslint-disable import/named */
2-
import React, { useMemo } from 'react';
2+
import React from 'react';
33
import { useIntl } from '@edx/frontend-platform/i18n';
44
import {
55
ModalDialog,
66
ActionRow,
77
} from '@openedx/paragon';
88

9-
import { courseOutlineQueryKeys, usePublishCourseItem } from '@src/course-outline/data/apiHooks';
9+
import { usePublishCourseItem } from '@src/course-outline/data/apiHooks';
1010
import type { UnitXBlock, XBlock } from '@src/data/types';
1111
import LoadingButton from '@src/generic/loading-button';
1212
import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
13-
import { useQueryClient } from '@tanstack/react-query';
1413
import messages from './messages';
1514
import { COURSE_BLOCK_NAMES } from '../constants';
1615

@@ -26,22 +25,6 @@ const PublishModal = () => {
2625
: undefined;
2726
const children: Array<XBlock | UnitXBlock> | undefined = childInfo?.children;
2827
const publishMutation = usePublishCourseItem();
29-
const queryClient = useQueryClient();
30-
31-
const childrenIds = useMemo(() => children?.reduce((
32-
result: string[],
33-
current: XBlock | UnitXBlock,
34-
): string[] => {
35-
let temp = [...result];
36-
if ('childInfo' in current) {
37-
const grandChildren = current.childInfo.children.filter((child) => child.hasChanges);
38-
temp = [...temp, ...grandChildren.map((child) => child.id)];
39-
}
40-
if (current.hasChanges) {
41-
temp.push(current.id);
42-
}
43-
return temp;
44-
}, []), [children]);
4528

4629
const onPublishSubmit = async () => {
4730
if (id) {
@@ -52,10 +35,6 @@ const PublishModal = () => {
5235
}, {
5336
onSettled: () => {
5437
closePublishModal();
55-
// Update query client to refresh the data of all children blocks
56-
childrenIds?.forEach((blockId) => {
57-
queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseItemId(blockId) });
58-
});
5938
},
6039
});
6140
}

src/course-outline/section-card/SectionCard.tsx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {
22
useContext, useEffect, useState, useRef, useCallback, ReactNode, useMemo,
33
} from 'react';
4-
import { useDispatch } from 'react-redux';
54
import {
65
Bubble, Button, useToggle,
76
} from '@openedx/paragon';
@@ -14,7 +13,6 @@ import SortableItem from '@src/course-outline/drag-helper/SortableItem';
1413
import { DragContext } from '@src/course-outline/drag-helper/DragContextProvider';
1514
import TitleButton from '@src/course-outline/card-header/TitleButton';
1615
import XBlockStatus from '@src/course-outline/xblock-status/XBlockStatus';
17-
import { fetchCourseSectionQuery } from '@src/course-outline/data/thunk';
1816
import { getItemStatus, getItemStatusBorder, scrollToElement } from '@src/course-outline/utils';
1917
import OutlineAddChildButtons from '@src/course-outline/OutlineAddChildButtons';
2018
import { ContainerType } from '@src/generic/key-utils';
@@ -60,7 +58,6 @@ const SectionCard = ({
6058
resetScrollState,
6159
}: SectionCardProps) => {
6260
const currentRef = useRef(null);
63-
const dispatch = useDispatch();
6461
const { activeId, overId } = useContext(DragContext);
6562
const { selectedContainerState, openContainerInfoSidebar, setSelectedContainerState } = useOutlineSidebarContext();
6663
const [searchParams] = useSearchParams();
@@ -111,6 +108,10 @@ const SectionCard = ({
111108
useEffect(() => {
112109
// istanbul ignore if
113110
if (moment(initialData.editedOnRaw).isAfter(moment(section.editedOnRaw))) {
111+
queryClient.cancelQueries({
112+
queryKey: courseOutlineQueryKeys.courseItemId(initialData.id),
113+
// eslint-disable-next-line no-console
114+
}).catch((error) => console.error('Error cancelling query:', error));
114115
queryClient.setQueryData(courseOutlineQueryKeys.courseItemId(initialData.id), initialData);
115116
}
116117
}, [initialData, section]);
@@ -167,11 +168,13 @@ const SectionCard = ({
167168
}, [locatorId, setIsExpanded]);
168169

169170
const handleOnPostChangeSync = useCallback(() => {
170-
dispatch(fetchCourseSectionQuery([section.id]));
171+
queryClient.invalidateQueries({
172+
queryKey: courseOutlineQueryKeys.courseItemId(section.id),
173+
});
171174
if (courseId) {
172175
invalidateLinksQuery(queryClient, courseId);
173176
}
174-
}, [dispatch, section, courseId, queryClient]);
177+
}, [section, courseId, queryClient]);
175178

176179
// re-create actions object for customizations
177180
const actions = { ...sectionActions };

src/course-outline/subsection-card/SubsectionCard.tsx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {
22
useContext, useEffect, useState, useRef, useCallback, ReactNode, useMemo,
33
} from 'react';
4-
import { useDispatch } from 'react-redux';
54
import { useSearchParams } from 'react-router-dom';
65
import { useIntl } from '@edx/frontend-platform/i18n';
76
import { useToggle } from '@openedx/paragon';
@@ -15,7 +14,6 @@ import SortableItem from '@src/course-outline/drag-helper/SortableItem';
1514
import { DragContext } from '@src/course-outline/drag-helper/DragContextProvider';
1615
import { useClipboard, PasteComponent } from '@src/generic/clipboard';
1716
import TitleButton from '@src/course-outline/card-header/TitleButton';
18-
import { fetchCourseSectionQuery } from '@src/course-outline/data/thunk';
1917
import XBlockStatus from '@src/course-outline/xblock-status/XBlockStatus';
2018
import { getItemStatus, getItemStatusBorder, scrollToElement } from '@src/course-outline/utils';
2119
import { ContainerType } from '@src/generic/key-utils';
@@ -65,7 +63,6 @@ const SubsectionCard = ({
6563
}: SubsectionCardProps) => {
6664
const currentRef = useRef(null);
6765
const intl = useIntl();
68-
const dispatch = useDispatch();
6966
const { activeId, overId } = useContext(DragContext);
7067
const { selectedContainerState, openContainerInfoSidebar, setSelectedContainerState } = useOutlineSidebarContext();
7168
const [searchParams] = useSearchParams();
@@ -146,6 +143,10 @@ const SubsectionCard = ({
146143
useEffect(() => {
147144
// istanbul ignore if
148145
if (moment(initialData.editedOnRaw).isAfter(moment(subsection.editedOnRaw))) {
146+
queryClient.cancelQueries({
147+
queryKey: courseOutlineQueryKeys.courseItemId(initialData.id),
148+
// eslint-disable-next-line no-console
149+
}).catch((error) => console.error('Error cancelling query:', error));
149150
queryClient.setQueryData(courseOutlineQueryKeys.courseItemId(initialData.id), initialData);
150151
}
151152
}, [initialData, subsection]);
@@ -171,11 +172,13 @@ const SubsectionCard = ({
171172
};
172173

173174
const handleOnPostChangeSync = useCallback(() => {
174-
dispatch(fetchCourseSectionQuery([section.id]));
175+
queryClient.invalidateQueries({
176+
queryKey: courseOutlineQueryKeys.courseItemId(section.id),
177+
});
175178
if (courseId) {
176179
invalidateLinksQuery(queryClient, courseId);
177180
}
178-
}, [dispatch, section, queryClient, courseId]);
181+
}, [section, queryClient, courseId]);
179182

180183
const handleSubsectionMoveUp = () => {
181184
onOrderChange(section, moveUpDetails);

0 commit comments

Comments
 (0)