Merge branch 'master' into keyboard-shortcut-scroll

Dockerfile

@@ -1,4 +1,4 @@
-FROM node:7.6
+FROM node:7
 
 # Create app directory
 RUN mkdir -p /usr/src/app

client/coral-admin/src/actions/moderation.js

@@ -4,5 +4,5 @@ export const toggleModal = open => ({type: actions.TOGGLE_MODAL, open});
 export const singleView = () => ({type: actions.SINGLE_VIEW});
 
 // Ban User Dialog
-export const showBanUserDialog = (user, commentId) => ({type: actions.SHOW_BANUSER_DIALOG, user, commentId});
+export const showBanUserDialog = (user, commentId, showRejectedNote) => ({type: actions.SHOW_BANUSER_DIALOG, user, commentId, showRejectedNote});
 export const hideBanUserDialog = (showDialog) => ({type: actions.HIDE_BANUSER_DIALOG, showDialog});

client/coral-admin/src/components/BanUserDialog.js

@@ -15,7 +15,7 @@ const onBanClick = (userId, commentId, handleBanUser, rejectComment, handleClose
   .then(() => rejectComment({commentId}));
 };
 
-const BanUserDialog = ({open, handleClose, handleBanUser, rejectComment, user, commentId}) => (
+const BanUserDialog = ({open, handleClose, handleBanUser, rejectComment, user, commentId, showRejectedNote}) => (
   <Dialog
     className={styles.dialog}
     id="banuserDialog"
@@ -30,7 +30,7 @@ const BanUserDialog = ({open, handleClose, handleBanUser, rejectComment, user, c
       </div>
       <div className={styles.separator}>
         <h3>{lang.t('bandialog.are_you_sure', user.name)}</h3>
-        <i>{lang.t('bandialog.note')}</i>
+        <i>{showRejectedNote && lang.t('bandialog.note')}</i>
       </div>
       <div className={styles.buttons}>
         <Button cStyle="cancel" className={styles.cancel} onClick={handleClose} raised>
@@ -47,6 +47,8 @@ const BanUserDialog = ({open, handleClose, handleBanUser, rejectComment, user, c
 BanUserDialog.propTypes = {
   handleBanUser: PropTypes.func.isRequired,
   handleClose: PropTypes.func.isRequired,
+  rejectComment: PropTypes.func.isRequired,
+  commentId: PropTypes.string,
   user: PropTypes.object.isRequired,
 };
 

client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js

@@ -178,6 +178,7 @@ class ModerationContainer extends Component {
           commentId={moderation.commentId}
           handleClose={props.hideBanUserDialog}
           handleBanUser={props.banUser}
+          showRejectedNote={moderation.showRejectedNote}
           rejectComment={props.rejectComment}
         />
       <ModerationKeysModal
@@ -200,7 +201,7 @@ const mapDispatchToProps = dispatch => ({
   singleView: () => dispatch(singleView()),
   updateAssets: assets => dispatch(updateAssets(assets)),
   fetchSettings: () => dispatch(fetchSettings()),
-  showBanUserDialog: (user, commentId) => dispatch(showBanUserDialog(user, commentId)),
+  showBanUserDialog: (user, commentId, showRejectedNote) => dispatch(showBanUserDialog(user, commentId, showRejectedNote)),
   hideBanUserDialog: () => dispatch(hideBanUserDialog(false)),
 });
 

client/coral-admin/src/containers/ModerationQueue/components/Comment.js

@@ -31,7 +31,7 @@ const Comment = ({actions = [], ...props}) => {
           <span className={styles.created}>
             {timeago().format(props.comment.created_at || (Date.now() - props.index * 60 * 1000), lang.getLocale().replace('-', '_'))}
           </span>
-          <BanUserButton user={props.comment.user} onClick={() => props.showBanUserDialog(props.comment.user, props.comment.id)} />
+          <BanUserButton user={props.comment.user} onClick={() => props.showBanUserDialog(props.comment.user, props.comment.id, props.comment.status !== 'REJECTED')} />
           <CommentType type={props.commentType} />
         </div>
           {props.comment.user.status === 'banned' ?

client/coral-admin/src/reducers/moderation.js

@@ -19,6 +19,7 @@ export default function moderation (state = initialState, action) {
       .merge({
         user: Map(action.user),
         commentId: action.commentId,
+        showRejectedNote: action.showRejectedNote,
         banDialog: true
       });
   case actions.SET_ACTIVE_TAB:

client/coral-embed/src/index.js

@@ -13,7 +13,7 @@ const snackbarStyles = {
   color: '#fff',
   borderRadius: '3px 3px 0 0',
   textAlign: 'center',
-  maxWidth: '300px',
+  maxWidth: '400px',
   left: '50%',
   opacity: 0,
   transform: 'translate(-50%, 20px)',

graph/loaders/comments.js

@@ -1,4 +1,8 @@
-const util = require('./util');
+const {
+  SharedCounterDataLoader,
+  singleJoinBy,
+  arrayJoinBy
+} = require('./util');
 const DataLoader = require('dataloader');
 
 const CommentModel = require('../../models/comment');
@@ -11,6 +15,38 @@ const CommentModel = require('../../models/comment');
  *                                    comments that we want to get
  */
 const getCountsByAssetID = (context, asset_ids) => {
+  return CommentModel.aggregate([
+    {
+      $match: {
+        asset_id: {
+          $in: asset_ids
+        },
+        status: {
+          $in: ['NONE', 'ACCEPTED']
+        }
+      }
+    },
+    {
+      $group: {
+        _id: '$asset_id',
+        count: {
+          $sum: 1
+        }
+      }
+    }
+  ])
+  .then(singleJoinBy(asset_ids, '_id'))
+  .then((results) => results.map((result) => result ? result.count : 0));
+};
+
+/**
+ * Returns the comment count for all comments that are public based on their
+ * asset ids.
+ * @param {Object}        context     graph context
+ * @param {Array<String>} asset_ids   the ids of assets for which there are
+ *                                    comments that we want to get
+ */
+const getParentCountsByAssetID = (context, asset_ids) => {
   return CommentModel.aggregate([
     {
       $match: {
@@ -32,7 +68,7 @@ const getCountsByAssetID = (context, asset_ids) => {
       }
     }
   ])
-  .then(util.singleJoinBy(asset_ids, '_id'))
+  .then(singleJoinBy(asset_ids, '_id'))
   .then((results) => results.map((result) => result ? result.count : 0));
 };
 
@@ -64,7 +100,7 @@ const getCountsByParentID = (context, parent_ids) => {
       }
     }
   ])
-  .then(util.singleJoinBy(parent_ids, '_id'))
+  .then(singleJoinBy(parent_ids, '_id'))
   .then((results) => results.map((result) => result ? result.count : 0));
 };
 
@@ -216,7 +252,7 @@ const genRecentReplies = (context, ids) => {
 
   ])
   .then((replies) => replies.map((reply) => reply.replies))
-  .then(util.arrayJoinBy(ids, 'parent_id'));
+  .then(arrayJoinBy(ids, 'parent_id'));
 };
 
 /**
@@ -267,7 +303,7 @@ const genRecentComments = (_, ids) => {
 
   ])
   .then((replies) => replies.map((reply) => reply.comments))
-  .then(util.arrayJoinBy(ids, 'asset_id'));
+  .then(arrayJoinBy(ids, 'asset_id'));
 };
 
 /**
@@ -294,7 +330,7 @@ const genComments = ({user}, ids) => {
       }
     });
   }
-  return comments.then(util.singleJoinBy(ids, 'id'));
+  return comments.then(singleJoinBy(ids, 'id'));
 };
 
 /**
@@ -307,8 +343,9 @@ module.exports = (context) => ({
     get: new DataLoader((ids) => genComments(context, ids)),
     getByQuery: (query) => getCommentsByQuery(context, query),
     getCountByQuery: (query) => getCommentCountByQuery(context, query),
-    countByAssetID: new util.SharedCacheDataLoader('Comments.countByAssetID', 3600, (ids) => getCountsByAssetID(context, ids)),
-    countByParentID: new util.SharedCacheDataLoader('Comments.countByParentID', 3600, (ids) => getCountsByParentID(context, ids)),
+    countByAssetID: new SharedCounterDataLoader('Comments.totalCommentCount', 3600, (ids) => getCountsByAssetID(context, ids)),
+    parentCountByAssetID: new SharedCounterDataLoader('Comments.countByAssetID', 3600, (ids) => getParentCountsByAssetID(context, ids)),
+    countByParentID: new SharedCounterDataLoader('Comments.countByParentID', 3600, (ids) => getCountsByParentID(context, ids)),
     genRecentReplies: new DataLoader((ids) => genRecentReplies(context, ids)),
     genRecentComments: new DataLoader((ids) => genRecentComments(context, ids))
   }

graph/loaders/util.js

@@ -121,6 +121,29 @@ class SharedCacheDataLoader extends DataLoader {
   }
 }
 
+/**
+ * SharedCounterDataLoader is identical to SharedCacheDataLoader with the
+ * exception in that it is designed to work with numerical cached data.
+ */
+class SharedCounterDataLoader extends SharedCacheDataLoader {
+
+  /**
+   * Increments the key in the cache if it already exists in the cache, if not
+   * it does nothing.
+   */
+  incr(key) {
+    return cache.incr(key, this._expiry, this._keyFunc);
+  }
+
+  /**
+   * Decrements the key in the cache if it already exists in the cache, if not
+   * it does nothing.
+   */
+  decr(key) {
+    return cache.decr(key, this._expiry, this._keyFunc);
+  }
+}
+
 /**
  * Maps an object's paths to a string that can be used as a cache key.
  * @param  {Array} paths paths on the object to be used to generate the cache
@@ -145,5 +168,6 @@ module.exports = {
   objectCacheKeyFn,
   arrayCacheKeyFn,
   SingletonResolver,
-  SharedCacheDataLoader
+  SharedCacheDataLoader,
+  SharedCounterDataLoader
 };

graph/mutators/comment.js

@@ -33,16 +33,17 @@ const createComment = ({user, loaders: {Comments}}, {body, asset_id, parent_id =
   })
   .then((comment) => {
 
-    // TODO: explore using an `INCR` operation to update the counts here
-
     // If the loaders are present, clear the caches for these values because we
-    // just added a new comment, hence the counts should be updated.
-    if (Comments && Comments.countByAssetID && Comments.countByParentID) {
+    // just added a new comment, hence the counts should be updated. We should
+    // perform these increments in the event that we do have a new comment that
+    // is approved or without a comment.
+    if (status === 'NONE' || status === 'APPROVED') {
       if (parent_id != null) {
-        Comments.countByParentID.clear(parent_id);
+        Comments.countByParentID.incr(parent_id);
       } else {
-        Comments.countByAssetID.clear(asset_id);
+        Comments.parentCountByAssetID.incr(asset_id);
       }
+      Comments.countByAssetID.incr(asset_id);
     }
 
     return comment;
@@ -182,15 +183,18 @@ const setCommentStatus = ({loaders: {Comments}}, {id, status}) => {
     .then((comment) => {
 
       // If the loaders are present, clear the caches for these values because we
-      // just added a new comment, hence the counts should be updated.
-      if (Comments && Comments.countByAssetID && Comments.countByParentID) {
-        if (comment.parent_id != null) {
-          Comments.countByParentID.clear(comment.parent_id);
-        } else {
-          Comments.countByAssetID.clear(comment.asset_id);
-        }
+      // just added a new comment, hence the counts should be updated. It would
+      // be nice if we could decrement the counters here, but that would result
+      // in us having to know the initial state of the comment, which would
+      // require another database query.
+      if (comment.parent_id != null) {
+        Comments.countByParentID.clear(comment.parent_id);
+      } else {
+        Comments.parentCountByAssetID.clear(comment.asset_id);
       }
 
+      Comments.countByAssetID.clear(comment.asset_id);
+
       return comment;
     });
 };

graph/resolvers/asset.js

@@ -10,7 +10,18 @@ const Asset = {
       parent_id: null
     });
   },
-  commentCount({id}, _, {loaders: {Comments}}) {
+  commentCount({id, commentCount}, _, {loaders: {Comments}}) {
+    if (commentCount != null) {
+      return commentCount;
+    }
+
+    return Comments.parentCountByAssetID.load(id);
+  },
+  totalCommentCount({id, totalCommentCount}, _, {loaders: {Comments}}) {
+    if (totalCommentCount != null) {
+      return totalCommentCount;
+    }
+
     return Comments.countByAssetID.load(id);
   },
   settings({settings = null}, _, {loaders: {Settings}}) {

graph/typeDefs.graphql

@@ -405,6 +405,9 @@ type Asset {
   # The count of top level comments on the asset.
   commentCount: Int
 
+  # The total count of all comments made on the asset.
+  totalCommentCount: Int
+
   # The settings (rectified with the global settings) that should be applied to
   # this asset.
   settings: Settings!
@@ -646,14 +649,14 @@ type SetCommentStatusResponse implements Response {
 type AddCommentTagResponse implements Response {
   # An array of errors relating to the mutation that occured.
   comment: Comment
-  errors: [UserError]  
+  errors: [UserError]
 }
 
 # Response to removeCommentTag mutation
 type RemoveCommentTagResponse implements Response {
   # An array of errors relating to the mutation that occured.
   comment: Comment
-  errors: [UserError]  
+  errors: [UserError]
 }
 
 # All mutations for the application are defined on this object.

package.json

@@ -83,7 +83,7 @@
     "passport-facebook": "^2.1.1",
     "passport-local": "^1.0.0",
     "react-apollo": "^0.10.0",
-    "redis": "^2.6.3",
+    "redis": "^2.6.5",
     "uuid": "^2.0.3"
   },
   "devDependencies": {
@@ -167,6 +167,6 @@
     "webpack": "^2.2.1"
   },
   "engines": {
-    "node": "~7.6.0"
+    "node": "^7.7.0"
   }
 }

services/cache.js

@@ -1,5 +1,6 @@
 const redis = require('./redis');
 const debug = require('debug')('talk:cache');
+const crypto = require('crypto');
 
 const cache = module.exports = {
   client: redis.createClient()
@@ -60,6 +61,157 @@ cache.wrap = (key, expiry, work, kf = keyfunc) => {
     });
 };
 
+// This is designed to increment a key and add an expiry iff the key already
+// exists.
+const INCR_SCRIPT = `
+if redis.call('GET', KEYS[1]) ~= false then
+  redis.call('INCR', KEYS[1])
+  redis.call('EXPIRE', KEYS[1], ARGV[1])
+end
+`;
+
+// Stores the SHA1 hash of INCR_SCRIPT, used for executing via EVALSHA.
+let INCR_SCRIPT_HASH;
+
+// This is designed to decrement a key and add an expiry iff the key already
+// exists.
+const DECR_SCRIPT = `
+if redis.call('GET', KEYS[1]) ~= false then
+  redis.call('DECR', KEYS[1])
+  redis.call('EXPIRE', KEYS[1], ARGV[1])
+end
+`;
+
+// Stores the SHA1 hash of DECR_SCRIPT, used for executing via EVALSHA.
+let DECR_SCRIPT_HASH;
+
+// Load the script into redis and track the script hash that we will use to exec
+// increments on.
+const loadScript = (name, script) => new Promise((resolve, reject) => {
+
+  let shasum = crypto.createHash('sha1');
+  shasum.update(script);
+
+  let hash = shasum.digest('hex');
+
+  cache.client
+    .script('EXISTS', hash, (err, [exists]) => {
+      if (err) {
+        return reject(err);
+      }
+
+      if (exists) {
+        debug(`already loaded ${name} as SHA[${hash}], not loading again`);
+
+        return resolve(hash);
+      }
+
+      debug(`${name} not loaded as SHA[${hash}], loading`);
+
+      cache.client
+        .script('load', script, (err, hash) => {
+          if (err) {
+            return reject(err);
+          }
+
+          debug(`loaded ${name} as SHA[${hash}]`);
+
+          resolve(hash);
+        });
+    });
+});
+
+// Load the INCR_SCRIPT and DECR_SCRIPT into Redis.
+Promise.all([
+  loadScript('INCR_SCRIPT', INCR_SCRIPT),
+  loadScript('DECR_SCRIPT', DECR_SCRIPT)
+])
+.then(([incrScriptHash, decrScriptHash]) => {
+  INCR_SCRIPT_HASH = incrScriptHash;
+  DECR_SCRIPT_HASH = decrScriptHash;
+})
+.catch((err) => {
+  throw err;
+});
+
+/**
+ * This will increment a key in redis and update the expiry iff it already
+ * exists, otherwise it will do nothing.
+ */
+cache.incr = (key, expiry, kf = keyfunc) => new Promise((resolve, reject) => {
+  cache.client
+    .evalsha(INCR_SCRIPT_HASH, 1, kf(key), expiry, (err) => {
+      if (err) {
+        return reject(err);
+      }
+
+      return resolve();
+    });
+});
+
+/**
+ * This will decrement a key in redis and update the expiry iff it already
+ * exists, otherwise it will do nothing.
+ */
+cache.decr = (key, expiry, kf = keyfunc) => new Promise((resolve, reject) => {
+  cache.client
+    .evalsha(DECR_SCRIPT_HASH, 1, kf(key), expiry, (err) => {
+      if (err) {
+        return reject(err);
+      }
+
+      return resolve();
+    });
+});
+
+/**
+ * This will increment many keys in redis and update the expiry iff it already
+ * exists, otherwise it will do nothing.
+ */
+cache.incrMany = (keys, expiry, kf = keyfunc) => {
+  let multi = cache.client.multi();
+
+  keys.forEach((key) => {
+
+    // Queue up the evalsha command.
+    multi.evalsha(INCR_SCRIPT_HASH, 1, kf(key), expiry);
+  });
+
+  return new Promise((resolve, reject) => {
+    multi.exec((err) => {
+      if (err) {
+        return reject(err);
+      }
+
+      resolve();
+    });
+  });
+};
+
+/**
+ * This will decrement many keys in redis and update the expiry iff it already
+ * exists, otherwise it will do nothing.
+ */
+cache.decrMany = (keys, expiry, kf = keyfunc) => {
+  let multi = cache.client.multi();
+
+  keys.forEach((key) => {
+
+    // Queue up the evalsha command.
+    multi.evalsha(DECR_SCRIPT_HASH, 1, kf(key), expiry);
+  });
+
+  return new Promise((resolve, reject) => {
+    multi.exec((err) => {
+      if (err) {
+        return reject(err);
+      }
+
+      resolve();
+    });
+  });
+};
+
 /**
  * [wrapMany description]
  * @param  {Array<String>} keys         Either an array of objects represening