Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -176,16 +176,12 @@ Directory Structure

* ``app.ts``

* The ``App`` object consumed by ``site.config.*.tsx`` — wires ``appId``, ``routes``, and ``providers`` for frontend-base to register.
* The ``App`` object consumed by ``site.config.*.tsx`` - wires ``appId``, ``routes``, and ``slots`` for frontend-base to register.

* ``routes.tsx``

* React Router route definitions for the app.

* ``providers.tsx``

* App-scoped context providers registered with frontend-base.

* ``slots``

* Slots this app exposes for host sites to plug into.
Expand Down
2 changes: 0 additions & 2 deletions src/app.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import { App } from '@openedx/frontend-base';
import { appId } from './constants';
import routes from './routes';
import providers from './providers';
import slots from './slots';

const app: App = {
appId,
routes,
providers,
slots,
};

Expand Down
5 changes: 2 additions & 3 deletions src/components/BulkManagementHistoryView/data/apiHook.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { useQuery } from '@tanstack/react-query';

import { useAssignmentTypes, useCourseIdWithGate } from '@src/data/apiHook';
import { useGradebookQuery } from '@src/data/query';

import { getBulkOperationHistory } from './api';
import { bulkOperationHistoryQueryKeys } from './queryKeys';
Expand All @@ -18,7 +17,7 @@ const EMPTY_ARRAY: never[] = [];
export const useBulkOperationHistory = (
courseId: string,
{ enabled = true }: { enabled?: boolean } = {},
) => useQuery({
) => useGradebookQuery({
queryKey: bulkOperationHistoryQueryKeys.byCourse(courseId),
queryFn: () => getBulkOperationHistory(courseId),
enabled: !!courseId && enabled,
Expand Down
6 changes: 3 additions & 3 deletions src/components/GradebookFilters/data/apiHook.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useQuery } from '@tanstack/react-query';
import { useGradebookQuery } from '@src/data/query';

import { getCohorts, getTracks } from './api';
import { cohortsQueryKeys, tracksQueryKeys } from './queryKeys';
Expand All @@ -11,7 +11,7 @@ import { cohortsQueryKeys, tracksQueryKeys } from './queryKeys';
export const useCohorts = (
courseId: string,
{ enabled = true }: { enabled?: boolean } = {},
) => useQuery({
) => useGradebookQuery({
queryKey: cohortsQueryKeys.byCourse(courseId),
queryFn: () => getCohorts(courseId),
enabled: !!courseId && enabled,
Expand All @@ -25,7 +25,7 @@ export const useCohorts = (
export const useTracks = (
courseId: string,
{ enabled = true }: { enabled?: boolean } = {},
) => useQuery({
) => useGradebookQuery({
queryKey: tracksQueryKeys.byCourse(courseId),
queryFn: () => getTracks(courseId),
enabled: !!courseId && enabled,
Expand Down
11 changes: 7 additions & 4 deletions src/components/GradesView/data/apiHook.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { useMemo } from 'react';
import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query';
import { useMutation, useQueryClient } from '@tanstack/react-query';

import lms from '@src/data/services/lms';
import { sortAlphaAsc } from '@src/data/formatUtils';
import { filtersSnapshot } from '@src/data/filtersSnapshot';
import { useGradebookUi } from '@src/data/gradebookUiContext';
import { useCourseIdWithGate } from '@src/data/apiHook';
import { useCourseId } from '@src/data/courseIdContext';
import { useGradebookQuery } from '@src/data/query';
import {
trackGradesDisplayed,
trackGradeOverrideSucceeded,
Expand All @@ -19,7 +20,7 @@ import {
import { bulkOperationHistoryQueryKeys } from '@src/components/BulkManagementHistoryView/data/queryKeys';

import { getGradeOverrideHistory, getGrades, getGradesPage } from './api';
import { gradeOverrideHistoryQueryKeys, gradesQueryKeys } from './queryKeys';
import { gradeMutationKeys, gradeOverrideHistoryQueryKeys, gradesQueryKeys } from './queryKeys';
import { buildGradesFetchParams } from './utils';

/**
Expand All @@ -45,7 +46,7 @@ export const useGrades = (
courseId: string,
gradesPageEndpoint: string | null,
{ enabled = true }: { enabled?: boolean } = {},
) => useQuery({
) => useGradebookQuery({
queryKey: [...gradesQueryKeys.byCourse(courseId), gradesPageEndpoint ?? 'base'],
queryFn: async () => {
const data = gradesPageEndpoint
Expand Down Expand Up @@ -143,7 +144,7 @@ export const useGradesData = (): GradesData => {
* is not worth retrying.
*/
export const useGradeOverrideHistory = (subsectionId?: string, userId?: string | number) => (
useQuery({
useGradebookQuery({
queryKey: gradeOverrideHistoryQueryKeys.byCell(subsectionId ?? null, userId ?? null),
queryFn: () => getGradeOverrideHistory(subsectionId as string, userId as string | number),
enabled: !!subsectionId && userId !== undefined && userId !== null,
Expand Down Expand Up @@ -187,6 +188,7 @@ export const useUpdateGrades = () => {
const courseId = useCourseId();
const { setShowSuccess, setGradesPageEndpoint, modalState } = useGradebookUi();
const mutation = useMutation({
mutationKey: gradeMutationKeys.updateGrades,
mutationFn: (updateData: GradeOverrideUpdate[]) => lms.api.updateGradebookData(courseId, updateData),
onSuccess: ({ data }, updateData) => {
trackGradeOverrideSucceeded(courseId, data);
Expand Down Expand Up @@ -238,6 +240,7 @@ export const useSubmitImportGradesButtonData = () => {
setCsvUploadErrors,
} = useGradebookUi();
const mutation = useMutation<unknown, CsvUploadError, FormData>({
mutationKey: gradeMutationKeys.uploadGradeCsv,
mutationFn: (formData) => lms.api.uploadGradeCsv(courseId, formData),
onMutate: () => {
resetCsvUpload();
Expand Down
6 changes: 4 additions & 2 deletions src/components/GradesView/data/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useFilters } from '@src/data/filtersContext';
import { useGradebookUi } from '@src/data/gradebookUiContext';
import { useCanViewGradebook } from '@src/data/apiHook';
import { useCourseId } from '@src/data/courseIdContext';
import { BASE_KEY } from '@src/data/queryKeys';
import {
useSelectedCohortEntry,
useSelectedTrackEntry,
Expand Down Expand Up @@ -87,12 +88,13 @@ export const useSelectedAssignmentLabel = () => useSelectedAssignmentData()?.lab
/**
* useShouldShowSpinner()
* The busy indicator: the roles gate combined with the grades query fetching or
* any in-flight mutation (grade override save / CSV upload).
* one of the gradebook's own mutations in flight (grade override save / CSV
* upload).
*/
export const useShouldShowSpinner = () => {
const canViewGradebook = useCanViewGradebook();
const { isFetching } = useGradesData();
const mutatingCount = useIsMutating();
const mutatingCount = useIsMutating({ mutationKey: BASE_KEY });
return canViewGradebook && (isFetching || mutatingCount > 0);
};

Expand Down
6 changes: 6 additions & 0 deletions src/components/GradesView/data/hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useFilters } from '@src/data/filtersContext';
import { useGradebookUi } from '@src/data/gradebookUiContext';
import { useCanViewGradebook } from '@src/data/apiHook';
import { useCourseId } from '@src/data/courseIdContext';
import { BASE_KEY } from '@src/data/queryKeys';
import {
useSelectedCohortEntry, useSelectedTrackEntry,
} from '@src/components/GradebookFilters/data/hooks';
Expand Down Expand Up @@ -217,6 +218,11 @@ describe('GradesView/data hooks', () => {
const { result } = renderHook(useShouldShowSpinner);
expect(result.current).toBe(true);
});

it('only counts the gradebook\'s own mutations', () => {
renderHook(useShouldShowSpinner);
expect(useIsMutatingMock).toHaveBeenCalledWith({ mutationKey: BASE_KEY });
});
});

describe('read-model hooks', () => {
Expand Down
5 changes: 5 additions & 0 deletions src/components/GradesView/data/queryKeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ export const gradesQueryKeys = {
byCourse: (courseId: string) => [...BASE_KEY, courseId, 'grades'] as const,
};

export const gradeMutationKeys = {
updateGrades: [...BASE_KEY, 'updateGrades'] as const,
uploadGradeCsv: [...BASE_KEY, 'uploadGradeCsv'] as const,
};

/** Grade-override history for one learner on one subsection (edit modal). */
export const gradeOverrideHistoryQueryKeys = {
all: [...BASE_KEY, 'gradeOverrideHistory'] as const,
Expand Down
3 changes: 2 additions & 1 deletion src/data/apiHook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ describe('root apiHook', () => {
});

it('returns false when the roles query errors out', async () => {
canViewMock.mockRejectedValue(new Error('boom'));
// 4xx-shaped so the shared retry rule (`query.ts`) doesn't retry it.
canViewMock.mockRejectedValue({ response: { status: 403 } });
const { result } = renderQueryHook(useCanViewGradebook);
await waitFor(() => expect(result.current).toBe(false));
});
Expand Down
7 changes: 3 additions & 4 deletions src/data/apiHook.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { useQuery } from '@tanstack/react-query';

import { getAssignmentTypes, getCanUserViewGradebook } from './api';
import { useCourseId } from './courseIdContext';
import { useGradebookQuery } from './query';
import { assignmentTypesQueryKeys, rolesQueryKeys } from './queryKeys';

/**
Expand All @@ -12,7 +11,7 @@ import { assignmentTypesQueryKeys, rolesQueryKeys } from './queryKeys';
*/
export const useCanUserViewGradebook = () => {
const courseId = useCourseId();
return useQuery({
return useGradebookQuery({
queryKey: rolesQueryKeys.byCourse(courseId),
queryFn: () => getCanUserViewGradebook(courseId),
enabled: !!courseId,
Expand All @@ -28,7 +27,7 @@ export const useCanUserViewGradebook = () => {
export const useAssignmentTypes = (
courseId: string,
{ enabled = true }: { enabled?: boolean } = {},
) => useQuery({
) => useGradebookQuery({
queryKey: assignmentTypesQueryKeys.byCourse(courseId),
queryFn: () => getAssignmentTypes(courseId),
enabled: !!courseId && enabled,
Expand Down
60 changes: 60 additions & 0 deletions src/data/query.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { useQuery } from '@tanstack/react-query';
import { renderHook } from '@testing-library/react';

import { gradebookQueryOptions, retryUnlessClientError, useGradebookQuery } from './query';

jest.mock('@tanstack/react-query', () => ({
...jest.requireActual('@tanstack/react-query'),
useQuery: jest.fn(),
}));

const useQueryMock = useQuery as jest.Mock;

describe('gradebookQueryOptions', () => {
it('uses a 5-minute staleTime and disables refetchOnWindowFocus', () => {
expect(gradebookQueryOptions.staleTime).toBe(5 * 60 * 1000);
expect(gradebookQueryOptions.refetchOnWindowFocus).toBe(false);
expect(gradebookQueryOptions.retry).toBe(retryUnlessClientError);
});

describe('retryUnlessClientError', () => {
it('does not retry 4xx client errors', () => {
expect(retryUnlessClientError(0, { response: { status: 400 } })).toBe(false);
expect(retryUnlessClientError(0, { response: { status: 404 } })).toBe(false);
expect(retryUnlessClientError(0, { response: { status: 499 } })).toBe(false);
});

it('retries 5xx server errors up to 3 attempts', () => {
expect(retryUnlessClientError(0, { response: { status: 500 } })).toBe(true);
expect(retryUnlessClientError(2, { response: { status: 502 } })).toBe(true);
expect(retryUnlessClientError(3, { response: { status: 500 } })).toBe(false);
});

it('retries network errors (no response) up to 3 attempts', () => {
expect(retryUnlessClientError(0, new Error('network'))).toBe(true);
expect(retryUnlessClientError(3, new Error('network'))).toBe(false);
});
});
});

describe('useGradebookQuery', () => {
const queryFn = jest.fn();

beforeEach(() => {
jest.clearAllMocks();
});

it('applies the gradebook options to the query', () => {
renderHook(() => useGradebookQuery({ queryKey: ['test'], queryFn }));
expect(useQueryMock).toHaveBeenCalledWith({
...gradebookQueryOptions,
queryKey: ['test'],
queryFn,
});
});

it('lets the caller override them', () => {
renderHook(() => useGradebookQuery({ queryKey: ['test'], queryFn, retry: false }));
expect(useQueryMock).toHaveBeenCalledWith(expect.objectContaining({ retry: false }));
});
});
38 changes: 38 additions & 0 deletions src/data/query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import {
DefaultError, useQuery, UseQueryOptions, UseQueryResult,
} from '@tanstack/react-query';

/** Subset of an Axios-style error we inspect, to avoid depending on axios types. */
interface HttpErrorLike {
response?: { status?: number };
}

const isClientError = (error: unknown): boolean => {
const status = (error as HttpErrorLike)?.response?.status;
return typeof status === 'number' && status >= 400 && status < 500;
};

export const retryUnlessClientError = (failureCount: number, error: unknown): boolean => {
if (isClientError(error)) {
return false;
}
return failureCount < 3;
};

export const gradebookQueryOptions = {
staleTime: 5 * 60 * 1000, // 5 minutes
refetchOnWindowFocus: false,
retry: retryUnlessClientError,
} as const;

/**
* These can't be defaults on a QueryClient: app providers wrap the whole site,
* so an app-owned client would impose them on every other app sharing it. The
* shell's client sets no defaults, so a query that skips this wrapper silently
* falls back to stock React Query behavior.
*/
export function useGradebookQuery<TQueryFnData, TError = DefaultError, TData = TQueryFnData>(
options: UseQueryOptions<TQueryFnData, TError, TData>,
): UseQueryResult<TData, TError> {
return useQuery<TQueryFnData, TError, TData>({ ...gradebookQueryOptions, ...options });
}
37 changes: 0 additions & 37 deletions src/data/queryClient.test.ts

This file was deleted.

Loading
Loading