Skip to content

Commit

Permalink
feat: refactor notices provider to useQuery (#992)
Browse files Browse the repository at this point in the history
* feat: refactor notices provider to useQuery

* chore: merge fix

* chore: cleanup

* fix: redirects without momentary render

* fix: debugging

* chore: Testing and cleanup

* chore: PR feedback
  • Loading branch information
brobro10000 authored and adamstankiewicz committed Mar 6, 2024
1 parent d0b1f41 commit cffd0b4
Show file tree
Hide file tree
Showing 13 changed files with 143 additions and 129 deletions.
5 changes: 2 additions & 3 deletions src/components/app/Root.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { Hyperlink } from '@openedx/paragon';

import DelayedFallbackContainer from '../DelayedFallback/DelayedFallbackContainer';
import { Toasts, ToastsProvider } from '../Toasts';
import NoticesProvider from '../notices-provider';
import { ErrorPage } from '../error-page';
import { useNProgressLoader } from './data';

Expand Down Expand Up @@ -47,15 +46,15 @@ const Root = () => {

// User is authenticated, so render the child routes (rest of the app).
return (
<NoticesProvider>
<>
<ToastsProvider>
<Toasts />
<Suspense fallback={<DelayedFallbackContainer />}>
<Outlet />
</Suspense>
</ToastsProvider>
<ScrollRestoration />
</NoticesProvider>
</>
);
};

Expand Down
15 changes: 12 additions & 3 deletions src/components/app/routes/data/queries/ensureEnterpriseAppData.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import dayjs from 'dayjs';
import { getConfig } from '@edx/frontend-platform';

import { activateLicense, requestAutoAppliedUserLicense } from '../services';
import { activateOrAutoApplySubscriptionLicense } from '../utils';
Expand All @@ -12,6 +13,7 @@ import {
querySubscriptions,
queryBrowseAndRequestConfiguration,
} from './subsidies';
import queryNotices from './notices';

/**
* TODO
Expand All @@ -26,7 +28,7 @@ export default async function ensureEnterpriseAppData({
requestUrl,
}) {
const subscriptionsQuery = querySubscriptions(enterpriseCustomer.uuid);
const enterpriseAppData = await Promise.all([
const enterpriseAppDataQueries = [
// Enterprise Customer User Subsidies
queryClient.ensureQueryData(subscriptionsQuery).then(async (subscriptionsData) => {
// Auto-activate the user's subscription license, if applicable.
Expand Down Expand Up @@ -88,7 +90,14 @@ export default async function ensureEnterpriseAppData({
queryClient.ensureQueryData(
queryContentHighlightsConfiguration(enterpriseCustomer.uuid),
),
]);

];
if (getConfig().ENABLE_NOTICES) {
enterpriseAppDataQueries.push(
queryClient.ensureQueryData(
queryNotices(),
),
);
}
const enterpriseAppData = await Promise.all(enterpriseAppDataQueries);
return enterpriseAppData;
}
1 change: 1 addition & 0 deletions src/components/app/routes/data/queries/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export { default as queryCourseMetadata } from './courseMetadata';
export { default as queryEnterpriseCourseEnrollments } from './enterpriseCourseEnrollments';
export { default as queryUserEntitlements } from './userEntitlements';
export { default as ensureEnterpriseAppData } from './ensureEnterpriseAppData';
export { default as queryNotices } from './notices';

export {
queryEnterpriseLearner,
Expand Down
12 changes: 12 additions & 0 deletions src/components/app/routes/data/queries/notices.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { queries } from '../../../../../utils/queryKeyFactory';

/**
* Helper function to assist querying with useQuery package
*
* @returns {Types.QueryObject} - The query object for notices.
* @property {[string]} QueryObject.queryKey - The query key for the object
* @property {func} QueryObject.queryFn - The asynchronous API request "fetchNotices"
*/
export default function queryNotices() {
return queries.user.notices;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ import { queries } from '../../../../../../utils/queryKeyFactory';
import { SUBSIDY_REQUEST_STATE } from '../../../../../enterprise-subsidy-requests';

/**
* Helper function to assist querying with useQuery package
* queries
* .enterprise
* .enterpriseCustomer(enterpriseUuid)
* ._ctx.subsidies
* ._ctx.browseAndRequest(userEmail)
* ._ctx.configuration
* @returns
* Helper function to assist querying with useQuery package.
*
* @param {string} enterpriseUuid - The UUID of the enterprise.
* @param {string} userEmail - The email of the user.
* @returns {QueryObject} - The query object for the enterprise configuration.
* @property {[string]} QueryObject.queryKey - The query key for the object
* @property {func} QueryObject.queryFn - The asynchronous API request "fetchBrowseAndRequestConfiguration"
*/
export function queryBrowseAndRequestConfiguration(enterpriseUuid) {
return queries
Expand Down Expand Up @@ -43,6 +42,7 @@ export function queryLicenseRequests(enterpriseUuid, userEmail, state = SUBSIDY_

/**
* Helper function to assist querying with useQuery package
*
* queries
* .enterprise
* .enterpriseCustomer(enterpriseUuid)
Expand Down
25 changes: 25 additions & 0 deletions src/components/app/routes/data/services.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { camelCaseObject, getConfig } from '@edx/frontend-platform';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
import { logError, logInfo } from '@edx/frontend-platform/logging';
import {
ENTERPRISE_OFFER_STATUS,
ENTERPRISE_OFFER_USAGE_TYPE,
Expand Down Expand Up @@ -384,6 +385,30 @@ export async function requestAutoAppliedUserLicense(customerAgreementId) {
return camelCaseObject(response.data);
}

// Notices
export const fetchNotices = async () => {
const url = `${getConfig().LMS_BASE_URL}/notices/api/v1/unacknowledged`;
try {
const { data } = await getAuthenticatedHttpClient().get(url);
if (data?.results.length > 0) {
const { results } = data;
window.location.assign(`${results[0]}?next=${window.location.href}`);
throw new Error('Redirecting to notice');
}
return data;
} catch (error) {
// we will just swallow error, as that probably means the notices app is not installed.
// Notices are not necessary for the rest of dashboard to function.
const httpErrorStatus = getErrorResponseStatusCode(error);
if (httpErrorStatus === 404) {
logInfo(`${error}. This probably happened because the notices plugin is not installed on platform.`);
} else {
logError(error);
}
}
return null;
};

/**
* Helper function to `updateActiveEnterpriseCustomerUser` to make the POST API
* request, updating the active enterprise customer for the learner.
Expand Down
76 changes: 76 additions & 0 deletions src/components/app/routes/data/tests/services.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/* eslint-disable react/jsx-filename-extension */
import { render, waitFor, screen } from '@testing-library/react';
import { useState } from 'react';
import userEvent from '@testing-library/user-event';
import MockAdapter from 'axios-mock-adapter';
import axios from 'axios';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
import { Button } from '@openedx/paragon';
import { fetchNotices } from '../services';

const APP_CONFIG = {
USE_API_CACHE: true,
DISCOVERY_API_BASE_URL: 'http://localhost:18381',
LMS_BASE_URL: 'http://localhost:18000',
};
jest.mock('@edx/frontend-platform/config', () => ({
...jest.requireActual('@edx/frontend-platform/config'),
getConfig: jest.fn(() => APP_CONFIG),
}));

jest.mock('@edx/frontend-platform/auth', () => ({
getAuthenticatedUser: jest.fn(() => ({ id: 12345 })),
getAuthenticatedHttpClient: jest.fn(),
}));

const axiosMock = new MockAdapter(axios);
getAuthenticatedHttpClient.mockReturnValue(axios);

describe('fetchNotices', () => {
const NOTICES_ENDPOINT = `${APP_CONFIG.LMS_BASE_URL }/notices/api/v1/unacknowledged`;
const ComponentWithNotices = () => {
const [output, setOuput] = useState(null);
const onClickHandler = async () => {
const apiOutput = await fetchNotices();
if (apiOutput?.results.length > 0) {
setOuput(apiOutput.results[0]);
return;
}
setOuput('No Results');
};
return (
<Button data-testid="fetchNotices" onClick={onClickHandler}>{output || 'hi'}</Button>
);
};

// Preserves original window location, and swaps it back after test is completed
const currentLocation = window.location;
beforeAll(() => {
delete window.location;
window.location = { ...currentLocation, assign: jest.fn() };
});
afterAll(() => {
window.location = currentLocation;
});
it('returns empty data results and does not assign the window location', async () => {
axiosMock.onGet(NOTICES_ENDPOINT).reply(200, { results: [] });
render(<ComponentWithNotices />);
userEvent.click(screen.getByTestId('fetchNotices'));
await waitFor(() => expect(window.location.assign).not.toHaveBeenCalled());
});
it('returns logInfo on 404', async () => {
axiosMock.onGet(NOTICES_ENDPOINT).reply(404, {});
render(<ComponentWithNotices />);
userEvent.click(screen.getByTestId('fetchNotices'));
await waitFor(() => expect(window.location.assign).not.toHaveBeenCalled());
});
it('assigns the window location on successful API response', async () => {
const currentHref = window.location.href;
axiosMock.onGet(NOTICES_ENDPOINT).reply(200, { results: [APP_CONFIG.LMS_BASE_URL] });
render(<ComponentWithNotices />);
userEvent.click(screen.getByTestId('fetchNotices'));
await waitFor(() => expect(window.location.assign).toHaveBeenCalledWith(
`${APP_CONFIG.LMS_BASE_URL }?next=${currentHref}`,
));
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { transformEnterpriseCustomer } from './utils';
import { transformEnterpriseCustomer } from '../utils';

const mockEnterpriseFeatures = {
'example-feature': true,
Expand Down
32 changes: 0 additions & 32 deletions src/components/notices-provider/NoticesProvider.jsx

This file was deleted.

53 changes: 0 additions & 53 deletions src/components/notices-provider/NoticesProvider.test.jsx

This file was deleted.

25 changes: 0 additions & 25 deletions src/components/notices-provider/api.js

This file was deleted.

3 changes: 0 additions & 3 deletions src/components/notices-provider/index.js

This file was deleted.

7 changes: 6 additions & 1 deletion src/utils/queryKeyFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
fetchEnterpriseOffers,
fetchEnterpriseCuration,
fetchCouponCodeRequests,
fetchNotices,
} from '../components/app/routes/data/services';

import { SUBSIDY_REQUEST_STATE } from '../components/enterprise-subsidy-requests';
Expand Down Expand Up @@ -103,11 +104,15 @@ export const enterprise = createQueryKeys('enterprise', {
}),
});

export const user = createQueryKeys('user', {
const user = createQueryKeys('user', {
entitlements: {
queryKey: null,
queryFn: async () => fetchUserEntitlements(),
},
notices: {
queryKey: null,
queryFn: async () => fetchNotices(),
},
});

export const queries = mergeQueryKeys(enterprise, user);

0 comments on commit cffd0b4

Please sign in to comment.