added support for configuring limiting, action optims

This commit is contained in:
Wyatt Johnson
2018-02-13 17:24:57 -07:00
parent 74a2ed4a41
commit 73034a20f0
11 changed files with 180 additions and 206 deletions
+3
View File
@@ -1,6 +1,9 @@
FROM coralproject/talk:latest
# Setup the build arguments
ONBUILD ARG TALK_ADDTL_COMMENTS_ON_LOAD_MORE=10
ONBUILD ARG TALK_ASSET_COMMENTS_LOAD_DEPTH=10
ONBUILD ARG TALK_REPLY_COMMENTS_LOAD_DEPTH=3
ONBUILD ARG TALK_THREADING_LEVEL=3
ONBUILD ARG TALK_DEFAULT_STREAM_TAB=all
ONBUILD ARG TALK_DEFAULT_LANG=en
@@ -1,14 +1,27 @@
import defaultTo from 'lodash/defaultTo';
const prefix = 'TALK_EMBED_STREAM';
export const SET_ACTIVE_REPLY_BOX = 'SET_ACTIVE_REPLY_BOX';
export const ADDTL_COMMENTS_ON_LOAD_MORE = 10;
export const VIEW_ALL_COMMENTS = 'VIEW_ALL_COMMENTS';
export const VIEW_COMMENT = 'VIEW_COMMENT';
export const ADDTL_COMMENTS_ON_LOAD_MORE = parseInt(
defaultTo(process.env.TALK_ADDTL_COMMENTS_ON_LOAD_MORE, '10')
);
export const ASSET_COMMENTS_LOAD_DEPTH = parseInt(
defaultTo(process.env.TALK_ASSET_COMMENTS_LOAD_DEPTH, '10')
);
export const REPLY_COMMENTS_LOAD_DEPTH = parseInt(
defaultTo(process.env.TALK_REPLY_COMMENTS_LOAD_DEPTH, '3')
);
export const THREADING_LEVEL = parseInt(
defaultTo(process.env.TALK_THREADING_LEVEL, '3')
);
export const ADD_COMMENT_BOX_TAG = `${prefix}_COMMENT_BOX_ADD_TAG`;
export const ADD_COMMENT_CLASSNAME = 'ADD_COMMENT_CLASSNAME';
export const CLEAR_COMMENT_BOX_TAGS = `${prefix}_COMMENT_BOX_CLEAR_TAGS`;
export const REMOVE_COMMENT_BOX_TAG = `${prefix}_COMMENT_BOX_REMOVE_TAG`;
export const REMOVE_COMMENT_CLASSNAME = 'REMOVE_COMMENT_CLASSNAME';
export const THREADING_LEVEL = process.env.TALK_THREADING_LEVEL;
export const SET_ACTIVE_REPLY_BOX = 'SET_ACTIVE_REPLY_BOX';
export const SET_ACTIVE_TAB = 'CORAL_STREAM_SET_ACTIVE_TAB';
export const SET_SORT = 'CORAL_STREAM_SET_SORT';
export const ADD_COMMENT_BOX_TAG = `${prefix}_COMMENT_BOX_ADD_TAG`;
export const REMOVE_COMMENT_BOX_TAG = `${prefix}_COMMENT_BOX_REMOVE_TAG`;
export const CLEAR_COMMENT_BOX_TAGS = `${prefix}_COMMENT_BOX_CLEAR_TAGS`;
export const VIEW_ALL_COMMENTS = 'VIEW_ALL_COMMENTS';
export const VIEW_COMMENT = 'VIEW_COMMENT';
@@ -4,7 +4,10 @@ import Comment from '../components/Comment';
import { withFragments } from 'coral-framework/hocs';
import { getSlotFragmentSpreads } from 'coral-framework/utils';
import { withSetCommentStatus } from 'coral-framework/graphql/mutations';
import { THREADING_LEVEL } from '../../../constants/stream';
import {
THREADING_LEVEL,
REPLY_COMMENTS_LOAD_DEPTH,
} from '../../../constants/stream';
import hoistStatics from 'recompose/hoistStatics';
import { nest } from '../../../graphql/utils';
@@ -118,7 +121,7 @@ const withCommentFragments = withFragments({
...CoralEmbedStream_Comment_SingleComment
${nest(
`
replies(query: {limit: 3, excludeIgnored: $excludeIgnored}) {
replies(query: {limit: ${REPLY_COMMENTS_LOAD_DEPTH}, excludeIgnored: $excludeIgnored}) {
nodes {
...CoralEmbedStream_Comment_SingleComment
...nest
@@ -4,6 +4,7 @@ import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import {
ADDTL_COMMENTS_ON_LOAD_MORE,
ASSET_COMMENTS_LOAD_DEPTH,
THREADING_LEVEL,
} from '../../../constants/stream';
import {
@@ -379,7 +380,7 @@ const fragments = {
requireEmailConfirmation
}
totalCommentCount @skip(if: $hasComment)
comments(query: {limit: 10, excludeIgnored: $excludeIgnored, sortOrder: $sortOrder, sortBy: $sortBy}) @skip(if: $hasComment) {
comments(query: {limit: ${ASSET_COMMENTS_LOAD_DEPTH}, excludeIgnored: $excludeIgnored, sortOrder: $sortOrder, sortBy: $sortBy}) @skip(if: $hasComment) {
nodes {
...CoralEmbedStream_Stream_comment
}
+26 -1
View File
@@ -492,4 +492,29 @@ tracing of GraphQL requests.
Setting this to `TRUE` will allow queries to execute without a limit (returns
all documents). This introduces a significant performance regression, and should
be used with caution. (Default `FALSE`)
be used with caution. (Default `FALSE`)
## TALK_ADDTL_COMMENTS_ON_LOAD_MORE
This is a **Build Variable** and must be consumed during build. If using the
[Docker-onbuild]({{ "/installation-from-docker/#onbuild" | relative_url }})
image you can specify it with `--build-arg TALK_ADDTL_COMMENTS_ON_LOAD_MORE=10`.
Specifies the number of additional comments to load when a user clicks `Load More`. (Default `10`)
## TALK_ASSET_COMMENTS_LOAD_DEPTH
This is a **Build Variable** and must be consumed during build. If using the
[Docker-onbuild]({{ "/installation-from-docker/#onbuild" | relative_url }})
image you can specify it with `--build-arg TALK_ASSET_COMMENTS_LOAD_DEPTH=10`.
Specifies the initial number of comments to load for an asset. (Default `10`)
## TALK_REPLY_COMMENTS_LOAD_DEPTH
This is a **Build Variable** and must be consumed during build. If using the
[Docker-onbuild]({{ "/installation-from-docker/#onbuild" | relative_url }})
image you can specify it with `--build-arg TALK_REPLY_COMMENTS_LOAD_DEPTH=3`.
Specifies the initial replies to load for a comment. (Default `3`)
+105 -25
View File
@@ -1,9 +1,7 @@
const DataLoader = require('dataloader');
const util = require('./util');
const ActionsService = require('../../services/actions');
const ActionModel = require('../../models/action');
const { first, get, remove, groupBy, reduce, defaultTo } = require('lodash');
/**
* Gets actions based on their item id's.
@@ -15,40 +13,122 @@ const genActionsByItemID = (_, item_ids) => {
};
/**
* Looks up actions based on the requested id's all bounded by the user.
* @param {Object} context the context of the request
* @param {Array} ids array of id's to get
* @return {Promise} resolves to the promises of the requested actions
* Looks up the actions for each of the items.
*
* @param {Object} ctx the graph context of the request
* @param {Array<String>} itemIDs the items that we need to get the actions for
*/
const genActionSummariessByItemID = ({ user = {} }, item_ids) => {
return ActionsService.getActionSummaries(item_ids, user.id).then(
util.arrayJoinBy(item_ids, 'item_id')
const genActionsAuthoredWithID = ({ user = {} }, itemIDs) =>
ActionsService.getUserActions(user.id, itemIDs).then(
util.arrayJoinBy(itemIDs, 'item_id')
);
};
/**
* Search for actions based on their action_type and item_type and ensures that
* the actions returned have unique item id's.
* @param {String} action_type the action to search by
* @param {String} item_type the item id to search by
* @return {Promise} resolves to distinct items actions
* Looks up the action summaries for a set of items.
*
* @param {Object} ctx the graph context of the request
* @param {Array<Object>} items the items that should have their items looked up for
*/
const getItemIdsByActionTypeAndItemType = (_, action_type, item_type) => {
return ActionModel.distinct('item_id', { action_type, item_type });
const genActionSummariesByItem = async (ctx, items) => {
const { loaders: { Actions } } = ctx;
// This is designed to match the action_counts value that is embedded on
// documents which cache action counts. For users that are not logged in, we
// don't need to hit the actions collection at all!
// This will match any action count that is specific for a group id.
const nonGroupIDTest = /^([A-Z]+)_([A-Z_]+)$/;
// We will literate over all the items that we're comparing.
return items.map(async ({ id, action_counts = {} }) => {
// Cache all those entries for which we got the group id of, because we
// don't want to include them twice.
const groupIDCache = {};
// Possibly get the list of user actions completed by the user. This will be
// used later to join together with the action summaries to provide context.
const userActions =
ctx.user && reduce(action_counts, (total, count) => total + count, 0) > 0
? await Actions.getAuthoredByID.load(id)
: [];
// Group the user actions in the same way that the action counts are
// grouped. This will let us extract it easy.
const groupedUserActions = groupBy(
userActions,
({ action_type, group_id }) =>
(group_id ? `${action_type}_${group_id}` : action_type).toUpperCase()
);
// Generate the action summaries for the item.
return Object.keys(action_counts)
.map(action_type => ({
count: action_counts[action_type],
action_type: action_type.toUpperCase(),
}))
.reduce((actionTypeList, { count, action_type }) => {
// Get the current user's actions (if they have any).
const current_user = defaultTo(
first(get(groupedUserActions, action_type, [])),
null
);
// Check to see if this is a action without a corresponding group id.
if (nonGroupIDTest.test(action_type)) {
// This action type does have a group id associated with it.
const results = nonGroupIDTest.exec(action_type);
const groupActionType = results[1];
const groupID = results[2];
// Purge out the summary if it already exists, and mark that this
// group id has been found so we don't include it in the future.
remove(
actionTypeList,
({ action_type }) => action_type === groupActionType
);
groupIDCache[groupActionType] = true;
// Push the new entry in.
actionTypeList.push({
action_type: groupActionType,
group_id: groupID,
count,
current_user,
});
} else {
// This does not have a group id. Check to see if this group id
// already has an specific (group id) entry.
if (groupIDCache[action_type]) {
// It does. Don't add anything.
return actionTypeList;
}
// It does not, add the entry.
actionTypeList.push({
action_type,
group_id: null,
count,
current_user,
});
}
return actionTypeList;
}, []);
});
};
/**
* Creates a set of loaders based on a GraphQL context.
* @param {Object} context the context of the GraphQL request
* @param {Object} ctx the context of the GraphQL request
* @return {Object} object of loaders
*/
module.exports = context => ({
module.exports = ctx => ({
Actions: {
getByID: new DataLoader(ids => genActionsByItemID(context, ids)),
getSummariesByItemID: new DataLoader(ids =>
genActionSummariessByItemID(context, ids)
getByID: new DataLoader(ids => genActionsByItemID(ctx, ids)),
getSummariesByItem: new DataLoader(
items => genActionSummariesByItem(ctx, items),
{ cacheKeyFn: ({ id }) => id }
),
getByTypes: ({ action_type, item_type }) =>
getItemIdsByActionTypeAndItemType(context, action_type, item_type),
getAuthoredByID: new DataLoader(ids => genActionsAuthoredWithID(ctx, ids)),
},
});
+4 -4
View File
@@ -40,12 +40,12 @@ const Comment = {
return Actions.getByID.load(id);
},
action_summaries({ id, action_summaries }, _, { loaders: { Actions } }) {
if (action_summaries) {
return action_summaries;
action_summaries(comment, _, { loaders: { Actions } }) {
if (comment.action_summaries) {
return comment.action_summaries;
}
return Actions.getSummariesByItemID.load(id);
return Actions.getSummariesByItem.load(comment);
},
asset({ asset_id }, _, { loaders: { Assets } }) {
return Assets.getByID.load(asset_id);
+2 -2
View File
@@ -10,8 +10,8 @@ const {
} = require('../../perms/constants');
const User = {
action_summaries({ id }, _, { loaders: { Actions } }) {
return Actions.getSummariesByItemID.load(id);
action_summaries(user, _, { loaders: { Actions } }) {
return Actions.getSummariesByItem.load(user);
},
actions({ id }, _, { user, loaders: { Actions } }) {
// Only return the actions if the user is not an admin.
+9 -100
View File
@@ -104,110 +104,19 @@ module.exports = class ActionsService {
}
/**
* Fetches the action summaries for the given asset, and comments around the
* given user id.
* Get the actions for a specific user on the specific items.
*
* @param {[type]} asset_id [description]
* @param {[type]} comments [description]
* @param {String} [current_user_id=''] [description]
* @return {[type]} [description]
* @param {String} userID the id of the user to find their actions for
* @param {Array<String>} itemIDs the ids of the items to find their actions
* for
*/
static getActionSummariesFromComments(
asset_id = '',
comments,
current_user_id = ''
) {
// Get the user id's from the author id's as a unique array that gets
// sorted.
let userIDs = _.uniq(comments.map(comment => comment.author_id)).sort();
// Fetch the actions for pretty much everything at this point.
return ActionsService.getActionSummaries(
_.uniq(
[
// Actions can be on assets...
asset_id,
// Comments...
...comments.map(comment => comment.id),
// Or Authors...
...userIDs,
].filter(e => e)
),
current_user_id
);
}
/**
* Returns summaries of actions for an array of ids.
*
* @param {String} ids array of user identifiers (uuid)
*/
static getActionSummaries(item_ids, current_user_id = '') {
// only grab items that match the specified item id's
let $match = {
static getUserActions(userID, itemIDs) {
return ActionModel.find({
user_id: userID,
item_id: {
$in: item_ids,
$in: itemIDs,
},
};
let $group = {
// group unique documents by these properties, we are leveraging the
// fact that each uuid is completely unique.
_id: {
item_id: '$item_id',
action_type: '$action_type',
group_id: '$group_id',
},
// and sum up all actions matching the above grouping criteria
count: {
$sum: 1,
},
// we are leveraging the fact that each uuid is completely unique and
// just grabbing the last instance of the item type here.
item_type: {
$first: '$item_type',
},
current_user: {
$max: {
$cond: {
if: {
$eq: ['$user_id', current_user_id],
},
then: '$$CURRENT',
else: null,
},
},
},
};
let $project = {
// suppress the _id field
_id: false,
// map the fields from the _id grouping down a level
item_id: '$_id.item_id',
action_type: '$_id.action_type',
group_id: '$_id.group_id',
// map the field directly
count: '$count',
item_type: '$item_type',
// set the current user to false here
current_user: '$current_user',
};
return ActionModel.aggregate([
{ $match },
{ $group },
{ $project },
{ $sort: { action_type: 1, group_id: 1 } },
]);
});
}
/**
-63
View File
@@ -151,67 +151,4 @@ describe('services.ActionsService', () => {
);
});
});
describe('#getActionSummaries()', () => {
it('should return properly formatted summaries from an array of item_ids', () => {
return ActionsService.getActionSummaries([comment.id, '789']).then(
summaries => {
expect(summaries).to.have.length(2);
expect(summaries).to.deep.include({
action_type: 'LIKE',
count: 1,
item_id: comment.id,
item_type: 'COMMENTS',
current_user: null,
});
expect(summaries).to.deep.include({
action_type: 'FLAG',
count: 2,
item_id: comment.id,
item_type: 'COMMENTS',
current_user: null,
});
}
);
});
it('should include a current user when one is passed', () => {
return ActionsService.getActionSummaries(
[comment.id],
'flagginguserid'
).then(summaries => {
expect(summaries).to.have.length(2);
let summary = summaries.find(
s => s.item_id === comment.id && s.action_type === 'FLAG'
);
expect(summary).to.not.be.undefined;
expect(summary.current_user).to.not.be.null;
expect(summary.current_user).to.have.property('item_id', comment.id);
expect(summary.current_user).to.have.property('item_type', 'COMMENTS');
expect(summary.current_user).to.have.property(
'user_id',
'flagginguserid'
);
expect(summary.current_user).to.have.property('action_type', 'FLAG');
});
});
it("should not include a current user when one is passed for a user that doesn't have an action", () => {
return ActionsService.getActionSummaries(
[comment.id],
'flagginguserid2'
).then(summaries => {
expect(summaries).to.have.length(2);
summaries.forEach(summary => {
expect(summary).to.not.be.undefined;
expect(summary).to.have.property('current_user', null);
});
});
});
});
});
+3
View File
@@ -128,6 +128,9 @@ const config = {
new webpack.EnvironmentPlugin({
TALK_PLUGINS_JSON: '{}',
TALK_THREADING_LEVEL: '3',
TALK_ADDTL_COMMENTS_ON_LOAD_MORE: '10',
TALK_ASSET_COMMENTS_LOAD_DEPTH: '10',
TALK_REPLY_COMMENTS_LOAD_DEPTH: '3',
TALK_DEFAULT_STREAM_TAB: 'all',
TALK_DEFAULT_LANG: 'en',
}),