From 3ec478fb0459199ca64bf72cb083ff23a6ce184a Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Fri, 15 Dec 2023 02:04:37 +0000 Subject: [PATCH 01/11] feat: WIP add file structure and write mocks for tests --- src/plugins/directPlugins/DirectPlugin.jsx | 33 ++++++++++ .../directPlugins/DirectPluginSlot.jsx | 2 + src/plugins/directPlugins/index.js | 7 +++ .../directPlugins/test/DirectPlugin.test.jsx | 42 +++++++++++++ .../test/DirectPluginSlot.test.jsx | 4 ++ .../test/mocks/DefaultComponentMock.jsx | 33 ++++++++++ .../test/mocks/PluginComponentsMock.jsx | 63 +++++++++++++++++++ 7 files changed, 184 insertions(+) create mode 100644 src/plugins/directPlugins/DirectPlugin.jsx create mode 100644 src/plugins/directPlugins/DirectPluginSlot.jsx create mode 100644 src/plugins/directPlugins/index.js create mode 100644 src/plugins/directPlugins/test/DirectPlugin.test.jsx create mode 100644 src/plugins/directPlugins/test/DirectPluginSlot.test.jsx create mode 100644 src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx create mode 100644 src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx diff --git a/src/plugins/directPlugins/DirectPlugin.jsx b/src/plugins/directPlugins/DirectPlugin.jsx new file mode 100644 index 00000000..9a93aa10 --- /dev/null +++ b/src/plugins/directPlugins/DirectPlugin.jsx @@ -0,0 +1,33 @@ +/** ///// Code for plugins ///// */ + +// DirectPluginOperation + +/** + +export const DirectPluginOperation = { + Insert: 'insert', + Hide: 'hide', + Modify: 'modify', + Wrap: 'wrap', +}; + +it makes sense to have the definitions of ChangeOperation live in here, but the actual changes are tightly living in +pluginslot because of the newContents array that is being changed. Ideally, the PluginSlot itself doesn't care about +the plugins inside it, it only cares about returning some component with the empty React tags <> + +export type DirectPluginOperation = + | { op: DirectPluginOperation.Insert; widget: UISlotWidget } + | { op: DirectPluginOperation.Hide; widgetId: string } + | { op: DirectPluginOperation.Modify; widgetId: string, fn: (widget: UISlotWidget) => UISlotWidget } + | { op: DirectPluginOperation.Wrap; widgetId: string, wrapper: React.FC<{widget: React.ReactElement}> }; + +*/ + +// idea: move enabledPlugins and all of the logic that contents receives into DirectPlugin.jsx +// this will mean that all of the logic for changing plugins will live in there. +// all that pluginSlot cares about is that if given contents then it will render it accordingly. + +// also consider maybe moving defaultContents into the same config as the plugin operations object. +// This will maybe bring it closer to the scenario where the defaultContents is just part of the config object +// alongside the plugin changes. Like, why would want to separate those two objects from each other when they're +// part of the same flow. diff --git a/src/plugins/directPlugins/DirectPluginSlot.jsx b/src/plugins/directPlugins/DirectPluginSlot.jsx new file mode 100644 index 00000000..a662af9f --- /dev/null +++ b/src/plugins/directPlugins/DirectPluginSlot.jsx @@ -0,0 +1,2 @@ +/** Context which makes the list of enabled plugins available to the components below it in the React tree */ +// export const DirectPluginsContext = React.createContext([]); diff --git a/src/plugins/directPlugins/index.js b/src/plugins/directPlugins/index.js new file mode 100644 index 00000000..7085dc66 --- /dev/null +++ b/src/plugins/directPlugins/index.js @@ -0,0 +1,7 @@ +export { + DirectPluginSlot, + DirectPluginsContext, +} from './DirectPluginSlot'; +export { + DirectPluginOperations, +} from './DirectPlugin'; diff --git a/src/plugins/directPlugins/test/DirectPlugin.test.jsx b/src/plugins/directPlugins/test/DirectPlugin.test.jsx new file mode 100644 index 00000000..40e64583 --- /dev/null +++ b/src/plugins/directPlugins/test/DirectPlugin.test.jsx @@ -0,0 +1,42 @@ +/* eslint-disable import/no-extraneous-dependencies */ +/* eslint react/prop-types: off */ + +// import React from 'react'; +// import { render } from '@testing-library/react'; +// import { fireEvent } from '@testing-library/dom'; +// import '@testing-library/jest-dom'; + +// import { +// FormattedMessage, +// IntlProvider, +// } from '@edx/frontend-platform/i18n'; + +// import { isAdminHelper, navLinksPlugin } from './mocks/PluginComponentsMock'; + +/* +test for +when given a plugin config with default content but no changes, return only default content +when using insert, plugin is unchanged +when using modify, plugin is changed +when using hide, plugin is missing +when using wrap (use plugin components mock for this), plugin renders based on condition +when using wrap, plugin has wrapped component (refer to classname) +*/ + +describe('When given a pluginConfig', () => { + describe('when there is no defaultContent', () => { + it('should return an empty array when there are no changes', () => { + }); + it('should return an array of only new plugins inserted', () => { + + }); + }); + describe('when there is defaultContent', () => { + it('should return the same default plugins if no changes', () => { + + }); + it('should return an array with each plugin including property "hidden" if operation is Hidden', () => { + + }); + }); +}); diff --git a/src/plugins/directPlugins/test/DirectPluginSlot.test.jsx b/src/plugins/directPlugins/test/DirectPluginSlot.test.jsx new file mode 100644 index 00000000..7bb97d30 --- /dev/null +++ b/src/plugins/directPlugins/test/DirectPluginSlot.test.jsx @@ -0,0 +1,4 @@ +// when no additional plugins, only render default +// when no defaults, only plugin is rendered +// when no plugins and no defaults, nothing is rendered +// when more than one plugin, priority is followed diff --git a/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx b/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx new file mode 100644 index 00000000..8d76c4d2 --- /dev/null +++ b/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx @@ -0,0 +1,33 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import { Icon } from '@edx/paragon'; +import { DirectPluginSlot } from '../..'; +import { navLinksPlugin } from './PluginComponentsMock'; + +// TODO: remove DirectPluginsContext and enabledPlugins from here once we have an example app +// these simply demonstrate how the root App would have needed this setup in order to pass in the plugin config +// and make it available to a given DirectPluginSlot + +// const enabledPlugins = [ +// linksPlugin, +// ]; + +const MyApp = () => ( + // +
+ ( + + {link.content.label} + + )} + /> +
+ // +); + +export default MyApp; diff --git a/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx b/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx new file mode 100644 index 00000000..4fee8d1b --- /dev/null +++ b/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx @@ -0,0 +1,63 @@ +/* eslint-disable react/prop-types */ +// eslint-disable-next-line import/no-extraneous-dependencies +import React from 'react'; +import { House, Star, InsertDriveFile, Login } from '@edx/paragon/icons'; +import { DirectPluginOperations } from '../..'; + +/** This is for us to be able to mock in tests */ +const isAdminHelper = () => true; + +/** This is a React widget that wraps its children and makes them visible only to administrators */ +const HideExceptForAdmin = ({ widget }) => { + const isAdmin = isAdminHelper(); + return {isAdmin ? widget : null}; +}; + +const navLinksPlugin = { + id: 'links-demo', // id isn't used anywhere, but can be extended to + defaultComponentProps: [ + { + id: 'home', + priority: 5, + content: { url: '/', icon: House, label: 'Home' }, + }, + { + id: 'lookup', + priority: 25, + content: { url: '/lookup', icon: Star, label: 'Lookup' }, + }, + { + id: 'drafts', + priority: 35, + content: { url: '/drafts', icon: InsertDriveFile, label: 'Drafts' }, + }, + ], + getDirectSlotChanges() { + return { + 'side-bar-nav': [ // slot id that is used by directpluginslot + // Hide the "Drafts" link, except for administrators: + { + op: DirectPluginOperations.Wrap, + widgetId: 'drafts', + wrapper: HideExceptForAdmin, + }, + // Add a new login link after the rest of default plugins: + { + op: DirectPluginOperations.Insert, + widget: { + id: 'login', + priority: 50, + content: { + url: '/login', icon: Login, label: 'Login', + }, + }, + }, + ], + }; + }, +}; + +export { + isAdminHelper, + navLinksPlugin, +}; From 1da6b94c6b22d227794a23694933e5b54b195ffc Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Mon, 8 Jan 2024 23:42:53 +0000 Subject: [PATCH 02/11] test: add testing for the DirectPluginOperations logic --- src/plugins/directPlugins/DirectPlugin.jsx | 10 +- .../directPlugins/DirectPluginSlot.jsx | 11 ++ src/plugins/directPlugins/hooks.js | 46 +++++ src/plugins/directPlugins/index.js | 5 +- .../directPlugins/test/DirectPlugin.test.jsx | 20 +-- src/plugins/directPlugins/test/hooks.test.js | 158 ++++++++++++++++++ .../test/mocks/DefaultComponentMock.jsx | 40 ++--- .../test/mocks/PluginComponentsMock.jsx | 6 +- 8 files changed, 253 insertions(+), 43 deletions(-) create mode 100644 src/plugins/directPlugins/hooks.js create mode 100644 src/plugins/directPlugins/test/hooks.test.js diff --git a/src/plugins/directPlugins/DirectPlugin.jsx b/src/plugins/directPlugins/DirectPlugin.jsx index 9a93aa10..01055f0e 100644 --- a/src/plugins/directPlugins/DirectPlugin.jsx +++ b/src/plugins/directPlugins/DirectPlugin.jsx @@ -1,16 +1,18 @@ -/** ///// Code for plugins ///// */ +import React from 'react'; -// DirectPluginOperation +/** Context which makes the list of enabled plugins available to the components below it in the React tree */ +export const DirectPluginsContext = React.createContext([]); -/** +// DirectPluginOperation -export const DirectPluginOperation = { +export const DirectPluginOperations = { Insert: 'insert', Hide: 'hide', Modify: 'modify', Wrap: 'wrap', }; +/** it makes sense to have the definitions of ChangeOperation live in here, but the actual changes are tightly living in pluginslot because of the newContents array that is being changed. Ideally, the PluginSlot itself doesn't care about the plugins inside it, it only cares about returning some component with the empty React tags <> diff --git a/src/plugins/directPlugins/DirectPluginSlot.jsx b/src/plugins/directPlugins/DirectPluginSlot.jsx index a662af9f..900856e4 100644 --- a/src/plugins/directPlugins/DirectPluginSlot.jsx +++ b/src/plugins/directPlugins/DirectPluginSlot.jsx @@ -1,2 +1,13 @@ /** Context which makes the list of enabled plugins available to the components below it in the React tree */ // export const DirectPluginsContext = React.createContext([]); + +// whenever the PluginSlot gets called and needs to prepare for rendering +// call the DirectPluginsContext to get the enabledPlugins + +// sift through the enabledPlugins to find the relevant one we need for this slot +// enabledPlugins.forEach(plugin => { +// const changes = plugin.getUiSlotChanges(); // Optional: Pass in any app-specific context that the plugin wants +// const slotChanges = changes[slotId] ?? []; +// TODO: above could be condensed to: +// const changes = plugin.getUiSlotChanges()[slotId] ?? []; +// call on usePreparePlugins, passing in changes, which includes the pluginChanges and defaultcontent diff --git a/src/plugins/directPlugins/hooks.js b/src/plugins/directPlugins/hooks.js new file mode 100644 index 00000000..c06f8d95 --- /dev/null +++ b/src/plugins/directPlugins/hooks.js @@ -0,0 +1,46 @@ +import React from 'react'; +// allows mocking state modules from +// eslint-disable-next-line import/no-self-import + +import { DirectPluginOperations } from './DirectPlugin'; + +export const organizePlugins = (defaultContents, pluginChanges) => { + const newContents = [...defaultContents]; + + pluginChanges.forEach(change => { + if (change.op === DirectPluginOperations.Insert) { + newContents.push(change.widget); + } else if (change.op === DirectPluginOperations.Hide) { + const widget = newContents.find((w) => w.id === change.widgetId); + if (widget) { widget.hidden = true; } + } else if (change.op === DirectPluginOperations.Modify) { + const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); + if (widgetIdx >= 0) { + const widget = { ...newContents[widgetIdx] }; + newContents[widgetIdx] = change.fn(widget); + } + } else if (change.op === DirectPluginOperations.Wrap) { + const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); + if (widgetIdx >= 0) { + const newWidget = { wrappers: [], ...newContents[widgetIdx] }; + // refactor suggestion: remove bottom line and write wrappers: [change.wrapper] + newWidget.wrappers.push(change.wrapper); + newContents[widgetIdx] = newWidget; + } + } else { + throw new Error(`unknown plugin UI change operation: ${(change).op}`); + } + }); + + // newContents.sort((a, b) => (a.priority - b.priority) * 10_000 + a.id.localeCompare(b.id)); + return newContents; +}; + +export const useGetPlugins = (defaultContents, enabledPlugins) => { + const contents = React.useMemo(() => { + organizePlugins(defaultContents, enabledPlugins); + }, [defaultContents, enabledPlugins]); + return contents; +}; + +export default { useGetPlugins, organizePlugins }; diff --git a/src/plugins/directPlugins/index.js b/src/plugins/directPlugins/index.js index 7085dc66..4a12fc51 100644 --- a/src/plugins/directPlugins/index.js +++ b/src/plugins/directPlugins/index.js @@ -1,7 +1,10 @@ export { DirectPluginSlot, - DirectPluginsContext, } from './DirectPluginSlot'; export { + DirectPluginsContext, DirectPluginOperations, } from './DirectPlugin'; +export { + useGetPlugins, +} from './hooks'; diff --git a/src/plugins/directPlugins/test/DirectPlugin.test.jsx b/src/plugins/directPlugins/test/DirectPlugin.test.jsx index 40e64583..d8995798 100644 --- a/src/plugins/directPlugins/test/DirectPlugin.test.jsx +++ b/src/plugins/directPlugins/test/DirectPlugin.test.jsx @@ -10,8 +10,10 @@ // FormattedMessage, // IntlProvider, // } from '@edx/frontend-platform/i18n'; +// import { Login } from '@edx/paragon/icons'; // import { isAdminHelper, navLinksPlugin } from './mocks/PluginComponentsMock'; +// import { DirectPluginOperations, usePreparePlugins } from '../DirectPlugin'; /* test for @@ -23,20 +25,6 @@ when using wrap (use plugin components mock for this), plugin renders based on c when using wrap, plugin has wrapped component (refer to classname) */ -describe('When given a pluginConfig', () => { - describe('when there is no defaultContent', () => { - it('should return an empty array when there are no changes', () => { - }); - it('should return an array of only new plugins inserted', () => { +// describe('When given a pluginConfig', () => { - }); - }); - describe('when there is defaultContent', () => { - it('should return the same default plugins if no changes', () => { - - }); - it('should return an array with each plugin including property "hidden" if operation is Hidden', () => { - - }); - }); -}); +// }); diff --git a/src/plugins/directPlugins/test/hooks.test.js b/src/plugins/directPlugins/test/hooks.test.js new file mode 100644 index 00000000..29763442 --- /dev/null +++ b/src/plugins/directPlugins/test/hooks.test.js @@ -0,0 +1,158 @@ +// import React from 'react'; +import '@testing-library/jest-dom'; + +import * as hooks from '../hooks'; +import { DirectPluginOperations } from '../DirectPlugin'; + +jest.unmock('../hooks'); + +let mockEnabledPlugins = [ + { + op: DirectPluginOperations.Wrap, + widgetId: 'drafts', + wrapper: jest.fn(), + }, + { + op: DirectPluginOperations.Hide, + widgetId: 'home', + }, + { + op: DirectPluginOperations.Modify, + widgetId: 'lookUp', + fn: jest.fn((widget) => widget), + }, + { + op: DirectPluginOperations.Insert, + widget: { + id: 'login', + priority: 50, + content: { + url: '/login', label: 'Login', + }, + }, + }, +]; + +const mockModifyComponent = (widget) => { + const newContent = { + url: '/search', + label: 'Search', + }; + const modifiedWidget = widget; + modifiedWidget.content = newContent; + return modifiedWidget; +}; + +const mockDefaultContent = [ + { + id: 'home', + priority: 5, + content: { url: '/', label: 'Home' }, + }, + { + id: 'lookUp', + priority: 25, + content: { url: '/lookup', label: 'Lookup' }, + }, + { + id: 'drafts', + priority: 35, + content: { url: '/drafts', label: 'Drafts' }, + }, +]; + +describe('organizePlugins', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('when there is no defaultContent', () => { + it('should return an empty array when there are no enabledPlugins', () => { + const plugins = hooks.organizePlugins([], []); + expect(plugins.length).toBe(0); + expect(plugins).toEqual([]); + }); + + it('should return an array of changes for non-default plugins', () => { + const plugins = hooks.organizePlugins([], mockEnabledPlugins); + expect(plugins.length).toEqual(1); + expect(plugins[0].id).toEqual('login'); + }); + }); + + describe('when there is defaultContent', () => { + it('should return an array of defaultContent if no enabledPlugins', () => { + mockEnabledPlugins = []; + const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + expect(plugins.length).toEqual(3); + expect(plugins).toEqual(mockDefaultContent); + }); + + it('should remove plugins with DirectOperation.Hide', () => { + const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const widget = plugins.find((w) => w.id === 'home'); + expect(plugins.length).toEqual(4); + expect(widget.hidden).toBe(true); + }); + + it('should modify plugins with DirectOperation.Modify', () => { + const pluginToModify = mockEnabledPlugins.find(w => w.widgetId === 'lookUp'); + pluginToModify.fn = mockModifyComponent; + const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const widget = plugins.find((w) => w.id === 'lookUp'); + + expect(plugins.length).toEqual(4); + expect(widget.content.url).toEqual('/search'); + }); + }); +}); + +describe('useGetPlugins', () => { + // navLinksPlugin.defaultComponentProps = jest.fn(() => []); + // navLinksPlugin.getDirectSlotChanges = jest.fn(() => ( + // { + // 'side-nav-bar': [], + // } + // )); + // beforeEach(jest.clearAllMocks); + + // it('should return an array if only new plugins inserted', () => { + // navLinksPlugin.getDirectSlotChanges = jest.fn(() => ( + // { + // 'side-nav-bar': [ + // { + // op: DirectPluginOperations.Insert, + // widget: { + // id: 'login', + // priority: 50, + // content: { + // url: '/login', icon: Login, label: 'Login', + // }, + // }, + // }, + // ], + // } + // )); + + // const expectedChanges = { + // 'side-nav-bar': [ + // { + // widget: { + // id: 'login', + // priority: 50, + // content: { + // url: '/login', icon: Login, label: 'Login', + // }, + // }, + // }, + // ], + // }; + + // expect(navLinksPlugin.useGetPlugins.length()).toBe(1); + // expect(navLinksPlugin.useGetPlugins).toBe(expectedChanges); + // }); +}); diff --git a/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx b/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx index 8d76c4d2..33f4ca18 100644 --- a/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx +++ b/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx @@ -1,33 +1,33 @@ // eslint-disable-next-line import/no-extraneous-dependencies import { Icon } from '@edx/paragon'; -import { DirectPluginSlot } from '../..'; +import { DirectPluginSlot, DirectPluginsContext } from '../..'; import { navLinksPlugin } from './PluginComponentsMock'; // TODO: remove DirectPluginsContext and enabledPlugins from here once we have an example app // these simply demonstrate how the root App would have needed this setup in order to pass in the plugin config // and make it available to a given DirectPluginSlot -// const enabledPlugins = [ -// linksPlugin, -// ]; +const enabledPlugins = [ + navLinksPlugin, +]; const MyApp = () => ( - // -
- ( - - {link.content.label} - - )} - /> -
- // + +
+ ( + + {link.content.label} + + )} + /> +
+ ); export default MyApp; diff --git a/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx b/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx index 4fee8d1b..8fd814d6 100644 --- a/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx +++ b/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx @@ -1,7 +1,9 @@ /* eslint-disable react/prop-types */ // eslint-disable-next-line import/no-extraneous-dependencies import React from 'react'; -import { House, Star, InsertDriveFile, Login } from '@edx/paragon/icons'; +import { + House, Star, InsertDriveFile, Login, +} from '@edx/paragon/icons'; import { DirectPluginOperations } from '../..'; /** This is for us to be able to mock in tests */ @@ -14,7 +16,7 @@ const HideExceptForAdmin = ({ widget }) => { }; const navLinksPlugin = { - id: 'links-demo', // id isn't used anywhere, but can be extended to + id: 'links-demo', // id isn't used anywhere, but can be extended to defaultComponentProps: [ { id: 'home', From af931ed7a9428b6442ef605c9cafabd971d09f39 Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Fri, 15 Dec 2023 02:04:37 +0000 Subject: [PATCH 03/11] feat: WIP add file structure and write mocks for tests --- src/plugins/directPlugins/DirectPlugin.jsx | 33 ++++++++++ .../directPlugins/DirectPluginSlot.jsx | 2 + src/plugins/directPlugins/index.js | 7 +++ .../directPlugins/test/DirectPlugin.test.jsx | 42 +++++++++++++ .../test/DirectPluginSlot.test.jsx | 4 ++ .../test/mocks/DefaultComponentMock.jsx | 33 ++++++++++ .../test/mocks/PluginComponentsMock.jsx | 63 +++++++++++++++++++ 7 files changed, 184 insertions(+) create mode 100644 src/plugins/directPlugins/DirectPlugin.jsx create mode 100644 src/plugins/directPlugins/DirectPluginSlot.jsx create mode 100644 src/plugins/directPlugins/index.js create mode 100644 src/plugins/directPlugins/test/DirectPlugin.test.jsx create mode 100644 src/plugins/directPlugins/test/DirectPluginSlot.test.jsx create mode 100644 src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx create mode 100644 src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx diff --git a/src/plugins/directPlugins/DirectPlugin.jsx b/src/plugins/directPlugins/DirectPlugin.jsx new file mode 100644 index 00000000..9a93aa10 --- /dev/null +++ b/src/plugins/directPlugins/DirectPlugin.jsx @@ -0,0 +1,33 @@ +/** ///// Code for plugins ///// */ + +// DirectPluginOperation + +/** + +export const DirectPluginOperation = { + Insert: 'insert', + Hide: 'hide', + Modify: 'modify', + Wrap: 'wrap', +}; + +it makes sense to have the definitions of ChangeOperation live in here, but the actual changes are tightly living in +pluginslot because of the newContents array that is being changed. Ideally, the PluginSlot itself doesn't care about +the plugins inside it, it only cares about returning some component with the empty React tags <> + +export type DirectPluginOperation = + | { op: DirectPluginOperation.Insert; widget: UISlotWidget } + | { op: DirectPluginOperation.Hide; widgetId: string } + | { op: DirectPluginOperation.Modify; widgetId: string, fn: (widget: UISlotWidget) => UISlotWidget } + | { op: DirectPluginOperation.Wrap; widgetId: string, wrapper: React.FC<{widget: React.ReactElement}> }; + +*/ + +// idea: move enabledPlugins and all of the logic that contents receives into DirectPlugin.jsx +// this will mean that all of the logic for changing plugins will live in there. +// all that pluginSlot cares about is that if given contents then it will render it accordingly. + +// also consider maybe moving defaultContents into the same config as the plugin operations object. +// This will maybe bring it closer to the scenario where the defaultContents is just part of the config object +// alongside the plugin changes. Like, why would want to separate those two objects from each other when they're +// part of the same flow. diff --git a/src/plugins/directPlugins/DirectPluginSlot.jsx b/src/plugins/directPlugins/DirectPluginSlot.jsx new file mode 100644 index 00000000..a662af9f --- /dev/null +++ b/src/plugins/directPlugins/DirectPluginSlot.jsx @@ -0,0 +1,2 @@ +/** Context which makes the list of enabled plugins available to the components below it in the React tree */ +// export const DirectPluginsContext = React.createContext([]); diff --git a/src/plugins/directPlugins/index.js b/src/plugins/directPlugins/index.js new file mode 100644 index 00000000..7085dc66 --- /dev/null +++ b/src/plugins/directPlugins/index.js @@ -0,0 +1,7 @@ +export { + DirectPluginSlot, + DirectPluginsContext, +} from './DirectPluginSlot'; +export { + DirectPluginOperations, +} from './DirectPlugin'; diff --git a/src/plugins/directPlugins/test/DirectPlugin.test.jsx b/src/plugins/directPlugins/test/DirectPlugin.test.jsx new file mode 100644 index 00000000..40e64583 --- /dev/null +++ b/src/plugins/directPlugins/test/DirectPlugin.test.jsx @@ -0,0 +1,42 @@ +/* eslint-disable import/no-extraneous-dependencies */ +/* eslint react/prop-types: off */ + +// import React from 'react'; +// import { render } from '@testing-library/react'; +// import { fireEvent } from '@testing-library/dom'; +// import '@testing-library/jest-dom'; + +// import { +// FormattedMessage, +// IntlProvider, +// } from '@edx/frontend-platform/i18n'; + +// import { isAdminHelper, navLinksPlugin } from './mocks/PluginComponentsMock'; + +/* +test for +when given a plugin config with default content but no changes, return only default content +when using insert, plugin is unchanged +when using modify, plugin is changed +when using hide, plugin is missing +when using wrap (use plugin components mock for this), plugin renders based on condition +when using wrap, plugin has wrapped component (refer to classname) +*/ + +describe('When given a pluginConfig', () => { + describe('when there is no defaultContent', () => { + it('should return an empty array when there are no changes', () => { + }); + it('should return an array of only new plugins inserted', () => { + + }); + }); + describe('when there is defaultContent', () => { + it('should return the same default plugins if no changes', () => { + + }); + it('should return an array with each plugin including property "hidden" if operation is Hidden', () => { + + }); + }); +}); diff --git a/src/plugins/directPlugins/test/DirectPluginSlot.test.jsx b/src/plugins/directPlugins/test/DirectPluginSlot.test.jsx new file mode 100644 index 00000000..7bb97d30 --- /dev/null +++ b/src/plugins/directPlugins/test/DirectPluginSlot.test.jsx @@ -0,0 +1,4 @@ +// when no additional plugins, only render default +// when no defaults, only plugin is rendered +// when no plugins and no defaults, nothing is rendered +// when more than one plugin, priority is followed diff --git a/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx b/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx new file mode 100644 index 00000000..8d76c4d2 --- /dev/null +++ b/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx @@ -0,0 +1,33 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import { Icon } from '@edx/paragon'; +import { DirectPluginSlot } from '../..'; +import { navLinksPlugin } from './PluginComponentsMock'; + +// TODO: remove DirectPluginsContext and enabledPlugins from here once we have an example app +// these simply demonstrate how the root App would have needed this setup in order to pass in the plugin config +// and make it available to a given DirectPluginSlot + +// const enabledPlugins = [ +// linksPlugin, +// ]; + +const MyApp = () => ( + // +
+ ( + + {link.content.label} + + )} + /> +
+ // +); + +export default MyApp; diff --git a/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx b/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx new file mode 100644 index 00000000..4fee8d1b --- /dev/null +++ b/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx @@ -0,0 +1,63 @@ +/* eslint-disable react/prop-types */ +// eslint-disable-next-line import/no-extraneous-dependencies +import React from 'react'; +import { House, Star, InsertDriveFile, Login } from '@edx/paragon/icons'; +import { DirectPluginOperations } from '../..'; + +/** This is for us to be able to mock in tests */ +const isAdminHelper = () => true; + +/** This is a React widget that wraps its children and makes them visible only to administrators */ +const HideExceptForAdmin = ({ widget }) => { + const isAdmin = isAdminHelper(); + return {isAdmin ? widget : null}; +}; + +const navLinksPlugin = { + id: 'links-demo', // id isn't used anywhere, but can be extended to + defaultComponentProps: [ + { + id: 'home', + priority: 5, + content: { url: '/', icon: House, label: 'Home' }, + }, + { + id: 'lookup', + priority: 25, + content: { url: '/lookup', icon: Star, label: 'Lookup' }, + }, + { + id: 'drafts', + priority: 35, + content: { url: '/drafts', icon: InsertDriveFile, label: 'Drafts' }, + }, + ], + getDirectSlotChanges() { + return { + 'side-bar-nav': [ // slot id that is used by directpluginslot + // Hide the "Drafts" link, except for administrators: + { + op: DirectPluginOperations.Wrap, + widgetId: 'drafts', + wrapper: HideExceptForAdmin, + }, + // Add a new login link after the rest of default plugins: + { + op: DirectPluginOperations.Insert, + widget: { + id: 'login', + priority: 50, + content: { + url: '/login', icon: Login, label: 'Login', + }, + }, + }, + ], + }; + }, +}; + +export { + isAdminHelper, + navLinksPlugin, +}; From ee48f04f903613ba826d24a91e094d8b335c4e68 Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Mon, 8 Jan 2024 23:42:53 +0000 Subject: [PATCH 04/11] test: add testing for the DirectPluginOperations logic --- src/plugins/directPlugins/DirectPlugin.jsx | 10 +- .../directPlugins/DirectPluginSlot.jsx | 11 ++ src/plugins/directPlugins/hooks.js | 46 +++++ src/plugins/directPlugins/index.js | 5 +- .../directPlugins/test/DirectPlugin.test.jsx | 20 +-- src/plugins/directPlugins/test/hooks.test.js | 158 ++++++++++++++++++ .../test/mocks/DefaultComponentMock.jsx | 40 ++--- .../test/mocks/PluginComponentsMock.jsx | 6 +- 8 files changed, 253 insertions(+), 43 deletions(-) create mode 100644 src/plugins/directPlugins/hooks.js create mode 100644 src/plugins/directPlugins/test/hooks.test.js diff --git a/src/plugins/directPlugins/DirectPlugin.jsx b/src/plugins/directPlugins/DirectPlugin.jsx index 9a93aa10..01055f0e 100644 --- a/src/plugins/directPlugins/DirectPlugin.jsx +++ b/src/plugins/directPlugins/DirectPlugin.jsx @@ -1,16 +1,18 @@ -/** ///// Code for plugins ///// */ +import React from 'react'; -// DirectPluginOperation +/** Context which makes the list of enabled plugins available to the components below it in the React tree */ +export const DirectPluginsContext = React.createContext([]); -/** +// DirectPluginOperation -export const DirectPluginOperation = { +export const DirectPluginOperations = { Insert: 'insert', Hide: 'hide', Modify: 'modify', Wrap: 'wrap', }; +/** it makes sense to have the definitions of ChangeOperation live in here, but the actual changes are tightly living in pluginslot because of the newContents array that is being changed. Ideally, the PluginSlot itself doesn't care about the plugins inside it, it only cares about returning some component with the empty React tags <> diff --git a/src/plugins/directPlugins/DirectPluginSlot.jsx b/src/plugins/directPlugins/DirectPluginSlot.jsx index a662af9f..900856e4 100644 --- a/src/plugins/directPlugins/DirectPluginSlot.jsx +++ b/src/plugins/directPlugins/DirectPluginSlot.jsx @@ -1,2 +1,13 @@ /** Context which makes the list of enabled plugins available to the components below it in the React tree */ // export const DirectPluginsContext = React.createContext([]); + +// whenever the PluginSlot gets called and needs to prepare for rendering +// call the DirectPluginsContext to get the enabledPlugins + +// sift through the enabledPlugins to find the relevant one we need for this slot +// enabledPlugins.forEach(plugin => { +// const changes = plugin.getUiSlotChanges(); // Optional: Pass in any app-specific context that the plugin wants +// const slotChanges = changes[slotId] ?? []; +// TODO: above could be condensed to: +// const changes = plugin.getUiSlotChanges()[slotId] ?? []; +// call on usePreparePlugins, passing in changes, which includes the pluginChanges and defaultcontent diff --git a/src/plugins/directPlugins/hooks.js b/src/plugins/directPlugins/hooks.js new file mode 100644 index 00000000..c06f8d95 --- /dev/null +++ b/src/plugins/directPlugins/hooks.js @@ -0,0 +1,46 @@ +import React from 'react'; +// allows mocking state modules from +// eslint-disable-next-line import/no-self-import + +import { DirectPluginOperations } from './DirectPlugin'; + +export const organizePlugins = (defaultContents, pluginChanges) => { + const newContents = [...defaultContents]; + + pluginChanges.forEach(change => { + if (change.op === DirectPluginOperations.Insert) { + newContents.push(change.widget); + } else if (change.op === DirectPluginOperations.Hide) { + const widget = newContents.find((w) => w.id === change.widgetId); + if (widget) { widget.hidden = true; } + } else if (change.op === DirectPluginOperations.Modify) { + const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); + if (widgetIdx >= 0) { + const widget = { ...newContents[widgetIdx] }; + newContents[widgetIdx] = change.fn(widget); + } + } else if (change.op === DirectPluginOperations.Wrap) { + const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); + if (widgetIdx >= 0) { + const newWidget = { wrappers: [], ...newContents[widgetIdx] }; + // refactor suggestion: remove bottom line and write wrappers: [change.wrapper] + newWidget.wrappers.push(change.wrapper); + newContents[widgetIdx] = newWidget; + } + } else { + throw new Error(`unknown plugin UI change operation: ${(change).op}`); + } + }); + + // newContents.sort((a, b) => (a.priority - b.priority) * 10_000 + a.id.localeCompare(b.id)); + return newContents; +}; + +export const useGetPlugins = (defaultContents, enabledPlugins) => { + const contents = React.useMemo(() => { + organizePlugins(defaultContents, enabledPlugins); + }, [defaultContents, enabledPlugins]); + return contents; +}; + +export default { useGetPlugins, organizePlugins }; diff --git a/src/plugins/directPlugins/index.js b/src/plugins/directPlugins/index.js index 7085dc66..4a12fc51 100644 --- a/src/plugins/directPlugins/index.js +++ b/src/plugins/directPlugins/index.js @@ -1,7 +1,10 @@ export { DirectPluginSlot, - DirectPluginsContext, } from './DirectPluginSlot'; export { + DirectPluginsContext, DirectPluginOperations, } from './DirectPlugin'; +export { + useGetPlugins, +} from './hooks'; diff --git a/src/plugins/directPlugins/test/DirectPlugin.test.jsx b/src/plugins/directPlugins/test/DirectPlugin.test.jsx index 40e64583..d8995798 100644 --- a/src/plugins/directPlugins/test/DirectPlugin.test.jsx +++ b/src/plugins/directPlugins/test/DirectPlugin.test.jsx @@ -10,8 +10,10 @@ // FormattedMessage, // IntlProvider, // } from '@edx/frontend-platform/i18n'; +// import { Login } from '@edx/paragon/icons'; // import { isAdminHelper, navLinksPlugin } from './mocks/PluginComponentsMock'; +// import { DirectPluginOperations, usePreparePlugins } from '../DirectPlugin'; /* test for @@ -23,20 +25,6 @@ when using wrap (use plugin components mock for this), plugin renders based on c when using wrap, plugin has wrapped component (refer to classname) */ -describe('When given a pluginConfig', () => { - describe('when there is no defaultContent', () => { - it('should return an empty array when there are no changes', () => { - }); - it('should return an array of only new plugins inserted', () => { +// describe('When given a pluginConfig', () => { - }); - }); - describe('when there is defaultContent', () => { - it('should return the same default plugins if no changes', () => { - - }); - it('should return an array with each plugin including property "hidden" if operation is Hidden', () => { - - }); - }); -}); +// }); diff --git a/src/plugins/directPlugins/test/hooks.test.js b/src/plugins/directPlugins/test/hooks.test.js new file mode 100644 index 00000000..29763442 --- /dev/null +++ b/src/plugins/directPlugins/test/hooks.test.js @@ -0,0 +1,158 @@ +// import React from 'react'; +import '@testing-library/jest-dom'; + +import * as hooks from '../hooks'; +import { DirectPluginOperations } from '../DirectPlugin'; + +jest.unmock('../hooks'); + +let mockEnabledPlugins = [ + { + op: DirectPluginOperations.Wrap, + widgetId: 'drafts', + wrapper: jest.fn(), + }, + { + op: DirectPluginOperations.Hide, + widgetId: 'home', + }, + { + op: DirectPluginOperations.Modify, + widgetId: 'lookUp', + fn: jest.fn((widget) => widget), + }, + { + op: DirectPluginOperations.Insert, + widget: { + id: 'login', + priority: 50, + content: { + url: '/login', label: 'Login', + }, + }, + }, +]; + +const mockModifyComponent = (widget) => { + const newContent = { + url: '/search', + label: 'Search', + }; + const modifiedWidget = widget; + modifiedWidget.content = newContent; + return modifiedWidget; +}; + +const mockDefaultContent = [ + { + id: 'home', + priority: 5, + content: { url: '/', label: 'Home' }, + }, + { + id: 'lookUp', + priority: 25, + content: { url: '/lookup', label: 'Lookup' }, + }, + { + id: 'drafts', + priority: 35, + content: { url: '/drafts', label: 'Drafts' }, + }, +]; + +describe('organizePlugins', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('when there is no defaultContent', () => { + it('should return an empty array when there are no enabledPlugins', () => { + const plugins = hooks.organizePlugins([], []); + expect(plugins.length).toBe(0); + expect(plugins).toEqual([]); + }); + + it('should return an array of changes for non-default plugins', () => { + const plugins = hooks.organizePlugins([], mockEnabledPlugins); + expect(plugins.length).toEqual(1); + expect(plugins[0].id).toEqual('login'); + }); + }); + + describe('when there is defaultContent', () => { + it('should return an array of defaultContent if no enabledPlugins', () => { + mockEnabledPlugins = []; + const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + expect(plugins.length).toEqual(3); + expect(plugins).toEqual(mockDefaultContent); + }); + + it('should remove plugins with DirectOperation.Hide', () => { + const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const widget = plugins.find((w) => w.id === 'home'); + expect(plugins.length).toEqual(4); + expect(widget.hidden).toBe(true); + }); + + it('should modify plugins with DirectOperation.Modify', () => { + const pluginToModify = mockEnabledPlugins.find(w => w.widgetId === 'lookUp'); + pluginToModify.fn = mockModifyComponent; + const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const widget = plugins.find((w) => w.id === 'lookUp'); + + expect(plugins.length).toEqual(4); + expect(widget.content.url).toEqual('/search'); + }); + }); +}); + +describe('useGetPlugins', () => { + // navLinksPlugin.defaultComponentProps = jest.fn(() => []); + // navLinksPlugin.getDirectSlotChanges = jest.fn(() => ( + // { + // 'side-nav-bar': [], + // } + // )); + // beforeEach(jest.clearAllMocks); + + // it('should return an array if only new plugins inserted', () => { + // navLinksPlugin.getDirectSlotChanges = jest.fn(() => ( + // { + // 'side-nav-bar': [ + // { + // op: DirectPluginOperations.Insert, + // widget: { + // id: 'login', + // priority: 50, + // content: { + // url: '/login', icon: Login, label: 'Login', + // }, + // }, + // }, + // ], + // } + // )); + + // const expectedChanges = { + // 'side-nav-bar': [ + // { + // widget: { + // id: 'login', + // priority: 50, + // content: { + // url: '/login', icon: Login, label: 'Login', + // }, + // }, + // }, + // ], + // }; + + // expect(navLinksPlugin.useGetPlugins.length()).toBe(1); + // expect(navLinksPlugin.useGetPlugins).toBe(expectedChanges); + // }); +}); diff --git a/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx b/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx index 8d76c4d2..33f4ca18 100644 --- a/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx +++ b/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx @@ -1,33 +1,33 @@ // eslint-disable-next-line import/no-extraneous-dependencies import { Icon } from '@edx/paragon'; -import { DirectPluginSlot } from '../..'; +import { DirectPluginSlot, DirectPluginsContext } from '../..'; import { navLinksPlugin } from './PluginComponentsMock'; // TODO: remove DirectPluginsContext and enabledPlugins from here once we have an example app // these simply demonstrate how the root App would have needed this setup in order to pass in the plugin config // and make it available to a given DirectPluginSlot -// const enabledPlugins = [ -// linksPlugin, -// ]; +const enabledPlugins = [ + navLinksPlugin, +]; const MyApp = () => ( - // -
- ( - - {link.content.label} - - )} - /> -
- // + +
+ ( + + {link.content.label} + + )} + /> +
+ ); export default MyApp; diff --git a/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx b/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx index 4fee8d1b..8fd814d6 100644 --- a/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx +++ b/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx @@ -1,7 +1,9 @@ /* eslint-disable react/prop-types */ // eslint-disable-next-line import/no-extraneous-dependencies import React from 'react'; -import { House, Star, InsertDriveFile, Login } from '@edx/paragon/icons'; +import { + House, Star, InsertDriveFile, Login, +} from '@edx/paragon/icons'; import { DirectPluginOperations } from '../..'; /** This is for us to be able to mock in tests */ @@ -14,7 +16,7 @@ const HideExceptForAdmin = ({ widget }) => { }; const navLinksPlugin = { - id: 'links-demo', // id isn't used anywhere, but can be extended to + id: 'links-demo', // id isn't used anywhere, but can be extended to defaultComponentProps: [ { id: 'home', From 17d1fd9e484e29f51ea8784dd6e6425a683095a9 Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Wed, 10 Jan 2024 17:34:43 +0000 Subject: [PATCH 05/11] test: add tests for DirectOperation.Wrap and error handling --- src/plugins/directPlugins/hooks.jsx | 45 +++++++++ .../test/{hooks.test.js => hooks.test.jsx} | 92 ++++++++++++++----- 2 files changed, 115 insertions(+), 22 deletions(-) create mode 100644 src/plugins/directPlugins/hooks.jsx rename src/plugins/directPlugins/test/{hooks.test.js => hooks.test.jsx} (65%) diff --git a/src/plugins/directPlugins/hooks.jsx b/src/plugins/directPlugins/hooks.jsx new file mode 100644 index 00000000..ca5efe71 --- /dev/null +++ b/src/plugins/directPlugins/hooks.jsx @@ -0,0 +1,45 @@ +import React from 'react'; +// allows mocking state modules from +// eslint-disable-next-line import/no-self-import + +import { DirectPluginOperations } from './DirectPlugin'; + +export const organizePlugins = (defaultContents, pluginChanges) => { + const newContents = [...defaultContents]; + + pluginChanges.forEach(change => { + if (change.op === DirectPluginOperations.Insert) { + newContents.push(change.widget); + } else if (change.op === DirectPluginOperations.Hide) { + const widget = newContents.find((w) => w.id === change.widgetId); + if (widget) { widget.hidden = true; } + } else if (change.op === DirectPluginOperations.Modify) { + const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); + if (widgetIdx >= 0) { + const widget = { ...newContents[widgetIdx] }; + newContents[widgetIdx] = change.fn(widget); + } + } else if (change.op === DirectPluginOperations.Wrap) { + const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); + if (widgetIdx >= 0) { + const newWidget = { wrappers: [], ...newContents[widgetIdx] }; + newWidget.wrappers.push(change.wrapper); + newContents[widgetIdx] = newWidget; + } + } else { + throw new Error('unknown direct plugin change operation'); + } + }); + + newContents.sort((a, b) => (a.priority - b.priority) * 10_000 + a.id.localeCompare(b.id)); + return newContents; +}; + +export const useGetPlugins = (defaultContents, enabledPlugins) => { + const contents = React.useMemo(() => { + organizePlugins(defaultContents, enabledPlugins); + }, [defaultContents, enabledPlugins]); + return contents; +}; + +export default { useGetPlugins, organizePlugins }; diff --git a/src/plugins/directPlugins/test/hooks.test.js b/src/plugins/directPlugins/test/hooks.test.jsx similarity index 65% rename from src/plugins/directPlugins/test/hooks.test.js rename to src/plugins/directPlugins/test/hooks.test.jsx index 29763442..9d985c85 100644 --- a/src/plugins/directPlugins/test/hooks.test.js +++ b/src/plugins/directPlugins/test/hooks.test.jsx @@ -1,4 +1,4 @@ -// import React from 'react'; +import React from 'react'; import '@testing-library/jest-dom'; import * as hooks from '../hooks'; @@ -6,11 +6,27 @@ import { DirectPluginOperations } from '../DirectPlugin'; jest.unmock('../hooks'); -let mockEnabledPlugins = [ +const mockModifyComponent = (widget) => { + const newContent = { + url: '/search', + label: 'Search', + }; + const modifiedWidget = widget; + modifiedWidget.content = newContent; + return modifiedWidget; +}; + +/** This is a React widget that wraps its children and makes them visible only to administrators */ +function mockWrapComponent({ widget }) { + const isAdmin = true; + return isAdmin ? widget : null; +} + +const mockEnabledPlugins = [ { op: DirectPluginOperations.Wrap, widgetId: 'drafts', - wrapper: jest.fn(), + wrapper: mockWrapComponent, }, { op: DirectPluginOperations.Hide, @@ -19,7 +35,7 @@ let mockEnabledPlugins = [ { op: DirectPluginOperations.Modify, widgetId: 'lookUp', - fn: jest.fn((widget) => widget), + fn: mockModifyComponent, }, { op: DirectPluginOperations.Insert, @@ -33,16 +49,6 @@ let mockEnabledPlugins = [ }, ]; -const mockModifyComponent = (widget) => { - const newContent = { - url: '/search', - label: 'Search', - }; - const modifiedWidget = widget; - modifiedWidget.content = newContent; - return modifiedWidget; -}; - const mockDefaultContent = [ { id: 'home', @@ -66,11 +72,11 @@ describe('organizePlugins', () => { jest.clearAllMocks(); }); - afterEach(() => { - jest.clearAllMocks(); - }); - describe('when there is no defaultContent', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + it('should return an empty array when there are no enabledPlugins', () => { const plugins = hooks.organizePlugins([], []); expect(plugins.length).toBe(0); @@ -85,9 +91,12 @@ describe('organizePlugins', () => { }); describe('when there is defaultContent', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + it('should return an array of defaultContent if no enabledPlugins', () => { - mockEnabledPlugins = []; - const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const plugins = hooks.organizePlugins(mockDefaultContent, []); expect(plugins.length).toEqual(3); expect(plugins).toEqual(mockDefaultContent); }); @@ -100,14 +109,53 @@ describe('organizePlugins', () => { }); it('should modify plugins with DirectOperation.Modify', () => { - const pluginToModify = mockEnabledPlugins.find(w => w.widgetId === 'lookUp'); - pluginToModify.fn = mockModifyComponent; const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); const widget = plugins.find((w) => w.id === 'lookUp'); expect(plugins.length).toEqual(4); expect(widget.content.url).toEqual('/search'); }); + + it('should wrap plugins with DirectOperation.Wrap', () => { + const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const widget = plugins.find((w) => w.id === 'drafts'); + expect(plugins.length).toEqual(4); + expect(widget.wrappers.length).toEqual(1); + }); + + it('should accept several wrappers for a single plugin with DirectOperation.Wrap', () => { + const newMockWrapComponent = ({ widget }) => { + const isStudent = false; + return isStudent ? null : widget; + }; + const newPluginChange = { + op: DirectPluginOperations.Wrap, + widgetId: 'drafts', + wrapper: newMockWrapComponent, + }; + mockEnabledPlugins.push(newPluginChange); + const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const widget = plugins.find((w) => w.id === 'drafts'); + expect(plugins.length).toEqual(4); + expect(widget.wrappers.length).toEqual(2); + expect(widget.wrappers[0]).toEqual(mockWrapComponent); + expect(widget.wrappers[1]).toEqual(newMockWrapComponent); + }); + + it('should raise an error for an operation that does not exist', async () => { + const badPluginChange = { + op: DirectPluginOperations.Destroy, + widgetId: 'drafts', + }; + mockEnabledPlugins.push(badPluginChange); + + expect.assertions(1); + try { + await hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + } catch (error) { + expect(error.message).toBe('unknown direct plugin change operation'); + } + }); }); }); From 6379a9c8293b52e7864ead1f65f3cdc5a629798c Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Wed, 10 Jan 2024 17:57:10 +0000 Subject: [PATCH 06/11] test: add test for priority sort of organizePlugins --- src/plugins/directPlugins/hooks.js | 46 ------------------- src/plugins/directPlugins/test/hooks.test.jsx | 23 +++++++++- 2 files changed, 21 insertions(+), 48 deletions(-) delete mode 100644 src/plugins/directPlugins/hooks.js diff --git a/src/plugins/directPlugins/hooks.js b/src/plugins/directPlugins/hooks.js deleted file mode 100644 index c06f8d95..00000000 --- a/src/plugins/directPlugins/hooks.js +++ /dev/null @@ -1,46 +0,0 @@ -import React from 'react'; -// allows mocking state modules from -// eslint-disable-next-line import/no-self-import - -import { DirectPluginOperations } from './DirectPlugin'; - -export const organizePlugins = (defaultContents, pluginChanges) => { - const newContents = [...defaultContents]; - - pluginChanges.forEach(change => { - if (change.op === DirectPluginOperations.Insert) { - newContents.push(change.widget); - } else if (change.op === DirectPluginOperations.Hide) { - const widget = newContents.find((w) => w.id === change.widgetId); - if (widget) { widget.hidden = true; } - } else if (change.op === DirectPluginOperations.Modify) { - const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); - if (widgetIdx >= 0) { - const widget = { ...newContents[widgetIdx] }; - newContents[widgetIdx] = change.fn(widget); - } - } else if (change.op === DirectPluginOperations.Wrap) { - const widgetIdx = newContents.findIndex((w) => w.id === change.widgetId); - if (widgetIdx >= 0) { - const newWidget = { wrappers: [], ...newContents[widgetIdx] }; - // refactor suggestion: remove bottom line and write wrappers: [change.wrapper] - newWidget.wrappers.push(change.wrapper); - newContents[widgetIdx] = newWidget; - } - } else { - throw new Error(`unknown plugin UI change operation: ${(change).op}`); - } - }); - - // newContents.sort((a, b) => (a.priority - b.priority) * 10_000 + a.id.localeCompare(b.id)); - return newContents; -}; - -export const useGetPlugins = (defaultContents, enabledPlugins) => { - const contents = React.useMemo(() => { - organizePlugins(defaultContents, enabledPlugins); - }, [defaultContents, enabledPlugins]); - return contents; -}; - -export default { useGetPlugins, organizePlugins }; diff --git a/src/plugins/directPlugins/test/hooks.test.jsx b/src/plugins/directPlugins/test/hooks.test.jsx index 9d985c85..c12de6d7 100644 --- a/src/plugins/directPlugins/test/hooks.test.jsx +++ b/src/plugins/directPlugins/test/hooks.test.jsx @@ -1,4 +1,3 @@ -import React from 'react'; import '@testing-library/jest-dom'; import * as hooks from '../hooks'; @@ -16,7 +15,6 @@ const mockModifyComponent = (widget) => { return modifiedWidget; }; -/** This is a React widget that wraps its children and makes them visible only to administrators */ function mockWrapComponent({ widget }) { const isAdmin = true; return isAdmin ? widget : null; @@ -142,6 +140,27 @@ describe('organizePlugins', () => { expect(widget.wrappers[1]).toEqual(newMockWrapComponent); }); + it('should return plugins arranged by priority', () => { + const newPluginChange = { + op: DirectPluginOperations.Insert, + widget: { + id: 'profile', + priority: 1, + content: { + url: '/profile', label: 'Profile', + }, + }, + }; + mockEnabledPlugins.push(newPluginChange); + const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + expect(plugins.length).toEqual(5); + expect(plugins[0].id).toBe('profile'); + expect(plugins[1].id).toBe('home'); + expect(plugins[2].id).toBe('lookUp'); + expect(plugins[3].id).toBe('drafts'); + expect(plugins[4].id).toBe('login'); + }); + it('should raise an error for an operation that does not exist', async () => { const badPluginChange = { op: DirectPluginOperations.Destroy, From c6d63ce9259c8a73ea98d28ef53c5f931cd3468e Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Wed, 10 Jan 2024 20:47:50 +0000 Subject: [PATCH 07/11] refactor: reorganize and add/remove comments for next steps and defining functions --- src/plugins/directPlugins/DirectPlugin.jsx | 44 ++++++----- .../directPlugins/DirectPluginSlot.jsx | 17 +---- src/plugins/directPlugins/index.js | 6 +- .../{test => }/mocks/DefaultComponentMock.jsx | 6 +- .../{test => }/mocks/PluginComponentsMock.jsx | 2 +- .../directPlugins/test/DirectPlugin.test.jsx | 30 -------- .../test/DirectPluginSlot.test.jsx | 4 - .../directPlugins/{hooks.jsx => utils.jsx} | 13 +--- .../{test/hooks.test.jsx => utils.test.jsx} | 74 +++---------------- 9 files changed, 45 insertions(+), 151 deletions(-) rename src/plugins/directPlugins/{test => }/mocks/DefaultComponentMock.jsx (86%) rename src/plugins/directPlugins/{test => }/mocks/PluginComponentsMock.jsx (97%) delete mode 100644 src/plugins/directPlugins/test/DirectPlugin.test.jsx delete mode 100644 src/plugins/directPlugins/test/DirectPluginSlot.test.jsx rename src/plugins/directPlugins/{hooks.jsx => utils.jsx} (76%) rename src/plugins/directPlugins/{test/hooks.test.jsx => utils.test.jsx} (69%) diff --git a/src/plugins/directPlugins/DirectPlugin.jsx b/src/plugins/directPlugins/DirectPlugin.jsx index 01055f0e..9068e83c 100644 --- a/src/plugins/directPlugins/DirectPlugin.jsx +++ b/src/plugins/directPlugins/DirectPlugin.jsx @@ -1,9 +1,19 @@ import React from 'react'; -/** Context which makes the list of enabled plugins available to the components below it in the React tree */ -export const DirectPluginsContext = React.createContext([]); +/** + * Context which makes the list of enabled plugins available to the components below it in the React tree + */ +export const DirectPluginContext = React.createContext([]); -// DirectPluginOperation +/** + * @description DirectPluginOperation defines the changes to be made to either the default widget(s) or to any + * that are inserted + * @property {string} Insert - inserts a new widget into the DirectPluginSlot + * @property {string} Hide - used to hide a default widget based on the widgetId + * @property {string} Modify - used to modify/replace a widget's content + * @property {string} Wrap - wraps a widget with a React element or fragment + * + */ export const DirectPluginOperations = { Insert: 'insert', @@ -13,23 +23,11 @@ export const DirectPluginOperations = { }; /** -it makes sense to have the definitions of ChangeOperation live in here, but the actual changes are tightly living in -pluginslot because of the newContents array that is being changed. Ideally, the PluginSlot itself doesn't care about -the plugins inside it, it only cares about returning some component with the empty React tags <> - -export type DirectPluginOperation = - | { op: DirectPluginOperation.Insert; widget: UISlotWidget } - | { op: DirectPluginOperation.Hide; widgetId: string } - | { op: DirectPluginOperation.Modify; widgetId: string, fn: (widget: UISlotWidget) => UISlotWidget } - | { op: DirectPluginOperation.Wrap; widgetId: string, wrapper: React.FC<{widget: React.ReactElement}> }; - -*/ - -// idea: move enabledPlugins and all of the logic that contents receives into DirectPlugin.jsx -// this will mean that all of the logic for changing plugins will live in there. -// all that pluginSlot cares about is that if given contents then it will render it accordingly. - -// also consider maybe moving defaultContents into the same config as the plugin operations object. -// This will maybe bring it closer to the scenario where the defaultContents is just part of the config object -// alongside the plugin changes. Like, why would want to separate those two objects from each other when they're -// part of the same flow. + * A placeholder for what the pluginChanges configuration should include depending on the operation: + * pluginChanges = [ + * { op: DirectPluginOperation.Insert; widget: } + * { op: DirectPluginOperation.Hide; widgetId: string } + * { op: DirectPluginOperation.Modify; widgetId: string, fn: (widget: ) => } + * { op: DirectPluginOperation.Wrap; widgetId: string, wrapper: React.FC<{widget: React.ReactElement }> }; + * ] + */ diff --git a/src/plugins/directPlugins/DirectPluginSlot.jsx b/src/plugins/directPlugins/DirectPluginSlot.jsx index 900856e4..23e2a0ab 100644 --- a/src/plugins/directPlugins/DirectPluginSlot.jsx +++ b/src/plugins/directPlugins/DirectPluginSlot.jsx @@ -1,13 +1,4 @@ -/** Context which makes the list of enabled plugins available to the components below it in the React tree */ -// export const DirectPluginsContext = React.createContext([]); - -// whenever the PluginSlot gets called and needs to prepare for rendering -// call the DirectPluginsContext to get the enabledPlugins - -// sift through the enabledPlugins to find the relevant one we need for this slot -// enabledPlugins.forEach(plugin => { -// const changes = plugin.getUiSlotChanges(); // Optional: Pass in any app-specific context that the plugin wants -// const slotChanges = changes[slotId] ?? []; -// TODO: above could be condensed to: -// const changes = plugin.getUiSlotChanges()[slotId] ?? []; -// call on usePreparePlugins, passing in changes, which includes the pluginChanges and defaultcontent +// DirectPluginsSlot (defaultContents, slotId, renderWidget) +// call the DirectPluginsContext to get the enabledPlugins +// assign `contents` variable to a useMemo-wrapped usePreparePlugins (defaultContents, enabledPlugins) +// return logic to render plugins by mapping through contents diff --git a/src/plugins/directPlugins/index.js b/src/plugins/directPlugins/index.js index 4a12fc51..4fc549d9 100644 --- a/src/plugins/directPlugins/index.js +++ b/src/plugins/directPlugins/index.js @@ -2,9 +2,9 @@ export { DirectPluginSlot, } from './DirectPluginSlot'; export { - DirectPluginsContext, + DirectPluginContext, DirectPluginOperations, } from './DirectPlugin'; export { - useGetPlugins, -} from './hooks'; + organizePlugins, +} from './utils'; diff --git a/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx b/src/plugins/directPlugins/mocks/DefaultComponentMock.jsx similarity index 86% rename from src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx rename to src/plugins/directPlugins/mocks/DefaultComponentMock.jsx index 33f4ca18..32eaca15 100644 --- a/src/plugins/directPlugins/test/mocks/DefaultComponentMock.jsx +++ b/src/plugins/directPlugins/mocks/DefaultComponentMock.jsx @@ -1,6 +1,6 @@ // eslint-disable-next-line import/no-extraneous-dependencies import { Icon } from '@edx/paragon'; -import { DirectPluginSlot, DirectPluginsContext } from '../..'; +import { DirectPluginSlot, DirectPluginContext } from '..'; import { navLinksPlugin } from './PluginComponentsMock'; // TODO: remove DirectPluginsContext and enabledPlugins from here once we have an example app @@ -12,7 +12,7 @@ const enabledPlugins = [ ]; const MyApp = () => ( - +
( )} />
- + ); export default MyApp; diff --git a/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx b/src/plugins/directPlugins/mocks/PluginComponentsMock.jsx similarity index 97% rename from src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx rename to src/plugins/directPlugins/mocks/PluginComponentsMock.jsx index 8fd814d6..b85dd580 100644 --- a/src/plugins/directPlugins/test/mocks/PluginComponentsMock.jsx +++ b/src/plugins/directPlugins/mocks/PluginComponentsMock.jsx @@ -4,7 +4,7 @@ import React from 'react'; import { House, Star, InsertDriveFile, Login, } from '@edx/paragon/icons'; -import { DirectPluginOperations } from '../..'; +import { DirectPluginOperations } from '..'; /** This is for us to be able to mock in tests */ const isAdminHelper = () => true; diff --git a/src/plugins/directPlugins/test/DirectPlugin.test.jsx b/src/plugins/directPlugins/test/DirectPlugin.test.jsx deleted file mode 100644 index d8995798..00000000 --- a/src/plugins/directPlugins/test/DirectPlugin.test.jsx +++ /dev/null @@ -1,30 +0,0 @@ -/* eslint-disable import/no-extraneous-dependencies */ -/* eslint react/prop-types: off */ - -// import React from 'react'; -// import { render } from '@testing-library/react'; -// import { fireEvent } from '@testing-library/dom'; -// import '@testing-library/jest-dom'; - -// import { -// FormattedMessage, -// IntlProvider, -// } from '@edx/frontend-platform/i18n'; -// import { Login } from '@edx/paragon/icons'; - -// import { isAdminHelper, navLinksPlugin } from './mocks/PluginComponentsMock'; -// import { DirectPluginOperations, usePreparePlugins } from '../DirectPlugin'; - -/* -test for -when given a plugin config with default content but no changes, return only default content -when using insert, plugin is unchanged -when using modify, plugin is changed -when using hide, plugin is missing -when using wrap (use plugin components mock for this), plugin renders based on condition -when using wrap, plugin has wrapped component (refer to classname) -*/ - -// describe('When given a pluginConfig', () => { - -// }); diff --git a/src/plugins/directPlugins/test/DirectPluginSlot.test.jsx b/src/plugins/directPlugins/test/DirectPluginSlot.test.jsx deleted file mode 100644 index 7bb97d30..00000000 --- a/src/plugins/directPlugins/test/DirectPluginSlot.test.jsx +++ /dev/null @@ -1,4 +0,0 @@ -// when no additional plugins, only render default -// when no defaults, only plugin is rendered -// when no plugins and no defaults, nothing is rendered -// when more than one plugin, priority is followed diff --git a/src/plugins/directPlugins/hooks.jsx b/src/plugins/directPlugins/utils.jsx similarity index 76% rename from src/plugins/directPlugins/hooks.jsx rename to src/plugins/directPlugins/utils.jsx index ca5efe71..05abbed6 100644 --- a/src/plugins/directPlugins/hooks.jsx +++ b/src/plugins/directPlugins/utils.jsx @@ -1,7 +1,3 @@ -import React from 'react'; -// allows mocking state modules from -// eslint-disable-next-line import/no-self-import - import { DirectPluginOperations } from './DirectPlugin'; export const organizePlugins = (defaultContents, pluginChanges) => { @@ -35,11 +31,4 @@ export const organizePlugins = (defaultContents, pluginChanges) => { return newContents; }; -export const useGetPlugins = (defaultContents, enabledPlugins) => { - const contents = React.useMemo(() => { - organizePlugins(defaultContents, enabledPlugins); - }, [defaultContents, enabledPlugins]); - return contents; -}; - -export default { useGetPlugins, organizePlugins }; +export default { organizePlugins }; diff --git a/src/plugins/directPlugins/test/hooks.test.jsx b/src/plugins/directPlugins/utils.test.jsx similarity index 69% rename from src/plugins/directPlugins/test/hooks.test.jsx rename to src/plugins/directPlugins/utils.test.jsx index c12de6d7..ac7f08e0 100644 --- a/src/plugins/directPlugins/test/hooks.test.jsx +++ b/src/plugins/directPlugins/utils.test.jsx @@ -1,9 +1,9 @@ import '@testing-library/jest-dom'; -import * as hooks from '../hooks'; -import { DirectPluginOperations } from '../DirectPlugin'; +import * as utils from './utils'; +import { DirectPluginOperations } from './DirectPlugin'; -jest.unmock('../hooks'); +jest.unmock('./utils'); const mockModifyComponent = (widget) => { const newContent = { @@ -66,23 +66,19 @@ const mockDefaultContent = [ ]; describe('organizePlugins', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - describe('when there is no defaultContent', () => { afterEach(() => { jest.clearAllMocks(); }); it('should return an empty array when there are no enabledPlugins', () => { - const plugins = hooks.organizePlugins([], []); + const plugins = utils.organizePlugins([], []); expect(plugins.length).toBe(0); expect(plugins).toEqual([]); }); it('should return an array of changes for non-default plugins', () => { - const plugins = hooks.organizePlugins([], mockEnabledPlugins); + const plugins = utils.organizePlugins([], mockEnabledPlugins); expect(plugins.length).toEqual(1); expect(plugins[0].id).toEqual('login'); }); @@ -94,20 +90,20 @@ describe('organizePlugins', () => { }); it('should return an array of defaultContent if no enabledPlugins', () => { - const plugins = hooks.organizePlugins(mockDefaultContent, []); + const plugins = utils.organizePlugins(mockDefaultContent, []); expect(plugins.length).toEqual(3); expect(plugins).toEqual(mockDefaultContent); }); it('should remove plugins with DirectOperation.Hide', () => { - const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); const widget = plugins.find((w) => w.id === 'home'); expect(plugins.length).toEqual(4); expect(widget.hidden).toBe(true); }); it('should modify plugins with DirectOperation.Modify', () => { - const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); const widget = plugins.find((w) => w.id === 'lookUp'); expect(plugins.length).toEqual(4); @@ -115,7 +111,7 @@ describe('organizePlugins', () => { }); it('should wrap plugins with DirectOperation.Wrap', () => { - const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); const widget = plugins.find((w) => w.id === 'drafts'); expect(plugins.length).toEqual(4); expect(widget.wrappers.length).toEqual(1); @@ -132,7 +128,7 @@ describe('organizePlugins', () => { wrapper: newMockWrapComponent, }; mockEnabledPlugins.push(newPluginChange); - const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); const widget = plugins.find((w) => w.id === 'drafts'); expect(plugins.length).toEqual(4); expect(widget.wrappers.length).toEqual(2); @@ -152,7 +148,7 @@ describe('organizePlugins', () => { }, }; mockEnabledPlugins.push(newPluginChange); - const plugins = hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); expect(plugins.length).toEqual(5); expect(plugins[0].id).toBe('profile'); expect(plugins[1].id).toBe('home'); @@ -170,56 +166,10 @@ describe('organizePlugins', () => { expect.assertions(1); try { - await hooks.organizePlugins(mockDefaultContent, mockEnabledPlugins); + await utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); } catch (error) { expect(error.message).toBe('unknown direct plugin change operation'); } }); }); }); - -describe('useGetPlugins', () => { - // navLinksPlugin.defaultComponentProps = jest.fn(() => []); - // navLinksPlugin.getDirectSlotChanges = jest.fn(() => ( - // { - // 'side-nav-bar': [], - // } - // )); - // beforeEach(jest.clearAllMocks); - - // it('should return an array if only new plugins inserted', () => { - // navLinksPlugin.getDirectSlotChanges = jest.fn(() => ( - // { - // 'side-nav-bar': [ - // { - // op: DirectPluginOperations.Insert, - // widget: { - // id: 'login', - // priority: 50, - // content: { - // url: '/login', icon: Login, label: 'Login', - // }, - // }, - // }, - // ], - // } - // )); - - // const expectedChanges = { - // 'side-nav-bar': [ - // { - // widget: { - // id: 'login', - // priority: 50, - // content: { - // url: '/login', icon: Login, label: 'Login', - // }, - // }, - // }, - // ], - // }; - - // expect(navLinksPlugin.useGetPlugins.length()).toBe(1); - // expect(navLinksPlugin.useGetPlugins).toBe(expectedChanges); - // }); -}); From ead01819890244032a274925d8a7b5120ff96aa1 Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Thu, 11 Jan 2024 23:25:41 +0000 Subject: [PATCH 08/11] feat: add Direct Plugin Slot logic * added some further documentation to explain what the configuration of allSlotChanges and slotChanges should look like * changed variable naming from original source code to better define data objects * changed assumption of what is returned by DirectPluginContext to be a single object and not array --- src/plugins/directPlugins/DirectPlugin.jsx | 43 +++++++++++++---- .../directPlugins/DirectPluginSlot.jsx | 46 +++++++++++++++++-- src/plugins/directPlugins/utils.jsx | 13 ++++-- src/plugins/directPlugins/utils.test.jsx | 32 ++++++------- 4 files changed, 103 insertions(+), 31 deletions(-) diff --git a/src/plugins/directPlugins/DirectPlugin.jsx b/src/plugins/directPlugins/DirectPlugin.jsx index 9068e83c..7776c574 100644 --- a/src/plugins/directPlugins/DirectPlugin.jsx +++ b/src/plugins/directPlugins/DirectPlugin.jsx @@ -1,7 +1,8 @@ import React from 'react'; /** - * Context which makes the list of enabled plugins available to the components below it in the React tree + * Context which makes the list of all plugin changes (allPluginChanges) available to the components + * below it in the React tree */ export const DirectPluginContext = React.createContext([]); @@ -23,11 +24,37 @@ export const DirectPluginOperations = { }; /** - * A placeholder for what the pluginChanges configuration should include depending on the operation: - * pluginChanges = [ - * { op: DirectPluginOperation.Insert; widget: } - * { op: DirectPluginOperation.Hide; widgetId: string } - * { op: DirectPluginOperation.Modify; widgetId: string, fn: (widget: ) => } - * { op: DirectPluginOperation.Wrap; widgetId: string, wrapper: React.FC<{widget: React.ReactElement }> }; - * ] + A placeholder for what allSlotChanges configuration should look like when passed into DirectPluginContext + { + id: "allDirectPluginChanges", + getDirectSlotChanges() { + return { + "main-nav": [ + // Hide the "Drafts" link, except for administrators: + { + op: UiChangeOperation.Wrap, + widgetId: "drafts", + wrapper: HideExceptForAdmin, + }, + // Add a new login link: + { + op: UiChangeOperation.Insert, + widget: { id: "login", priority: 50, content: { + url: "/login", icon: "person-fill", label: + }}, + }, + ], + }; + }, + }; + */ + +/** + A placeholder for what the slotChanges configuration should include depending on the operation: + slotChanges = [ + { op: DirectPluginOperation.Insert; widget: }, + { op: DirectPluginOperation.Hide; widgetId: string }, + { op: DirectPluginOperation.Modify; widgetId: string, fn: (widget: ) => }, + { op: DirectPluginOperation.Wrap; widgetId: string, wrapper: React.FC<{widget: React.ReactElement }> }, + ] */ diff --git a/src/plugins/directPlugins/DirectPluginSlot.jsx b/src/plugins/directPlugins/DirectPluginSlot.jsx index 23e2a0ab..4608b5aa 100644 --- a/src/plugins/directPlugins/DirectPluginSlot.jsx +++ b/src/plugins/directPlugins/DirectPluginSlot.jsx @@ -1,4 +1,42 @@ -// DirectPluginsSlot (defaultContents, slotId, renderWidget) -// call the DirectPluginsContext to get the enabledPlugins -// assign `contents` variable to a useMemo-wrapped usePreparePlugins (defaultContents, enabledPlugins) -// return logic to render plugins by mapping through contents +import React from 'react'; +import PropTypes from 'prop-types'; + +import { DirectPluginContext } from './DirectPlugin'; +import organizePlugins from './utils'; + +const DirectPluginSlot = ({ defaultContents, slotId, renderWidget }) => { + const allPluginChanges = React.useContext(DirectPluginContext); + + const contents = React.useMemo(() => { + const slotChanges = allPluginChanges.getDirectSlotChanges()[slotId] ?? []; + return organizePlugins(defaultContents, slotChanges); + }, [allPluginChanges, defaultContents, slotId]); + + return ( + <> + {contents.map((c) => { + if (c.hidden) { + return null; + } + if (c.wrappers) { + // TODO: define how the reduce logic is able to wrap widgets and make it testable + // eslint-disable-next-line max-len + return c.wrappers.reduce((widget, wrapper) => React.createElement(wrapper, { widget, key: c.id }), renderWidget(c)); + } + return renderWidget(c); + })} + + ); +}; + +DirectPluginSlot.propTypes = { + defaultContents: PropTypes.shape([]), + slotId: PropTypes.string.isRequired, + renderWidget: PropTypes.func.isRequired, +}; + +DirectPluginSlot.defaultProps = { + defaultContents: [], +}; + +export default { DirectPluginSlot }; diff --git a/src/plugins/directPlugins/utils.jsx b/src/plugins/directPlugins/utils.jsx index 05abbed6..2b2ff21a 100644 --- a/src/plugins/directPlugins/utils.jsx +++ b/src/plugins/directPlugins/utils.jsx @@ -1,9 +1,16 @@ import { DirectPluginOperations } from './DirectPlugin'; -export const organizePlugins = (defaultContents, pluginChanges) => { +/** + * Called by DirectPluginSlot to prepare the plugin changes for the given slot + * + * @param {Array} defaultContents - The default widgets where the plugin slot exists. + * @param {Array} slotChanges - All of the changes assigned to the specific plugin slot + * @returns {Array} - A sorted list of widgets with any additional properties needed to render them in the plugin slot + */ +const organizePlugins = (defaultContents, slotChanges) => { const newContents = [...defaultContents]; - pluginChanges.forEach(change => { + slotChanges.forEach(change => { if (change.op === DirectPluginOperations.Insert) { newContents.push(change.widget); } else if (change.op === DirectPluginOperations.Hide) { @@ -31,4 +38,4 @@ export const organizePlugins = (defaultContents, pluginChanges) => { return newContents; }; -export default { organizePlugins }; +export default organizePlugins; diff --git a/src/plugins/directPlugins/utils.test.jsx b/src/plugins/directPlugins/utils.test.jsx index ac7f08e0..423bf3bb 100644 --- a/src/plugins/directPlugins/utils.test.jsx +++ b/src/plugins/directPlugins/utils.test.jsx @@ -1,6 +1,6 @@ import '@testing-library/jest-dom'; -import * as utils from './utils'; +import organizePlugins from './utils'; import { DirectPluginOperations } from './DirectPlugin'; jest.unmock('./utils'); @@ -20,7 +20,7 @@ function mockWrapComponent({ widget }) { return isAdmin ? widget : null; } -const mockEnabledPlugins = [ +const mockSlotChanges = [ { op: DirectPluginOperations.Wrap, widgetId: 'drafts', @@ -71,14 +71,14 @@ describe('organizePlugins', () => { jest.clearAllMocks(); }); - it('should return an empty array when there are no enabledPlugins', () => { - const plugins = utils.organizePlugins([], []); + it('should return an empty array when there are no changes or additions to slot', () => { + const plugins = organizePlugins([], []); expect(plugins.length).toBe(0); expect(plugins).toEqual([]); }); it('should return an array of changes for non-default plugins', () => { - const plugins = utils.organizePlugins([], mockEnabledPlugins); + const plugins = organizePlugins([], mockSlotChanges); expect(plugins.length).toEqual(1); expect(plugins[0].id).toEqual('login'); }); @@ -89,21 +89,21 @@ describe('organizePlugins', () => { jest.clearAllMocks(); }); - it('should return an array of defaultContent if no enabledPlugins', () => { - const plugins = utils.organizePlugins(mockDefaultContent, []); + it('should return an array of defaultContent if no changes for plugins in slot', () => { + const plugins = organizePlugins(mockDefaultContent, []); expect(plugins.length).toEqual(3); expect(plugins).toEqual(mockDefaultContent); }); it('should remove plugins with DirectOperation.Hide', () => { - const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); const widget = plugins.find((w) => w.id === 'home'); expect(plugins.length).toEqual(4); expect(widget.hidden).toBe(true); }); it('should modify plugins with DirectOperation.Modify', () => { - const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); const widget = plugins.find((w) => w.id === 'lookUp'); expect(plugins.length).toEqual(4); @@ -111,7 +111,7 @@ describe('organizePlugins', () => { }); it('should wrap plugins with DirectOperation.Wrap', () => { - const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); + const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); const widget = plugins.find((w) => w.id === 'drafts'); expect(plugins.length).toEqual(4); expect(widget.wrappers.length).toEqual(1); @@ -127,8 +127,8 @@ describe('organizePlugins', () => { widgetId: 'drafts', wrapper: newMockWrapComponent, }; - mockEnabledPlugins.push(newPluginChange); - const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); + mockSlotChanges.push(newPluginChange); + const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); const widget = plugins.find((w) => w.id === 'drafts'); expect(plugins.length).toEqual(4); expect(widget.wrappers.length).toEqual(2); @@ -147,8 +147,8 @@ describe('organizePlugins', () => { }, }, }; - mockEnabledPlugins.push(newPluginChange); - const plugins = utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); + mockSlotChanges.push(newPluginChange); + const plugins = organizePlugins(mockDefaultContent, mockSlotChanges); expect(plugins.length).toEqual(5); expect(plugins[0].id).toBe('profile'); expect(plugins[1].id).toBe('home'); @@ -162,11 +162,11 @@ describe('organizePlugins', () => { op: DirectPluginOperations.Destroy, widgetId: 'drafts', }; - mockEnabledPlugins.push(badPluginChange); + mockSlotChanges.push(badPluginChange); expect.assertions(1); try { - await utils.organizePlugins(mockDefaultContent, mockEnabledPlugins); + await organizePlugins(mockDefaultContent, mockSlotChanges); } catch (error) { expect(error.message).toBe('unknown direct plugin change operation'); } From 967e674f4edf77c73e265316a424d335cd17f0c1 Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Tue, 16 Jan 2024 16:58:57 +0000 Subject: [PATCH 09/11] fix: import issue --- src/plugins/directPlugins/DirectPluginSlot.jsx | 2 +- src/plugins/directPlugins/index.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/directPlugins/DirectPluginSlot.jsx b/src/plugins/directPlugins/DirectPluginSlot.jsx index 4608b5aa..fa340a94 100644 --- a/src/plugins/directPlugins/DirectPluginSlot.jsx +++ b/src/plugins/directPlugins/DirectPluginSlot.jsx @@ -39,4 +39,4 @@ DirectPluginSlot.defaultProps = { defaultContents: [], }; -export default { DirectPluginSlot }; +export default DirectPluginSlot; diff --git a/src/plugins/directPlugins/index.js b/src/plugins/directPlugins/index.js index 4fc549d9..e37a65cb 100644 --- a/src/plugins/directPlugins/index.js +++ b/src/plugins/directPlugins/index.js @@ -1,10 +1,10 @@ export { - DirectPluginSlot, + default as DirectPluginSlot, } from './DirectPluginSlot'; export { DirectPluginContext, DirectPluginOperations, } from './DirectPlugin'; export { - organizePlugins, + default as organizePlugins, } from './utils'; From 75423f4dd77e5604bbe028b0b5b2dc9618b8443e Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Tue, 16 Jan 2024 20:51:36 +0000 Subject: [PATCH 10/11] docs: clarify some language around defining what plugin slot changes should look like --- src/plugins/directPlugins/DirectPlugin.jsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plugins/directPlugins/DirectPlugin.jsx b/src/plugins/directPlugins/DirectPlugin.jsx index 7776c574..a27c9528 100644 --- a/src/plugins/directPlugins/DirectPlugin.jsx +++ b/src/plugins/directPlugins/DirectPlugin.jsx @@ -24,7 +24,7 @@ export const DirectPluginOperations = { }; /** - A placeholder for what allSlotChanges configuration should look like when passed into DirectPluginContext + This is what the allSlotChanges configuration should look like when passed into DirectPluginContext { id: "allDirectPluginChanges", getDirectSlotChanges() { @@ -32,13 +32,13 @@ export const DirectPluginOperations = { "main-nav": [ // Hide the "Drafts" link, except for administrators: { - op: UiChangeOperation.Wrap, + op: DirectPluginChangeOperation.Wrap, widgetId: "drafts", wrapper: HideExceptForAdmin, }, // Add a new login link: { - op: UiChangeOperation.Insert, + op: DirectPluginChangeOperation.Insert, widget: { id: "login", priority: 50, content: { url: "/login", icon: "person-fill", label: }}, @@ -50,7 +50,7 @@ export const DirectPluginOperations = { */ /** - A placeholder for what the slotChanges configuration should include depending on the operation: + This is what a slotChanges configuration should include depending on the operation: slotChanges = [ { op: DirectPluginOperation.Insert; widget: }, { op: DirectPluginOperation.Hide; widgetId: string }, From 7b0326e077b4d5376f86384a917afa1b8f30b1cb Mon Sep 17 00:00:00 2001 From: Jason Wesson Date: Wed, 17 Jan 2024 20:23:43 +0000 Subject: [PATCH 11/11] refactor: change some variable naming and remove unused code --- .../directPlugins/DirectPluginSlot.jsx | 12 ++-- .../mocks/DefaultComponentMock.jsx | 33 ---------- .../mocks/PluginComponentsMock.jsx | 65 ------------------- src/plugins/directPlugins/utils.test.jsx | 12 ++-- 4 files changed, 11 insertions(+), 111 deletions(-) delete mode 100644 src/plugins/directPlugins/mocks/DefaultComponentMock.jsx delete mode 100644 src/plugins/directPlugins/mocks/PluginComponentsMock.jsx diff --git a/src/plugins/directPlugins/DirectPluginSlot.jsx b/src/plugins/directPlugins/DirectPluginSlot.jsx index fa340a94..b28afd52 100644 --- a/src/plugins/directPlugins/DirectPluginSlot.jsx +++ b/src/plugins/directPlugins/DirectPluginSlot.jsx @@ -7,23 +7,23 @@ import organizePlugins from './utils'; const DirectPluginSlot = ({ defaultContents, slotId, renderWidget }) => { const allPluginChanges = React.useContext(DirectPluginContext); - const contents = React.useMemo(() => { + const preparedWidgets = React.useMemo(() => { const slotChanges = allPluginChanges.getDirectSlotChanges()[slotId] ?? []; return organizePlugins(defaultContents, slotChanges); }, [allPluginChanges, defaultContents, slotId]); return ( <> - {contents.map((c) => { - if (c.hidden) { + {preparedWidgets.map((preppedWidget) => { + if (preppedWidget.hidden) { return null; } - if (c.wrappers) { + if (preppedWidget.wrappers) { // TODO: define how the reduce logic is able to wrap widgets and make it testable // eslint-disable-next-line max-len - return c.wrappers.reduce((widget, wrapper) => React.createElement(wrapper, { widget, key: c.id }), renderWidget(c)); + return preppedWidget.wrappers.reduce((widget, wrapper) => React.createElement(wrapper, { widget, key: preppedWidget.id }), renderWidget(preppedWidget)); } - return renderWidget(c); + return renderWidget(preppedWidget); })} ); diff --git a/src/plugins/directPlugins/mocks/DefaultComponentMock.jsx b/src/plugins/directPlugins/mocks/DefaultComponentMock.jsx deleted file mode 100644 index 32eaca15..00000000 --- a/src/plugins/directPlugins/mocks/DefaultComponentMock.jsx +++ /dev/null @@ -1,33 +0,0 @@ -// eslint-disable-next-line import/no-extraneous-dependencies -import { Icon } from '@edx/paragon'; -import { DirectPluginSlot, DirectPluginContext } from '..'; -import { navLinksPlugin } from './PluginComponentsMock'; - -// TODO: remove DirectPluginsContext and enabledPlugins from here once we have an example app -// these simply demonstrate how the root App would have needed this setup in order to pass in the plugin config -// and make it available to a given DirectPluginSlot - -const enabledPlugins = [ - navLinksPlugin, -]; - -const MyApp = () => ( - -
- ( - - {link.content.label} - - )} - /> -
- -); - -export default MyApp; diff --git a/src/plugins/directPlugins/mocks/PluginComponentsMock.jsx b/src/plugins/directPlugins/mocks/PluginComponentsMock.jsx deleted file mode 100644 index b85dd580..00000000 --- a/src/plugins/directPlugins/mocks/PluginComponentsMock.jsx +++ /dev/null @@ -1,65 +0,0 @@ -/* eslint-disable react/prop-types */ -// eslint-disable-next-line import/no-extraneous-dependencies -import React from 'react'; -import { - House, Star, InsertDriveFile, Login, -} from '@edx/paragon/icons'; -import { DirectPluginOperations } from '..'; - -/** This is for us to be able to mock in tests */ -const isAdminHelper = () => true; - -/** This is a React widget that wraps its children and makes them visible only to administrators */ -const HideExceptForAdmin = ({ widget }) => { - const isAdmin = isAdminHelper(); - return {isAdmin ? widget : null}; -}; - -const navLinksPlugin = { - id: 'links-demo', // id isn't used anywhere, but can be extended to - defaultComponentProps: [ - { - id: 'home', - priority: 5, - content: { url: '/', icon: House, label: 'Home' }, - }, - { - id: 'lookup', - priority: 25, - content: { url: '/lookup', icon: Star, label: 'Lookup' }, - }, - { - id: 'drafts', - priority: 35, - content: { url: '/drafts', icon: InsertDriveFile, label: 'Drafts' }, - }, - ], - getDirectSlotChanges() { - return { - 'side-bar-nav': [ // slot id that is used by directpluginslot - // Hide the "Drafts" link, except for administrators: - { - op: DirectPluginOperations.Wrap, - widgetId: 'drafts', - wrapper: HideExceptForAdmin, - }, - // Add a new login link after the rest of default plugins: - { - op: DirectPluginOperations.Insert, - widget: { - id: 'login', - priority: 50, - content: { - url: '/login', icon: Login, label: 'Login', - }, - }, - }, - ], - }; - }, -}; - -export { - isAdminHelper, - navLinksPlugin, -}; diff --git a/src/plugins/directPlugins/utils.test.jsx b/src/plugins/directPlugins/utils.test.jsx index 423bf3bb..961d40d4 100644 --- a/src/plugins/directPlugins/utils.test.jsx +++ b/src/plugins/directPlugins/utils.test.jsx @@ -3,9 +3,7 @@ import '@testing-library/jest-dom'; import organizePlugins from './utils'; import { DirectPluginOperations } from './DirectPlugin'; -jest.unmock('./utils'); - -const mockModifyComponent = (widget) => { +const mockModifyWidget = (widget) => { const newContent = { url: '/search', label: 'Search', @@ -15,7 +13,7 @@ const mockModifyComponent = (widget) => { return modifiedWidget; }; -function mockWrapComponent({ widget }) { +function mockWrapWidget({ widget }) { const isAdmin = true; return isAdmin ? widget : null; } @@ -24,7 +22,7 @@ const mockSlotChanges = [ { op: DirectPluginOperations.Wrap, widgetId: 'drafts', - wrapper: mockWrapComponent, + wrapper: mockWrapWidget, }, { op: DirectPluginOperations.Hide, @@ -33,7 +31,7 @@ const mockSlotChanges = [ { op: DirectPluginOperations.Modify, widgetId: 'lookUp', - fn: mockModifyComponent, + fn: mockModifyWidget, }, { op: DirectPluginOperations.Insert, @@ -132,7 +130,7 @@ describe('organizePlugins', () => { const widget = plugins.find((w) => w.id === 'drafts'); expect(plugins.length).toEqual(4); expect(widget.wrappers.length).toEqual(2); - expect(widget.wrappers[0]).toEqual(mockWrapComponent); + expect(widget.wrappers[0]).toEqual(mockWrapWidget); expect(widget.wrappers[1]).toEqual(newMockWrapComponent); });