Skip to content

Commit 625d241

Browse files
committed
fix: nits on the code and tests
1 parent 22b3f3e commit 625d241

3 files changed

Lines changed: 47 additions & 30 deletions

File tree

src/library-authoring/data/apiHooks.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,7 +1030,7 @@ export const useMigrationInfo = (sourcesKeys: string[], enabled: boolean = true)
10301030
/**
10311031
* Returns the draft history of a library block.
10321032
*/
1033-
export const useLibraryBlockDraftHistory = (usageKey: string | undefined) => (
1033+
export const useLibraryBlockDraftHistory = (usageKey?: string) => (
10341034
useQuery({
10351035
queryKey: xblockQueryKeys.draftHistory(usageKey!),
10361036
queryFn: usageKey ? () => api.getLibraryBlockDraftHistory(usageKey) : skipToken,
@@ -1040,7 +1040,7 @@ export const useLibraryBlockDraftHistory = (usageKey: string | undefined) => (
10401040
/**
10411041
* Returns the publish history of a library block.
10421042
*/
1043-
export const useLibraryBlockPublishHistory = (usageKey: string | undefined) => (
1043+
export const useLibraryBlockPublishHistory = (usageKey?: string) => (
10441044
useQuery({
10451045
queryKey: xblockQueryKeys.publishHistory(usageKey!),
10461046
queryFn: usageKey ? () => api.getLibraryBlockPublishHistory(usageKey) : skipToken,
@@ -1051,8 +1051,8 @@ export const useLibraryBlockPublishHistory = (usageKey: string | undefined) => (
10511051
* Returns the entries for a publish history group of a library block.
10521052
*/
10531053
export const useLibraryBlockPublishHistoryEntries = (
1054-
usageKey: string | undefined,
1055-
publishGroupId: string | undefined,
1054+
usageKey?: string,
1055+
publishGroupId?: string,
10561056
enabled: boolean = true,
10571057
) => (
10581058
useQuery({
@@ -1064,7 +1064,7 @@ export const useLibraryBlockPublishHistoryEntries = (
10641064
/**
10651065
* Returns the creation entry for a library block.
10661066
*/
1067-
export const useLibraryBlockCreationEntry = (usageKey: string | undefined) => (
1067+
export const useLibraryBlockCreationEntry = (usageKey?: string ) => (
10681068
useQuery({
10691069
queryKey: xblockQueryKeys.creationEntry(usageKey!),
10701070
queryFn: usageKey ? () => api.getLibraryBlockCreationEntry(usageKey) : skipToken,
Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1+
import { userEvent } from '@testing-library/user-event';
12
import {
23
initializeMocks,
34
render,
45
screen,
56
waitFor,
6-
fireEvent,
7+
findByDeepTextContent,
78
} from '@src/testUtils';
89

910
import {
1011
mockLibraryBlockDraftHistory,
1112
mockLibraryBlockPublishHistory,
1213
mockLibraryBlockPublishHistoryEntries,
1314
mockLibraryBlockCreationEntry,
14-
} from '../../data/api.mocks';
15+
} from '@src/library-authoring/data/api.mocks';
1516
import { HistoryComponentLog } from './HistoryLog';
1617

1718
mockLibraryBlockDraftHistory.applyMock();
@@ -23,19 +24,6 @@ const renderComponent = (componentId: string) => render(
2324
<HistoryComponentLog componentId={componentId} displayName="Test Component" />,
2425
);
2526

26-
/**
27-
* Finds the innermost element whose full textContent (normalized whitespace) matches the given regex.
28-
* Useful when text is split across child elements (e.g. by an icon).
29-
* Requiring that no direct child also matches ensures we get the deepest matching element.
30-
*/
31-
const findByTextContent = (pattern: RegExp) => screen.findByText((_, el) => {
32-
if (!el) return false;
33-
const normalizedText = (el.textContent ?? '').replace(/\s+/g, ' ').trim();
34-
if (!pattern.test(normalizedText)) return false;
35-
return !Array.from(el.children).some(
36-
(child) => pattern.test((child.textContent ?? '').replace(/\s+/g, ' ').trim()),
37-
);
38-
});
3927

4028
describe('<HistoryComponentLog />', () => {
4129
beforeEach(() => {
@@ -48,12 +36,13 @@ describe('<HistoryComponentLog />', () => {
4836
});
4937

5038
it('renders draft history group with entries when they exist', async () => {
39+
const user = userEvent.setup();
5140
renderComponent(mockLibraryBlockCreationEntry.usageKey);
52-
const trigger = await findByTextContent(/Test Component is a draft/i);
41+
const trigger = await findByDeepTextContent(/Test Component is a draft/i);
5342
expect(trigger).toBeInTheDocument();
54-
fireEvent.click(trigger);
55-
expect(await findByTextContent(/test_user_1 edited.*Electron Arcs/i)).toBeInTheDocument();
56-
expect(await findByTextContent(/test_user_2 renamed.*More on Quarks/i)).toBeInTheDocument();
43+
await user.click(trigger);
44+
expect(await findByDeepTextContent(/test_user_1 edited.*Electron Arcs/i)).toBeInTheDocument();
45+
expect(await findByDeepTextContent(/test_user_2 renamed.*More on Quarks/i)).toBeInTheDocument();
5746
});
5847

5948
it('does not render draft history group when there are no draft entries', async () => {
@@ -64,17 +53,18 @@ describe('<HistoryComponentLog />', () => {
6453

6554
it('renders publish history group when one exists', async () => {
6655
renderComponent(mockLibraryBlockCreationEntry.usageKey);
67-
expect(await findByTextContent(/author published.*Protons/i)).toBeInTheDocument();
56+
expect(await findByDeepTextContent(/author published.*Protons/i)).toBeInTheDocument();
6857
expect(await screen.findByText(/5 authors contributed/i)).toBeInTheDocument();
6958
});
7059

7160
it('loads and shows publish history entries after expanding the publish group', async () => {
61+
const user = userEvent.setup();
7262
renderComponent(mockLibraryBlockCreationEntry.usageKey);
7363
expect(await screen.findByText(/5 authors contributed/i)).toBeInTheDocument();
74-
const publishTrigger = await findByTextContent(/author published.*Protons/i);
64+
const publishTrigger = await findByDeepTextContent(/author published.*Protons/i);
7565

76-
fireEvent.click(publishTrigger);
77-
expect(await findByTextContent(/test_user edited.*Protons/i)).toBeInTheDocument();
66+
await user.click(publishTrigger);
67+
expect(await findByDeepTextContent(/test_user edited.*Protons/i)).toBeInTheDocument();
7868
await waitFor(() => expect(screen.queryByText(/5 authors contributed/i)).not.toBeInTheDocument());
7969
});
8070

@@ -86,6 +76,6 @@ describe('<HistoryComponentLog />', () => {
8676

8777
it('always renders the created group with fallback user when createdBy is null', async () => {
8878
renderComponent(mockLibraryBlockCreationEntry.usageKey);
89-
expect(await findByTextContent(/Author created.*Introduction to Testing 1/i)).toBeInTheDocument();
79+
expect(await findByDeepTextContent(/Author created.*Introduction to Testing 1/i)).toBeInTheDocument();
9080
});
9181
});

src/testUtils.tsx

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
1313
import { IntlProvider } from '@edx/frontend-platform/i18n';
1414
import { AppProvider } from '@edx/frontend-platform/react';
1515
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
16-
import { render, type RenderResult } from '@testing-library/react';
16+
import { render, screen, type RenderResult } from '@testing-library/react';
1717
import MockAdapter from 'axios-mock-adapter';
1818
import {
1919
generatePath,
@@ -245,9 +245,36 @@ const getInnerText = (element: Element | null): string => {
245245
.join(' ');
246246
};
247247

248+
/**
249+
* Returns a matcher for `getByText`/`findByText` that checks exact text match and element type.
250+
* - Requires specifying the element's nodeName (e.g. 'P', 'DIV').
251+
* - Uses exact string comparison (no regex).
252+
*
253+
* For partial/regex matching when text is split across child elements,
254+
* use `findByDeepTextContent` instead.
255+
*/
248256
export const matchInnerText = (
249257
nodeName: string,
250258
textToMatch: string,
251259
) => (_: string, element: Element | null) => !!element
252260
&& element.nodeName === nodeName
253261
&& getInnerText(element) === textToMatch;
262+
263+
/**
264+
* Finds the innermost element whose full textContent (normalized whitespace) matches a regex.
265+
* Unlike `matchInnerText`, this:
266+
* - Accepts a `RegExp` for partial/pattern matching.
267+
* - Does NOT require specifying the element type.
268+
* - Normalizes whitespace (collapses multiple spaces/newlines into one).
269+
* - Returns the deepest matching element (excludes elements where a direct child also matches).
270+
*
271+
* Useful when text is split across child elements (e.g. by an icon or inline tag).
272+
*/
273+
export const findByDeepTextContent = (pattern: RegExp) => screen.findByText((_, el) => {
274+
if (!el) return false;
275+
const normalizedText = (el.textContent ?? '').replace(/\s+/g, ' ').trim();
276+
if (!pattern.test(normalizedText)) return false;
277+
return !Array.from(el.children).some(
278+
(child) => pattern.test(((child as Element).textContent ?? '').replace(/\s+/g, ' ').trim()),
279+
);
280+
});

0 commit comments

Comments
 (0)