From 7051c76b40509dde3375dcede8645f8d352aae99 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 8 Sep 2017 17:37:27 +0700 Subject: [PATCH 01/10] Init step for plugins and bootstrap, introspection service --- client/coral-admin/src/index.js | 28 ++++---- client/coral-embed-stream/src/index.js | 67 ++++++++++--------- client/coral-framework/services/bootstrap.js | 42 +++++++++--- client/coral-framework/services/client.js | 6 +- .../coral-framework/services/introspection.js | 31 +++++++++ client/coral-framework/services/plugins.js | 8 +++ 6 files changed, 122 insertions(+), 60 deletions(-) create mode 100644 client/coral-framework/services/introspection.js diff --git a/client/coral-admin/src/index.js b/client/coral-admin/src/index.js index 5df2ce391..736d3efc9 100644 --- a/client/coral-admin/src/index.js +++ b/client/coral-admin/src/index.js @@ -12,23 +12,23 @@ import {toast} from 'react-toastify'; import {createNotificationService} from './services/notification'; import {hideShortcutsNote} from './actions/moderation'; -function hidrateStore({store, storage}) { +smoothscroll.polyfill(); + +function init({store, storage}) { if (storage && storage.getItem('coral:shortcutsNote') === 'hide') { store.dispatch(hideShortcutsNote()); } } -smoothscroll.polyfill(); +async function main() { + const notification = createNotificationService(toast); + const context = await createContext({reducers, graphqlExtension, pluginsConfig, notification, init}); + render( + + + + , document.querySelector('#root') + ); +} -const notification = createNotificationService(toast); -const context = createContext({reducers, graphqlExtension, pluginsConfig, notification}); - -// hidrate Store with external data. -hidrateStore(context); - -render( - - - - , document.querySelector('#root') -); +main(); diff --git a/client/coral-embed-stream/src/index.js b/client/coral-embed-stream/src/index.js index 5d72ea3e7..c8d3101ab 100644 --- a/client/coral-embed-stream/src/index.js +++ b/client/coral-embed-stream/src/index.js @@ -10,11 +10,6 @@ import reducers from './reducers'; import TalkProvider from 'coral-framework/components/TalkProvider'; import pluginsConfig from 'pluginsConfig'; -const context = createContext({reducers, graphqlExtension, pluginsConfig}); - -// TODO: move init code into `bootstrap` service after auth has been refactored. -const {store, pym} = context; - function inIframe() { try { return window.self !== window.top; @@ -23,37 +18,45 @@ function inIframe() { } } -function init(config = {}) { - store.dispatch(addExternalConfig(config)); - store.dispatch(checkLogin()); -} +// TODO: move init code into `bootstrap` service after auth has been refactored. +function preInit({store, pym}) { -// Don't run this in the popup. -if (!window.opener) { - if (inIframe()) { + // TODO: This is popup specific code and needs to be refactored. + if (!inIframe()) { + store.dispatch(addExternalConfig({})); + store.dispatch(checkLogin()); + return; + } + + pym.onMessage('login', (token) => { + if (token) { + store.dispatch(handleAuthToken(token)); + } + store.dispatch(checkLogin()); + }); + + pym.onMessage('logout', () => { + store.dispatch(logout()); + }); + + return new Promise((resolve) => { pym.sendMessage('getConfig'); pym.onMessage('config', (config) => { - init(JSON.parse(config)); - }); - - pym.onMessage('login', (token) => { - if (token) { - store.dispatch(handleAuthToken(token)); - } + store.dispatch(addExternalConfig(config)); store.dispatch(checkLogin()); + resolve(); }); - - pym.onMessage('logout', () => { - store.dispatch(logout()); - }); - } else { - init(); - } + }); } -render( - - - - , document.querySelector('#talk-embed-stream-container') -); +async function main() { + const context = await createContext({reducers, graphqlExtension, pluginsConfig, preInit}); + render( + + + + , document.querySelector('#talk-embed-stream-container') + ); +} + +main(); diff --git a/client/coral-framework/services/bootstrap.js b/client/coral-framework/services/bootstrap.js index a3ae5b18e..5b627125e 100644 --- a/client/coral-framework/services/bootstrap.js +++ b/client/coral-framework/services/bootstrap.js @@ -14,6 +14,8 @@ import {createGraphQLRegistry} from './graphqlRegistry'; import globalFragments from 'coral-framework/graphql/fragments'; import {createStorage} from 'coral-framework/services/storage'; import {createHistory} from 'coral-framework/services/history'; +import {createIntrospection} from 'coral-framework/services/introspection'; +import introspectionData from 'coral-framework/graphql/introspection.json'; /** * getStaticConfiguration will return a singleton of the static configuration @@ -60,17 +62,27 @@ const getAuthToken = (store, storage) => { /** * createContext setups and returns Talk dependencies that should be * passed to `TalkProvider`. - * @param {Object} [config] configuration - * @param {Object} [config.reducers] extra reducers to add to redux - * @param {Array} [config.pluginsConfig] plugin configuration as returned by importing pluginsConfig - * @param {Object} [config.graphqlExtensions] additional extension to the graphql framework - * @param {Object} [config.notification] replace default notification service - * @return {Object} context + * @param {Object} [config] configuration + * @param {Object} [config.reducers] extra reducers to add to redux + * @param {Array} [config.pluginsConfig] plugin configuration as returned by importing pluginsConfig + * @param {Object} [config.graphqlExtensions] additional extension to the graphql framework + * @param {Object} [config.notification] replace default notification service + * @param {Function} [config.init] run initialization e.g. to hydrate redux store + * @param {Function} [config.preInit] same as init but run and resolve before init and plugins init + * @return {Object} context */ -export function createContext({reducers = {}, pluginsConfig = [], graphqlExtension = {}, notification} = {}) { +export async function createContext({ + reducers = {}, + pluginsConfig = [], + graphqlExtension = {}, + notification, + preInit, + init, +} = {}) { const eventEmitter = new EventEmitter({wildcard: true}); const storage = createStorage(); const history = createHistory(BASE_PATH); + const introspection = createIntrospection(introspectionData); let store = null; const token = () => { @@ -104,6 +116,7 @@ export function createContext({reducers = {}, pluginsConfig = [], graphqlExtensi uri: `${BASE_PATH}api/v1/graph/ql`, liveUri, token, + introspectionData, }); const plugins = createPluginsService(pluginsConfig); const graphqlRegistry = createGraphQLRegistry(plugins.getSlotFragments.bind(plugins)); @@ -123,6 +136,7 @@ export function createContext({reducers = {}, pluginsConfig = [], graphqlExtensi notification, storage, history, + introspection, }; // Load framework fragments. @@ -155,8 +169,14 @@ export function createContext({reducers = {}, pluginsConfig = [], graphqlExtensi createReduxEmitter(eventEmitter), ]); - return { - ...context, - store, - }; + context.store = store; + + // Run pre initialization. + if (preInit) { + await preInit(context); + } + + // Run initialization. + await Promise.all([init, plugins.executeInit(context)]); + return context; } diff --git a/client/coral-framework/services/client.js b/client/coral-framework/services/client.js index 5907488e3..52088ad79 100644 --- a/client/coral-framework/services/client.js +++ b/client/coral-framework/services/client.js @@ -1,7 +1,6 @@ import ApolloClient, {addTypename, IntrospectionFragmentMatcher, createNetworkInterface} from 'apollo-client'; import {SubscriptionClient, addGraphQLSubscriptions} from 'subscriptions-transport-ws'; import MessageTypes from 'subscriptions-transport-ws/dist/message-types'; -import introspectionQueryResultData from '../graphql/introspection.json'; // Redux middleware to report any errors to the console. export const apolloErrorReporter = () => (next) => (action) => { @@ -21,10 +20,11 @@ function resolveToken(token) { * @param {string|function} [options.token] auth token * @param {string} [options.uri] uri of the graphql server * @param {string} [options.liveUri] uri of the graphql subscription server + * @param {Object} [options.introspectionData] introspection query result data * @return {Object} apollo client */ export function createClient(options = {}) { - const {token, uri, liveUri} = options; + const {token, uri, liveUri, introspectionData} = options; const wsClient = new SubscriptionClient(liveUri, { reconnect: true, lazy: true, @@ -63,7 +63,7 @@ export function createClient(options = {}) { const client = new ApolloClient({ connectToDevTools: true, addTypename: true, - fragmentMatcher: new IntrospectionFragmentMatcher({introspectionQueryResultData}), + fragmentMatcher: new IntrospectionFragmentMatcher({introspectionQueryResultData: introspectionData}), queryTransformer: addTypename, dataIdFromObject: (result) => { if (result.id && result.__typename) { // eslint-disable-line no-underscore-dangle diff --git a/client/coral-framework/services/introspection.js b/client/coral-framework/services/introspection.js new file mode 100644 index 000000000..fd82d41d6 --- /dev/null +++ b/client/coral-framework/services/introspection.js @@ -0,0 +1,31 @@ +class Introspection { + _enums=null; + + constructor(data) { + this._enums = data.__schema.types + .filter((type) => type.kind === 'ENUM') + .reduce((obj, enumType) => { + obj[enumType.name] = enumType.enumValues.map((value) => value.name); + return obj; + }, {}); + } + + /** + * isValidEnumValue returns true when given enum and value exists. + * @param {string} name + * @param {string} value + * @return {boolean} + */ + isValidEnumValue(name, value) { + return this._enums[name] && this._enums[name].indexOf(value) >= 0; + } +} + +/** + * createIntrospection returns a introspection service + * @param {Object} data introspection query data + * @return {Object} introspection service + */ +export function createIntrospection(data) { + return new Introspection(data); +} diff --git a/client/coral-framework/services/plugins.js b/client/coral-framework/services/plugins.js index 301428c12..dbda672ba 100644 --- a/client/coral-framework/services/plugins.js +++ b/client/coral-framework/services/plugins.js @@ -172,6 +172,14 @@ class PluginsService { .map((o) => ({[camelize(o.name)] : o.module.reducer})) ); } + + async executeInit(context) { + const results = this.plugins + .map((o) => o.module.init) + .filter((fn) => fn) + .map((fn) => fn(context)); + await Promise.all(results); + } } /** From e5bd8e2a3ba377eba836cb9607bf05123e05d5d0 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 8 Sep 2017 17:47:02 +0700 Subject: [PATCH 02/10] No need to check for loading config anymore --- client/coral-embed-stream/src/containers/Embed.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/coral-embed-stream/src/containers/Embed.js b/client/coral-embed-stream/src/containers/Embed.js index cd374291d..132e7f5e7 100644 --- a/client/coral-embed-stream/src/containers/Embed.js +++ b/client/coral-embed-stream/src/containers/Embed.js @@ -215,6 +215,6 @@ const mapDispatchToProps = (dispatch) => export default compose( connect(mapStateToProps, mapDispatchToProps), - branch((props) => !props.auth.checkedInitialLogin && props.config, renderComponent(Spinner)), + branch((props) => !props.auth.checkedInitialLogin, renderComponent(Spinner)), withEmbedQuery, )(EmbedContainer); From c8d1da5715d64a328a261a45e8cf9bc9dc7b643a Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 8 Sep 2017 17:50:22 +0700 Subject: [PATCH 03/10] Remember sort plugin --- .eslintignore | 1 + .gitignore | 1 + .../talk-plugin-remember-sort/client/.babelrc | 14 ++++++++ .../client/.eslintrc.json | 23 +++++++++++++ .../talk-plugin-remember-sort/client/index.js | 34 +++++++++++++++++++ plugins/talk-plugin-remember-sort/index.js | 1 + 6 files changed, 74 insertions(+) create mode 100644 plugins/talk-plugin-remember-sort/client/.babelrc create mode 100644 plugins/talk-plugin-remember-sort/client/.eslintrc.json create mode 100644 plugins/talk-plugin-remember-sort/client/index.js create mode 100644 plugins/talk-plugin-remember-sort/index.js diff --git a/.eslintignore b/.eslintignore index 4805e34a4..c558f1256 100644 --- a/.eslintignore +++ b/.eslintignore @@ -22,5 +22,6 @@ plugins/* !plugins/talk-plugin-author-menu !plugins/talk-plugin-member-since !plugins/talk-plugin-ignore-user +!plugins/talk-plugin-remember-sort node_modules diff --git a/.gitignore b/.gitignore index 0a2ac00df..05e41eac3 100644 --- a/.gitignore +++ b/.gitignore @@ -38,5 +38,6 @@ plugins/* !plugins/talk-plugin-author-menu !plugins/talk-plugin-member-since !plugins/talk-plugin-ignore-user +!plugins/talk-plugin-remember-sort **/node_modules/* diff --git a/plugins/talk-plugin-remember-sort/client/.babelrc b/plugins/talk-plugin-remember-sort/client/.babelrc new file mode 100644 index 000000000..60be246eb --- /dev/null +++ b/plugins/talk-plugin-remember-sort/client/.babelrc @@ -0,0 +1,14 @@ +{ + "presets": [ + "es2015" + ], + "plugins": [ + "add-module-exports", + "transform-class-properties", + "transform-decorators-legacy", + "transform-object-assign", + "transform-object-rest-spread", + "transform-async-to-generator", + "transform-react-jsx" + ] +} \ No newline at end of file diff --git a/plugins/talk-plugin-remember-sort/client/.eslintrc.json b/plugins/talk-plugin-remember-sort/client/.eslintrc.json new file mode 100644 index 000000000..9fe56bd14 --- /dev/null +++ b/plugins/talk-plugin-remember-sort/client/.eslintrc.json @@ -0,0 +1,23 @@ +{ + "env": { + "browser": true, + "es6": true, + "mocha": true + }, + "parserOptions": { + "sourceType": "module", + "ecmaFeatures": { + "experimentalObjectRestSpread": true, + "jsx": true + } + }, + "parser": "babel-eslint", + "plugins": [ + "react" + ], + "rules": { + "react/jsx-uses-react": "error", + "react/jsx-uses-vars": "error", + "no-console": ["warn", { "allow": ["warn", "error"] }] + } +} diff --git a/plugins/talk-plugin-remember-sort/client/index.js b/plugins/talk-plugin-remember-sort/client/index.js new file mode 100644 index 000000000..515eda186 --- /dev/null +++ b/plugins/talk-plugin-remember-sort/client/index.js @@ -0,0 +1,34 @@ +import {setSort} from 'plugin-api/beta/client/actions/stream'; +import {sortOrderSelector, sortBySelector} from 'plugin-api/beta/client/selectors/stream'; + +const STORAGE_PATH = 'talkPluginRememberSort'; + +export default { + init: ({store, storage, introspection}) => { + + // TODO: workaround as this plugin is included in any target and + // embeds (e.g. admin), but should only be included inside the stream. + + // Detect if we are currently running inside the stream. + if (!store.getState().stream) { + return; + } + + const sort = JSON.parse(storage.getItem(STORAGE_PATH)); + if ( + sort && + introspection.isValidEnumValue('SORT_ORDER', sort.sortOrder) && + introspection.isValidEnumValue('SORT_COMMENTS_BY', sort.sortBy) + ) { + store.dispatch(setSort(sort)); + } + store.subscribe(() => { + const state = store.getState(); + const sortOrder = sortOrderSelector(state); + const sortBy = sortBySelector(state); + if (!sort || sort.sortOrder !== sortOrder || sort.sortBy !== sortBy) { + storage.setItem(STORAGE_PATH, JSON.stringify({sortOrder, sortBy})); + } + }); + } +}; diff --git a/plugins/talk-plugin-remember-sort/index.js b/plugins/talk-plugin-remember-sort/index.js new file mode 100644 index 000000000..f053ebf79 --- /dev/null +++ b/plugins/talk-plugin-remember-sort/index.js @@ -0,0 +1 @@ +module.exports = {}; From 244a4b108de743f9a6258204b7eb8e815ace12e4 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 8 Sep 2017 18:16:49 +0700 Subject: [PATCH 04/10] Correctly detect subsequent change --- plugins/talk-plugin-remember-sort/client/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/talk-plugin-remember-sort/client/index.js b/plugins/talk-plugin-remember-sort/client/index.js index 515eda186..4f1850b3f 100644 --- a/plugins/talk-plugin-remember-sort/client/index.js +++ b/plugins/talk-plugin-remember-sort/client/index.js @@ -14,7 +14,7 @@ export default { return; } - const sort = JSON.parse(storage.getItem(STORAGE_PATH)); + let sort = JSON.parse(storage.getItem(STORAGE_PATH)); if ( sort && introspection.isValidEnumValue('SORT_ORDER', sort.sortOrder) && @@ -26,8 +26,11 @@ export default { const state = store.getState(); const sortOrder = sortOrderSelector(state); const sortBy = sortBySelector(state); + + // Save sorting choice to storage if it has changed. if (!sort || sort.sortOrder !== sortOrder || sort.sortBy !== sortBy) { - storage.setItem(STORAGE_PATH, JSON.stringify({sortOrder, sortBy})); + sort = {sortOrder, sortBy}; + storage.setItem(STORAGE_PATH, JSON.stringify(sort)); } }); } From 9f33a77745e1acc0c895572d6ac02bce4a1ffc1d Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Sat, 9 Sep 2017 00:11:15 +0700 Subject: [PATCH 05/10] Implement and use pymStorage --- .../src/containers/Comment.js | 6 -- client/coral-embed/src/Stream.js | 4 + client/coral-embed/src/index.js | 2 +- client/coral-framework/services/bootstrap.js | 4 +- client/coral-framework/services/storage.js | 88 +++++++++++++++++++ .../talk-plugin-remember-sort/client/index.js | 9 +- 6 files changed, 102 insertions(+), 11 deletions(-) diff --git a/client/coral-embed-stream/src/containers/Comment.js b/client/coral-embed-stream/src/containers/Comment.js index 733708a97..bd3160643 100644 --- a/client/coral-embed-stream/src/containers/Comment.js +++ b/client/coral-embed-stream/src/containers/Comment.js @@ -92,12 +92,6 @@ const singleCommentFragment = gql` const withCommentFragments = withFragments({ root: gql` fragment CoralEmbedStream_Comment_root on RootQuery { - me { - ignoredUsers { - id - } - } - __typename me { ignoredUsers { id diff --git a/client/coral-embed/src/Stream.js b/client/coral-embed/src/Stream.js index b81964529..cb8bb5bca 100644 --- a/client/coral-embed/src/Stream.js +++ b/client/coral-embed/src/Stream.js @@ -3,6 +3,7 @@ import pym from 'pym.js'; import EventEmitter from 'eventemitter2'; import {buildUrl} from 'coral-framework/utils/url'; import Snackbar from './Snackbar'; +import {createStorage, connectStorageToPym} from 'coral-framework/services/storage'; const NOTIFICATION_OFFSET = 200; @@ -134,6 +135,9 @@ export default class Stream { // If the user clicks outside the embed, then tell the embed. document.addEventListener('click', this.handleClick.bind(this), true); + + // Listens to storage requests on pym and relay it to local storage. + connectStorageToPym(createStorage(), this.pym); } login(token) { diff --git a/client/coral-embed/src/index.js b/client/coral-embed/src/index.js index 2b17ed465..34bee1797 100644 --- a/client/coral-embed/src/index.js +++ b/client/coral-embed/src/index.js @@ -83,7 +83,7 @@ Talk.render = (el, opts) => { console.warn( 'This page does not include a canonical link tag. Talk has inferred this asset_url from the window object. Query params have been stripped, which may cause a single thread to be present across multiple pages.' ); - + if (!window.location.origin) { window.location.origin = `${window.location.protocol}//${window.location.hostname}${window.location.port ? `:${window.location.port}` : ''}`; } diff --git a/client/coral-framework/services/bootstrap.js b/client/coral-framework/services/bootstrap.js index 5b627125e..492f6cc40 100644 --- a/client/coral-framework/services/bootstrap.js +++ b/client/coral-framework/services/bootstrap.js @@ -12,7 +12,7 @@ import {createPluginsService} from './plugins'; import {createNotificationService} from './notification'; import {createGraphQLRegistry} from './graphqlRegistry'; import globalFragments from 'coral-framework/graphql/fragments'; -import {createStorage} from 'coral-framework/services/storage'; +import {createStorage, createPymStorage} from 'coral-framework/services/storage'; import {createHistory} from 'coral-framework/services/history'; import {createIntrospection} from 'coral-framework/services/introspection'; import introspectionData from 'coral-framework/graphql/introspection.json'; @@ -81,6 +81,7 @@ export async function createContext({ } = {}) { const eventEmitter = new EventEmitter({wildcard: true}); const storage = createStorage(); + const pymStorage = createPymStorage(pym); const history = createHistory(BASE_PATH); const introspection = createIntrospection(introspectionData); let store = null; @@ -137,6 +138,7 @@ export async function createContext({ storage, history, introspection, + pymStorage, }; // Load framework fragments. diff --git a/client/coral-framework/services/storage.js b/client/coral-framework/services/storage.js index 9f06595fa..91ff10680 100644 --- a/client/coral-framework/services/storage.js +++ b/client/coral-framework/services/storage.js @@ -1,3 +1,4 @@ +import uuid from 'uuid/v4'; function getStorage(type) { let storage = window[type], x = '__storage_test__'; @@ -41,3 +42,90 @@ function getStorage(type) { export function createStorage() { return getStorage('localStorage'); } + +/** + * Creates a storage that relay requests over to pym. + * This is the counterpart of `connectStorageToPym`. + * @param {string} pym pym + * @return {Object} storage + */ +export function createPymStorage(pym) { + + // A Map of requestID => {resolve, reject} + const requests = {}; + + // Requests method with parameters over pym. + const call = (method, parameters) => { + const id = uuid(); + return new Promise((resolve, reject) => { + requests[id] = {resolve, reject}; + pym.sendMessage('pymStorage.request', JSON.stringify({id, method, parameters})); + }); + }; + + // Receive successful responses. + pym.onMessage('pymStorage.response', (msg) => { + const {id, result} = JSON.parse(msg); + requests[id].resolve(result); + delete requests[id]; + }); + + // Receive error responses. + pym.onMessage('pymStorage.error', (msg) => { + const {id, error} = JSON.parse(msg); + requests[id].reject(error); + delete requests[id]; + }); + + return { + setItem: (key, value) => call('setItem', {key, value}), + getItem: (key, value) => call('getItem', {key, value}), + removeItem: (key) => call('removeItem', {key}), + }; +} + +/** + * Listens to `pym` and relay storage requests to `storage`. + * This is the counterpart of `createPymStorage`. + * @param {string} pym pym + * @return {Object} storage + */ +export function connectStorageToPym(storage, pym) { + pym.onMessage('pymStorage.request', (msg) => { + const {id, method, parameters} = JSON.parse(msg); + const {key, value} = parameters; + const prefixedKey = `talkPymStorage:${key}`; + + // Variable for the method return value. + let result; + + const sendError = (error) => { + console.error(error); + pym.sendMessage('pymStorage.error', JSON.stringify({id, error})); + }; + + try { + switch(method) { + case 'setItem': + result = storage.setItem(prefixedKey, value); + break; + case 'getItem': + result = storage.getItem(prefixedKey); + break; + case 'removeItem': + result = storage.removeItem(prefixedKey); + break; + default: + sendError(`Unknown method ${method}`); + return; + } + } + catch(err) { + sendError(err.toString()); + return; + } + + pym.sendMessage('pymStorage.response', JSON.stringify({id, result})); + }); +} + diff --git a/plugins/talk-plugin-remember-sort/client/index.js b/plugins/talk-plugin-remember-sort/client/index.js index 4f1850b3f..061f71809 100644 --- a/plugins/talk-plugin-remember-sort/client/index.js +++ b/plugins/talk-plugin-remember-sort/client/index.js @@ -4,7 +4,7 @@ import {sortOrderSelector, sortBySelector} from 'plugin-api/beta/client/selector const STORAGE_PATH = 'talkPluginRememberSort'; export default { - init: ({store, storage, introspection}) => { + init: async ({store, pymStorage, introspection}) => { // TODO: workaround as this plugin is included in any target and // embeds (e.g. admin), but should only be included inside the stream. @@ -14,7 +14,10 @@ export default { return; } - let sort = JSON.parse(storage.getItem(STORAGE_PATH)); + // We use pymStorage instead to persist the data directly on the parent page, + // in order to mitigate strict cross domain security settings. + + let sort = JSON.parse(await pymStorage.getItem(STORAGE_PATH)); if ( sort && introspection.isValidEnumValue('SORT_ORDER', sort.sortOrder) && @@ -30,7 +33,7 @@ export default { // Save sorting choice to storage if it has changed. if (!sort || sort.sortOrder !== sortOrder || sort.sortBy !== sortBy) { sort = {sortOrder, sortBy}; - storage.setItem(STORAGE_PATH, JSON.stringify(sort)); + pymStorage.setItem(STORAGE_PATH, JSON.stringify(sort)); } }); } From 6b74877d016d08b5b4e7e9dff8c6057d0a61e5fc Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Sat, 9 Sep 2017 00:15:19 +0700 Subject: [PATCH 06/10] Implement and use pymStorage --- client/coral-framework/services/storage.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/client/coral-framework/services/storage.js b/client/coral-framework/services/storage.js index 91ff10680..11aa5df98 100644 --- a/client/coral-framework/services/storage.js +++ b/client/coral-framework/services/storage.js @@ -87,14 +87,15 @@ export function createPymStorage(pym) { /** * Listens to `pym` and relay storage requests to `storage`. * This is the counterpart of `createPymStorage`. - * @param {string} pym pym - * @return {Object} storage + * @param {Object} storage storage to perform requests on + * @param {Object} pym pym to listen to storage requests + * @param {string} prefix namespace requests by prepending a prefix to the keys */ -export function connectStorageToPym(storage, pym) { +export function connectStorageToPym(storage, pym, prefix = 'talkPymStorage:') { pym.onMessage('pymStorage.request', (msg) => { const {id, method, parameters} = JSON.parse(msg); const {key, value} = parameters; - const prefixedKey = `talkPymStorage:${key}`; + const prefixedKey = `${prefix}${key}`; // Variable for the method return value. let result; From ce3e29813bd8566250dc95ed532bd6e4370c0880 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 8 Sep 2017 15:41:43 -0600 Subject: [PATCH 07/10] implemented status switch when mismatched --- graph/mutators/comment.js | 30 +++-- services/assets.js | 25 +++-- services/comments.js | 103 +++++++++++------ test/server/services/comments.js | 187 +++++++++++++++++-------------- 4 files changed, 208 insertions(+), 137 deletions(-) diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index 9142687f6..a75e319f4 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -194,16 +194,32 @@ const createComment = async (context, {tags = [], body, asset_id, parent_id = nu * @param {String} [asset_id] id of asset comment is posted on * @return {Object} resolves to the wordlist results */ -const filterNewComment = (context, {body, asset_id}) => { +const filterNewComment = async (context, {body, asset_id}) => { + + // Load the settings. + const [ + settings, + asset, + ] = await Promise.all([ + context.loaders.Settings.load(), + context.loaders.Assets.getByID.load(asset_id), + ]); // Create a new instance of the Wordlist. const wl = new Wordlist(); + // Load the wordlist. + wl.upsert(settings.wordlist); + // Load the wordlist and filter the comment content. - return Promise.all([ - wl.load().then(() => wl.scan('body', body)), - asset_id && AssetsService.rectifySettings(AssetsService.findById(asset_id)) - ]); + return [ + + // Scan the word. + wl.scan('body', body), + + // Return the asset's settings. + AssetsService.rectifySettings(asset, settings) + ]; }; /** @@ -247,7 +263,7 @@ const resolveNewCommentStatus = async (context, {asset_id, body, status}, wordli // Return `premod` if pre-moderation is enabled and an empty "new" status // in the event that it is not in pre-moderation mode. - let {moderation, charCountEnable, charCount} = await AssetsService.rectifySettings(asset); + let {moderation, charCountEnable, charCount} = await AssetsService.rectifySettings(asset, settings); // Reject if the comment is too long if (charCountEnable && body.length > charCount) { @@ -365,7 +381,7 @@ const edit = async (context, {id, asset_id, edit: {body}}) => { const status = await resolveNewCommentStatus(context, {asset_id, body}, wordlist, settings); // Execute the edit. - const comment = await CommentsService.edit(id, context.user.id, {body, status}); + const comment = await CommentsService.edit({id, author_id: context.user.id, body, status}); // Publish the edited comment via the subscription. context.pubsub.publish('commentEdited', comment); diff --git a/services/assets.js b/services/assets.js index 34c8ff465..cddfcfb7c 100644 --- a/services/assets.js +++ b/services/assets.js @@ -3,6 +3,7 @@ const AssetModel = require('../models/asset'); const SettingsService = require('./settings'); const domainlist = require('./domainlist'); const errors = require('../errors'); +const merge = require('lodash/merge'); module.exports = class AssetsService { @@ -28,19 +29,21 @@ module.exports = class AssetsService { * @param {Promise} assetQuery an asset query that returns a single asset. * @return {Promise} */ - static rectifySettings(assetQuery) { - return Promise.all([ - SettingsService.retrieve(), - assetQuery - ]).then(([settings, asset]) => { + static async rectifySettings(assetQuery, settings = null) { + const [ + globalSettings, + asset, + ] = await Promise.all([ + settings || SettingsService.retrieve(), + assetQuery, + ]); - // If the asset exists and has settings then return the merged object. - if (asset && asset.settings) { - settings.merge(asset.settings); - } + // If the asset exists and has settings then return the merged object. + if (asset && asset.settings) { + settings = merge({}, globalSettings, asset.settings); + } - return settings; - }); + return settings; } /** diff --git a/services/comments.js b/services/comments.js index db0714233..1c39fdafb 100644 --- a/services/comments.js +++ b/services/comments.js @@ -5,6 +5,7 @@ const ActionsService = require('./actions'); const SettingsService = require('./settings'); const sc = require('snake-case'); +const cloneDeep = require('lodash/cloneDeep'); const errors = require('../errors'); const events = require('./events'); const { @@ -52,13 +53,34 @@ module.exports = class CommentsService { } /** - * Edit a Comment - * @param {String} id comment.id you want to edit (or its ID) - * @param {String} author_id user.id of the user trying to edit the comment (will err if not comment author) + * lastUnmoderatedStatus will retrieve the last status before this one. + * + * @param {Object} comment the comment to get the last status of + */ + static lastUnmoderatedStatus(comment) { + const UNMODERATED_STATUSES = [ + 'NONE', + 'PREMOD', + ]; + + for (let i = comment.status_history.length - 1; i >= 0; i--) { + const {type} = comment.status_history[i]; + + if (UNMODERATED_STATUSES.includes(type)) { + return type; + } + } + } + + /** + * Edit a Comment. + * + * @param {String} id comment.id you want to edit (or its ID) + * @param {String} author_id user.id of the user trying to edit the comment (will err if not comment author) * @param {String} body the new Comment body * @param {String} status the new Comment status */ - static async edit(id, author_id, {body, status, ignoreEditWindow = false}) { + static async edit({id, author_id, body, status}) { const query = { id, author_id, @@ -69,14 +91,11 @@ module.exports = class CommentsService { // Establish the edit window (if it exists) and add the condition to the // original query. - let lastEditableCommentCreatedAt; - if (!ignoreEditWindow) { - const {editCommentWindowLength: editWindowMs} = await SettingsService.retrieve(); - lastEditableCommentCreatedAt = new Date((new Date()).getTime() - editWindowMs); - query.created_at = { - $gt: lastEditableCommentCreatedAt, - }; - } + const {editCommentWindowLength: editWindowMs} = await SettingsService.retrieve(); + const lastEditableCommentCreatedAt = new Date(Date.now() - editWindowMs); + query.created_at = { + $gt: lastEditableCommentCreatedAt, + }; const originalComment = await CommentModel.findOneAndUpdate(query, { $set: { @@ -117,7 +136,7 @@ module.exports = class CommentsService { } // Check to see if the edit window expired. - if (!ignoreEditWindow && comment.created_at <= lastEditableCommentCreatedAt) { + if (comment.created_at <= lastEditableCommentCreatedAt) { debug('rejecting comment edit because outside edit time window'); throw errors.ErrEditWindowHasEnded; } @@ -126,7 +145,7 @@ module.exports = class CommentsService { } // Mutate the comment like Mongo would have. - const editedComment = originalComment; + const editedComment = cloneDeep(originalComment); editedComment.status = status; editedComment.body = body; editedComment.body_history.push({ @@ -138,6 +157,43 @@ module.exports = class CommentsService { created_at: new Date(), }); + // We should adjust the comment's status such that if it was approved + // previously, we should mark the comment as 'NONE' or 'PREMOD', which ever + // was most recent if the new comment is destined to be `NONE` or `PREMOD`. + if (originalComment.status === 'ACCEPTED' && ['NONE', 'PREMOD'].includes(status)) { + + const lastUnmoderatedStatus = CommentsService.lastUnmoderatedStatus(originalComment); + + // If the last moderated status was found and the current comment doesn't + // match this already. + if (lastUnmoderatedStatus && status !== lastUnmoderatedStatus) { + + // Update the comment model (if at this point, the status is still + // accepted) with the previously unmoderated status + await CommentModel.update({ + id, + status, + }, { + $set: { + status: lastUnmoderatedStatus, + }, + $push: { + status_history: { + type: lastUnmoderatedStatus, + created_at: new Date(), + } + }, + }); + + // Update the returned comment. + editedComment.status = lastUnmoderatedStatus; + editedComment.status_history.push({ + type: lastUnmoderatedStatus, + created_at: new Date(), + }); + } + } + await events.emitAsync(COMMENTS_EDIT, originalComment, editedComment); return editedComment; @@ -215,23 +271,6 @@ module.exports = class CommentsService { return CommentModel.find({status}); } - /** - * Find comments that need to be moderated (aka moderation queue). - * @param {String} asset_id - * @return {Promise} - */ - static moderationQueue(status = 'NONE', asset_id = null) { - - // Fetch the comments with statuses. - let comments = CommentModel.find({status}); - - if (asset_id) { - comments = comments.where({asset_id}); - } - - return comments; - } - /** * Pushes a new status in for the user. * @param {String} id identifier of the comment (uuid) @@ -356,7 +395,7 @@ events.on(COMMENTS_NEW, async (comment) => { if ( !comment || // Check that the comment is defined. (!comment.parent_id || comment.parent_id.length === 0) || // Check that the comment has a parent (is a reply). - !(comment.status === 'NONE' || comment.status === 'APPROVED') // Check that the comment is visible. + !(comment.status === 'NONE' || comment.status === 'ACCEPTED') // Check that the comment is visible. ) { return; } diff --git a/test/server/services/comments.js b/test/server/services/comments.js index e33089325..2be05f87c 100644 --- a/test/server/services/comments.js +++ b/test/server/services/comments.js @@ -10,7 +10,6 @@ const CommentsService = require('../../../services/comments'); const settings = {id: '1', moderation: 'PRE', wordlist: {banned: ['bad words'], suspect: ['suspect words']}}; const chai = require('chai'); -chai.use(require('chai-as-promised')); chai.use(require('sinon-chai')); const expect = chai.expect; @@ -96,102 +95,118 @@ describe('services.CommentsService', () => { user_id: '456' }]; - beforeEach(() => { - return SettingsService.init(settings).then(() => { - return Promise.all([ - CommentModel.create(comments), - UsersService.createLocalUsers(users), - ActionModel.create(actions) - ]); - }); + beforeEach(async () => { + await SettingsService.init(settings); + + await Promise.all([ + CommentModel.create(comments), + UsersService.createLocalUsers(users), + ActionModel.create(actions) + ]); }); describe('#publicCreate()', () => { - it('creates a new comment', () => { - return CommentsService - .publicCreate({ - body: 'This is a comment!', - status: 'ACCEPTED' - }).then((c) => { - expect(c).to.not.be.null; - expect(c.id).to.not.be.null; - expect(c.id).to.be.uuid; - expect(c.status).to.be.equal('ACCEPTED'); - }); + it('creates a new comment', async () => { + const c = await CommentsService.publicCreate({ + body: 'This is a comment!', + status: 'ACCEPTED' + }); + + expect(c).to.not.be.null; + expect(c.id).to.not.be.null; + expect(c.id).to.be.uuid; + expect(c.status).to.be.equal('ACCEPTED'); }); - it('creates many new comments', () => { - return CommentsService - .publicCreate([{ - body: 'This is a comment!', - status: 'ACCEPTED' - }, { - body: 'This is another comment!' - }, { - body: 'This is a rejected comment!', - status: 'REJECTED' - }]).then(([c1, c2, c3]) => { - expect(c1).to.not.be.null; - expect(c1.id).to.be.uuid; - expect(c1.status).to.be.equal('ACCEPTED'); + it('creates many new comments', async () => { + const [ + c1, + c2, + c3, + ] = await CommentsService.publicCreate([{ + body: 'This is a comment!', + status: 'ACCEPTED' + }, { + body: 'This is another comment!' + }, { + body: 'This is a rejected comment!', + status: 'REJECTED' + }]); - expect(c2).to.not.be.null; - expect(c2.id).to.be.uuid; - expect(c2.status).to.be.equal('NONE'); + expect(c1).to.not.be.null; + expect(c1.status).to.be.equal('ACCEPTED'); - expect(c3).to.not.be.null; - expect(c3.id).to.be.uuid; - expect(c3.status).to.be.equal('REJECTED'); - }); + expect(c2).to.not.be.null; + expect(c2.status).to.be.equal('NONE'); + + expect(c3).to.not.be.null; + expect(c3.status).to.be.equal('REJECTED'); }); + }); + describe.only('#edit', () => { + it('changes the comment status back to premod if it was accepted', async () => { + const originalComment = await CommentsService.publicCreate({ + body: 'this is a body!', + status: 'PREMOD', + author_id: '123', + }); + + expect(originalComment.status_history).to.have.length(1); + + await CommentsService.pushStatus(originalComment.id, 'ACCEPTED'); + + let retrivedComment = await CommentsService.findById(originalComment.id); + + expect(retrivedComment).to.have.property('status', 'ACCEPTED'); + expect(retrivedComment.status_history).to.have.length(2); + expect(retrivedComment.status_history[1]).to.have.property('type', 'ACCEPTED'); + + const editedComment = await CommentsService.edit({ + id: originalComment.id, + author_id: '123', + body: 'This is a body!', + status: 'NONE', + }); + + expect(editedComment).to.have.property('status', 'PREMOD'); + expect(editedComment.status_history).to.have.length(4); + expect(editedComment.status_history[3]).to.have.property('type', 'PREMOD'); + + retrivedComment = await CommentsService.findById(originalComment.id); + + expect(retrivedComment).to.have.property('status', 'PREMOD'); + expect(retrivedComment.status_history).to.have.length(4); + expect(retrivedComment.status_history[3]).to.have.property('type', 'PREMOD'); + }); }); describe('#findById()', () => { - it('should find a comment by id', () => { - return CommentsService - .findById('1') - .then((result) => { - expect(result).to.not.be.null; - expect(result).to.have.property('body', 'comment 10'); - }); + it('should find a comment by id', async () => { + const comment = await CommentsService.findById('1'); + expect(comment).to.not.be.null; + expect(comment).to.have.property('body', 'comment 10'); }); }); describe('#findByAssetId()', () => { - it('should find an array of all comments by asset id', () => { - return CommentsService - .findByAssetId('123') - .then((result) => { - expect(result).to.have.length(3); - result.sort((a, b) => { - if (a.body < b.body) {return -1;} - else {return 1;} - }); - expect(result[0]).to.have.property('body', 'comment 10'); - expect(result[1]).to.have.property('body', 'comment 20'); - expect(result[2]).to.have.property('body', 'comment 40'); - }); + it('should find an array of all comments by asset id', async () => { + const comments = await CommentsService.findByAssetId('123'); + expect(comments).to.have.length(3); + comments.sort((a, b) => { + if (a.body < b.body) {return -1;} + else {return 1;} + }); + expect(comments[0]).to.have.property('body', 'comment 10'); + expect(comments[1]).to.have.property('body', 'comment 20'); + expect(comments[2]).to.have.property('body', 'comment 40'); }); }); - describe('#moderationQueue()', () => { - - it('should find an array of new comments to moderate when pre-moderation', () => { - return CommentsService - .moderationQueue('PREMOD') - .then((result) => { - expect(result).to.not.be.null; - expect(result).to.have.lengthOf(2); - }); - }); - - }); - describe('#changeStatus', () => { it('should change the status of a comment from no status', async () => { @@ -220,20 +235,18 @@ describe('services.CommentsService', () => { expect(c3.status_history[0]).to.have.property('assigned_by', '123'); }); - it('should change the status of a comment from accepted', () => { - return CommentsService.pushStatus(comments[1].id, 'REJECTED', '123') - .then(() => CommentsService.findById(comments[1].id)) - .then((c) => { - expect(c).to.have.property('status_history'); - expect(c).to.have.property('status'); - expect(c.status).to.equal('REJECTED'); - expect(c.status_history).to.have.length(2); - expect(c.status_history[0]).to.have.property('type', 'ACCEPTED'); - expect(c.status_history[0]).to.have.property('assigned_by', null); + it('should change the status of a comment from accepted', async () => { + await CommentsService.pushStatus(comments[1].id, 'REJECTED', '123'); + const c = await CommentsService.findById(comments[1].id); + expect(c).to.have.property('status_history'); + expect(c).to.have.property('status'); + expect(c.status).to.equal('REJECTED'); + expect(c.status_history).to.have.length(2); + expect(c.status_history[0]).to.have.property('type', 'ACCEPTED'); + expect(c.status_history[0]).to.have.property('assigned_by', null); - expect(c.status_history[1]).to.have.property('type', 'REJECTED'); - expect(c.status_history[1]).to.have.property('assigned_by', '123'); - }); + expect(c.status_history[1]).to.have.property('type', 'REJECTED'); + expect(c.status_history[1]).to.have.property('assigned_by', '123'); }); }); }); From d20bd20005dface4b3d58126edf90ea9774011d4 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 8 Sep 2017 15:46:04 -0600 Subject: [PATCH 08/10] linting --- test/server/services/comments.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/server/services/comments.js b/test/server/services/comments.js index 2be05f87c..77711d321 100644 --- a/test/server/services/comments.js +++ b/test/server/services/comments.js @@ -145,7 +145,7 @@ describe('services.CommentsService', () => { }); }); - describe.only('#edit', () => { + describe('#edit', () => { it('changes the comment status back to premod if it was accepted', async () => { const originalComment = await CommentsService.publicCreate({ body: 'this is a body!', From 9af21b4226779b8a346764ae6521ce74f0e8bcf9 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 8 Sep 2017 16:23:31 -0600 Subject: [PATCH 09/10] test fixes --- graph/mutators/comment.js | 2 +- services/assets.js | 4 +++- services/comments.js | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index a75e319f4..c6e335018 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -218,7 +218,7 @@ const filterNewComment = async (context, {body, asset_id}) => { wl.scan('body', body), // Return the asset's settings. - AssetsService.rectifySettings(asset, settings) + await AssetsService.rectifySettings(asset, settings) ]; }; diff --git a/services/assets.js b/services/assets.js index cddfcfb7c..8f93de009 100644 --- a/services/assets.js +++ b/services/assets.js @@ -34,13 +34,15 @@ module.exports = class AssetsService { globalSettings, asset, ] = await Promise.all([ - settings || SettingsService.retrieve(), + settings !== null ? settings : SettingsService.retrieve(), assetQuery, ]); // If the asset exists and has settings then return the merged object. if (asset && asset.settings) { settings = merge({}, globalSettings, asset.settings); + } else { + settings = globalSettings; } return settings; diff --git a/services/comments.js b/services/comments.js index 1c39fdafb..82914bcd0 100644 --- a/services/comments.js +++ b/services/comments.js @@ -160,7 +160,7 @@ module.exports = class CommentsService { // We should adjust the comment's status such that if it was approved // previously, we should mark the comment as 'NONE' or 'PREMOD', which ever // was most recent if the new comment is destined to be `NONE` or `PREMOD`. - if (originalComment.status === 'ACCEPTED' && ['NONE', 'PREMOD'].includes(status)) { + if (originalComment.status === 'ACCEPTED' && status === 'NONE') { const lastUnmoderatedStatus = CommentsService.lastUnmoderatedStatus(originalComment); From ee41cc690cbc9874d980bbc89554691a7a4fed8c Mon Sep 17 00:00:00 2001 From: Boaz Sender Date: Sat, 9 Sep 2017 18:05:08 -0400 Subject: [PATCH 10/10] add link to new location for config vars, and include min needed docker env vars in example --- docs/_docs/01-02-install-docker.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/_docs/01-02-install-docker.md b/docs/_docs/01-02-install-docker.md index 9d177049e..7fd6f6ddf 100644 --- a/docs/_docs/01-02-install-docker.md +++ b/docs/_docs/01-02-install-docker.md @@ -53,6 +53,10 @@ services: environment: - TALK_MONGO_URL=mongodb://mongo/talk - TALK_REDIS_URL=redis://redis + - TALK_ROOT_URL=http://localhost:5000 + - TALK_JWT_SECRET=password + - TALK_FACEBOOK_APP_ID=12345 + - TALK_FACEBOOK_APP_SECRET=123abc mongo: image: mongo:3.2 restart: always @@ -70,10 +74,10 @@ volumes: external: false ``` -At this stage, you should refer to the `README.md` for configuration variables -that are specific to your installation. Some pre-defined fields have been filled -in the above example which are consistent with Docker Compose naming conventions -for [Docker Links](https://docs.docker.com/compose/networking/#links). +At this stage, you should refer to the [configuration](https://coralproject.github.io/talk/docs/running/configuration/) +for configuration variables that are specific to your installation. Some +pre-defined fields have been filled in the above example which are consistent +with Docker Compose naming conventions for [Docker Links](https://docs.docker.com/compose/networking/#links). ### Scaling