From 8a3e6155c4bb1aed1657664a94c089cd08d36431 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Wed, 23 Jun 2021 20:56:48 -0400 Subject: [PATCH 1/8] Separate helper for creating the default redux store for www pages --- src/lib/configure-store.js | 26 ++++++++++++++++++++++++++ src/lib/render.jsx | 21 ++------------------- 2 files changed, 28 insertions(+), 19 deletions(-) create mode 100644 src/lib/configure-store.js diff --git a/src/lib/configure-store.js b/src/lib/configure-store.js new file mode 100644 index 000000000..7818d4422 --- /dev/null +++ b/src/lib/configure-store.js @@ -0,0 +1,26 @@ +const redux = require('redux'); +const thunk = require('redux-thunk').default; + +const reducer = require('../redux/reducer.js'); + +const configureStore = (reducers, initialState, enhancer) => { + const allReducers = reducer(reducers); + + const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || redux.compose; + const enhancers = enhancer ? + composeEnhancers( + redux.applyMiddleware(thunk), + enhancer + ) : + composeEnhancers( + redux.applyMiddleware(thunk) + ); + const store = redux.createStore( + allReducers, + initialState || {}, + enhancers + ); + return store; +}; + +module.exports = configureStore; diff --git a/src/lib/render.jsx b/src/lib/render.jsx index dce966204..e17358616 100644 --- a/src/lib/render.jsx +++ b/src/lib/render.jsx @@ -1,5 +1,3 @@ -const redux = require('redux'); -const thunk = require('redux-thunk').default; // JSX syntax transforms to React.createElement const React = require('react'); // eslint-disable-line const ReactDOM = require('react-dom'); @@ -8,7 +6,7 @@ const StoreProvider = require('react-redux').Provider; const IntlProvider = require('./intl.jsx').IntlProvider; const permissionsActions = require('../redux/permissions.js'); const sessionActions = require('../redux/session.js'); -const reducer = require('../redux/reducer.js'); +const configureStore = require('./configure-store.js'); require('../main.scss'); @@ -36,22 +34,7 @@ const render = (jsx, element, reducers, initialState, enhancer) => { messages = window._messages[locale]; } - const allReducers = reducer(reducers); - - const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || redux.compose; - const enhancers = enhancer ? - composeEnhancers( - redux.applyMiddleware(thunk), - enhancer - ) : - composeEnhancers( - redux.applyMiddleware(thunk) - ); - const store = redux.createStore( - allReducers, - initialState || {}, - enhancers - ); + const store = configureStore(reducers, initialState, enhancer); // Render view component ReactDOM.render( From a8ed7410a433a06ce545ed5bef909914f296be5f Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Wed, 23 Jun 2021 20:57:30 -0400 Subject: [PATCH 2/8] Separate studio reducers and initial state into a helper --- src/views/studio/studio-redux.js | 37 ++++++++++++++++++++++++++++++++ src/views/studio/studio.jsx | 36 ++++--------------------------- 2 files changed, 41 insertions(+), 32 deletions(-) create mode 100644 src/views/studio/studio-redux.js diff --git a/src/views/studio/studio-redux.js b/src/views/studio/studio-redux.js new file mode 100644 index 000000000..9cfaf6ee0 --- /dev/null +++ b/src/views/studio/studio-redux.js @@ -0,0 +1,37 @@ +import { + projects, + curators, + managers, + activity, + userProjects +} from './lib/redux-modules'; + +const {getInitialState, studioReducer} = require('../../redux/studio'); +const {studioReportReducer} = require('../../redux/studio-report'); +const {commentsReducer} = require('../../redux/comments'); +const {studioMutationsReducer} = require('../../redux/studio-mutations'); + + +const reducers = { + [projects.key]: projects.reducer, + [curators.key]: curators.reducer, + [managers.key]: managers.reducer, + [activity.key]: activity.reducer, + [userProjects.key]: userProjects.reducer, + comments: commentsReducer, + studio: studioReducer, + studioMutations: studioMutationsReducer, + studioReport: studioReportReducer +}; + +const initialState = { + studio: { + ...getInitialState(), + // Include the studio id in the initial state to allow us + // to stop passing around the studio id in components + // when it is only needed for data fetching, not for rendering. + id: window.location.pathname.split('/')[2] + } +}; + +export {reducers, initialState}; diff --git a/src/views/studio/studio.jsx b/src/views/studio/studio.jsx index a88871e8c..51054bbc5 100644 --- a/src/views/studio/studio.jsx +++ b/src/views/studio/studio.jsx @@ -27,18 +27,8 @@ import StudioMeta from './studio-meta.jsx'; import StudioAdminPanel from './studio-admin-panel.jsx'; import StudioDeleted from './studio-deleted.jsx'; -import { - projects, - curators, - managers, - activity, - userProjects -} from './lib/redux-modules'; - -const {getInitialState, studioReducer, selectStudioLoadFailed, getInfo} = require('../../redux/studio'); -const {studioReportReducer} = require('../../redux/studio-report'); -const {commentsReducer} = require('../../redux/comments'); -const {studioMutationsReducer} = require('../../redux/studio-mutations'); +import {reducers, initialState} from './studio-redux.js'; +import {selectStudioLoadFailed, getInfo} from '../../redux/studio'; import './studio.scss'; import {selectIsAdmin, selectMuteStatus} from '../../redux/session.js'; @@ -140,24 +130,6 @@ render( , document.getElementById('app'), - { - [projects.key]: projects.reducer, - [curators.key]: curators.reducer, - [managers.key]: managers.reducer, - [activity.key]: activity.reducer, - [userProjects.key]: userProjects.reducer, - comments: commentsReducer, - studio: studioReducer, - studioMutations: studioMutationsReducer, - studioReport: studioReportReducer - }, - { - studio: { - ...getInitialState(), - // Include the studio id in the initial state to allow us - // to stop passing around the studio id in components - // when it is only needed for data fetching, not for rendering. - id: window.location.pathname.split('/')[2] - } - } + reducers, + initialState ); From c9ba9d443c17af03d47836b982267970bc66563f Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Wed, 23 Jun 2021 21:28:33 -0400 Subject: [PATCH 3/8] Add tests for all studio-member-actions --- src/views/studio/lib/studio-member-actions.js | 3 +- test/unit/redux/studio-member-actions.test.js | 377 ++++++++++++++++++ 2 files changed, 379 insertions(+), 1 deletion(-) create mode 100644 test/unit/redux/studio-member-actions.test.js diff --git a/src/views/studio/lib/studio-member-actions.js b/src/views/studio/lib/studio-member-actions.js index c8eb56db0..36cd3e3ab 100644 --- a/src/views/studio/lib/studio-member-actions.js +++ b/src/views/studio/lib/studio-member-actions.js @@ -14,7 +14,8 @@ const Errors = keyMirror({ USER_MUTED: null, UNKNOWN_USERNAME: null, RATE_LIMIT: null, - MANAGER_LIMIT: null + MANAGER_LIMIT: null, + UNHANDLED: null }); const normalizeError = (err, body, res) => { diff --git a/test/unit/redux/studio-member-actions.test.js b/test/unit/redux/studio-member-actions.test.js new file mode 100644 index 000000000..1800aeb10 --- /dev/null +++ b/test/unit/redux/studio-member-actions.test.js @@ -0,0 +1,377 @@ +import {selectStudioManagerCount} from '../../../src/redux/studio'; +import { + Errors, + removeManager, + loadManagers, + loadCurators, + removeCurator, + inviteCurator, + promoteCurator, + acceptInvitation +} from '../../../src/views/studio/lib/studio-member-actions'; +import {managers, curators} from '../../../src/views/studio/lib/redux-modules'; +import {reducers, initialState} from '../../../src/views/studio/studio-redux'; +import configureStore from '../../../src/lib/configure-store'; + +jest.mock('../../../src/lib/api'); +import api from '../../../src/lib/api'; + +let store; + +beforeEach(() => { + api.mockClear(); +}); + +describe('loadManagers', () => { + test('it populates the managers list', () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + api.mockImplementation((opts, callback) => { + const body = [{username: 'user1'}, {username: 'user2'}, {username: 'user3'}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(loadManagers()); + let items = managers.selector(store.getState()).items; + expect(api.mock.calls[0][0].uri).toBe('/studios/123123/managers/'); + expect(api.mock.calls[0][0].params.offset).toBe(0); + expect(items.length).toBe(3); + expect(items[0].username).toBe('user1'); + + // Include the new offset next time it is called + store.dispatch(loadManagers()); + expect(api.mock.calls[1][0].params.offset).toBe(3); + items = managers.selector(store.getState()).items; + expect(items.length).toBe(6); + }); + + test('it correctly uses the admin route when possible', () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123}, + session: { + session: { + user: {token: 'a-token'}, + permissions: {admin: true} + } + } + }); + api.mockImplementation((opts, callback) => { + expect(opts.uri).toBe('/admin/studios/123123/managers/'); + expect(opts.authentication).toBe('a-token'); + }); + store.dispatch(loadManagers()); + }); + + test('errors are set on the managers state', () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + api.mockImplementation((opts, callback) => { + callback(null, null, {statusCode: 500}); + }); + store.dispatch(loadManagers()); + expect(managers.selector(store.getState()).error).toBe(Errors.SERVER); + }); +}); + + +describe('loadCurators', () => { + test('it populates the curators list', () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + api.mockImplementation((opts, callback) => { + const body = [{username: 'user1'}, {username: 'user2'}, {username: 'user3'}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(loadCurators()); + let items = curators.selector(store.getState()).items; + expect(api.mock.calls[0][0].uri).toBe('/studios/123123/curators/'); + expect(api.mock.calls[0][0].params.offset).toBe(0); + expect(items.length).toBe(3); + expect(items[0].username).toBe('user1'); + + // Include the new offset next time it is called + store.dispatch(loadCurators()); + expect(api.mock.calls[1][0].params.offset).toBe(3); + items = curators.selector(store.getState()).items; + expect(items.length).toBe(6); + }); + + test('it correctly uses the admin route when possible', () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123}, + session: { + session: { + user: {token: 'a-token'}, + permissions: {admin: true} + } + } + }); + api.mockImplementation((opts, callback) => { + expect(opts.uri).toBe('/admin/studios/123123/curators/'); + expect(opts.authentication).toBe('a-token'); + }); + store.dispatch(loadCurators()); + }); + + test('errors are set on the curators state', () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + api.mockImplementation((opts, callback) => { + callback(null, null, {statusCode: 500}); + }); + store.dispatch(loadCurators()); + expect(curators.selector(store.getState()).error).toBe(Errors.SERVER); + }); +}); + +describe('removeManager', () => { + beforeEach(() => { + store = configureStore(reducers, { + ...initialState, + studio: { + id: 123123, + managers: 3, + manager: true + }, + managers: { + items: [ + {username: 'user1'}, + {username: 'user2'}, + {username: 'user3'} + ] + }, + session: { + session: { + user: {username: 'user2'} + } + } + }); + }); + + test('removes the manager by username and decrements the count', async () => { + api.mockImplementation((opts, callback) => { + expect(opts.uri).toBe('/site-api/users/curators-in/123123/remove/'); + callback(null, {}, {statusCode: 200}); + }); + + await store.dispatch(removeManager('user2')); + const state = store.getState(); + + // Ensure it removes the correct manager (index=1) + expect(selectStudioManagerCount(state)).toBe(2); + expect(managers.selector(state).items[0].username).toBe('user1'); + expect(managers.selector(state).items[1].username).toBe('user3'); + + // Ensure roles change if you are removing yourself + expect(state.studio.manager).toBe(false); + }); + + test('on error, promise rejects without any changing count or list', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {}, {statusCode: 403}); + }); + + await expect(store.dispatch(removeManager('user2'))) + .rejects.toBe(Errors.PERMISSION); + + const state = store.getState(); + const {items} = managers.selector(state); + expect(selectStudioManagerCount(state)).toBe(3); + expect(items.length).toBe(3); + }); +}); + +describe('removeCurator', () => { + beforeEach(() => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123}, + curators: { + items: [ + {username: 'user1'}, + {username: 'user2'}, + {username: 'user3'} + ] + } + }); + }); + + test('removes the curator by username', async () => { + api.mockImplementation((opts, callback) => { + expect(opts.uri).toBe('/site-api/users/curators-in/123123/remove/'); + callback(null, {}, {statusCode: 200}); + }); + + await store.dispatch(removeCurator('user2')); + const state = store.getState(); + + // Ensure it removes the correct curator (index=1) + expect(curators.selector(state).items[0].username).toBe('user1'); + expect(curators.selector(state).items[1].username).toBe('user3'); + }); + + test('on error, promise rejects without changing anything', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {}, {statusCode: 500}); + }); + + await expect(store.dispatch(removeCurator('user2'))) + .rejects.toBe(Errors.SERVER); + + const state = store.getState(); + const {items} = curators.selector(state); + expect(items.length).toBe(3); + }); +}); + +describe('inviteCurator', () => { + beforeEach(() => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + }); + + test('invites the curator on success', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {}, {statusCode: 200}); + }); + + const result = await store.dispatch(inviteCurator('user2')); + expect(result).toBe('user2'); + expect(api.mock.calls[0][0].uri).toBe('/site-api/users/curators-in/123123/invite_curator/'); + expect(api.mock.calls[0][0].params.usernames).toBe('user2'); + }); + + test('error because of unknown user', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {}, {statusCode: 404}); + }); + await expect(store.dispatch(inviteCurator('user2'))) + .rejects.toBe(Errors.UNKNOWN_USERNAME); + }); + test('error because of duplicate curator', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {status: 'error', message: 'already a curator'}, {statusCode: 200}); + }); + await expect(store.dispatch(inviteCurator('user2'))) + .rejects.toBe(Errors.DUPLICATE); + }); + test('unhandled error response', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {status: 'error', message: 'xyz'}, {statusCode: 200}); + }); + await expect(store.dispatch(inviteCurator('user2'))) + .rejects.toBe(Errors.UNHANDLED); + }); +}); + +describe('promoteCurator', () => { + beforeEach(() => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123, managers: 0}, + curators: { + items: [{username: 'curatorName'}] + } + }); + }); + + test('promotes the curator on success', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {}, {statusCode: 200}); + }); + + await store.dispatch(promoteCurator('curatorName')); + const state = store.getState(); + const {items: curatorList} = curators.selector(state); + const {items: managerList} = managers.selector(state); + + expect(api.mock.calls[0][0].uri).toBe('/site-api/users/curators-in/123123/promote/'); + expect(api.mock.calls[0][0].params.usernames).toBe('curatorName'); + expect(managerList.length).toBe(1); + expect(managerList[0].username).toBe('curatorName'); + expect(curatorList.length).toBe(0); + expect(selectStudioManagerCount(state)).toBe(1); + }); + + test('on error, promise rejects and nothing is modified', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {}, {statusCode: 403}); + }); + await expect(store.dispatch(promoteCurator('curatorName'))) + .rejects.toBe(Errors.PERMISSION); + const state = store.getState(); + const {items: curatorList} = curators.selector(state); + const {items: managerList} = managers.selector(state); + expect(managerList.length).toBe(0); + expect(curatorList.length).toBe(1); + expect(selectStudioManagerCount(state)).toBe(0); + }); + + test('error because of exceeding manager limit', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {message: 'too many owners'}, {statusCode: 400}); + }); + await expect(store.dispatch(promoteCurator('curatorName'))) + .rejects.toBe(Errors.MANAGER_LIMIT); + }); +}); + +describe('acceptInvitation', () => { + beforeEach(() => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123, invited: true, curator: false}, + session: { + session: { + user: { + username: 'me' + } + } + } + }); + }); + + test('accepts the invitation on success', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {username: 'me'}, {statusCode: 200}); + }); + jest.useFakeTimers(); + await store.dispatch(acceptInvitation()); + let state = store.getState(); + const {items: curatorList} = curators.selector(state); + expect(api.mock.calls[0][0].uri).toBe('/site-api/users/curators-in/123123/add/'); + expect(api.mock.calls[0][0].params.usernames).toBe('me'); + expect(curatorList.length).toBe(1); + expect(curatorList[0].username).toBe('me'); + expect(state.studio.invited).toBe(true); // Should remain true until timers run + jest.runAllTimers(); // delay to show success alert before toggling invited back to false + state = store.getState(); + expect(state.studio.invited).toBe(false); + expect(state.studio.curator).toBe(true); + jest.useRealTimers(); + }); + + test('on error, promise rejects and nothing is modified', async () => { + api.mockImplementation((opts, callback) => { + callback(null, {}, {statusCode: 403}); + }); + await expect(store.dispatch(acceptInvitation())) + .rejects.toBe(Errors.PERMISSION); + const state = store.getState(); + const {items: curatorList} = curators.selector(state); + expect(curatorList.length).toBe(0); + expect(state.studio.invited).toBe(true); + expect(state.studio.curator).toBe(false); + }); +}); From a9d0ca47d706df7c5de4fddc35230e6a3d19c5b6 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Thu, 24 Jun 2021 08:47:52 -0400 Subject: [PATCH 4/8] Remove unused args --- test/unit/redux/studio-member-actions.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/redux/studio-member-actions.test.js b/test/unit/redux/studio-member-actions.test.js index 1800aeb10..86062b93f 100644 --- a/test/unit/redux/studio-member-actions.test.js +++ b/test/unit/redux/studio-member-actions.test.js @@ -57,7 +57,7 @@ describe('loadManagers', () => { } } }); - api.mockImplementation((opts, callback) => { + api.mockImplementation((opts) => { expect(opts.uri).toBe('/admin/studios/123123/managers/'); expect(opts.authentication).toBe('a-token'); }); @@ -113,7 +113,7 @@ describe('loadCurators', () => { } } }); - api.mockImplementation((opts, callback) => { + api.mockImplementation((opts) => { expect(opts.uri).toBe('/admin/studios/123123/curators/'); expect(opts.authentication).toBe('a-token'); }); From 1027b82f66f4d7f079775a8f80aaef11c05374e3 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Fri, 25 Jun 2021 10:43:33 -0400 Subject: [PATCH 5/8] Include unit tests for more of the action creators --- .../studio/lib/studio-activity-actions.js | 2 +- .../studio/lib/studio-project-actions.js | 2 +- .../redux/project-comment-actions.test.js | 125 ++++++++++++ .../redux/studio-activity-actions.test.js | 54 +++++ .../unit/redux/studio-comment-actions.test.js | 139 +++++++++++++ test/unit/redux/studio-member-actions.test.js | 30 +++ .../unit/redux/studio-project-actions.test.js | 191 ++++++++++++++++++ 7 files changed, 541 insertions(+), 2 deletions(-) create mode 100644 test/unit/redux/project-comment-actions.test.js create mode 100644 test/unit/redux/studio-activity-actions.test.js create mode 100644 test/unit/redux/studio-comment-actions.test.js create mode 100644 test/unit/redux/studio-project-actions.test.js diff --git a/src/views/studio/lib/studio-activity-actions.js b/src/views/studio/lib/studio-activity-actions.js index 7800bd187..cc7290750 100644 --- a/src/views/studio/lib/studio-activity-actions.js +++ b/src/views/studio/lib/studio-activity-actions.js @@ -42,4 +42,4 @@ const loadActivity = () => ((dispatch, getState) => { }); }); -export {loadActivity}; +export {Errors, loadActivity}; diff --git a/src/views/studio/lib/studio-project-actions.js b/src/views/studio/lib/studio-project-actions.js index 0cf048cd6..ca9fb5d9d 100644 --- a/src/views/studio/lib/studio-project-actions.js +++ b/src/views/studio/lib/studio-project-actions.js @@ -19,7 +19,7 @@ const Errors = keyMirror({ const normalizeError = (err, body, res) => { if (err) return Errors.NETWORK; - if (res.statusCode === 403 && body.mute_status) return Errors.USER_MUTED; + if (res.statusCode === 403 && body && body.mute_status) return Errors.USER_MUTED; if (res.statusCode === 401 || res.statusCode === 403) return Errors.PERMISSION; if (res.statusCode === 404) return Errors.UNKNOWN_PROJECT; if (res.statusCode === 409) return Errors.DUPLICATE; diff --git a/test/unit/redux/project-comment-actions.test.js b/test/unit/redux/project-comment-actions.test.js new file mode 100644 index 000000000..d0ddc0289 --- /dev/null +++ b/test/unit/redux/project-comment-actions.test.js @@ -0,0 +1,125 @@ +import actions from '../../../src/redux/project-comment-actions'; +import configureStore from '../../../src/lib/configure-store'; +import {commentsReducer} from '../../../src/redux/comments'; + +jest.mock('../../../src/lib/api'); +import api from '../../../src/lib/api'; + +let store; + +beforeEach(() => { + api.mockClear(); + // TODO Ideally this would be the entire project page reducer list + store = configureStore({comments: commentsReducer}, {}); +}); + +describe('getTopLevelComments', () => { + test('replies are only loaded for comments with a reply_count > 0', async () => { + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/users/u/projects/123123/comments'); + const body = [ + {id: 1, reply_count: 0}, + {id: 50, reply_count: 1}, + {id: 60, reply_count: 0}, + {id: 70, reply_count: 1} + ]; + callback(null, body, {statusCode: 200}); + }) + .mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/users/u/projects/123123/comments/50/replies'); + const body = [{id: 4, parent_id: 50}]; + callback(null, body, {statusCode: 200}); + }) + .mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/users/u/projects/123123/comments/70/replies'); + const body = [{id: 5, parent_id: 70}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(actions.getTopLevelComments(123123, 0, 'u')); + const state = store.getState(); + expect(state.comments.comments.length).toBe(4); + expect(state.comments.replies[50].length).toBe(1); + expect(state.comments.replies[70].length).toBe(1); + expect(state.comments.replies[1]).toBeUndefined(); + expect(state.comments.replies[60]).toBeUndefined(); + }); + test('admin route is used correctly', async () => { + api.mockImplementationOnce((opts) => { + // NB: this route doesn't include the owner username + expect(opts.uri).toBe('/admin/projects/123123/comments'); + expect(opts.authentication).toBe('a-token'); + }); + store.dispatch(actions.getTopLevelComments(123123, 0, 'u', true, 'a-token')); + }); +}); + +describe('getCommentById', () => { + test('getting a top level comment will not load replies if there arent any', async () => { + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/users/u/projects/123123/comments/111'); + const body = {id: 111, parent_id: null, reply_count: 0}; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(actions.getCommentById(123123, 111, 'u')); + const state = store.getState(); + expect(state.comments.comments.length).toBe(1); + expect(state.comments.replies[111]).toBeUndefined(); + }); + + test('admin route is used correctly', async () => { + api.mockImplementationOnce((opts) => { + // NB: this route doesn't include the owner username + expect(opts.uri).toBe('/admin/projects/123123/comments/111'); + expect(opts.authentication).toBe('a-token'); + }); + store.dispatch(actions.getCommentById(123123, 111, 'u', true, 'a-token')); + }); + + test('getting a top level comment will load replies', async () => { + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/users/u/projects/123123/comments/111'); + const body = {id: 111, parent_id: null, reply_count: 2}; + callback(null, body, {statusCode: 200}); + }).mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/users/u/projects/123123/comments/111/replies'); + const body = [{id: 1, parent_id: 111}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(actions.getCommentById(123123, 111, 'u')); + const state = store.getState(); + expect(state.comments.comments.length).toBe(1); + expect(state.comments.replies[111].length).toBe(1); + }); + + test('getting a reply comment will load the parent comment and its other replies', async () => { + // Expect 3 requests. First 111, which is a reply comment, maybe linked to from messages + // Second is for 111's parent, which is 555. + // Third is for 555's replies, which returns 111 and 112 + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/users/u/projects/123123/comments/111'); + const body = {id: 111, parent_id: 555}; + callback(null, body, {statusCode: 200}); + }) + .mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/users/u/projects/123123/comments/555'); + const body = {id: 555, reply_count: 2}; + callback(null, body, {statusCode: 200}); + }) + .mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/users/u/projects/123123/comments/555/replies'); + const body = [{id: 111, parent_id: 555}, {id: 112, parent_id: 555}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(actions.getCommentById(123123, 111, 'u')); + const state = store.getState(); + expect(state.comments.comments.length).toBe(1); + expect(state.comments.replies[555].length).toBe(2); + }); +}); + +describe.skip('addNewComment', () => { }); +describe.skip('deleteComment', () => { }); +describe.skip('reportComment', () => { }); +describe.skip('resetComments', () => { }); +describe.skip('reportComment', () => { }); +describe.skip('getReplies', () => { }); diff --git a/test/unit/redux/studio-activity-actions.test.js b/test/unit/redux/studio-activity-actions.test.js new file mode 100644 index 000000000..a3587a1e9 --- /dev/null +++ b/test/unit/redux/studio-activity-actions.test.js @@ -0,0 +1,54 @@ +import { + Errors, + loadActivity +} from '../../../src/views/studio/lib/studio-activity-actions'; +import {activity} from '../../../src/views/studio/lib/redux-modules'; +import {reducers, initialState} from '../../../src/views/studio/studio-redux'; +import configureStore from '../../../src/lib/configure-store'; + +jest.mock('../../../src/lib/api'); +import api from '../../../src/lib/api'; + +let store; + +beforeEach(() => { + api.mockClear(); + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); +}); + +describe('loadActivity', () => { + test('it populates the activity list', () => { + api.mockImplementation((opts, callback) => { + const body = [{id: 1}, {id: 2}, {id: 3, datetime_created: 'abc'}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(loadActivity()); + let items = activity.selector(store.getState()).items; + expect(api.mock.calls[0][0].uri).toBe('/studios/123123/activity/'); + expect(api.mock.calls[0][0].params.offset).toBeUndefined(); + expect(items.length).toBe(3); + expect(items[0].id).toBe(1); + + // On next loadActivity request, it should include the last activity items + // datetime_created as the dateLimit. It should de-duplicate based on id + api.mockImplementation((opts, callback) => { + const body = [{id: 3}, {id: 4}, {id: 5, datetime_created: 'def'}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(loadActivity()); + expect(api.mock.calls[1][0].params.dateLimit).toBe('abc'); + items = activity.selector(store.getState()).items; + expect(items.length).toBe(5); // id=3 should get de-duplicated + }); + + test('errors are set on the activity state', () => { + api.mockImplementation((opts, callback) => { + callback(null, null, {statusCode: 500}); + }); + store.dispatch(loadActivity()); + expect(activity.selector(store.getState()).error).toBe(Errors.SERVER); + }); +}); diff --git a/test/unit/redux/studio-comment-actions.test.js b/test/unit/redux/studio-comment-actions.test.js new file mode 100644 index 000000000..1ba8eae5c --- /dev/null +++ b/test/unit/redux/studio-comment-actions.test.js @@ -0,0 +1,139 @@ +import actions from '../../../src/redux/studio-comment-actions'; +import {reducers, initialState} from '../../../src/views/studio/studio-redux'; +import configureStore from '../../../src/lib/configure-store'; + +jest.mock('../../../src/lib/api'); +import api from '../../../src/lib/api'; + +let store; + +beforeEach(() => { + api.mockClear(); +}); + +describe('getTopLevelComments', () => { + test('replies are only loaded for comments with a reply_count > 0', async () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/comments'); + const body = [ + {id: 1, reply_count: 0}, + {id: 50, reply_count: 1}, + {id: 60, reply_count: 0}, + {id: 70, reply_count: 1} + ]; + callback(null, body, {statusCode: 200}); + }) + .mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/comments/50/replies'); + const body = [{id: 4, parent_id: 50}]; + callback(null, body, {statusCode: 200}); + }) + .mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/comments/70/replies'); + const body = [{id: 5, parent_id: 70}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(actions.getTopLevelComments()); + const state = store.getState(); + expect(state.comments.comments.length).toBe(4); + expect(state.comments.replies[50].length).toBe(1); + expect(state.comments.replies[70].length).toBe(1); + expect(state.comments.replies[1]).toBeUndefined(); + expect(state.comments.replies[60]).toBeUndefined(); + }); + test('admin route is used when the session shows the user is an admin', async () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123}, + session: { + session: { + user: {token: 'a-token'}, + permissions: {admin: true} + } + } + }); + api.mockImplementationOnce((opts) => { + expect(opts.uri).toBe('/admin/studios/123123/comments'); + expect(opts.authentication).toBe('a-token'); + }); + store.dispatch(actions.getTopLevelComments()); + }); +}); + +describe('getCommentById', () => { + test('getting a top level comment will not load replies if there arent any', async () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/comments/111'); + const body = {id: 111, parent_id: null, reply_count: 0}; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(actions.getCommentById(111)); + const state = store.getState(); + expect(state.comments.comments.length).toBe(1); + expect(state.comments.replies[111]).toBeUndefined(); + }); + + test('getting a top level comment will load replies', async () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/comments/111'); + const body = {id: 111, parent_id: null, reply_count: 2}; + callback(null, body, {statusCode: 200}); + }).mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/comments/111/replies'); + const body = [{id: 1, parent_id: 111}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(actions.getCommentById(111)); + const state = store.getState(); + expect(state.comments.comments.length).toBe(1); + expect(state.comments.replies[111].length).toBe(1); + }); + + test('getting a reply comment will load the parent comment and its other replies', async () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + // Expect 3 requests. First 111, which is a reply comment, maybe linked to from messages + // Second is for 111's parent, which is 555. + // Third is for 555's replies, which returns 111 and 112 + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/comments/111'); + const body = {id: 111, parent_id: 555}; + callback(null, body, {statusCode: 200}); + }) + .mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/comments/555'); + const body = {id: 555, reply_count: 2}; + callback(null, body, {statusCode: 200}); + }) + .mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/comments/555/replies'); + const body = [{id: 111, parent_id: 555}, {id: 112, parent_id: 555}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(actions.getCommentById(111)); + const state = store.getState(); + expect(state.comments.comments.length).toBe(1); + expect(state.comments.replies[555].length).toBe(2); + }); +}); + +describe.skip('addNewComment', () => { }); +describe.skip('deleteComment', () => { }); +describe.skip('reportComment', () => { }); +describe.skip('resetComments', () => { }); +describe.skip('reportComment', () => { }); +describe.skip('getReplies', () => { }); diff --git a/test/unit/redux/studio-member-actions.test.js b/test/unit/redux/studio-member-actions.test.js index 86062b93f..7a9bece76 100644 --- a/test/unit/redux/studio-member-actions.test.js +++ b/test/unit/redux/studio-member-actions.test.js @@ -22,6 +22,36 @@ beforeEach(() => { api.mockClear(); }); + +const storage = { + max: undefined, + items: [] +}; + +Object.defineProperty(storage, 'max', {writable: false, value: 5000}); + +let currentStorage = 'undefined'; + +const storageUsed = () => { + if (currentStorage) { + return currentStorage; + } + currentStorage = 0; + for (const i = 0; i < storage.length(); i++) { + currentStorage += storage.items[i].weigth; + } + return currentStorage; +}; + +const add = (item) => { + if (storage.max - item.weight >= storageUsed()) { + storage.items.push(item); + currentStorage += iten.weight; + } +}; + +add({weight: 100}); + describe('loadManagers', () => { test('it populates the managers list', () => { store = configureStore(reducers, { diff --git a/test/unit/redux/studio-project-actions.test.js b/test/unit/redux/studio-project-actions.test.js new file mode 100644 index 000000000..30cb3eca9 --- /dev/null +++ b/test/unit/redux/studio-project-actions.test.js @@ -0,0 +1,191 @@ +import {selectStudioManagerCount} from '../../../src/redux/studio'; +import { + Errors, + loadProjects, + addProject, + removeProject +} from '../../../src/views/studio/lib/studio-project-actions'; +import {projects} from '../../../src/views/studio/lib/redux-modules'; +import {reducers, initialState} from '../../../src/views/studio/studio-redux'; +import configureStore from '../../../src/lib/configure-store'; + +jest.mock('../../../src/lib/api'); +import api from '../../../src/lib/api'; + +let store; + +beforeEach(() => { + api.mockClear(); +}); + +describe('loadProjects', () => { + test('it populates the projects list', () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + api.mockImplementation((opts, callback) => { + const body = [{id: 1}, {id: 2}, {id: 3}]; + callback(null, body, {statusCode: 200}); + }); + store.dispatch(loadProjects()); + let items = projects.selector(store.getState()).items; + expect(api.mock.calls[0][0].uri).toBe('/studios/123123/projects/'); + expect(api.mock.calls[0][0].params.offset).toBe(0); + expect(items.length).toBe(3); + expect(items[0].id).toBe(1); + + // Include the new offset next time it is called + store.dispatch(loadProjects()); + expect(api.mock.calls[1][0].params.offset).toBe(3); + items = projects.selector(store.getState()).items; + expect(items.length).toBe(6); + }); + + test('it correctly uses the admin route when possible', () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123}, + session: { + session: { + user: {token: 'a-token'}, + permissions: {admin: true} + } + } + }); + api.mockImplementation((opts) => { + expect(opts.uri).toBe('/admin/studios/123123/projects/'); + expect(opts.authentication).toBe('a-token'); + }); + store.dispatch(loadProjects()); + }); + + test('errors are set on the projects state', () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + api.mockImplementation((opts, callback) => { + callback(null, null, {statusCode: 500}); + }); + store.dispatch(loadProjects()); + expect(projects.selector(store.getState()).error).toBe(Errors.SERVER); + }); +}); + +describe('addProject', () => { + beforeEach(() => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123} + }); + }); + test('makes a POST and a GET and then combines the result and puts it in redux', async () => { + const postResponse = { + projectId: '111', + actorId: 'actor-id' + }; + const getResponse = { + title: 'project-title', + image: 'project-image', + author: { + id: 'author-id', + username: 'author-username', + profile: {images: [1, 2, 3]} + } + }; + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/project/111'); + expect(opts.method).toBe('POST'); + callback(null, postResponse, {statusCode: 200}); + }).mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/projects/111'); + callback(null, getResponse, {statusCode: 200}); + }); + await store.dispatch(addProject('scratch.mit.edu/projects/111')); + const {items} = projects.selector(store.getState()); + expect(items.length).toBe(1); + // Item in redux is a combination of get/post that matches the shape of the studio projects endpoint + expect(items[0]).toMatchObject({ + id: 111, + actor_id: 'actor-id', + title: 'project-title', + image: 'project-image', + creator_id: 'author-id', + username: 'author-username', + avatar: [1, 2, 3] + }); + }); + test('submitting an invalid returns error without network requests', async () => { + await expect(store.dispatch(addProject('abc'))) + .rejects.toBe(Errors.UNKNOWN_PROJECT); + expect(api.mock.calls.length).toBe(0); + }); + test('submitting an existing project returns error without network requests', async () => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123}, + projects: {items: [{id: 999}]} + }); + await expect(store.dispatch(addProject('localhost:800/projects/999'))) + .rejects.toBe(Errors.DUPLICATE); + expect(api.mock.calls.length).toBe(0); + }); + test('rate limit server response', async () => { + api.mockImplementationOnce((opts, callback) => { + callback(null, null, {statusCode: 429}); + }); + await expect(store.dispatch(addProject('localhost:800/projects/999'))) + .rejects.toBe(Errors.RATE_LIMIT); + }); + test('unknown project server response', async () => { + + api.mockImplementationOnce((opts, callback) => { + callback(null, null, {statusCode: 404}); + }); + await expect(store.dispatch(addProject('localhost:800/projects/999'))) + .rejects.toBe(Errors.UNKNOWN_PROJECT); + }); + test('not allowed server response', async () => { + api.mockImplementationOnce((opts, callback) => { + callback(null, null, {statusCode: 403}); + }); + await expect(store.dispatch(addProject('localhost:800/projects/999'))) + .rejects.toBe(Errors.PERMISSION); + }); + test('muted server response', async () => { + api.mockImplementationOnce((opts, callback) => { + callback(null, {mute_status: {}}, {statusCode: 403}); + }); + await expect(store.dispatch(addProject('localhost:800/projects/999'))) + .rejects.toBe(Errors.USER_MUTED); + }); +}); + +describe('removeProject', () => { + beforeEach(() => { + store = configureStore(reducers, { + ...initialState, + studio: {id: 123123}, + projects: {items: [{id: 999}]} + }); + }); + test('makes a DELETE and removes the item from redux', async () => { + api.mockImplementationOnce((opts, callback) => { + expect(opts.uri).toBe('/studios/123123/project/999'); + expect(opts.method).toBe('DELETE'); + callback(null, {}, {statusCode: 200}); + }); + await store.dispatch(removeProject(999)); + const {items} = projects.selector(store.getState()); + expect(items.length).toBe(0); + }); + + test('errors are set on the projects state', async () => { + api.mockImplementationOnce((opts, callback) => { + callback(null, {}, {statusCode: 500}); + }); + await expect(store.dispatch(removeProject(999))) + .rejects.toBe(Errors.SERVER); + }); +}); From caeb6e476c665a224abcc4f42378d06951ecba7c Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Mon, 28 Jun 2021 09:54:53 -0400 Subject: [PATCH 6/8] Fixup incorrectly added code to test files --- test/unit/redux/studio-member-actions.test.js | 30 ------------------- .../unit/redux/studio-project-actions.test.js | 1 - 2 files changed, 31 deletions(-) diff --git a/test/unit/redux/studio-member-actions.test.js b/test/unit/redux/studio-member-actions.test.js index 7a9bece76..86062b93f 100644 --- a/test/unit/redux/studio-member-actions.test.js +++ b/test/unit/redux/studio-member-actions.test.js @@ -22,36 +22,6 @@ beforeEach(() => { api.mockClear(); }); - -const storage = { - max: undefined, - items: [] -}; - -Object.defineProperty(storage, 'max', {writable: false, value: 5000}); - -let currentStorage = 'undefined'; - -const storageUsed = () => { - if (currentStorage) { - return currentStorage; - } - currentStorage = 0; - for (const i = 0; i < storage.length(); i++) { - currentStorage += storage.items[i].weigth; - } - return currentStorage; -}; - -const add = (item) => { - if (storage.max - item.weight >= storageUsed()) { - storage.items.push(item); - currentStorage += iten.weight; - } -}; - -add({weight: 100}); - describe('loadManagers', () => { test('it populates the managers list', () => { store = configureStore(reducers, { diff --git a/test/unit/redux/studio-project-actions.test.js b/test/unit/redux/studio-project-actions.test.js index 30cb3eca9..9e27e1c42 100644 --- a/test/unit/redux/studio-project-actions.test.js +++ b/test/unit/redux/studio-project-actions.test.js @@ -1,4 +1,3 @@ -import {selectStudioManagerCount} from '../../../src/redux/studio'; import { Errors, loadProjects, From e66a31abd8c92a7f0fa85cffcafcafaecaaae37a Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Wed, 30 Jun 2021 11:15:11 -0400 Subject: [PATCH 7/8] test(studios): add missing test situations for studio-member-actions --- test/unit/redux/studio-member-actions.test.js | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/unit/redux/studio-member-actions.test.js b/test/unit/redux/studio-member-actions.test.js index 86062b93f..1738b1bc5 100644 --- a/test/unit/redux/studio-member-actions.test.js +++ b/test/unit/redux/studio-member-actions.test.js @@ -175,6 +175,25 @@ describe('removeManager', () => { expect(state.studio.manager).toBe(false); }); + test('removing a manager that hasnt been loaded yet still works', async () => { + // This covers an edge case the code allows where you can remove a manager + // even if that manager hasn't been loaded into the paginated managers state yet. + api.mockImplementation((opts, callback) => { + callback(null, {}, {statusCode: 200}); // Server says that manager was removed + }); + + await store.dispatch(removeManager('user4')); + const state = store.getState(); + + // Manager count should still be updated + expect(selectStudioManagerCount(state)).toBe(98); + // The removed manager isn't the current user, so manager permission should be unchanged + expect(state.studio.manager).toBe(false); + // No change to the manager items list + expect(managers.selector(state).items.length).toBe(1); + expect(managers.selector(state).items[0].username).toBe('user1'); + }); + test('on error, promise rejects without any changing count or list', async () => { api.mockImplementation((opts, callback) => { callback(null, {}, {statusCode: 403}); @@ -266,6 +285,13 @@ describe('inviteCurator', () => { await expect(store.dispatch(inviteCurator('user2'))) .rejects.toBe(Errors.DUPLICATE); }); + test('error because of rate limit', async () => { + api.mockImplementation((opts, callback) => { + callback(null, null, {statusCode: 429}); + }); + await expect(store.dispatch(inviteCurator('user2'))) + .rejects.toBe(Errors.RATE_LIMIT); + }); test('unhandled error response', async () => { api.mockImplementation((opts, callback) => { callback(null, {status: 'error', message: 'xyz'}, {statusCode: 200}); From 0ca3431ec342e9ad21d31af919f4d0028c8e1057 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Wed, 30 Jun 2021 11:26:54 -0400 Subject: [PATCH 8/8] test: fix the new test for studio removeManager edge case --- test/unit/redux/studio-member-actions.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/unit/redux/studio-member-actions.test.js b/test/unit/redux/studio-member-actions.test.js index 1738b1bc5..f242de702 100644 --- a/test/unit/redux/studio-member-actions.test.js +++ b/test/unit/redux/studio-member-actions.test.js @@ -186,12 +186,11 @@ describe('removeManager', () => { const state = store.getState(); // Manager count should still be updated - expect(selectStudioManagerCount(state)).toBe(98); + expect(selectStudioManagerCount(state)).toBe(2); // The removed manager isn't the current user, so manager permission should be unchanged - expect(state.studio.manager).toBe(false); + expect(state.studio.manager).toBe(true); // No change to the manager items list - expect(managers.selector(state).items.length).toBe(1); - expect(managers.selector(state).items[0].username).toBe('user1'); + expect(managers.selector(state).items.length).toBe(3); }); test('on error, promise rejects without any changing count or list', async () => {