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: Added PluginSlot to wrap IDVerificationPage component #1097

Merged
merged 2 commits into from
Aug 16, 2024
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"build": "fedx-scripts webpack",
"i18n_extract": "fedx-scripts formatjs extract",
"lint": "fedx-scripts eslint --ext .js --ext .jsx .",
"lint:fix": "npm run lint -- --fix",
"snapshot": "fedx-scripts jest --updateSnapshot",
"start": "fedx-scripts webpack-dev-server --progress",
"test": "TZ=UTC fedx-scripts jest --coverage --passWithNoTests"
Expand Down
7 changes: 5 additions & 2 deletions src/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import FooterSlot from '@openedx/frontend-slot-footer';

import configureStore from './data/configureStore';
import AccountSettingsPage, { NotFoundPage } from './account-settings';
import IdVerificationPage from './id-verification';
import IdVerificationPageSlot from './plugin-slots/IdVerificationPageSlot';
import messages from './i18n';

import './index.scss';
Expand All @@ -40,7 +40,10 @@ subscribe(APP_READY, () => {
>
<Route path="/notifications/:courseId" element={<NotificationPreferences />} />
<Route path="/notifications" element={<NotificationCourses />} />
<Route path="/id-verification/*" element={<IdVerificationPage />} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a change like this, if we deploy this to production, the existing IDV flow will be gone.
I think we need a period of this code where the plugin slot and the existing IDV flow co-exist for a while. This way, while we build out the plugin slot, the existing IDV flow still serves our learners.
Then we need a waffle flag to do the switch from the entry link, which is probably 3 different places.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever component is wrapped by the PluginSlot (i.e. IdVerificationPage) is the "default component" and will be rendered by default if there is no plugin configuration specified (e.g. in an env.config.[js|jsx] file in edx-internal) or no overriding plugin in the said plugin configuration, so this deployment strategy isn't necessary. If we merge this as-is, there will be no changes to the IDV page in the learning MFE for 2U or any other deployments. There are more details the frontend-plugin-framework docs. Does that assuage your concerns?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I tested it locally but with no configuration and the <IdVerificationPage> is rendered by default.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I didn't know plugin slot handles this the way it should be. Thank you for pointing it out.

<Route
path="/id-verification/*"
element={<IdVerificationPageSlot />}
/>
<Route path="/" element={<AccountSettingsPage />} />
<Route path="/notfound" element={<NotFoundPage />} />
<Route path="*" element={<NotFoundPage />} />
Expand Down
45 changes: 45 additions & 0 deletions src/plugin-slots/IdVerificationPageSlot/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Footer Slot

### Slot ID: `id_verification_page_plugin`

## Description

This slot is used to replace/modify the IDV Page.

The implementation of the `IdVerificationPageSlot` component lives in `src/plugin-slots/IdVerificationPageSlot/index.jsx`.

## Example

The following `env.config.jsx` will replace the default IDV Page.

![Screenshot of Default IDV Page](./images/default_id-verification-page.png)

```jsx
import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework';

const config = {
pluginSlots: {
id_verification_page_plugin: {
plugins: [
{
// Insert a custom IDV Page
op: PLUGIN_OPERATIONS.Insert,
widget: {
id: 'id_verification_page_plugin',
type: DIRECT_PLUGIN,
RenderWidget: () => (
<div>
<p>This is the new IDV page</p>
<a href="/">Go Home</a>
</div>
),
},
},
],
},
},
};

export default config;

```
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions src/plugin-slots/IdVerificationPageSlot/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { PluginSlot } from '@openedx/frontend-plugin-framework';
import IdVerificationPage from '../../id-verification';

const IdVerificationPageSlot = () => (
<PluginSlot id="id_verification_page_plugin">

Check warning on line 5 in src/plugin-slots/IdVerificationPageSlot/index.jsx

View check run for this annotation

Codecov / codecov/patch

src/plugin-slots/IdVerificationPageSlot/index.jsx#L4-L5

Added lines #L4 - L5 were not covered by tests
<IdVerificationPage />
</PluginSlot>
);

export default IdVerificationPageSlot;
1 change: 1 addition & 0 deletions src/plugin-slots/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# `frontend-app-account` Plugin Slots

* [`footer_slot`](./FooterSlot/)
* [`id_verification_page_plugin`](./IdVerificationPageSlot/)
8 changes: 8 additions & 0 deletions src/setupTest.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
import 'core-js/stable';
import 'regenerator-runtime/runtime';
import '@testing-library/jest-dom';

import MockedPluginSlot from './tests/MockedPluginSlot';

jest.mock('@openedx/frontend-plugin-framework', () => ({
...jest.requireActual('@openedx/frontend-plugin-framework'),
Plugin: () => 'Plugin',
PluginSlot: MockedPluginSlot,
}));
26 changes: 26 additions & 0 deletions src/tests/MockedPluginSlot.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import React from 'react';
import PropTypes from 'prop-types';

const MockedPluginSlot = ({ children, id }) => (
<div data-testid={id}>
PluginSlot_{id}
{ children && <div>{children}</div> }
</div>
);

MockedPluginSlot.displayName = 'PluginSlot';

MockedPluginSlot.propTypes = {
children: PropTypes.oneOfType([
PropTypes.arrayOf(PropTypes.node),
PropTypes.node,
]),
id: PropTypes.string,
};

MockedPluginSlot.defaultProps = {
children: undefined,
id: undefined,
};

export default MockedPluginSlot;
43 changes: 43 additions & 0 deletions src/tests/MockedPluginSlot.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import MockedPluginSlot from './MockedPluginSlot';

describe('MockedPluginSlot', () => {
it('renders mock plugin with "PluginSlot" text', () => {
render(<MockedPluginSlot id="test_plugin" />);

const component = screen.getByText('PluginSlot_test_plugin');
expect(component).toBeInTheDocument();
});

it('renders as the slot children directly if there is content within', () => {
render(
<div role="article">
<MockedPluginSlot>
<q role="note">How much wood could a woodchuck chuck if a woodchuck could chuck wood?</q>
</MockedPluginSlot>
</div>,
);

const component = screen.getByRole('article');
expect(component).toBeInTheDocument();

// Direct children
const quote = component.querySelector(':scope > q');
expect(quote.getAttribute('role')).toBe('note');
});

it('renders mock plugin with a data-testid ', () => {
render(
<MockedPluginSlot id="guybrush">
<q role="note">I am selling these fine leather jackets.</q>
</MockedPluginSlot>,
);

const component = screen.getByTestId('guybrush');
expect(component).toBeInTheDocument();

const quote = component.querySelector('[role=note]');
expect(quote).toBeInTheDocument();
});
});
Loading