Skip to content

Commit 935f75b

Browse files
chore: Refactor code into generalized hooks and utils
1 parent 7d42f1e commit 935f75b

File tree

12 files changed

+722
-624
lines changed

12 files changed

+722
-624
lines changed

src/components/Admin/DownloadButtonWrapper.jsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ const DownloadButtonWrapper = ({
1010
}) => {
1111
const { tablesWithData } = useTableData();
1212

13-
// Check if this is a LearnerActivityTable
14-
const isLearnerActivityTable = [
13+
// Identify tables that rely on the useTableData hook and have been migrated to the new DataTable component.
14+
const isTableUsingDataState = [
1515
'learners-active-week',
1616
'learners-inactive-week',
1717
'learners-inactive-month',
@@ -21,7 +21,7 @@ const DownloadButtonWrapper = ({
2121
<DownloadCsvButton
2222
id={tableMetadata.csvButtonId}
2323
fetchMethod={tableMetadata.csvFetchMethod}
24-
disabled={isLearnerActivityTable ? !tablesWithData[actionSlug] : isTableDataMissing(actionSlug)}
24+
disabled={isTableUsingDataState ? !tablesWithData[actionSlug] : isTableDataMissing(actionSlug)}
2525
buttonLabel={downloadButtonLabel}
2626
/>
2727
);

src/components/LearnerActivityTable/LearnerActivityTable.test.jsx

Lines changed: 12 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,13 @@ import { Provider } from 'react-redux';
66
import { mount } from 'enzyme';
77

88
import LearnerActivityTable from '.';
9-
import useCourseEnrollments from './data/hooks/useCourseEnrollments';
10-
import mockUseCourseEnrollments from './data/tests/constants';
9+
import usePaginatedTableData from '../../hooks/usePaginatedTableData';
10+
import { mockUseCourseEnrollments, mockEmptyCourseEnrollmentsData } from './data/tests/constants';
1111

1212
const enterpriseId = 'test-enterprise';
1313
const mockStore = configureMockStore([thunk]);
1414

15-
jest.mock('./data/hooks/useCourseEnrollments.js', () => (
16-
jest.fn().mockReturnValue({})
17-
));
15+
jest.mock('../../hooks/usePaginatedTableData', () => jest.fn());
1816

1917
const store = mockStore({
2018
portalConfiguration: {
@@ -26,33 +24,26 @@ const LearnerActivityTableWrapper = props => (
2624
<MemoryRouter>
2725
<IntlProvider locale="en">
2826
<Provider store={store}>
29-
<LearnerActivityTable
30-
{...props}
31-
/>
27+
<LearnerActivityTable {...props} />
3228
</Provider>
3329
</IntlProvider>
3430
</MemoryRouter>
3531
);
3632

3733
const verifyLearnerActivityTableRendered = (tableId, activity, columnTitles, rowsData) => {
3834
const wrapper = mount(<LearnerActivityTableWrapper id={tableId} activity={activity} />);
39-
4035
const table = wrapper.find('[role="table"]');
4136
const headerColumns = table.find('thead th');
4237
const tableRows = table.find('tbody tr');
4338

44-
// Verify the number of columns
4539
expect(headerColumns).toHaveLength(columnTitles.length);
4640

47-
// Verify column titles
4841
headerColumns.forEach((column, index) => {
4942
expect(column.text()).toContain(columnTitles[index]);
5043
});
5144

52-
// Verify the number of rows
5345
expect(tableRows).toHaveLength(rowsData.length);
5446

55-
// Verify row data
5647
tableRows.forEach((row, rowIndex) => {
5748
const cells = row.find('td');
5849
cells.forEach((cell, colIndex) => {
@@ -63,32 +54,19 @@ const verifyLearnerActivityTableRendered = (tableId, activity, columnTitles, row
6354

6455
describe('LearnerActivityTable', () => {
6556
beforeEach(() => {
66-
useCourseEnrollments.mockReturnValue(mockUseCourseEnrollments);
57+
usePaginatedTableData.mockReturnValue(mockUseCourseEnrollments);
6758
});
6859

6960
afterEach(() => jest.clearAllMocks());
7061

71-
// Replacing the snapshot tests with explicit assertions
7262
it('renders empty table correctly', () => {
73-
useCourseEnrollments.mockReturnValue({
74-
isLoading: false,
75-
courseEnrollments: {
76-
itemCount: 0,
77-
pageCount: 0,
78-
results: [],
79-
},
80-
fetchCourseEnrollments: jest.fn(),
81-
hasData: false,
82-
});
63+
usePaginatedTableData.mockReturnValue(mockEmptyCourseEnrollmentsData);
8364

8465
const wrapper = mount(
8566
<LearnerActivityTableWrapper id="active-week" activity="active_past_week" />,
8667
);
8768

88-
// Verify the table exists
8969
expect(wrapper.find('[role="table"]').exists()).toBe(true);
90-
91-
// Verify no data rows are displayed
9270
expect(wrapper.find('tbody tr').length).toBe(0);
9371
});
9472

@@ -108,21 +86,15 @@ describe('LearnerActivityTable', () => {
10886
];
10987

11088
const wrapper = mount(<LearnerActivityTableWrapper id={tableId} activity={activity} />);
111-
112-
// Verify the table exists
11389
const table = wrapper.find('[role="table"]');
114-
expect(table.exists()).toBe(true);
115-
116-
// Verify correct columns are displayed
11790
const headerColumns = table.find('thead th');
118-
expect(headerColumns).toHaveLength(columnTitles.length);
11991

120-
// Verify column titles
92+
expect(table.exists()).toBe(true);
93+
expect(headerColumns).toHaveLength(columnTitles.length);
12194
headerColumns.forEach((column, index) => {
12295
expect(column.text()).toContain(columnTitles[index]);
12396
});
12497

125-
// Verify data is rendered
12698
expect(wrapper.find('tbody tr').length).toBeGreaterThan(0);
12799
});
128100

@@ -141,25 +113,17 @@ describe('LearnerActivityTable', () => {
141113
];
142114

143115
const wrapper = mount(<LearnerActivityTableWrapper id={tableId} activity={activity} />);
144-
145-
// Verify the table exists
146116
const table = wrapper.find('[role="table"]');
147-
expect(table.exists()).toBe(true);
148-
149-
// Verify correct columns are displayed (should not include Passed Date)
150117
const headerColumns = table.find('thead th');
151-
expect(headerColumns).toHaveLength(columnTitles.length);
152118

153-
// Verify column titles
119+
expect(table.exists()).toBe(true);
120+
expect(headerColumns).toHaveLength(columnTitles.length);
154121
headerColumns.forEach((column, index) => {
155122
expect(column.text()).toContain(columnTitles[index]);
156123
});
157124

158-
// Verify "Passed Date" column is not present
159125
const columnHeaders = headerColumns.map(col => col.text());
160126
expect(columnHeaders.includes('Passed Date')).toBe(false);
161-
162-
// Verify data is rendered
163127
expect(wrapper.find('tbody tr').length).toBeGreaterThan(0);
164128
});
165129

@@ -178,25 +142,17 @@ describe('LearnerActivityTable', () => {
178142
];
179143

180144
const wrapper = mount(<LearnerActivityTableWrapper id={tableId} activity={activity} />);
181-
182-
// Verify the table exists
183145
const table = wrapper.find('[role="table"]');
184-
expect(table.exists()).toBe(true);
185-
186-
// Verify correct columns are displayed (should not include Passed Date)
187146
const headerColumns = table.find('thead th');
188-
expect(headerColumns).toHaveLength(columnTitles.length);
189147

190-
// Verify column titles
148+
expect(table.exists()).toBe(true);
149+
expect(headerColumns).toHaveLength(columnTitles.length);
191150
headerColumns.forEach((column, index) => {
192151
expect(column.text()).toContain(columnTitles[index]);
193152
});
194153

195-
// Verify "Passed Date" column is not present
196154
const columnHeaders = headerColumns.map(col => col.text());
197155
expect(columnHeaders.includes('Passed Date')).toBe(false);
198-
199-
// Verify data is rendered
200156
expect(wrapper.find('tbody tr').length).toBeGreaterThan(0);
201157
});
202158

Lines changed: 7 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,95 +1,11 @@
1-
import {
2-
useCallback, useMemo, useRef, useState,
3-
} from 'react';
4-
import { camelCaseObject } from '@edx/frontend-platform/utils';
5-
import debounce from 'lodash.debounce';
6-
import { logError } from '@edx/frontend-platform/logging';
1+
import usePaginatedTableData from '../../../../hooks/usePaginatedTableData';
72
import EnterpriseDataApiService from '../../../../data/services/EnterpriseDataApiService';
8-
import EVENT_NAMES from '../../../../eventTracking';
9-
import { trackDataTableEvent } from '../utils';
103

11-
const applySortByToOptions = (sortBy, apiFieldsForColumnAccessor, options) => {
12-
if (!sortBy || sortBy.length === 0) {
13-
return;
14-
}
15-
const orderingStrings = sortBy.map(({ id, desc }) => {
16-
const fieldForColumnAccessor = apiFieldsForColumnAccessor[id];
17-
if (!fieldForColumnAccessor) {
18-
return undefined;
19-
}
20-
const apiFieldKey = fieldForColumnAccessor.key;
21-
return desc ? `-${apiFieldKey}` : apiFieldKey;
22-
}).filter(orderingString => !!orderingString);
23-
Object.assign(options, {
24-
ordering: orderingStrings.join(','),
25-
});
26-
};
27-
28-
const useCourseEnrollments = (enterpriseId, tableId, apiFieldsForColumnAccessor) => {
29-
const shouldTrackFetchEvents = useRef(false);
30-
const [isLoading, setIsLoading] = useState(true);
31-
const [courseEnrollments, setCourseEnrollments] = useState({
32-
itemCount: 0,
33-
pageCount: 0,
34-
results: [],
35-
});
36-
37-
const fetchCourseEnrollments = useCallback(async (args) => {
38-
try {
39-
setIsLoading(true);
40-
const options = {
41-
page: args.pageIndex + 1,
42-
pageSize: args.pageSize,
43-
};
44-
applySortByToOptions(args.sortBy, apiFieldsForColumnAccessor, options);
45-
46-
const response = await EnterpriseDataApiService.fetchCourseEnrollments(enterpriseId, options);
47-
const data = camelCaseObject(response.data);
48-
setCourseEnrollments({
49-
itemCount: data.count,
50-
pageCount: data.numPages ?? Math.floor(data.count / options.pageSize),
51-
results: data.results,
52-
});
53-
trackDataTableEvent({
54-
shouldTrackRef: shouldTrackFetchEvents,
55-
enterpriseId,
56-
eventName: EVENT_NAMES.PROGRESS_REPORT.DATATABLE_SORT_BY_OR_FILTER,
57-
tableId,
58-
options,
59-
});
60-
} catch (error) {
61-
// Enhanced error logging with table state context
62-
logError(error, {
63-
tableState: {
64-
tableId,
65-
enterpriseId,
66-
filters: args.filters || 'none',
67-
sortBy: JSON.stringify(args.sortBy || []),
68-
},
69-
message: `Error fetching course enrollments for table ${tableId}`,
70-
});
71-
} finally {
72-
setIsLoading(false);
73-
}
74-
}, [enterpriseId, tableId, apiFieldsForColumnAccessor]);
75-
76-
const debouncedFetchCourseEnrollments = useMemo(
77-
() => debounce(fetchCourseEnrollments, 300),
78-
[fetchCourseEnrollments],
79-
);
80-
81-
// Add a computed property to check if data exists
82-
const hasData = useMemo(
83-
() => courseEnrollments.results.length > 0,
84-
[courseEnrollments.results],
85-
);
86-
87-
return {
88-
isLoading,
89-
courseEnrollments,
90-
fetchCourseEnrollments: debouncedFetchCourseEnrollments,
91-
hasData,
92-
};
93-
};
4+
const useCourseEnrollments = (enterpriseId, tableId, apiFieldsForColumnAccessor) => usePaginatedTableData({
5+
enterpriseId,
6+
tableId,
7+
apiFieldsForColumnAccessor,
8+
fetchFunction: EnterpriseDataApiService.fetchCourseEnrollments,
9+
});
9410

9511
export default useCourseEnrollments;

0 commit comments

Comments
 (0)