Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add paginators on reporting configurations page #1362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
99 changes: 77 additions & 22 deletions src/components/ReportingConfig/index.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Collapsible, Icon } from '@openedx/paragon';
import { Collapsible, Icon, Pagination } from '@openedx/paragon';
import { Check, Close } from '@openedx/paragon/icons';
import { camelCaseObject } from '@edx/frontend-platform';
import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n';
Expand All @@ -12,6 +12,7 @@
import ErrorPage from '../ErrorPage';

const STATUS_FULFILLED = 'fulfilled';
const DEFAULT_PAGE_SIZE = 10;

class ReportingConfig extends React.Component {
// eslint-disable-next-line react/state-in-constructor
Expand All @@ -33,7 +34,15 @@
LMSApiService.fetchReportingConfigTypes(this.props.enterpriseId),
])
.then((responses) => {
let totalPages = responses[0].status === STATUS_FULFILLED ? responses[0].value.data.num_pages : 1;
if (!totalPages) {
totalPages = 1;
}

this.setState({
totalPages,
currentPage: 1,
totalRecords: responses[0].status === STATUS_FULFILLED ? responses[0].value.data.count : 0,
reportingConfigs: responses[0].status === STATUS_FULFILLED ? responses[0].value.data.results : undefined,
availableCatalogs: responses[1].status === STATUS_FULFILLED ? responses[1].value.data.results : undefined,
reportingConfigTypes: responses[2].status === STATUS_FULFILLED ? responses[2].value.data : undefined,
Expand All @@ -52,17 +61,25 @@
* @param {FormData} formData
*/
createConfig = async (formData) => {
// snake_case the data before sending it to the backend
const transformedData = snakeCaseFormData(formData);
try {
const response = await LMSApiService.postNewReportingConfig(transformedData);
this.setState(prevState => ({
reportingConfigs: [
...prevState.reportingConfigs,
response.data,
],
}));
// Transform data to snake_case format
const transformedData = snakeCaseFormData(formData);

// Post the new configuration to the backend
await LMSApiService.postNewReportingConfig(transformedData);

const { totalRecords, totalPages } = this.state;

// Determine the target page to navigate to
const shouldAddNewPage = totalRecords % DEFAULT_PAGE_SIZE === 0 && totalRecords !== 0;
const targetPage = shouldAddNewPage ? totalPages + 1 : totalPages;

// Navigate to the appropriate page
this.handlePageSelect(targetPage);

// Close the new config form
this.newConfigFormRef.current.close();

return undefined;
} catch (error) {
return error;
Expand All @@ -72,18 +89,17 @@
deleteConfig = async (uuid) => {
try {
await LMSApiService.deleteReportingConfig(uuid);
const deletedIndex = this.state.reportingConfigs
.findIndex(config => config.uuid === uuid);

this.setState((state) => {
// Copy the existing, needs to be updated, list of reporting configs
const newReportingConfig = [...state.reportingConfigs];
// Splice out the one that's being deleted
newReportingConfig.splice(deletedIndex, 1);
return {
reportingConfigs: newReportingConfig,
};
});

const isLastPage = this.state.currentPage === this.state.totalPages;
const hasOneRecord = this.state.reportingConfigs.length === 1;
const isOnlyRecordOnLastPage = hasOneRecord && isLastPage;

if (isOnlyRecordOnLastPage && this.state.currentPage > 1) {
this.handlePageSelect(this.state.totalPages - 1);

Check warning on line 98 in src/components/ReportingConfig/index.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/ReportingConfig/index.jsx#L98

Added line #L98 was not covered by tests
} else {
this.handlePageSelect(this.state.currentPage);
}

return undefined;
} catch (error) {
return error;
Expand Down Expand Up @@ -111,13 +127,41 @@
}
};

/**
* Handles page select event and fetches the data for the selected page
* @param {number} page - The page number to fetch data for
*/
handlePageSelect = async (page) => {
this.setState({
loading: true,
});

try {
const response = await LMSApiService.fetchReportingConfigs(this.props.enterpriseId, page);
this.setState({
totalPages: response.data.num_pages || 1,
totalRecords: response.data.count,
currentPage: page,
reportingConfigs: response.data.results,
loading: false,
});
} catch (error) {
this.setState({
loading: false,
error,
});
}
};

render() {
const {
reportingConfigs,
loading,
error,
availableCatalogs,
reportingConfigTypes,
currentPage,
totalPages,
} = this.state;
const { intl } = this.props;
if (loading) {
Expand Down Expand Up @@ -200,6 +244,17 @@
</Collapsible>
</div>
))}

{reportingConfigs && reportingConfigs.length > 0 && (
<Pagination
variant="reduced"
onPageSelect={this.handlePageSelect}
pageCount={totalPages}
currentPage={currentPage}
paginationLabel="reporting configurations pagination"
/>
)}

<Collapsible
styling="basic"
title={intl.formatMessage({
Expand Down
186 changes: 185 additions & 1 deletion src/components/ReportingConfig/index.test.jsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import React from 'react';
import { mount } from 'enzyme';
import { IntlProvider } from '@edx/frontend-platform/i18n';
import { act } from 'react-dom/test-utils';

import { IntlProvider } from '@edx/frontend-platform/i18n';
import { Pagination } from '@openedx/paragon';

import ReportingConfig from './index';
import LmsApiService from '../../data/services/LmsApiService';
import ErrorPage from '../ErrorPage';

const defaultProps = {
location: {
Expand Down Expand Up @@ -97,6 +101,35 @@ const reportingConfigTypes = {
],
};

const mockConfigsData = {
data: {
results: [{
enterpriseCustomerId: 'test-customer-uuid',
active: true,
delivery_method: 'email',
email: ['[email protected]'],
emailRaw: '[email protected]',
frequency: 'monthly',
dayOfMonth: 1,
dayOfWeek: null,
hourOfDay: 1,
sftpHostname: '',
sftpPort: null,
sftpUsername: '',
sftpFilePath: '',
dataType: 'progress_v3',
data_type: 'csv',
pgpEncryptionKey: '',
uuid: 'test-config-uuid',
enterpriseCustomerCatalogs: [{
uuid: 'test-enterprise-customer-catalog',
title: 'All Content',
}],
encryptedPassword: '#$dsfrtga@',
}],
},
};

// Mocking the LMSApiService.deleteReportingConfig function
jest.mock('../../data/services/LmsApiService', () => ({
fetchReportingConfigs: jest.fn().mockResolvedValue({
Expand Down Expand Up @@ -170,4 +203,155 @@ describe('<ReportingConfig />', () => {
const afterClickingDeleteButton = wrapper.find('button[data-testid="deleteConfigButton"]');
expect(afterClickingDeleteButton.exists()).toBe(false);
});
it('handles fetchReportingConfigs failure gracefully after deleting a record', async () => {
// Mock fetchReportingConfigs to return a valid response once
LmsApiService.fetchReportingConfigs = jest.fn().mockResolvedValueOnce(mockConfigsData).mockRejectedValueOnce(new Error('Failed to fetch reporting configs'));

let wrapper;

await act(async () => {
wrapper = mount(
<IntlProvider locale="en">
<ReportingConfig {...defaultProps} intl={mockIntl} />
</IntlProvider>,
);
});

const configUuidToDelete = 'test-config-uuid';
// Update the wrapper after the state changes
wrapper.setState({
loading: false,
});
wrapper.update();

// Find the collapsible component and set its "isOpen" prop to true
const collapsibleTrigger = wrapper.find('.collapsible-trigger').at(0);
await act(async () => { collapsibleTrigger.simulate('click'); });
wrapper.update();
// Find the delete button using its data-testid and simulate a click event
const deleteButton = wrapper.find('button[data-testid="deleteConfigButton"]');
expect(deleteButton.exists()).toBe(true); // Ensure the button exists
await act(async () => { deleteButton.simulate('click'); });
wrapper.update();

// Verify that the deleteConfig function was called with the correct UUID
expect(LmsApiService.deleteReportingConfig).toHaveBeenCalledWith(configUuidToDelete);

const afterClickingDeleteButton = wrapper.find('button[data-testid="deleteConfigButton"]');
expect(afterClickingDeleteButton.exists()).toBe(false);

// Check for error handling
const errorMessage = wrapper.find(ErrorPage); // Adjust selector based on your error display logic
expect(errorMessage.exists()).toBe(true);
});
it('should not render Pagination when reportingConfigs is empty', async () => {
LmsApiService.fetchReportingConfigs.mockResolvedValue({
data: {
results: [],
count: 0,
num_pages: 0,
},
});

let wrapper;
await act(async () => {
wrapper = mount(
<IntlProvider locale="en">
<ReportingConfig {...defaultProps} intl={mockIntl} />
</IntlProvider>,
);
});

wrapper.update();

// Check that Pagination component is not rendered when no configs
const paginationComponent = wrapper.find(Pagination);
expect(paginationComponent.exists()).toBe(false);
});
it('should render Pagination when reportingConfigs has items', async () => {
let wrapper;

LmsApiService.fetchReportingConfigs.mockResolvedValue({
data: {
results: [{
enterpriseCustomerId: 'test-customer-uuid',
active: true,
delivery_method: 'email',
uuid: 'test-config-uuid',
}],
count: 1,
num_pages: 1,
},
});

await act(async () => {
wrapper = mount(
<IntlProvider locale="en">
<ReportingConfig {...defaultProps} intl={mockIntl} />
</IntlProvider>,
);
});

wrapper.update();

// Check that Pagination component is rendered when configs exist
const paginationComponent = wrapper.find(Pagination);
expect(paginationComponent.exists()).toBe(true);
});
it('calls createConfig function and handles new configuration creation', async () => {
// Mock the necessary API service methods
LmsApiService.postNewReportingConfig = jest.fn().mockResolvedValue({
data: {
uuid: 'new-config-uuid',
active: true,
delivery_method: 'email',
data_type: 'progress_v3',
frequency: 'monthly',
},
});

let wrapper;
await act(async () => {
wrapper = mount(
<IntlProvider locale="en">
<ReportingConfig {...defaultProps} intl={mockIntl} />
</IntlProvider>,
);
});

// Wait for initial loading to complete
await act(async () => {
wrapper.update();
});

// Find and click the "Add a reporting configuration" collapsible
const addConfigCollapsible = wrapper.find('div.collapsible-trigger').last();
await act(async () => {
addConfigCollapsible.simulate('click');
});
wrapper.update();

// Prepare mock form data
const mockFormData = new FormData();
mockFormData.append('delivery_method', 'email');
mockFormData.append('data_type', 'progress_v3');
mockFormData.append('frequency', 'monthly');

// Find the ReportingConfigForm within the new config collapsible
const reportingConfigForm = wrapper.find('ReportingConfigForm').last();

// Mock the form submission
await act(async () => {
reportingConfigForm.prop('createConfig')(mockFormData);
});

// Verify that the postNewReportingConfig method was called
expect(LmsApiService.postNewReportingConfig).toHaveBeenCalled();

// Verify that a new page was fetched after configuration creation
expect(LmsApiService.fetchReportingConfigs).toHaveBeenCalledWith(
defaultProps.enterpriseId,
expect.any(Number),
);
});
});
9 changes: 7 additions & 2 deletions src/data/services/LmsApiService.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,13 @@ class LmsApiService {
return LmsApiService.apiClient().post(requestCodesUrl, postParams);
}

static fetchReportingConfigs(uuid) {
return LmsApiService.apiClient().get(`${LmsApiService.reportingConfigUrl}?enterprise_customer=${uuid}&page_size=100`);
static fetchReportingConfigs(uuid, pageNumber) {
let url = `${LmsApiService.reportingConfigUrl}?enterprise_customer=${uuid}`;
if (pageNumber) {
url += `&page=${pageNumber}`;
}

return LmsApiService.apiClient().get(url);
}

static fetchReportingConfigTypes(uuid) {
Expand Down
2 changes: 1 addition & 1 deletion src/data/services/tests/LmsApiService.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ describe('LmsApiService', () => {
}],
},
});
const response = await LmsApiService.fetchReportingConfigs();
const response = await LmsApiService.fetchReportingConfigs('test-enterprise-customer', 1);
expect(response).toEqual({
status: 200,
data: {
Expand Down
Loading