From 5d805c5d6b9ea57275910ae8603212a3ec800a1f Mon Sep 17 00:00:00 2001 From: Mauricio Date: Wed, 29 Jun 2022 13:46:46 -0400 Subject: [PATCH 1/5] Added starting point for the new admin interface page --- .../server/config/permission_groups.json | 3 +- .../server/express_validators.js | 3 + .../obojobo-express/server/obo_express_dev.js | 3 +- .../app/obojobo-repository/server/index.js | 1 + .../server/models/admin_interface.js | 30 +++++++ .../server/models/admin_interface.test.js | 0 .../obojobo-repository/server/routes/admin.js | 27 ++++++ .../server/routes/admin.test.js | 0 .../obojobo-repository/server/routes/api.js | 14 +++ .../shared/actions/admin-actions.js | 34 ++++++++ .../shared/actions/admin-actions.test.js | 0 .../shared/components/admin-hoc.js | 11 +++ .../shared/components/admin-hoc.test.js | 34 ++++++++ .../shared/components/admin.jsx | 87 +++++++++++++++++++ .../shared/components/admin.scss | 42 +++++++++ .../components/pages/page-admin-client.jsx | 8 ++ .../components/pages/page-admin-server.jsx | 24 +++++ .../pages/page-admin-server.test.js | 35 ++++++++ .../shared/components/repository-nav.jsx | 5 ++ .../shared/reducers/admin-reducer.js | 17 ++++ .../shared/reducers/admin-reducer.test.js | 0 21 files changed, 376 insertions(+), 2 deletions(-) create mode 100644 packages/app/obojobo-repository/server/models/admin_interface.js create mode 100644 packages/app/obojobo-repository/server/models/admin_interface.test.js create mode 100644 packages/app/obojobo-repository/server/routes/admin.js create mode 100644 packages/app/obojobo-repository/server/routes/admin.test.js create mode 100644 packages/app/obojobo-repository/shared/actions/admin-actions.js create mode 100644 packages/app/obojobo-repository/shared/actions/admin-actions.test.js create mode 100644 packages/app/obojobo-repository/shared/components/admin-hoc.js create mode 100644 packages/app/obojobo-repository/shared/components/admin-hoc.test.js create mode 100644 packages/app/obojobo-repository/shared/components/admin.jsx create mode 100644 packages/app/obojobo-repository/shared/components/admin.scss create mode 100644 packages/app/obojobo-repository/shared/components/pages/page-admin-client.jsx create mode 100644 packages/app/obojobo-repository/shared/components/pages/page-admin-server.jsx create mode 100644 packages/app/obojobo-repository/shared/components/pages/page-admin-server.test.js create mode 100644 packages/app/obojobo-repository/shared/reducers/admin-reducer.js create mode 100644 packages/app/obojobo-repository/shared/reducers/admin-reducer.test.js diff --git a/packages/app/obojobo-express/server/config/permission_groups.json b/packages/app/obojobo-express/server/config/permission_groups.json index 4ecb9e960c..23242fb2e4 100644 --- a/packages/app/obojobo-express/server/config/permission_groups.json +++ b/packages/app/obojobo-express/server/config/permission_groups.json @@ -39,7 +39,8 @@ ], "canViewAsStudent": ["Learner", "urn:lti:role:ims/lis/Learner"], "canViewStatsPage": [], - "canViewSystemStats": [] + "canViewSystemStats": [], + "canViewAdminPage": [] }, "test": { "canDoThing": ["roleName"] diff --git a/packages/app/obojobo-express/server/express_validators.js b/packages/app/obojobo-express/server/express_validators.js index 9b84e8c613..bc40fac153 100644 --- a/packages/app/obojobo-express/server/express_validators.js +++ b/packages/app/obojobo-express/server/express_validators.js @@ -120,6 +120,9 @@ exports.requireCanViewStatsPage = (req, res, next) => exports.requireCanViewSystemStats = (req, res, next) => requireCurrentUser(req, res, next, 'canViewSystemStats') +exports.requireCanViewAdminPage = (req, res, next) => + requireCurrentUser(req, res, next, 'canViewAdminPage') + exports.checkValidationRules = (req, res, next) => { const errors = validationResult(req) if (!errors.isEmpty()) { diff --git a/packages/app/obojobo-express/server/obo_express_dev.js b/packages/app/obojobo-express/server/obo_express_dev.js index c608054d65..04897dd4a3 100644 --- a/packages/app/obojobo-express/server/obo_express_dev.js +++ b/packages/app/obojobo-express/server/obo_express_dev.js @@ -16,7 +16,8 @@ const POSSIBLE_PERMS = [ 'canDeleteDrafts', 'canPreviewDrafts', 'canViewStatsPage', - 'canViewSystemStats' + 'canViewSystemStats', + 'canViewAdminPage', ] // Normally the query running in User.saveOrCreate would auto-fill the new user's id, diff --git a/packages/app/obojobo-repository/server/index.js b/packages/app/obojobo-repository/server/index.js index 95f15a494a..1556e965ec 100644 --- a/packages/app/obojobo-repository/server/index.js +++ b/packages/app/obojobo-repository/server/index.js @@ -22,6 +22,7 @@ app.on('mount', app => { app.use('/', require('./routes/dashboard')) app.use('/', require('./routes/library')) app.use('/', require('./routes/stats')) + app.use('/', require('./routes/admin')) // register the event listeners require('./events') diff --git a/packages/app/obojobo-repository/server/models/admin_interface.js b/packages/app/obojobo-repository/server/models/admin_interface.js new file mode 100644 index 0000000000..69922ac860 --- /dev/null +++ b/packages/app/obojobo-repository/server/models/admin_interface.js @@ -0,0 +1,30 @@ +const db = require('obojobo-express/server/db') +const logger = require('obojobo-express/server/logger') + +class AdminInterface { + constructor({ id = null, title = '' }) { + console.log("AdminInterface built") + this.id = id + this.title = title + } + + // INSERT INTO user_perms VALUES (4, '{canViewStatsPage}'); + // INSERT INTO user_perms VALUES (4, '{canViewSystemStats}'); + static doSomething(id) { + console.log("doSomething id:") + console.log(id) + return db + .one(` + INSERT INTO user_perms + VALUES (1000, '{canViewStatsPage}') + `) + .then(selectResult => { + return new AdminInterface(selectResult) + }) + .catch(error => { + throw logger.logError('AdminInterface doSomething Error', error) + }) + } +} + +module.exports = AdminInterface \ No newline at end of file diff --git a/packages/app/obojobo-repository/server/models/admin_interface.test.js b/packages/app/obojobo-repository/server/models/admin_interface.test.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/app/obojobo-repository/server/routes/admin.js b/packages/app/obojobo-repository/server/routes/admin.js new file mode 100644 index 0000000000..18f3651816 --- /dev/null +++ b/packages/app/obojobo-repository/server/routes/admin.js @@ -0,0 +1,27 @@ +const express = require('express') +const router = express.Router() +const { webpackAssetPath } = require('obojobo-express/server/asset_resolver') +const { + requireCurrentUser, + requireCanViewAdminPage +} = require('obojobo-express/server/express_validators') + +// Admin page +// mounted as /admin +// NOTE: is an isomorphic react page +router + .route('/admin') + .get([requireCurrentUser, requireCanViewAdminPage]) + .get((req, res) => { + console.log("REACHED ADMIN ROUTE") + const props = { + title: 'Admin', + currentUser: req.currentUser, + // must use webpackAssetPath for all webpack assets to work in dev and production! + appCSSUrl: webpackAssetPath('admin.css'), + appJsUrl: webpackAssetPath('admin.js') + } + res.render('pages/page-admin-server.jsx', props) + }) + +module.exports = router diff --git a/packages/app/obojobo-repository/server/routes/admin.test.js b/packages/app/obojobo-repository/server/routes/admin.test.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/app/obojobo-repository/server/routes/api.js b/packages/app/obojobo-repository/server/routes/api.js index 9fb75cd19a..8fadfaf5ef 100644 --- a/packages/app/obojobo-repository/server/routes/api.js +++ b/packages/app/obojobo-repository/server/routes/api.js @@ -6,6 +6,7 @@ const Draft = require('obojobo-express/server/models/draft') const DraftSummary = require('../models/draft_summary') const DraftPermissions = require('../models/draft_permissions') const DraftsMetadata = require('../models/drafts_metadata') +const AdminInterface = require('../models/admin_interface') const { requireCanPreviewDrafts, requireCurrentUser, @@ -444,4 +445,17 @@ router } }) +// Do something with the admin interface +router + .route('/admin-do-something') + .post((req, res) => { + console.log("In the router, req.currentUser.id:") + console.log(req.currentUser.id) + + return AdminInterface.doSomething("an id") + .then(res.success) + .catch(res.unexpected) + }) + + module.exports = router diff --git a/packages/app/obojobo-repository/shared/actions/admin-actions.js b/packages/app/obojobo-repository/shared/actions/admin-actions.js new file mode 100644 index 0000000000..a6ed0291da --- /dev/null +++ b/packages/app/obojobo-repository/shared/actions/admin-actions.js @@ -0,0 +1,34 @@ +// =================== API ======================= + +const { apiGetAssessmentDetailsForMultipleDrafts } = require('./shared-api-methods') + +const JSON_MIME_TYPE = 'application/json' + +const defaultOptions = () => ({ + method: 'GET', + credentials: 'include', + headers: { + Accept: JSON_MIME_TYPE, + 'Content-Type': JSON_MIME_TYPE + } +}) + +const apiAdminDoSomething = () => { + console.log("apiAdminDoSomething function called") + const url = '/admin-do-something' + const options = { ...defaultOptions(), method: 'POST' } + return fetch(url, options).then(res => res.json()) +} + +// ================== ACTIONS =================== + +const DO_SOMETHING = 'DO_SOMETHING' +const doSomething = () => ({ + type: DO_SOMETHING, + promise: apiAdminDoSomething() +}) + +module.exports = { + DO_SOMETHING, + doSomething +} \ No newline at end of file diff --git a/packages/app/obojobo-repository/shared/actions/admin-actions.test.js b/packages/app/obojobo-repository/shared/actions/admin-actions.test.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/app/obojobo-repository/shared/components/admin-hoc.js b/packages/app/obojobo-repository/shared/components/admin-hoc.js new file mode 100644 index 0000000000..851e1cf540 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/admin-hoc.js @@ -0,0 +1,11 @@ +const Admin = require('./admin') +const connect = require('react-redux').connect +const { doSomething } = require('../actions/admin-actions') +const mapStoreStateToProps = state => state +const mapActionsToProps = { + doSomething +} +module.exports = connect( + mapStoreStateToProps, + mapActionsToProps +)(Admin) diff --git a/packages/app/obojobo-repository/shared/components/admin-hoc.test.js b/packages/app/obojobo-repository/shared/components/admin-hoc.test.js new file mode 100644 index 0000000000..dcc7a1b365 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/admin-hoc.test.js @@ -0,0 +1,34 @@ +jest.mock('react-redux') +jest.mock('../actions/admin-actions') +jest.mock('./admin', () => ({})) + +import AdminActions from '../actions/admin-actions' +import Admin from './admin' + +describe('admin HOC', () => { + test('redux collect is called with the correct arguments', () => { + const ReactRedux = require('react-redux') + + const mockReduxConnectReturn = jest.fn() + ReactRedux.connect = jest.fn(mapStoreStateToProps => { + // The first argument to 'connect' is a private (untestable) method in the HOC + // that method will never be 'covered' unless we call it like this + // since we also know that it should just return what it's given, we can test that too + const mapStoreStateToPropsArgs = { testKey: 'testProp' } + const mapStoreStateToPropsReturn = mapStoreStateToProps(mapStoreStateToPropsArgs) + expect(mapStoreStateToPropsReturn).toStrictEqual(mapStoreStateToPropsArgs) + return mockReduxConnectReturn + }) + + require('./admin-hoc') + + expect(ReactRedux.connect).toHaveBeenCalledTimes(1) + expect(ReactRedux.connect).toHaveBeenCalledWith(expect.any(Function), { + loadUserModuleList: AdminActions.loadUserModuleList, + loadModuleAssessmentDetails: AdminActions.loadModuleAssessmentDetails + }) + + expect(mockReduxConnectReturn).toHaveBeenCalledTimes(1) + expect(mockReduxConnectReturn).toHaveBeenCalledWith(Admin) + }) +}) diff --git a/packages/app/obojobo-repository/shared/components/admin.jsx b/packages/app/obojobo-repository/shared/components/admin.jsx new file mode 100644 index 0000000000..6af9796b0f --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/admin.jsx @@ -0,0 +1,87 @@ +require('./modal.scss') +require('./admin.scss') + +const React = require('react') +const { useState, useEffect } = require('react') +const RepositoryNav = require('./repository-nav') +const RepositoryBanner = require('./repository-banner') + +function Admin({ + currentUser, + title, + doSomething +}) { + + const [selectedDrafts, setSelectedDrafts] = useState([]) + const [search, setSearch] = useState('') + const [selectedUser, setSelectedUser] = useState(null) + const [selectedModule, setSelectedModule] = useState(null) + + // When the component is mounted in the browser, request the list of available modules for the current user + useEffect(() => { + loadUserModuleList() + }, []) + + const loadStats = () => { + loadModuleAssessmentDetails(selectedDrafts) + } + + const onSearchChange = event => { + setSearch(event.target.value) + } + + const onAddModuleAccess = () => { + console.log("onAddModuleAccess called") + doSomething() + } + + const onRemoveModuleAccess = () => { + + } + + // Renders any tools involving adding, deleting, or modifying module access + let toolsModuleAccess = [ +
+

Add module access

+ +
+

Select user:

+ +
+
+

Select module:

+ +
+ +
, +
+

Remove module access

+ +
+

Select user:

+ +
+
+

Select module:

+ +
+ +
+ ] + + return ( + + + + {toolsModuleAccess} + + ) +} + +module.exports = Admin diff --git a/packages/app/obojobo-repository/shared/components/admin.scss b/packages/app/obojobo-repository/shared/components/admin.scss new file mode 100644 index 0000000000..fafd6d395c --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/admin.scss @@ -0,0 +1,42 @@ +@import '../../client/css/defaults'; + +#admin-root { + p { + background-color: purple; + color: red; + } + + div { + background-color: green; + color: pink; + } + + .title { + background-color: salmon; + color: orange; + } + + .repository--main-content { + max-width: inherit; + } + + .repository--section-wrapper { + max-width: inherit; + } + + .repository--data-grid-drafts-container { + max-height: 30em; + overflow: scroll; + margin-bottom: 1em; + } + + .repository--assessment-stats { + height: 50em; + } + + .repository--drafts-search { + padding: 0.25em; + width: 14em; + margin-bottom: 1em; + } +} diff --git a/packages/app/obojobo-repository/shared/components/pages/page-admin-client.jsx b/packages/app/obojobo-repository/shared/components/pages/page-admin-client.jsx new file mode 100644 index 0000000000..d05e363127 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/pages/page-admin-client.jsx @@ -0,0 +1,8 @@ +import { hydrateEl } from '../../react-utils' +import Admin from '../admin-hoc' +import AdminReducer from '../../reducers/admin-reducer' + +// include LayoutDefault so client side react has a copy of it +import LayoutDefault from '../layouts/default' //eslint-disable-line no-unused-vars + +hydrateEl(Admin, AdminReducer, '#react-hydrate-root') diff --git a/packages/app/obojobo-repository/shared/components/pages/page-admin-server.jsx b/packages/app/obojobo-repository/shared/components/pages/page-admin-server.jsx new file mode 100644 index 0000000000..390a086d68 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/pages/page-admin-server.jsx @@ -0,0 +1,24 @@ +const React = require('react') +const LayoutDefault = require('../layouts/default') +const Admin = require('../admin-hoc') +const { propsToStore, createCommonReactApp, convertPropsToString } = require('../../react-utils') +const AdminReducer = require('../../reducers/admin-reducer') + +const PageAdminServer = props => { + return ( + + + {createCommonReactApp(Admin, propsToStore(AdminReducer, props))} + + + ) +} + +PageAdminServer.defaultProps = {} + +module.exports = PageAdminServer diff --git a/packages/app/obojobo-repository/shared/components/pages/page-admin-server.test.js b/packages/app/obojobo-repository/shared/components/pages/page-admin-server.test.js new file mode 100644 index 0000000000..74c4e06c60 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/pages/page-admin-server.test.js @@ -0,0 +1,35 @@ +import React from 'react' +import PageAdminServer from './page-admin-server' +import { shallow } from 'enzyme' + +jest.mock('../../react-utils') +jest.mock('../admin-hoc') +jest.mock('../../reducers/admin-reducer') + +const ReactUtils = require('../../react-utils') +const Admin = require('../admin-hoc') +const AdminReducer = require('../../reducers/admin-reducer') +const LayoutDefault = require('../layouts/default') + +describe('Server-side Admin Page', () => { + test('renders and runs functions correctly with default props', () => { + const mockStore = { + mockStoreKey: 'mockStoreVal' + } + + ReactUtils.propsToStore.mockReturnValueOnce(mockStore) + + const component = shallow() + + expect(ReactUtils.convertPropsToString).toHaveBeenCalledTimes(1) + expect(ReactUtils.convertPropsToString).toHaveBeenCalledWith(PageAdminServer.defaultProps) + + expect(ReactUtils.propsToStore).toHaveBeenCalledTimes(1) + expect(ReactUtils.propsToStore).toHaveBeenCalledWith(AdminReducer, PageAdminServer.defaultProps) + + expect(ReactUtils.createCommonReactApp).toHaveBeenCalledTimes(1) + expect(ReactUtils.createCommonReactApp).toHaveBeenCalledWith(Admin, mockStore) + + expect(component.find(LayoutDefault).length).toBe(1) + }) +}) diff --git a/packages/app/obojobo-repository/shared/components/repository-nav.jsx b/packages/app/obojobo-repository/shared/components/repository-nav.jsx index 361dfd0fd1..f95c072c74 100644 --- a/packages/app/obojobo-repository/shared/components/repository-nav.jsx +++ b/packages/app/obojobo-repository/shared/components/repository-nav.jsx @@ -46,6 +46,11 @@ const RepositoryNav = props => { Stats ) : null} + {props.userPerms.indexOf('canViewAdminPage') > -1 ? ( +
+ Admin +
+ ) : null} {props.userId !== 0 ? (
`; +exports[`RepositoryNav renders admin section with canViewAdminPage 1`] = ` +
+ +
+`; + exports[`RepositoryNav renders correctly with standard expected props 1`] = `
state const mapActionsToProps = { - doSomething + loadModuleList, + loadUserList, + addUserPermission, + removeUserPermission } module.exports = connect( mapStoreStateToProps, diff --git a/packages/app/obojobo-repository/shared/components/admin.jsx b/packages/app/obojobo-repository/shared/components/admin.jsx index 6af9796b0f..40843386cc 100644 --- a/packages/app/obojobo-repository/shared/components/admin.jsx +++ b/packages/app/obojobo-repository/shared/components/admin.jsx @@ -5,68 +5,135 @@ const React = require('react') const { useState, useEffect } = require('react') const RepositoryNav = require('./repository-nav') const RepositoryBanner = require('./repository-banner') +const Button = require('./button') + +const POSSIBLE_ROLES = [ + 'canViewEditor', + 'canCreateDrafts', + 'canDeleteDrafts', + 'canPreviewDrafts', + 'canViewStatsPage', + 'canViewSystemStats', + 'canViewAdminPage', +] + +const NO_SELECTION_USER = 'no-select-user' +const NO_SELECTION_MODULE = 'no-select-module' +const NO_SELECTION_PERMISSION = 'no-select-permission' function Admin({ currentUser, title, - doSomething + loadUserList, + addUserPermission, + removeUserPermission, }) { + const [users, setUsers] = useState([]) + const [fetching, setFetching] = useState(true) + const [selectedUser, setSelectedUser] = useState(NO_SELECTION_USER) + const [selectedPermission, setSelectedPermission] = useState(NO_SELECTION_PERMISSION) - const [selectedDrafts, setSelectedDrafts] = useState([]) - const [search, setSearch] = useState('') - const [selectedUser, setSelectedUser] = useState(null) - const [selectedModule, setSelectedModule] = useState(null) - - // When the component is mounted in the browser, request the list of available modules for the current user useEffect(() => { - loadUserModuleList() + // Load all modules on this obo instance + loadAllUsers() + .then(() => setFetching(null)) }, []) - const loadStats = () => { - loadModuleAssessmentDetails(selectedDrafts) + const loadAllUsers = () => { + return new Promise(async resolve => { + const allUsers = await loadUserList() + if (allUsers && + allUsers.payload && + allUsers.payload.value && + allUsers.payload.value && + allUsers.payload.value.length > 0) { + + setUsers(allUsers.payload.value) + }else { + setUsers([]) + } + + resolve() + }) } - const onSearchChange = event => { - setSearch(event.target.value) + const onAddUserPermission = async () => { + if (selectedUser === NO_SELECTION_USER) return + if (selectedPermission === NO_SELECTION_PERMISSION) return + + await addUserPermission(selectedUser, selectedPermission) } - const onAddModuleAccess = () => { - console.log("onAddModuleAccess called") - doSomething() + const onRemoveUserPermission = async () => { + if (selectedUser === NO_SELECTION_USER) return + if (selectedPermission === NO_SELECTION_PERMISSION) return + + await removeUserPermission(selectedUser, selectedPermission) } - const onRemoveModuleAccess = () => { + const renderPermissionSelectList = () => { + return ( + + ) + } + const renderUserSelectList = () => { + return ( + + ) } - // Renders any tools involving adding, deleting, or modifying module access - let toolsModuleAccess = [ + // Contains any tools involving modifying module access, user permission, etc + const getTools = () => [
-

Add module access

- +

Manage User Permissions

Select user:

- + {renderUserSelectList()}
-

Select module:

- +

Select permission:

+ {renderPermissionSelectList()}
- -
, -
-

Remove module access

- -
-

Select user:

- +
+ +
-
-

Select module:

- -
- -
+
, ] return ( @@ -79,7 +146,11 @@ function Admin({ noticeCount={0} /> - {toolsModuleAccess} +
+
+ {fetching ?

Loading...

: getTools()} +
+
) } diff --git a/packages/app/obojobo-repository/shared/components/admin.scss b/packages/app/obojobo-repository/shared/components/admin.scss index fafd6d395c..bcc0f6ff49 100644 --- a/packages/app/obojobo-repository/shared/components/admin.scss +++ b/packages/app/obojobo-repository/shared/components/admin.scss @@ -1,25 +1,6 @@ @import '../../client/css/defaults'; #admin-root { - p { - background-color: purple; - color: red; - } - - div { - background-color: green; - color: pink; - } - - .title { - background-color: salmon; - color: orange; - } - - .repository--main-content { - max-width: inherit; - } - .repository--section-wrapper { max-width: inherit; } @@ -39,4 +20,38 @@ width: 14em; margin-bottom: 1em; } + + .title { + background-color: $color-banner-bg; + color: $color-action-minor; + border-radius: 0.25em; + padding: 1em; + margin: 0; + } + + select { + padding: 0.8em; + border-radius: 0.25em; + min-width: 30%; + } + + .repository--button { + margin-top: 20px; + margin-bottom: 20px; + } + + .row { + display: flex; + align-items: center; + justify-content: space-between; + } + + .row.buttons { + justify-content: left; + gap: 1em; + } + + .tool-button { + width: 6em; + } } diff --git a/packages/app/obojobo-repository/shared/components/repository-nav.test.js b/packages/app/obojobo-repository/shared/components/repository-nav.test.js index fe5b672055..421fb7cd72 100644 --- a/packages/app/obojobo-repository/shared/components/repository-nav.test.js +++ b/packages/app/obojobo-repository/shared/components/repository-nav.test.js @@ -88,6 +88,21 @@ describe('RepositoryNav', () => { expect(component.toJSON()).toMatchSnapshot() }) + test('renders admin section with canViewAdminPage', () => { + const component = create() + + expect( + component.root.findAllByProps({ className: 'repository--nav--current-user' }).length + ).toBe(1) + expect( + component.root.findByProps({ className: 'repository--nav--current-user--name' }).children[0] + ).toBe(navProps.displayName) + expect(component.root.findAllByProps({ href: '/login' }).length).toBe(0) + expect(component.root.findAllByProps({ href: '/admin' }).length).toBe(1) + + expect(component.toJSON()).toMatchSnapshot() + }) + test('clicking current user button toggles a class on a menu element', () => { const reusableComponent = const component = create(reusableComponent) From 4f2165a843531d5f22efc60351729afddcfa9145 Mon Sep 17 00:00:00 2001 From: Mauricio Date: Sat, 27 Aug 2022 14:36:10 -0400 Subject: [PATCH 3/5] Added additional unit tests to improve coverage --- .../server/models/admin_interface.js | 71 +++++++---- .../server/models/admin_interface.test.js | 67 ++++++++++ .../obojobo-repository/server/routes/admin.js | 1 - .../server/routes/admin.test.js | 115 ++++++++++++++++++ .../__snapshots__/admin.test.js.snap | 109 +++++++++++++++++ .../shared/components/admin.test.js | 34 ++++++ .../shared/reducers/admin-reducer.js | 9 -- .../shared/reducers/admin-reducer.test.js | 13 ++ 8 files changed, 383 insertions(+), 36 deletions(-) create mode 100644 packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap create mode 100644 packages/app/obojobo-repository/shared/components/admin.test.js diff --git a/packages/app/obojobo-repository/server/models/admin_interface.js b/packages/app/obojobo-repository/server/models/admin_interface.js index ff87804b03..6ac4ca6275 100644 --- a/packages/app/obojobo-repository/server/models/admin_interface.js +++ b/packages/app/obojobo-repository/server/models/admin_interface.js @@ -5,50 +5,69 @@ const User = require('obojobo-express/server/models/user') class AdminInterface { static addPermission(userId, permission) { - // First fetch user - User.fetchById(userId) - .then(u => { - let perms = u.perms - if (perms.includes(permission)) return - perms = [...perms, permission] - - // Then add new permission (if it is new) - return this._updateUserPerms(userId, perms) - }) - .catch(() => { - throw logger.logError(`AdminInterface error finding user with id ${userId}`) + return new Promise((resolve, reject) => { + // First fetch user + User.fetchById(userId) + .then(u => { + let perms = u.perms + if (perms.includes(permission)) return + perms = [...perms, permission] + + // Then add new permission (if it is new) + this._updateUserPerms(userId, perms) + .then(id => resolve(id)) + .catch(() => { + reject() + throw logger.logError(`AdminInterface error adding permission`) + }) + }) + .catch(err => { + reject() + throw logger.logError(`AdminInterface error finding user with id ${userId}`) + }) }) } static removePermission(userId, permission) { - // First fetch user - User.fetchById(userId) - .then(u => { - const perms = u.perms + return new Promise((resolve, reject) => { + // First fetch user + User.fetchById(userId) + .then(u => { + const perms = u.perms - const ix = perms.indexOf(permission) - if (ix === -1) return - perms.splice(ix, 1) + const ix = perms.indexOf(permission) + if (ix === -1) return + perms.splice(ix, 1) - // Then remove permission (if present) - return this._updateUserPerms(userId, perms) - }) - .catch(() => { - throw logger.logError(`AdminInterface Error finding user with id ${userId}`) + // Then remove permission (if present) + this._updateUserPerms(userId, perms) + .then(id => resolve(id)) + .catch(() => { + reject() + throw logger.logError(`AdminInterface error removing permission`) + }) + }) + .catch(() => { + reject() + throw logger.logError(`AdminInterface Error finding user with id ${userId}`) + }) }) } static _updateUserPerms(userId, perms) { - return db - .oneOrNone(` + return new Promise((resolve, reject) => { + db.oneOrNone(` UPDATE user_perms SET perms = $[perms] WHERE user_id = $[userId] `, { userId, perms }) + .then(() => resolve(userId)) .catch(error => { + reject() throw logger.logError('AdminInterface _updateUserPerms Error', error) }) + }) } } diff --git a/packages/app/obojobo-repository/server/models/admin_interface.test.js b/packages/app/obojobo-repository/server/models/admin_interface.test.js index e69de29bb2..d6b7ff67d8 100644 --- a/packages/app/obojobo-repository/server/models/admin_interface.test.js +++ b/packages/app/obojobo-repository/server/models/admin_interface.test.js @@ -0,0 +1,67 @@ +describe('AdminInterface Model', () => { + jest.mock('obojobo-express/server/db') + jest.mock('obojobo-express/server/logger') + let db + let logger + let AdminInterface + + beforeEach(() => { + jest.resetModules() + jest.resetAllMocks() + db = require('obojobo-express/server/db') + logger = require('obojobo-express/server/logger') + + AdminInterface = require('./admin_interface') + }) + afterEach(() => {}) + + test('addPermission correctly fetches id and adds new permission (if any)', () => { + db.one.mockResolvedValueOnce({ + id: 5, + created_at: 'mocked-date', + email: 'guest@obojobo.ucf.edu', + first_name: 'Guest', + last_name: 'Guest', + roles: [], + username: 'guest' + }) + + db.oneOrNone.mockResolvedValueOnce({}) + + return AdminInterface.addPermission(5, 'canViewAdminPage') + .then(ret => expect(ret).toBe(5)) + }) + + test('addPermission catches error when fetching user with invalid id', () => { + logger.logError = jest.fn() + + db.one.mockRejectedValueOnce('mock-error') + + return AdminInterface.addPermission(123456, 'canViewAdminPage') + .then(ret => { + console.log("ret:") + console.log(ret) + }) + .catch(() => { + console.log("here") + expect(logger.logError).toHaveBeenCalledWith('AdminInterface error finding user with id 123456') + }) + }) + + test('removePermission correctly fetches id and removes permission', () => { + db.one.mockResolvedValueOnce({ + id: 5, + created_at: 'mocked-date', + email: 'guest@obojobo.ucf.edu', + first_name: 'Guest', + last_name: 'Guest', + perms: ['canViewAdminPage'], + username: 'guest' + }) + + db.oneOrNone.mockResolvedValueOnce({}) + + return AdminInterface.removePermission(5, 'canViewAdminPage') + .then(ret => expect(ret).toBe(5)) + }) +}) \ No newline at end of file diff --git a/packages/app/obojobo-repository/server/routes/admin.js b/packages/app/obojobo-repository/server/routes/admin.js index 18f3651816..82966a4de2 100644 --- a/packages/app/obojobo-repository/server/routes/admin.js +++ b/packages/app/obojobo-repository/server/routes/admin.js @@ -13,7 +13,6 @@ router .route('/admin') .get([requireCurrentUser, requireCanViewAdminPage]) .get((req, res) => { - console.log("REACHED ADMIN ROUTE") const props = { title: 'Admin', currentUser: req.currentUser, diff --git a/packages/app/obojobo-repository/server/routes/admin.test.js b/packages/app/obojobo-repository/server/routes/admin.test.js index e69de29bb2..9dd86a606f 100644 --- a/packages/app/obojobo-repository/server/routes/admin.test.js +++ b/packages/app/obojobo-repository/server/routes/admin.test.js @@ -0,0 +1,115 @@ +jest.unmock('fs') // need fs working for view rendering +jest.unmock('express') // we'll use supertest + express for this +jest.mock( + 'obojobo-express/server/asset_resolver', + () => ({ + assetForEnv: path => path, + webpackAssetPath: path => path + }), + { virtual: true } +) + +jest.setTimeout(10000) // extend test timeout? + +// override requireCurrentUser for tests to provide our own user +let mockCurrentUser + +jest.mock('obojobo-express/server/express_current_user', () => (req, res, next) => { + req.requireCurrentUser = () => { + req.currentUser = mockCurrentUser + return Promise.resolve(mockCurrentUser) + } + req.getCurrentUser = () => { + req.currentUser = mockCurrentUser + return Promise.resolve(mockCurrentUser) + } + req.requireCanViewAdminPage = () => { + req.currentUser = mockCurrentUser + return Promise.resolve(mockCurrentUser) + } + next() +}) + +let mockAdminComponent +let mockAdminComponentConstructor +jest.mock('obojobo-repository/shared/components/pages/page-admin-server') + +const componentPropsDesiredProperties = ['title', 'currentUser', 'allModules'] + +// setup express server +const path = require('path') +const request = require('supertest') +const express = require('express') +const bodyParser = require('body-parser') +const app = express() + +// register express-react-views template engine if not already registered +app.engine('jsx', require('express-react-views-custom').createEngine()) + +app.set('view engine', 'jsx') + +let viewPaths = app.get('views') +if (!Array.isArray(viewPaths)) viewPaths = [viewPaths] +viewPaths.push(path.resolve(`${__dirname}/../../shared/components`)) // add the components dir so babel can transpile the jsx +app.set('views', viewPaths) + +app.use(bodyParser.json()) +app.use(bodyParser.urlencoded({ extended: true })) + +app.use(require('obojobo-express/server/express_current_user')) + +app.use('/', require('obojobo-express/server/express_response_decorator')) +app.use('/', require('obojobo-repository/server/routes/admin')) + +describe('repository admin route', () => { + beforeEach(() => { + jest.resetAllMocks() + mockCurrentUser = { + id: 99, + // return true when the perm being asked about is 'canViewAdminPage + hasPermission: perm => perm === 'canViewAdminPage' + } + + //there's extra express garbage attached to the props we care about + //this roundabout solution exists to only pull out the ones we want + mockAdminComponentConstructor = jest.fn() + mockAdminComponent = require('obojobo-repository/shared/components/pages/page-admin-server') + mockAdminComponent.mockImplementation(props => { + const desiredProps = {} + componentPropsDesiredProperties.forEach(prop => { + desiredProps[prop] = props[prop] + }) + mockAdminComponentConstructor(desiredProps) + return '' + }) + }) + + test('get /admin returns a "not authorized" if the viewer does not have canViewAdminPage', () => { + // always return false - a.k.a. the user does not have the right perms to use this + mockCurrentUser.hasPermission = () => false + + expect.hasAssertions() + + return request(app) + .get('/admin') + .then(response => { + expect(mockAdminComponent).toHaveBeenCalledTimes(0) + expect(response.statusCode).toBe(401) + }) + }) + + test('get /admin sends the correct props to the Admin component when the user has canViewAdminPage', () => { + expect.hasAssertions() + + return request(app) + .get('/admin') + .then(response => { + expect(mockAdminComponent).toHaveBeenCalledTimes(1) + expect(mockAdminComponentConstructor).toHaveBeenCalledWith({ + title: 'Admin', + currentUser: mockCurrentUser + }) + expect(response.statusCode).toBe(200) + }) + }) +}) diff --git a/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap b/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap new file mode 100644 index 0000000000..a08ef7b7ea --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap @@ -0,0 +1,109 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Admin Renders "tools loaded" state correctly 1`] = ` + +
+ +
+
+
+
+

+

+
+
+
+

+ Loading... +

+
+
+ +`; diff --git a/packages/app/obojobo-repository/shared/components/admin.test.js b/packages/app/obojobo-repository/shared/components/admin.test.js new file mode 100644 index 0000000000..33ce001a84 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/admin.test.js @@ -0,0 +1,34 @@ +import React from 'react' +import { create, act } from 'react-test-renderer' +// import Button from './button' + +import Admin from './admin' + +describe('Admin', () => { + beforeEach(() => { + jest.resetAllMocks() + }) + + const getTestProps = () => ({ + currentUser: { + id: 99, + avatarUrl: '/path/to/avatar', + firstName: 'firstName', + lastName: 'lastName', + perms: [ + 'canViewEditor', + 'canEditDrafts', + 'canDeleteDrafts', + 'canCreateDrafts', + 'canPreviewDrafts' + ] + }, + loadUserList: jest.fn() + }) + + test('Renders "tools loaded" state correctly', () => { + const component = create() + const tree = component.toJSON() + expect(tree).toMatchSnapshot() + }) +}) \ No newline at end of file diff --git a/packages/app/obojobo-repository/shared/reducers/admin-reducer.js b/packages/app/obojobo-repository/shared/reducers/admin-reducer.js index 51ec6aa2df..ad48af3a48 100644 --- a/packages/app/obojobo-repository/shared/reducers/admin-reducer.js +++ b/packages/app/obojobo-repository/shared/reducers/admin-reducer.js @@ -1,14 +1,5 @@ -const { handle } = require('redux-pack') - -const { - LOAD_STATS_PAGE_MODULES_FOR_USER, - LOAD_MODULE_ASSESSMENT_DETAILS -} = require('../actions/admin-actions') - function AdminReducer(state, action) { switch (action.type) { - case LOAD_STATS_PAGE_MODULES_FOR_USER: - case LOAD_MODULE_ASSESSMENT_DETAILS: default: return state } diff --git a/packages/app/obojobo-repository/shared/reducers/admin-reducer.test.js b/packages/app/obojobo-repository/shared/reducers/admin-reducer.test.js index e69de29bb2..123059bd44 100644 --- a/packages/app/obojobo-repository/shared/reducers/admin-reducer.test.js +++ b/packages/app/obojobo-repository/shared/reducers/admin-reducer.test.js @@ -0,0 +1,13 @@ +const adminReducer = require('./admin-reducer') + +describe('Admin Reducer', () => { + test('Admin actions in reducer return the original state', () => { + const initialState = {} + + const action = { + type: 'test-admin-action' + } + + expect(adminReducer(initialState, action)).toBe(initialState) + }) +}) \ No newline at end of file From 16477e24e697e0bf41b02ce6737663589a38d91b Mon Sep 17 00:00:00 2001 From: Brandon Stull Date: Fri, 9 Jun 2023 15:02:44 -0400 Subject: [PATCH 4/5] #2010 Changed tactics for user permission management to use a user search instead of a dropdown with all possible users. Made necessary backend and frontend adjustments to correctly differentiate between role-based implicit permissions and explicit permissions. Added/updated unit tests. --- .../__tests__/express_validators.test.js | 22 + .../app/obojobo-express/server/models/user.js | 16 - .../server/models/admin_interface.js | 108 +++-- .../server/models/admin_interface.test.js | 230 ++++++--- .../obojobo-repository/server/routes/api.js | 56 +-- .../server/routes/api.test.js | 61 +++ .../server/services/search.js | 25 +- .../server/services/search.test.js | 25 +- .../shared/actions/admin-actions.js | 94 ++-- .../shared/actions/admin-actions.test.js | 163 +++++++ .../__snapshots__/admin.test.js.snap | 22 +- .../shared/components/admin-hoc.js | 12 +- .../shared/components/admin-hoc.test.js | 6 +- .../shared/components/admin.jsx | 220 +++++---- .../shared/components/admin.scss | 5 - .../shared/components/admin.test.js | 458 +++++++++++++++++- .../shared/reducers/admin-reducer.js | 20 +- .../shared/reducers/admin-reducer.test.js | 91 +++- .../shared/util/implicit-perms.js | 17 + .../shared/util/implicit-perms.test.js | 31 ++ 20 files changed, 1326 insertions(+), 356 deletions(-) create mode 100644 packages/app/obojobo-repository/shared/util/implicit-perms.js create mode 100644 packages/app/obojobo-repository/shared/util/implicit-perms.test.js diff --git a/packages/app/obojobo-express/__tests__/express_validators.test.js b/packages/app/obojobo-express/__tests__/express_validators.test.js index 1bed32b95a..23afbe2fc2 100644 --- a/packages/app/obojobo-express/__tests__/express_validators.test.js +++ b/packages/app/obojobo-express/__tests__/express_validators.test.js @@ -519,6 +519,28 @@ describe('current user middleware', () => { }) }) + // requireCanViewAdminPage + + test('requireCanViewAdminPage calls next and has no validation errors', () => { + mockUser.hasPermission = perm => perm === 'canViewAdminPage' + mockReq.requireCurrentUser = jest.fn().mockResolvedValue(mockUser) + return Validators.requireCanViewAdminPage(mockReq, mockRes, mockNext).then(() => { + expect(mockNext).toHaveBeenCalledTimes(1) + expect(mockRes.notAuthorized).toHaveBeenCalledTimes(0) + expect(mockReq._validationErrors).toBeUndefined() + }) + }) + + test('requireCanViewAdminPage doesnt call next and has errors', () => { + mockUser.hasPermission = () => false + mockReq.requireCurrentUser = jest.fn().mockResolvedValue(mockUser) + return Validators.requireCanViewAdminPage(mockReq, mockRes, mockNext).then(() => { + expect(mockNext).toHaveBeenCalledTimes(0) + expect(mockRes.notAuthorized).toHaveBeenCalledTimes(1) + expect(mockReq._validationErrors).toBeUndefined() + }) + }) + // checkValidationRules test('checkValidationRules calls next with no errors', () => { diff --git a/packages/app/obojobo-express/server/models/user.js b/packages/app/obojobo-express/server/models/user.js index 204c950fd7..e2c420667a 100644 --- a/packages/app/obojobo-express/server/models/user.js +++ b/packages/app/obojobo-express/server/models/user.js @@ -102,22 +102,6 @@ class User { }) } - static getAll() { - return db - .manyOrNone( - `SELECT - * - FROM users - ORDER BY first_name, last_name - LIMIT 25` - ) - .then(results => results.map(r => User.dbResultToModel(r))) - .catch(error => { - logger.logError('Error getAll', error) - throw Error('Error fetching all users.') - }) - } - saveOrCreate() { return db .one( diff --git a/packages/app/obojobo-repository/server/models/admin_interface.js b/packages/app/obojobo-repository/server/models/admin_interface.js index 6ac4ca6275..ba1c7677df 100644 --- a/packages/app/obojobo-repository/server/models/admin_interface.js +++ b/packages/app/obojobo-repository/server/models/admin_interface.js @@ -3,72 +3,94 @@ const logger = require('obojobo-express/server/logger') const User = require('obojobo-express/server/models/user') +const { PERMS_PER_ROLE } = require('../../shared/util/implicit-perms') + class AdminInterface { static addPermission(userId, permission) { return new Promise((resolve, reject) => { - // First fetch user User.fetchById(userId) - .then(u => { - let perms = u.perms - if (perms.includes(permission)) return - perms = [...perms, permission] - - // Then add new permission (if it is new) - this._updateUserPerms(userId, perms) - .then(id => resolve(id)) + .then(u => { + let perms = u.perms + if (perms.includes(permission)) return resolve(u) + + perms = [...u.perms, permission] + + const allRolePerms = new Set() + + u.roles.forEach(role => { + PERMS_PER_ROLE[role].forEach(perm => allRolePerms.add(perm)) + }) + + perms = perms.filter(p => !allRolePerms.has(p)) + + const dedupedPerms = new Set([...u.perms, ...perms]) + u.perms = [...dedupedPerms] + + // Then add new permission (if it is new) + this._updateUserPerms(userId, perms) + .then(() => resolve(u)) + .catch(() => { + reject(logger.logError(`AdminInterface error adding permission`)) + }) + }) .catch(() => { - reject() - throw logger.logError(`AdminInterface error adding permission`) + reject(logger.logError(`AdminInterface error finding user with id ${userId}`)) }) - }) - .catch(err => { - reject() - throw logger.logError(`AdminInterface error finding user with id ${userId}`) - }) }) } static removePermission(userId, permission) { return new Promise((resolve, reject) => { - // First fetch user User.fetchById(userId) - .then(u => { - const perms = u.perms + .then(u => { + const allRolePerms = new Set() + + u.roles.forEach(role => { + PERMS_PER_ROLE[role].forEach(perm => allRolePerms.add(perm)) + }) + + const perms = u.perms.filter(p => !allRolePerms.has(p)) + + const ix = perms.indexOf(permission) + if (ix === -1) return resolve(u) + perms.splice(ix, 1) - const ix = perms.indexOf(permission) - if (ix === -1) return - perms.splice(ix, 1) + // add any remaining perms to the 'allRolePerms' set so we can use it to set the user's new combined perms + perms.forEach(p => allRolePerms.add(p)) - // Then remove permission (if present) - this._updateUserPerms(userId, perms) - .then(id => resolve(id)) + u.perms = [...allRolePerms] + + // Then remove permission (if present) + this._updateUserPerms(userId, perms) + .then(() => resolve(u)) + .catch(() => { + reject(logger.logError(`AdminInterface error removing permission`)) + }) + }) .catch(() => { - reject() - throw logger.logError(`AdminInterface error removing permission`) + reject(logger.logError(`AdminInterface error finding user with id ${userId}`)) }) - }) - .catch(() => { - reject() - throw logger.logError(`AdminInterface Error finding user with id ${userId}`) - }) }) } static _updateUserPerms(userId, perms) { return new Promise((resolve, reject) => { - db.oneOrNone(` - UPDATE user_perms - SET perms = $[perms] - WHERE user_id = $[userId] + db.oneOrNone( + ` + INSERT INTO user_perms (user_id, perms) + VALUES ($[userId], $[perms]) + ON CONFLICT (user_id) + DO UPDATE SET perms = $[perms] + WHERE user_perms.user_id = $[userId] `, - { userId, perms }) - .then(() => resolve(userId)) - .catch(error => { - reject() - throw logger.logError('AdminInterface _updateUserPerms Error', error) - }) + { userId, perms } + ) + .then(() => resolve(userId)) + .catch(error => { + reject(logger.logError('AdminInterface _updateUserPerms error', error)) + }) }) } } -module.exports = AdminInterface \ No newline at end of file +module.exports = AdminInterface diff --git a/packages/app/obojobo-repository/server/models/admin_interface.test.js b/packages/app/obojobo-repository/server/models/admin_interface.test.js index d6b7ff67d8..01341aa48e 100644 --- a/packages/app/obojobo-repository/server/models/admin_interface.test.js +++ b/packages/app/obojobo-repository/server/models/admin_interface.test.js @@ -1,67 +1,183 @@ +jest.mock('obojobo-express/server/db') +jest.mock('obojobo-express/server/logger') +jest.mock('obojobo-express/server/models/user') +jest.mock('../../shared/util/implicit-perms', () => ({ + PERMS_PER_ROLE: { + mockRole: ['mockExistingPermission'] + } +})) + +const db = require('obojobo-express/server/db') +const logger = require('obojobo-express/server/logger') + +const User = require('obojobo-express/server/models/user') + +const AdminInterface = require('./admin_interface') + describe('AdminInterface Model', () => { - jest.mock('obojobo-express/server/db') - jest.mock('obojobo-express/server/logger') - let db - let logger - let AdminInterface + let expectedResponseUser - beforeEach(() => { + beforeEach(() => { jest.resetModules() jest.resetAllMocks() - db = require('obojobo-express/server/db') - logger = require('obojobo-express/server/logger') - AdminInterface = require('./admin_interface') + expectedResponseUser = { + perms: [], + roles: [] + } + + // provide this by defualt, override in individual tests if necessary + // if every test ends up overriding this, just remove this one + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + }) + + test('addPermission does nothing if trying to add a permission the given user already has', () => { + expect.assertions(2) + + // set the user's permissions such that they already have the one we're trying to give them + // ideally we could check implicit and explicit perms separately, but they're added to a single + // array inside the User model so our only option is to check the one location + expectedResponseUser.perms = ['someExistingPermission'] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.addPermission(5, 'someExistingPermission').then(u => { + expect(u).toEqual(expectedResponseUser) + expect(db.oneOrNone).not.toHaveBeenCalled() + }) + }) + + test('addPermission only saves explicitly granted permissions', () => { + expect.assertions(3) + + db.oneOrNone.mockResolvedValueOnce(5) + + // 'mockRole' will account for the 'mockExistingPermission' perm below + expectedResponseUser.roles = ['mockRole'] + expectedResponseUser.perms = ['someExistingPermission', 'mockExistingPermission'] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.addPermission(5, 'someNewPermission').then(u => { + expect(u).toEqual({ + ...expectedResponseUser, + perms: ['someExistingPermission', 'mockExistingPermission', 'someNewPermission'] + }) + expect(db.oneOrNone).toHaveBeenCalledTimes(1) + // first argument to db function is the query string, no need to check that + expect(db.oneOrNone.mock.calls[0][1]).toEqual({ + userId: 5, + // since 'mockExistingPermission' is a perm-based/implicit perm, + // it should not have been saved explicitly + perms: ['someExistingPermission', 'someNewPermission'] + }) + }) + }) + + test('addPermission catches error when fetching user with invalid id', () => { + User.fetchById = jest.fn().mockRejectedValueOnce('mock-error') + + expect.hasAssertions() + + return AdminInterface.addPermission(123456, 'someNewPermission').catch(() => { + expect(logger.logError).toHaveBeenCalledWith( + 'AdminInterface error finding user with id 123456' + ) + }) + }) + + test('addPermission catches error when updating user perms', () => { + expect.hasAssertions() + + db.oneOrNone.mockRejectedValueOnce('mock-error') + + return AdminInterface.addPermission(5, 'someNewPermission').catch(() => { + expect(logger.logError).toHaveBeenCalledTimes(2) + expect(logger.logError).toHaveBeenCalledWith( + 'AdminInterface _updateUserPerms error', + 'mock-error' + ) + expect(logger.logError).toHaveBeenCalledWith('AdminInterface error adding permission') + }) + }) + + test('removePermission does nothing if trying to remove a permission the given user does not have', () => { + expect.assertions(2) + + // set the user's permissions such that they already have the one we're trying to give them + // ideally we could check implicit and explicit perms separately, but they're added to a single + // array inside the User model so our only option is to check the one location + expectedResponseUser.perms = ['someOtherPermission'] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.removePermission(5, 'someExistingPermission').then(u => { + expect(u).toEqual(expectedResponseUser) + expect(db.oneOrNone).not.toHaveBeenCalled() + }) + }) + + test('removePermission does nothing if trying to remove a permission the given user has implicitly', () => { + expect.assertions(2) + + expectedResponseUser.roles = ['mockRole'] + expectedResponseUser.perms = ['someExistingPermission', 'mockExistingPermission'] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.removePermission(5, 'mockExistingPermission').then(u => { + expect(u).toEqual(expectedResponseUser) + expect(db.oneOrNone).not.toHaveBeenCalled() + }) }) - afterEach(() => {}) - - test('addPermission correctly fetches id and adds new permission (if any)', () => { - db.one.mockResolvedValueOnce({ - id: 5, - created_at: 'mocked-date', - email: 'guest@obojobo.ucf.edu', - first_name: 'Guest', - last_name: 'Guest', - roles: [], - username: 'guest' + + test('removePermission saves explicit permissions after removing one from the given user', () => { + db.oneOrNone.mockResolvedValueOnce(5) + + // 'mockRole' will account for the 'mockExistingPermission' perm below + expectedResponseUser.roles = ['mockRole'] + expectedResponseUser.perms = [ + 'someExistingPermission', + 'someOtherExistingPermission', + 'mockExistingPermission' + ] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.removePermission(5, 'someOtherExistingPermission').then(u => { + expect(u).toEqual({ + ...expectedResponseUser, + perms: ['mockExistingPermission', 'someExistingPermission'] + }) + expect(db.oneOrNone).toHaveBeenCalledTimes(1) + expect(db.oneOrNone.mock.calls[0][1]).toEqual({ + userId: 5, + perms: ['someExistingPermission'] + }) }) + }) + + test('removePermission catches error when fetching user with invalid id', () => { + User.fetchById = jest.fn().mockRejectedValueOnce('mock-error') + + expect.hasAssertions() - db.oneOrNone.mockResolvedValueOnce({}) - - return AdminInterface.addPermission(5, 'canViewAdminPage') - .then(ret => expect(ret).toBe(5)) - }) - - test('addPermission catches error when fetching user with invalid id', () => { - logger.logError = jest.fn() - - db.one.mockRejectedValueOnce('mock-error') - - return AdminInterface.addPermission(123456, 'canViewAdminPage') - .then(ret => { - console.log("ret:") - console.log(ret) - }) - .catch(() => { - console.log("here") - expect(logger.logError).toHaveBeenCalledWith('AdminInterface error finding user with id 123456') - }) - }) - - test('removePermission correctly fetches id and removes permission', () => { - db.one.mockResolvedValueOnce({ - id: 5, - created_at: 'mocked-date', - email: 'guest@obojobo.ucf.edu', - first_name: 'Guest', - last_name: 'Guest', - perms: ['canViewAdminPage'], - username: 'guest' + return AdminInterface.removePermission(123456, 'someExistingPermission').catch(() => { + expect(logger.logError).toHaveBeenCalledWith( + 'AdminInterface error finding user with id 123456' + ) }) + }) + + test('removePermission catches error when updating user perms', () => { + expect.hasAssertions() + + db.oneOrNone.mockRejectedValueOnce('mock-error') - db.oneOrNone.mockResolvedValueOnce({}) - - return AdminInterface.removePermission(5, 'canViewAdminPage') - .then(ret => expect(ret).toBe(5)) - }) -}) \ No newline at end of file + expectedResponseUser.perms = ['someExistingPermission'] + + return AdminInterface.removePermission(5, 'someExistingPermission').catch(() => { + expect(logger.logError).toHaveBeenCalledTimes(2) + expect(logger.logError).toHaveBeenCalledWith( + 'AdminInterface _updateUserPerms error', + 'mock-error' + ) + expect(logger.logError).toHaveBeenCalledWith('AdminInterface error removing permission') + }) + }) +}) diff --git a/packages/app/obojobo-repository/server/routes/api.js b/packages/app/obojobo-repository/server/routes/api.js index b16b87438c..b912aa8688 100644 --- a/packages/app/obojobo-repository/server/routes/api.js +++ b/packages/app/obojobo-repository/server/routes/api.js @@ -16,7 +16,7 @@ const { requireCanDeleteDrafts, check, requireCanViewStatsPage, - requireCanViewAdminPage, + requireCanViewAdminPage } = require('obojobo-express/server/express_validators') const UserModel = require('obojobo-express/server/models/user') const { searchForUserByString } = require('../services/search') @@ -172,47 +172,34 @@ router }) router - .route('/users/all') - .get([requireCanViewAdminPage]) - .get(async (req, res) => { + .route('/permissions/add') + .post([requireCanViewAdminPage]) + .post(async (req, res) => { + const userId = req.body.userId + const perm = req.body.perm + try { - let users = await UserModel.getAll() - users = users.map(u => u.toJSON()) - res.success(users) + const modifiedUser = await AdminInterface.addPermission(userId, perm) + res.success(modifiedUser) } catch (error) { res.unexpected(error) } }) router -.route('/permissions/add') -.post([requireCanViewAdminPage]) -.post(async (req, res) => { - const userId = req.body.userId - const perm = req.body.perm - - try { - await AdminInterface.addPermission(userId, perm) - res.success() - } catch (error) { - res.unexpected(error) - } -}) + .route('/permissions/remove') + .post([requireCanViewAdminPage]) + .post(async (req, res) => { + const userId = req.body.userId + const perm = req.body.perm -router -.route('/permissions/remove') -.post([requireCanViewAdminPage]) -.post(async (req, res) => { - const userId = req.body.userId - const perm = req.body.perm - - try { - await AdminInterface.removePermission(userId, perm) - res.success() - } catch (error) { - res.unexpected(error) - } -}) + try { + const modifiedUser = await AdminInterface.removePermission(userId, perm) + res.success(modifiedUser) + } catch (error) { + res.unexpected(error) + } + }) // Copy a draft to the current user // mounted as /api/drafts/:draftId/copy @@ -541,5 +528,4 @@ router } }) - module.exports = router diff --git a/packages/app/obojobo-repository/server/routes/api.test.js b/packages/app/obojobo-repository/server/routes/api.test.js index db541d935e..538d94debd 100644 --- a/packages/app/obojobo-repository/server/routes/api.test.js +++ b/packages/app/obojobo-repository/server/routes/api.test.js @@ -10,6 +10,7 @@ jest.mock('../services/collections') jest.mock('../services/count') jest.mock('obojobo-express/server/models/user') jest.mock('obojobo-express/server/insert_event') +jest.mock('../models/admin_interface') jest.unmock('fs') // need fs working for view rendering jest.unmock('express') // we'll use supertest + express for this @@ -24,6 +25,7 @@ let CountServices let UserModel let insertEvent let DraftPermissions +let AdminInterface // override requireCurrentUser for tests to provide our own user let mockCurrentUser @@ -98,6 +100,7 @@ describe('repository api route', () => { CountServices = require('../services/count') DraftPermissions = require('../models/draft_permissions') UserModel = require('obojobo-express/server/models/user') + AdminInterface = require('../models/admin_interface') insertEvent = require('obojobo-express/server/insert_event') }) @@ -494,6 +497,64 @@ describe('repository api route', () => { }) }) + test('post /permissions/add returns the expected response', () => { + expect.hasAssertions() + + // this should be a user object but for testing purposes it doesn't matter + AdminInterface.addPermission.mockResolvedValueOnce(true) + + return request(app) + .post('/permissions/add') + .send({ userId: 5, perm: 'someNewPerm' }) + .then(response => { + expect(AdminInterface.addPermission).toHaveBeenCalledTimes(1) + expect(AdminInterface.addPermission).toHaveBeenCalledWith(5, 'someNewPerm') + expect(response.statusCode).toBe(200) + }) + }) + test('post /permissions/add handles thrown errors', () => { + expect.hasAssertions() + + AdminInterface.addPermission.mockRejectedValueOnce('database error') + + return request(app) + .post('/permissions/add') + .send({ userId: 5, perm: 'someNewPerm' }) + .then(response => { + expect(AdminInterface.addPermission).toHaveBeenCalledTimes(1) + expect(AdminInterface.addPermission).toHaveBeenCalledWith(5, 'someNewPerm') + expect(response.statusCode).toBe(500) + }) + }) + test('post /permissions/remove returns the expected response', () => { + expect.hasAssertions() + + AdminInterface.removePermission.mockResolvedValueOnce(true) + + return request(app) + .post('/permissions/remove') + .send({ userId: 5, perm: 'someExistingPerm' }) + .then(response => { + expect(AdminInterface.removePermission).toHaveBeenCalledTimes(1) + expect(AdminInterface.removePermission).toHaveBeenCalledWith(5, 'someExistingPerm') + expect(response.statusCode).toBe(200) + }) + }) + test('post /permissions/remove handles thrown errors', () => { + expect.hasAssertions() + + AdminInterface.removePermission.mockRejectedValueOnce('database error') + + return request(app) + .post('/permissions/remove') + .send({ userId: 5, perm: 'someExistingPerm' }) + .then(response => { + expect(AdminInterface.removePermission).toHaveBeenCalledTimes(1) + expect(AdminInterface.removePermission).toHaveBeenCalledWith(5, 'someExistingPerm') + expect(response.statusCode).toBe(500) + }) + }) + test('post /drafts/:draftId/copy returns the expected response when user can copy draft', () => { expect.hasAssertions() diff --git a/packages/app/obojobo-repository/server/services/search.js b/packages/app/obojobo-repository/server/services/search.js index 19dd0c5080..1ba978780a 100644 --- a/packages/app/obojobo-repository/server/services/search.js +++ b/packages/app/obojobo-repository/server/services/search.js @@ -6,18 +6,21 @@ const searchForUserByString = async searchString => { return db .manyOrNone( `SELECT - id, - first_name AS "firstName", - last_name AS "lastName", - email, - username, - created_at AS "createdAt", - roles + users.id, + users.first_name AS "firstName", + users.last_name AS "lastName", + users.email, + users.username, + users.created_at AS "createdAt", + users.roles, + user_perms.perms FROM users - WHERE obo_immutable_concat_ws(' ', first_name, last_name) ILIKE $[search] - OR email ILIKE $[search] - OR username ILIKE $[search] - ORDER BY first_name, last_name + LEFT JOIN user_perms + ON users.id = user_perms.user_id + WHERE obo_immutable_concat_ws(' ', users.first_name, users.last_name) ILIKE $[search] + OR users.email ILIKE $[search] + OR users.username ILIKE $[search] + ORDER BY users.first_name, users.last_name LIMIT 25`, { search: `%${searchString}%` } ) diff --git a/packages/app/obojobo-repository/server/services/search.test.js b/packages/app/obojobo-repository/server/services/search.test.js index 789f406285..61c748db44 100644 --- a/packages/app/obojobo-repository/server/services/search.test.js +++ b/packages/app/obojobo-repository/server/services/search.test.js @@ -36,18 +36,21 @@ describe('Search Services', () => { expect.hasAssertions() const manyOrNoneQueryString = `SELECT - id, - first_name AS "firstName", - last_name AS "lastName", - email, - username, - created_at AS "createdAt", - roles + users.id, + users.first_name AS "firstName", + users.last_name AS "lastName", + users.email, + users.username, + users.created_at AS "createdAt", + users.roles, + user_perms.perms FROM users - WHERE obo_immutable_concat_ws(' ', first_name, last_name) ILIKE $[search] - OR email ILIKE $[search] - OR username ILIKE $[search] - ORDER BY first_name, last_name + LEFT JOIN user_perms + ON users.id = user_perms.user_id + WHERE obo_immutable_concat_ws(' ', users.first_name, users.last_name) ILIKE $[search] + OR users.email ILIKE $[search] + OR users.username ILIKE $[search] + ORDER BY users.first_name, users.last_name LIMIT 25` return SearchServices.searchForUserByString('searchString').then(response => { diff --git a/packages/app/obojobo-repository/shared/actions/admin-actions.js b/packages/app/obojobo-repository/shared/actions/admin-actions.js index 393aea53c9..ab594a955b 100644 --- a/packages/app/obojobo-repository/shared/actions/admin-actions.js +++ b/packages/app/obojobo-repository/shared/actions/admin-actions.js @@ -1,31 +1,44 @@ -// =================== API ======================= +const debouncePromise = require('debounce-promise') -const apiGetModules = () => { - const JSON_MIME_TYPE = 'application/json' - const fetchOptions = { - method: 'GET', - credentials: 'include', - headers: { - Accept: JSON_MIME_TYPE, - 'Content-Type': JSON_MIME_TYPE - } +const JSON_MIME_TYPE = 'application/json' +const defaultOptions = () => ({ + method: 'GET', + credentials: 'include', + headers: { + Accept: JSON_MIME_TYPE, + 'Content-Type': JSON_MIME_TYPE } - return fetch('/api/drafts', fetchOptions).then(res => res.json()) +}) + +const throwIfNotOk = res => { + if (!res.ok) throw Error(`Error requesting ${res.url}, status code: ${res.status}`) + return res } -const apiGetUsers = () => { - const JSON_MIME_TYPE = 'application/json' - const fetchOptions = { - method: 'GET', - credentials: 'include', - headers: { - Accept: JSON_MIME_TYPE, - 'Content-Type': JSON_MIME_TYPE - } - } - return fetch('/api/users/all', fetchOptions).then(res => res.json()) +const apiSearchForUser = searchString => { + return fetch(`/api/users/search?q=${searchString}`, defaultOptions()) + .then(throwIfNotOk) + .then(res => res.json()) } +const apiSearchForUserDebounced = debouncePromise(apiSearchForUser, 300) + +// =================== API ======================= + +// not using this yet +// const apiGetModules = () => { +// const JSON_MIME_TYPE = 'application/json' +// const fetchOptions = { +// method: 'GET', +// credentials: 'include', +// headers: { +// Accept: JSON_MIME_TYPE, +// 'Content-Type': JSON_MIME_TYPE +// } +// } +// return fetch('/api/drafts', fetchOptions).then(res => res.json()) +// } + const apiAddUserPermission = (userId, perm) => { const JSON_MIME_TYPE = 'application/json' const fetchOptions = { @@ -58,16 +71,17 @@ const apiRemoveUserPermission = (userId, perm) => { // ================== ACTIONS =================== -const LOAD_ALL_MODULES = 'LOAD_ALL_MODULES' -const loadModuleList = () => ({ - type: LOAD_ALL_MODULES, - promise: apiGetModules() -}) +// const LOAD_ALL_MODULES = 'LOAD_ALL_MODULES' +// const loadModuleList = () => ({ +// type: LOAD_ALL_MODULES, +// promise: apiGetModules() +// }) -const LOAD_ALL_USERS = 'LOAD_ALL_USERS' -const loadUserList = () => ({ - type: LOAD_ALL_USERS, - promise: apiGetUsers() +const LOAD_USER_SEARCH = 'LOAD_USER_SEARCH' +const searchForUser = searchString => ({ + type: LOAD_USER_SEARCH, + meta: { searchString }, + promise: apiSearchForUserDebounced(searchString) }) const ADD_USER_PERMISSION = 'ADD_USER_PERMISSION' @@ -82,16 +96,22 @@ const removeUserPermission = (userId, perm) => ({ promise: apiRemoveUserPermission(userId, perm) }) -module.exports = { - LOAD_ALL_MODULES, - loadModuleList, +const CLEAR_PEOPLE_SEARCH_RESULTS = 'CLEAR_PEOPLE_SEARCH_RESULTS' +const clearPeopleSearchResults = () => ({ type: CLEAR_PEOPLE_SEARCH_RESULTS }) - LOAD_ALL_USERS, - loadUserList, +module.exports = { + // LOAD_ALL_MODULES, + // loadModuleList, ADD_USER_PERMISSION, addUserPermission, REMOVE_USER_PERMISSION, removeUserPermission, -} \ No newline at end of file + + LOAD_USER_SEARCH, + searchForUser, + + CLEAR_PEOPLE_SEARCH_RESULTS, + clearPeopleSearchResults +} diff --git a/packages/app/obojobo-repository/shared/actions/admin-actions.test.js b/packages/app/obojobo-repository/shared/actions/admin-actions.test.js index e69de29bb2..07b0fc4491 100644 --- a/packages/app/obojobo-repository/shared/actions/admin-actions.test.js +++ b/packages/app/obojobo-repository/shared/actions/admin-actions.test.js @@ -0,0 +1,163 @@ +const dayjs = require('dayjs') +const advancedFormat = require('dayjs/plugin/advancedFormat') + +jest.mock('./shared-api-methods', () => ({ + apiGetAssessmentDetailsForDraft: () => Promise.resolve() +})) + +dayjs.extend(advancedFormat) + +describe('Admin Actions', () => { + let standardFetchResponse + + let AdminActions + + // this is lifted straight out of admin-actions, for ease of comparison + // barring any better ways of using it + const defaultFetchOptions = { + method: 'GET', + credentials: 'include', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json' + } + } + + const originalFetch = global.fetch + + beforeAll(() => { + global.fetch = jest.fn() + }) + + beforeEach(() => { + jest.resetModules() + jest.resetAllMocks() + jest.useFakeTimers() + + global.alert = jest.fn() + + delete window.location + window.location = { + reload: jest.fn() + } + + standardFetchResponse = { + ok: true, + json: jest.fn(() => ({ value: 'mockVal' })) + } + + AdminActions = require('./admin-actions') + }) + + afterAll(() => { + global.fetch = originalFetch + }) + + test('searchForUser returns the expected output and calls other functions correctly - server ok', () => { + global.fetch.mockResolvedValueOnce({ ...standardFetchResponse, ok: true }) + + const actionReply = AdminActions.searchForUser('mockSearchString') + jest.runAllTimers() + + expect(global.fetch).toHaveBeenCalledWith( + '/api/users/search?q=mockSearchString', + defaultFetchOptions + ) + + expect(actionReply).toEqual({ + type: AdminActions.LOAD_USER_SEARCH, + meta: { + searchString: 'mockSearchString' + }, + promise: expect.any(Object) + }) + + return actionReply.promise.then(() => { + expect(standardFetchResponse.json).toHaveBeenCalled() + }) + }) + test('searchForUser returns the expected output and calls other functions correctly - server error', () => { + const mockFetchUrl = '/api/users/search?q=mockSearchString' + global.fetch.mockResolvedValueOnce({ + ok: false, + url: mockFetchUrl, + status: 500 + }) + + const actionReply = AdminActions.searchForUser('mockSearchString') + + expect(actionReply).toEqual({ + type: AdminActions.LOAD_USER_SEARCH, + meta: { + searchString: 'mockSearchString' + }, + promise: expect.any(Object) + }) + + jest.runAllTimers() + return actionReply.promise.catch(error => { + expect(error).toBeInstanceOf(Error) + expect(error.message).toBe( + 'Error requesting /api/users/search?q=mockSearchString, status code: 500' + ) + + expect(global.fetch).toHaveBeenCalledWith( + '/api/users/search?q=mockSearchString', + defaultFetchOptions + ) + }) + }) + + test('addUserPermission returns the expected output and calls other functions', () => { + global.fetch.mockResolvedValueOnce({ ...standardFetchResponse, ok: true }) + + const actionReply = AdminActions.addUserPermission(5, 'someNewPermission') + jest.runAllTimers() + + expect(global.fetch).toHaveBeenCalledWith('/api/permissions/add', { + ...defaultFetchOptions, + method: 'POST', + body: JSON.stringify({ userId: 5, perm: 'someNewPermission' }) + }) + + expect(actionReply).toEqual({ + type: AdminActions.ADD_USER_PERMISSION, + promise: expect.any(Object) + }) + + return actionReply.promise.then(() => { + expect(standardFetchResponse.json).toHaveBeenCalled() + }) + }) + + test('removeUserPermission returns the expected output and calls other functions', () => { + global.fetch.mockResolvedValueOnce({ ...standardFetchResponse, ok: true }) + + const actionReply = AdminActions.removeUserPermission(5, 'someExistingPermission') + jest.runAllTimers() + + expect(global.fetch).toHaveBeenCalledWith('/api/permissions/remove', { + ...defaultFetchOptions, + method: 'POST', + body: JSON.stringify({ userId: 5, perm: 'someExistingPermission' }) + }) + + expect(actionReply).toEqual({ + type: AdminActions.REMOVE_USER_PERMISSION, + promise: expect.any(Object) + }) + + return actionReply.promise.then(() => { + expect(standardFetchResponse.json).toHaveBeenCalled() + }) + }) + + test('clearPeopleSearchResults returns the expected output', () => { + const actionReply = AdminActions.clearPeopleSearchResults() + + expect(global.fetch).not.toHaveBeenCalled() + expect(actionReply).toEqual({ + type: AdminActions.CLEAR_PEOPLE_SEARCH_RESULTS + }) + }) +}) diff --git a/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap b/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap index a08ef7b7ea..485364a0c2 100644 --- a/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap +++ b/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Admin Renders "tools loaded" state correctly 1`] = ` +exports[`Admin renders default view correctly 1`] = ` @@ -100,9 +100,23 @@ exports[`Admin Renders "tools loaded" state correctly 1`] = `
-

- Loading... -

+
+

+ Find Users to Manage +

+ +
+
+
    +
diff --git a/packages/app/obojobo-repository/shared/components/admin-hoc.js b/packages/app/obojobo-repository/shared/components/admin-hoc.js index 406cc202c4..25accf38ea 100644 --- a/packages/app/obojobo-repository/shared/components/admin-hoc.js +++ b/packages/app/obojobo-repository/shared/components/admin-hoc.js @@ -1,17 +1,17 @@ const Admin = require('./admin') const connect = require('react-redux').connect const { - loadModuleList, - loadUserList, addUserPermission, - removeUserPermission + removeUserPermission, + searchForUser, + clearPeopleSearchResults } = require('../actions/admin-actions') const mapStoreStateToProps = state => state const mapActionsToProps = { - loadModuleList, - loadUserList, addUserPermission, - removeUserPermission + removeUserPermission, + searchForUser, + clearPeopleSearchResults } module.exports = connect( mapStoreStateToProps, diff --git a/packages/app/obojobo-repository/shared/components/admin-hoc.test.js b/packages/app/obojobo-repository/shared/components/admin-hoc.test.js index dcc7a1b365..50103f49b5 100644 --- a/packages/app/obojobo-repository/shared/components/admin-hoc.test.js +++ b/packages/app/obojobo-repository/shared/components/admin-hoc.test.js @@ -24,8 +24,10 @@ describe('admin HOC', () => { expect(ReactRedux.connect).toHaveBeenCalledTimes(1) expect(ReactRedux.connect).toHaveBeenCalledWith(expect.any(Function), { - loadUserModuleList: AdminActions.loadUserModuleList, - loadModuleAssessmentDetails: AdminActions.loadModuleAssessmentDetails + addUserPermission: AdminActions.addUserPermission, + clearPeopleSearchResults: AdminActions.clearPeopleSearchResults, + removeUserPermission: AdminActions.removeUserPermission, + searchForUser: AdminActions.searchForUser }) expect(mockReduxConnectReturn).toHaveBeenCalledTimes(1) diff --git a/packages/app/obojobo-repository/shared/components/admin.jsx b/packages/app/obojobo-repository/shared/components/admin.jsx index 40843386cc..301378dd7f 100644 --- a/packages/app/obojobo-repository/shared/components/admin.jsx +++ b/packages/app/obojobo-repository/shared/components/admin.jsx @@ -5,70 +5,40 @@ const React = require('react') const { useState, useEffect } = require('react') const RepositoryNav = require('./repository-nav') const RepositoryBanner = require('./repository-banner') +const Search = require('./search') const Button = require('./button') +const PeopleListItem = require('./people-list-item') + +const { POSSIBLE_PERMS, PERMS_PER_ROLE } = require('../util/implicit-perms') -const POSSIBLE_ROLES = [ - 'canViewEditor', - 'canCreateDrafts', - 'canDeleteDrafts', - 'canPreviewDrafts', - 'canViewStatsPage', - 'canViewSystemStats', - 'canViewAdminPage', -] - -const NO_SELECTION_USER = 'no-select-user' -const NO_SELECTION_MODULE = 'no-select-module' const NO_SELECTION_PERMISSION = 'no-select-permission' -function Admin({ - currentUser, - title, - loadUserList, - addUserPermission, - removeUserPermission, -}) { - const [users, setUsers] = useState([]) - const [fetching, setFetching] = useState(true) - const [selectedUser, setSelectedUser] = useState(NO_SELECTION_USER) +const Admin = props => { + // storing selected user in state by choosing one from the list fetched by the search may not work + // perms attached to a user via the search didn't previously include explicitly added perms, just role perms + // that was changed but may slow down the initial fetch due to a join in the query + // may be faster to instead run a new action to fetch the single user explicitly, which will get all perms + // but that would put the selected user in props, as opposed to doing this + const [selectedUser, setSelectedUser] = useState(null) const [selectedPermission, setSelectedPermission] = useState(NO_SELECTION_PERMISSION) + // make sure user/module search results are zero'd out on first render useEffect(() => { - // Load all modules on this obo instance - loadAllUsers() - .then(() => setFetching(null)) + props.clearPeopleSearchResults() }, []) - const loadAllUsers = () => { - return new Promise(async resolve => { - const allUsers = await loadUserList() - if (allUsers && - allUsers.payload && - allUsers.payload.value && - allUsers.payload.value && - allUsers.payload.value.length > 0) { - - setUsers(allUsers.payload.value) - }else { - setUsers([]) - } - - resolve() - }) - } - const onAddUserPermission = async () => { - if (selectedUser === NO_SELECTION_USER) return if (selectedPermission === NO_SELECTION_PERMISSION) return - await addUserPermission(selectedUser, selectedPermission) + const action = await props.addUserPermission(selectedUser.id, selectedPermission) + if (action.payload.status === 'ok') setSelectedUser(action.payload.value) } const onRemoveUserPermission = async () => { - if (selectedUser === NO_SELECTION_USER) return if (selectedPermission === NO_SELECTION_PERMISSION) return - await removeUserPermission(selectedUser, selectedPermission) + const action = await props.removeUserPermission(selectedUser.id, selectedPermission) + if (action.payload.status === 'ok') setSelectedUser(action.payload.value) } const renderPermissionSelectList = () => { @@ -78,78 +48,128 @@ function Admin({ value={selectedPermission} onChange={e => setSelectedPermission(e.target.value)} > - - {POSSIBLE_ROLES.map((r, i) => { + {POSSIBLE_PERMS.map((r, i) => { return ( - + ) })} ) } - const renderUserSelectList = () => { + const renderListItem = u => { + if (u.id !== props.currentUser.id) { + return ( + + + + ) + } + } + + const renderContent = () => { + if (selectedUser) { + const allRolePerms = new Set() + const rolePermsRender = selectedUser.roles.map(role => { + const roles = PERMS_PER_ROLE[role].map(perm => { + allRolePerms.add(perm) + + return
  • {perm}
    • + }) + return ( +
    + +
      {roles}
    +
    + ) + }) + + const nonRolePerms = selectedUser.perms + .filter(p => !allRolePerms.has(p)) + .map(p =>
  • {p}
    • ) + + return ( + +
    +

    Managing:

    + +
    +
    +

    Manage User Permissions

    +
    + {selectedUser.roles.length > 0 ? ( + + + Current implicit permissions from role{selectedUser.roles.length > 1 ? 's' : ''} + : + + {rolePermsRender} + + ) : ( + 'None' + )} +
    +
    + Current explicit permissions: + {nonRolePerms.length < 1 ? 'None' :
      {nonRolePerms}
    } +
    +
    +

    Select permission:

    + {renderPermissionSelectList()} +
    +
    + + +
    +
    +     + ) + } return ( - + +
    +

    Find Users to Manage

    + +
    +
    +
      + {props.searchUsers && props.searchUsers.items + ? props.searchUsers.items.map(renderListItem) + : null} +
    +
    +     ) } - // Contains any tools involving modifying module access, user permission, etc - const getTools = () => [ -
    -

    Manage User Permissions

    -
    -

    Select user:

    - {renderUserSelectList()} -
    -
    -

    Select permission:

    - {renderPermissionSelectList()} -
    -
    - - -
    -
    , - ] - return ( - +
    -
    - {fetching ?

    Loading...

    : getTools()} -
    +
    {renderContent()}
     ) diff --git a/packages/app/obojobo-repository/shared/components/admin.scss b/packages/app/obojobo-repository/shared/components/admin.scss index bcc0f6ff49..03367097db 100644 --- a/packages/app/obojobo-repository/shared/components/admin.scss +++ b/packages/app/obojobo-repository/shared/components/admin.scss @@ -35,11 +35,6 @@ min-width: 30%; } - .repository--button { - margin-top: 20px; - margin-bottom: 20px; - } - .row { display: flex; align-items: center; diff --git a/packages/app/obojobo-repository/shared/components/admin.test.js b/packages/app/obojobo-repository/shared/components/admin.test.js index 33ce001a84..819dae0806 100644 --- a/packages/app/obojobo-repository/shared/components/admin.test.js +++ b/packages/app/obojobo-repository/shared/components/admin.test.js @@ -2,16 +2,34 @@ import React from 'react' import { create, act } from 'react-test-renderer' // import Button from './button' +jest.mock('./people-list-item', () => props => { + return {props.children} +}) +import PeopleListItem from './people-list-item' + +jest.mock('./search', () => props => { + return {props.children} +}) +import Search from './search' + +jest.mock('../util/implicit-perms', () => ({ + POSSIBLE_PERMS: ['perm1', 'perm2', 'perm3', 'perm4', 'mockExtraPerm'], + PERMS_PER_ROLE: { + role1: ['perm1', 'perm2', 'perm3'], + role2: ['perm4'] + } +})) + import Admin from './admin' describe('Admin', () => { - beforeEach(() => { + beforeEach(() => { jest.resetAllMocks() }) - const getTestProps = () => ({ - currentUser: { - id: 99, + const defaultProps = { + currentUser: { + id: 99, avatarUrl: '/path/to/avatar', firstName: 'firstName', lastName: 'lastName', @@ -22,13 +40,425 @@ describe('Admin', () => { 'canCreateDrafts', 'canPreviewDrafts' ] - }, - loadUserList: jest.fn() - }) - - test('Renders "tools loaded" state correctly', () => { - const component = create() - const tree = component.toJSON() - expect(tree).toMatchSnapshot() - }) -}) \ No newline at end of file + }, + addUserPermission: jest.fn(), + removeUserPermission: jest.fn(), + searchForUser: jest.fn(), + clearPeopleSearchResults: jest.fn() + } + + test('renders default view correctly', () => { + const component = create() + + // should only be a search input visible by default, no search results + expect(component.root.findAllByType(PeopleListItem).length).toBe(0) + expect(component.root.findByProps({ className: 'user-list' }).children.length).toBe(0) + + const tree = component.toJSON() + expect(tree).toMatchSnapshot() + }) + + test('renders user search results correctly', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { id: 1, firstName: 'Mock', lastName: 'User1' }, + { id: 2, firstName: 'Mock', lastName: 'User2' }, + { id: 3, firstName: 'Mock', lastName: 'User3' }, + // just to make sure the current user doesn't show up in the list + { ...defaultProps.currentUser } + ] + } + + const component = create() + const searchResults = component.root.findAllByType(PeopleListItem) + expect(searchResults.length).toBe(3) + // bit inelegant but does the job + searchResults.forEach(i => { + expect(i.props.isMe).toBe(false) + expect(i.props.firstName).not.toBe(defaultProps.currentUser.firstName) + expect(i.props.lastName).not.toBe(defaultProps.currentUser.lastName) + }) + }) + + test('calls functions appropriately when search value changes', () => { + const component = create() + act(() => { + // ordinarily there'd be targets and values to parse, but the Search component does that for us + component.root.findByType(Search).props.onChange('mock-new-search-value') + }) + + expect(defaultProps.searchForUser).toHaveBeenCalledTimes(1) + expect(defaultProps.searchForUser).toHaveBeenCalledWith('mock-new-search-value') + }) + + test('sets selected user correctly - no roles or explicit perms', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { + id: 1, + firstName: 'Mock', + lastName: 'User1', + roles: [], + perms: [] + } + ] + } + + const component = create() + + // pre-selection state - make sure the search tools are visible + expect(component.root.findAllByType('mock-Search').length).toBe(1) + expect(component.root.findAllByProps({ className: 'tool' }).length).toBe(0) + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // post-selection state - make sure the search tools are no longer available + expect(component.root.findAllByType('mock-Search').length).toBe(0) + expect(component.root.findAllByProps({ className: 'tool' }).length).toBe(1) + + const implicitContainer = component.root.findByProps({ className: 'implicit-perms-container' }) + expect(implicitContainer.findAllByType('ul').length).toBe(0) + expect(implicitContainer.children[implicitContainer.children.length - 1]).toBe('None') + + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findAllByType('ul').length).toBe(0) + expect(explicitContainer.children[explicitContainer.children.length - 1]).toBe('None') + + // for good measure, make sure the permissions list is populated correctly and the add/remove buttons appear + // this is a bit brute force-y, but at least we know for sure + // also a bit magical since we happen to know what the mocked values are + const expectedSelectOptions = [ + 'no-select-permission', // this will always be there + 'perm1', + 'perm2', + 'perm3', + 'perm4', + 'mockExtraPerm' + ] + const permSelect = component.root.findByType('select') + expectedSelectOptions.forEach((v, i) => { + expect(permSelect.findAllByType('option')[i].props.value).toBe(v) + }) + + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + expect(buttons.length).toBe(2) + expect(buttons[0].children[0].children[0]).toBe('Add') + expect(buttons[1].children[0].children[0]).toBe('Remove') + }) + + test('sets selected user correctly - one role and explicit perms', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { + id: 1, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3', 'mockExtraPerm'] + } + ] + } + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + const implicitContainer = component.root.findByProps({ className: 'implicit-perms-container' }) + // the 'single' rendered string is actually three strings, since the singular/plural part is interpolated + // checking to make sure 'role' isn't 'roles' since any number greater than one requires the extra 's' + expect(implicitContainer.findByType('span').children).toEqual([ + 'Current implicit permissions from role', + '', + ':' + ]) + expect(implicitContainer.findAllByType('ul').length).toBe(1) + // we happen to know what this should be based on our mock of the role-based perms at the top of this file + // ...but ideally this shouldn't be a magical number + expect(implicitContainer.findByType('ul').children.length).toBe(3) + + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('mockExtraPerm') + }) + + test('sets selected user correctly - multiple roles', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { + id: 1, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1', 'role2'], + perms: ['perm1', 'perm2', 'perm3', 'perm4', 'mockExtraPerm'] + } + ] + } + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + const implicitContainer = component.root.findByProps({ className: 'implicit-perms-container' }) + + expect(implicitContainer.findByType('span').children).toEqual([ + 'Current implicit permissions from role', + 's', + ':' + ]) + expect(implicitContainer.findAllByType('ul').length).toBe(2) + // same situation - we know what order these will be in because we mocked them to show up this way, but... magical + expect(implicitContainer.findAllByType('ul')[0].children.length).toBe(3) + expect(implicitContainer.findAllByType('ul')[0].children[0].children[0]).toBe('perm1') + expect(implicitContainer.findAllByType('ul')[0].children[1].children[0]).toBe('perm2') + expect(implicitContainer.findAllByType('ul')[0].children[2].children[0]).toBe('perm3') + + expect(implicitContainer.findAllByType('ul')[1].children.length).toBe(1) + expect(implicitContainer.findAllByType('ul')[1].children[0].children[0]).toBe('perm4') + + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('mockExtraPerm') + }) + + test('does nothing when trying to add or remove before selecting a valid permission', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { + id: 1, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3'] + } + ] + } + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // without selecting a new permission, try adding and removing to make sure they don't do anything + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + + // clicking the 'Add' button should do nothing + act(() => { + buttons[0].props.onClick() + }) + expect(newProps.addUserPermission).not.toHaveBeenCalled() + + // clicking the 'Remove' button should do nothing + act(() => { + buttons[1].props.onClick() + }) + expect(newProps.removeUserPermission).not.toHaveBeenCalled() + }) + + test('sends the correct permission to the API utility when adding - success response', async () => { + const mockUser = { + id: 43, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3'] + } + + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [{ ...mockUser }] + } + + newProps.addUserPermission = jest.fn().mockResolvedValueOnce({ + payload: { + status: 'ok', + value: { ...mockUser, perms: ['perm1', 'perm2', 'perm3', 'perm4'] } + } + }) + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // make sure there are no explicit perms before we add one + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findAllByType('ul').length).toBe(0) + + // as a bonus we're also making sure the permission selection update works properly + // since there's no real way of checking that independently + // make sure clicking the 'Add' button calls functions appropriately + const permSelect = component.root.findByType('select') + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + act(() => { + // ordinarily this value would come from the selected option attached to the change event + // we can just simulate the change event's target's value here + const mockChangeEvent = { target: { value: 'perm4' } } + permSelect.props.onChange(mockChangeEvent) + }) + await act(async () => { + await buttons[0].props.onClick() + }) + + expect(newProps.addUserPermission).toHaveBeenCalledTimes(1) + expect(newProps.addUserPermission).toHaveBeenCalledWith(mockUser.id, 'perm4') + + // on success, the implicit/explicit perms areas should update with any changes + // we added an explicit change this time so we can check that + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('perm4') + }) + test('sends the correct permission to the API utility when adding - non-success response', async () => { + const mockUser = { + id: 43, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3'] + } + + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [{ ...mockUser }] + } + + newProps.addUserPermission = jest.fn().mockResolvedValueOnce({ + payload: { status: 'not-ok' } + }) + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // make sure there are no explicit perms before we try adding one + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findAllByType('ul').length).toBe(0) + + const permSelect = component.root.findByType('select') + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + act(() => { + const mockChangeEvent = { target: { value: 'perm4' } } + permSelect.props.onChange(mockChangeEvent) + }) + await act(async () => { + await buttons[0].props.onClick() + }) + + expect(newProps.addUserPermission).toHaveBeenCalledTimes(1) + expect(newProps.addUserPermission).toHaveBeenCalledWith(mockUser.id, 'perm4') + + // response wasn't successful, make sure nothing changed + expect(explicitContainer.findAllByType('ul').length).toBe(0) + }) + + test('sends the correct permission to the API utility when removing - success response', async () => { + const mockUser = { + id: 43, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3', 'perm4'] + } + + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [{ ...mockUser }] + } + + newProps.removeUserPermission = jest.fn().mockResolvedValueOnce({ + payload: { + status: 'ok', + value: { ...mockUser, perms: ['perm1', 'perm2', 'perm3'] } + } + }) + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // make sure the single explicit perm is shown before we remove it + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('perm4') + + // make sure clicking the 'Remove' button calls functions appropriately + const permSelect = component.root.findByType('select') + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + act(() => { + const mockChangeEvent = { target: { value: 'perm4' } } + permSelect.props.onChange(mockChangeEvent) + }) + await act(async () => { + await buttons[1].props.onClick() + }) + + expect(newProps.removeUserPermission).toHaveBeenCalledTimes(1) + expect(newProps.removeUserPermission).toHaveBeenCalledWith(mockUser.id, 'perm4') + + // we removed the only explicit perm, make sure that's reflected properly + expect(explicitContainer.findAllByType('ul').length).toBe(0) + }) + test('sends the correct permission to the API utility when removing - success response', async () => { + const mockUser = { + id: 43, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3', 'perm4'] + } + + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [{ ...mockUser }] + } + + newProps.removeUserPermission = jest.fn().mockResolvedValueOnce({ + payload: { + status: 'not-ok' + } + }) + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('perm4') + + const permSelect = component.root.findByType('select') + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + act(() => { + const mockChangeEvent = { target: { value: 'perm4' } } + permSelect.props.onChange(mockChangeEvent) + }) + await act(async () => { + await buttons[1].props.onClick() + }) + + expect(newProps.removeUserPermission).toHaveBeenCalledTimes(1) + expect(newProps.removeUserPermission).toHaveBeenCalledWith(mockUser.id, 'perm4') + + // response wasn't successful, make sure nothing changed + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('perm4') + }) +}) diff --git a/packages/app/obojobo-repository/shared/reducers/admin-reducer.js b/packages/app/obojobo-repository/shared/reducers/admin-reducer.js index ad48af3a48..5b4f05f5c8 100644 --- a/packages/app/obojobo-repository/shared/reducers/admin-reducer.js +++ b/packages/app/obojobo-repository/shared/reducers/admin-reducer.js @@ -1,8 +1,18 @@ +const { handle } = require('redux-pack') + +const { LOAD_USER_SEARCH } = require('../actions/admin-actions') + function AdminReducer(state, action) { - switch (action.type) { - default: - return state - } + switch (action.type) { + case LOAD_USER_SEARCH: + return handle(state, action, { + start: prevState => ({ ...prevState, userSearchString: action.meta.searchString }), + success: prevState => ({ ...prevState, searchUsers: { items: action.payload.value } }) + }) + + default: + return state + } } -module.exports = AdminReducer \ No newline at end of file +module.exports = AdminReducer diff --git a/packages/app/obojobo-repository/shared/reducers/admin-reducer.test.js b/packages/app/obojobo-repository/shared/reducers/admin-reducer.test.js index 123059bd44..823cbe7d0d 100644 --- a/packages/app/obojobo-repository/shared/reducers/admin-reducer.test.js +++ b/packages/app/obojobo-repository/shared/reducers/admin-reducer.test.js @@ -1,13 +1,84 @@ +jest.mock('redux-pack', () => { + return { + handle: jest.fn((prevState, action, steps) => ({ prevState, action, steps })) + } +}) +const Pack = require('redux-pack') + +const { LOAD_USER_SEARCH } = require('../actions/admin-actions') + const adminReducer = require('./admin-reducer') +const handleStart = handler => { + return handler.steps.start(handler.prevState) +} +const handleSuccess = handler => { + return handler.steps.success(handler.prevState) +} + describe('Admin Reducer', () => { - test('Admin actions in reducer return the original state', () => { - const initialState = {} - - const action = { - type: 'test-admin-action' - } - - expect(adminReducer(initialState, action)).toBe(initialState) - }) -}) \ No newline at end of file + const defaultSearchResultsState = { + isFetching: false, + hasFetched: false, + items: [] + } + + beforeEach(() => { + Pack.handle.mockClear() + }) + + test('LOAD_USER_SEARCH action modifies state correctly', () => { + const initialState = { + userSearchString: 'oldSearchString', + searchUsers: { ...defaultSearchResultsState } + } + + const mockUserList = [ + { + id: 0, + displayName: 'firstName lastName' + }, + { + id: 1, + displayName: 'firstName2 lastName2' + } + ] + const action = { + type: LOAD_USER_SEARCH, + meta: { + searchString: 'newSearchString' + }, + payload: { + value: mockUserList + } + } + + // asynchronous action - state changes on success + const handler = adminReducer(initialState, action) + let newState + + newState = handleStart(handler) + expect(newState.userSearchString).toEqual('newSearchString') + expect(newState.searchUsers).toEqual(initialState.searchUsers) + + newState = handleSuccess(handler) + expect(newState.searchUsers).not.toEqual(initialState.searchUsers) + expect(newState.searchUsers).toEqual({ + items: mockUserList + }) + }) + + test('unrecognized action types just return the current state', () => { + const initialState = { + key: 'initialValue' + } + + const action = { + type: 'UNSUPPORTED_TYPE', + key: 'someOtherValue' + } + + const newState = adminReducer(initialState, action) + expect(newState.key).toEqual(initialState.key) + }) +}) diff --git a/packages/app/obojobo-repository/shared/util/implicit-perms.js b/packages/app/obojobo-repository/shared/util/implicit-perms.js new file mode 100644 index 0000000000..d809f2527a --- /dev/null +++ b/packages/app/obojobo-repository/shared/util/implicit-perms.js @@ -0,0 +1,17 @@ +// translate all possible perm options from a list and collate them per role +const defaultPermissions = require('obojobo-express/server/config/permission_groups.json').default + +const POSSIBLE_PERMS = Object.keys(defaultPermissions) +const PERMS_PER_ROLE = {} +POSSIBLE_PERMS.forEach(p => { + const roles = defaultPermissions[p] + roles.forEach(r => { + if (!PERMS_PER_ROLE[r]) PERMS_PER_ROLE[r] = [] + PERMS_PER_ROLE[r].push(p) + }) +}) + +module.exports = { + POSSIBLE_PERMS, + PERMS_PER_ROLE +} diff --git a/packages/app/obojobo-repository/shared/util/implicit-perms.test.js b/packages/app/obojobo-repository/shared/util/implicit-perms.test.js new file mode 100644 index 0000000000..bb40ee11bd --- /dev/null +++ b/packages/app/obojobo-repository/shared/util/implicit-perms.test.js @@ -0,0 +1,31 @@ +const mockStructureAsObject = { + mockPerm1: [ + 'Role1', + 'Role2', + 'urn:lti:role:standard/lis/Role1', + 'urn:lti:role:standard/lis/Role2' + ], + mockPerm2: ['Role1', 'Role2', 'urn:lti:role:standard/lis/Role1'], + mockPerm3: ['Role1', 'urn:lti:role:standard/lis/Role1'], + mockPerm4: ['Role1'] +} + +const { POSSIBLE_PERMS, PERMS_PER_ROLE } = require('./implicit-perms') + +jest.mock('obojobo-express/server/config/permission_groups.json', () => ({ + default: mockStructureAsObject +})) + +describe('implicit permission collation shortcut', () => { + test('POSSIBLE_PERMS and PERMS_PER_ROLE are generated correctly based on default role/perm associations', () => { + const expectedPossiblePerms = ['mockPerm1', 'mockPerm2', 'mockPerm3', 'mockPerm4'] + const expectedPermsPerRole = { + Role1: ['mockPerm1', 'mockPerm2', 'mockPerm3', 'mockPerm4'], + Role2: ['mockPerm1', 'mockPerm2'], + 'urn:lti:role:standard/lis/Role1': ['mockPerm1', 'mockPerm2', 'mockPerm3'], + 'urn:lti:role:standard/lis/Role2': ['mockPerm1'] + } + expect(POSSIBLE_PERMS).toEqual(expectedPossiblePerms) + expect(PERMS_PER_ROLE).toEqual(expectedPermsPerRole) + }) +}) From 9ce813d3a28e9ff84954c6cecf3a9342ebfde207 Mon Sep 17 00:00:00 2001 From: Brandon Stull Date: Fri, 9 Jun 2023 15:09:22 -0400 Subject: [PATCH 5/5] #2010 Prettier fixes. --- packages/app/obojobo-express/server/obo_express_dev.js | 2 +- packages/app/obojobo-express/webpack.config.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/app/obojobo-express/server/obo_express_dev.js b/packages/app/obojobo-express/server/obo_express_dev.js index 04897dd4a3..4698df6825 100644 --- a/packages/app/obojobo-express/server/obo_express_dev.js +++ b/packages/app/obojobo-express/server/obo_express_dev.js @@ -17,7 +17,7 @@ const POSSIBLE_PERMS = [ 'canPreviewDrafts', 'canViewStatsPage', 'canViewSystemStats', - 'canViewAdminPage', + 'canViewAdminPage' ] // Normally the query running in User.saveOrCreate would auto-fill the new user's id, diff --git a/packages/app/obojobo-express/webpack.config.js b/packages/app/obojobo-express/webpack.config.js index c0e5763e12..9c94aae71c 100644 --- a/packages/app/obojobo-express/webpack.config.js +++ b/packages/app/obojobo-express/webpack.config.js @@ -35,7 +35,7 @@ module.exports = watchOptions: { ignored: '/node_modules/' }, - stats: { children: false, modules: false }, + stats: { children: false, modules: false } }, entry: entriesFromObojoboModules, output: {