From cc4c943a21c7c6950b194c891b7b9ad347dfdbe5 Mon Sep 17 00:00:00 2001 From: IAmSamHankins Date: Fri, 9 Jun 2017 17:27:35 -0400 Subject: [PATCH 1/4] hover states, link styling, alignment fixes --- .../coral-admin/src/components/ui/Header.css | 2 +- .../routes/Dashboard/components/Widget.css | 22 +++++----- .../Moderation/components/CommentCount.css | 1 + .../Moderation/components/UserDetail.css | 4 +- .../routes/Moderation/components/styles.css | 44 ++++++++++++------- locales/en.yml | 4 +- 6 files changed, 45 insertions(+), 32 deletions(-) diff --git a/client/coral-admin/src/components/ui/Header.css b/client/coral-admin/src/components/ui/Header.css index 7cb68ecbf..d99a7f295 100644 --- a/client/coral-admin/src/components/ui/Header.css +++ b/client/coral-admin/src/components/ui/Header.css @@ -77,7 +77,7 @@ letter-spacing: .8; &:hover { - background-color: #232323; + background-color: #404040; } &.active { diff --git a/client/coral-admin/src/routes/Dashboard/components/Widget.css b/client/coral-admin/src/routes/Dashboard/components/Widget.css index 587c51ada..cf83fcbdc 100644 --- a/client/coral-admin/src/routes/Dashboard/components/Widget.css +++ b/client/coral-admin/src/routes/Dashboard/components/Widget.css @@ -17,8 +17,9 @@ .heading { margin: 0; padding-left: 10px; - font-size: 1.5rem; - font-weight: bold; + font-size: 1.3rem; + font-weight: 600; + color: #2c2c2c; } .widgetTable { @@ -32,7 +33,8 @@ } .widgetHead p { - color: rgb(35, 102, 223); + color: #2c2c2c; + font-weight: 500; padding: 10px; text-align: left; text-transform: capitalize; @@ -47,11 +49,11 @@ } .rowLinkify { - cursor: pointer; border-bottom: 1px solid lightgrey; color: #555; height: var(--row-height); padding: 10px; + transition: background-color 200ms; } .rowLinkify:last-child { @@ -60,6 +62,7 @@ .rowLinkify:hover { background-color: #f8f8f8; + pointer: default; } .linkToAsset { @@ -68,16 +71,17 @@ } .linkToModerate { - background-color: #e0e0e0; + background-color: #BDBDBD; padding: 10px 14px; text-decoration: none; color: black; float: right; margin-left: 15px; + transition: background-color 200ms; } .linkToModerate:hover { - background-color: #ccc; + background-color: #9E9E9E; } .lede { @@ -89,14 +93,10 @@ color: #555; text-decoration: none; font-size: 1.2em; - font-weight: normal; + font-weight: 500; margin: 0; } -.assetTitle:hover { - text-decoration: underline; -} - .widgetCount { color: #555; font-size: 1.3em; diff --git a/client/coral-admin/src/routes/Moderation/components/CommentCount.css b/client/coral-admin/src/routes/Moderation/components/CommentCount.css index b684d0f4a..4d24d5702 100644 --- a/client/coral-admin/src/routes/Moderation/components/CommentCount.css +++ b/client/coral-admin/src/routes/Moderation/components/CommentCount.css @@ -12,4 +12,5 @@ right: 0; margin-top: -2px; font-size: 12px; + color: white; } diff --git a/client/coral-admin/src/routes/Moderation/components/UserDetail.css b/client/coral-admin/src/routes/Moderation/components/UserDetail.css index d4b5f0db7..710f84759 100644 --- a/client/coral-admin/src/routes/Moderation/components/UserDetail.css +++ b/client/coral-admin/src/routes/Moderation/components/UserDetail.css @@ -15,7 +15,7 @@ display: flex; .stat { - margin: 0 4px 12px; + margin: 0 4px 10px 0px; } .stat:last-child { @@ -49,7 +49,7 @@ li { display: inline-block; - margin: 0 10px; + margin-right: 10px; cursor: pointer; padding: 0 10px; } diff --git a/client/coral-admin/src/routes/Moderation/components/styles.css b/client/coral-admin/src/routes/Moderation/components/styles.css index d10311f74..fc825eccb 100644 --- a/client/coral-admin/src/routes/Moderation/components/styles.css +++ b/client/coral-admin/src/routes/Moderation/components/styles.css @@ -18,14 +18,20 @@ .tab { flex: 1; - color: #EEEEEE; + color: #C0C0C0; text-transform: capitalize; font-weight: 100; - font-size: 15px; + font-size: 14px; letter-spacing: 1px; transition: border-bottom 200ms; - padding: 0px 5px; - margin-right: 30px; + transition: color 200ms; + padding: 0px 10px; + margin-right: 20px; + &:hover { + color: white; + border-bottom: solid 2px #F36451; + box-sizing: border-box; + } } .active { @@ -33,6 +39,10 @@ box-sizing: border-box; border-bottom: solid 4px #F36451; font-weight: 400; + &:hover { + border-bottom: solid 4px #F36451; + font-weight: 400; + } } .active > span { @@ -103,19 +113,18 @@ span { font-weight: 400; font-size: 15px; letter-spacing: 1px; - transition: opacity 200ms; + transition: background-color 200ms; opacity: 1; - &:hover { - opacity: .8; - cursor: pointer; - } - &:first-child { text-align: left; } &:nth-child(2) { + &:hover { + cursor: pointer; + background-color: #212121; + } span { text-align: center; text-overflow: ellipsis; @@ -245,7 +254,6 @@ span { padding: 5px; color: #262626; font-size: 14px; - margin-left: 15px; line-height: 1px; font-weight: 300; } @@ -421,17 +429,21 @@ span { .tabIcon { position: relative; - top: 2px; + top: 3px; font-size: 16px; } .username { - color: blue; - text-decoration: underline; + color: #393B44; + text-decoration: none; cursor: pointer; - + font-weight: 600; + padding: 2px 5px; + border-radius: 2px; + margin-left: -5px; + transition: background-color 200ms ease; &:hover { - background-color: rgba(255, 0, 0, .1); + background-color: #E0E0E0; } } diff --git a/locales/en.yml b/locales/en.yml index ce9abdce5..930a4eadb 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -144,8 +144,8 @@ en: auto_update: "Data automatically updates every five minutes or when you Reload." comment_count: comments flags: Flags - most_flags: "Articles with the most flags" - most_conversations: "Articles with the most conversations" + most_flags: "Stories with the most flags" + most_conversations: "Stories with the most conversations" next_update: "{0} minutes until next update." no_activity: "There haven't been any comments anywhere in the last five minutes." no_flags: "There have been no flags in the last 5 minutes! Hooray!" From aed265efcc2fbc8e69783270b6e8ef6f13667601 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 13 Jun 2017 22:01:58 +0700 Subject: [PATCH 2/4] Use correct fragment name --- client/coral-framework/graphql/fragments.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/coral-framework/graphql/fragments.js b/client/coral-framework/graphql/fragments.js index 3c64090bd..110f0796b 100644 --- a/client/coral-framework/graphql/fragments.js +++ b/client/coral-framework/graphql/fragments.js @@ -7,7 +7,7 @@ export default { 'SuspendUserResponse', 'RejectUsernameResponse', 'SetUserStatusResponse', - 'PostCommentResponse', + 'CreateFlagResponse', 'EditCommentResponse', 'PostFlagResponse', 'CreateDontAgreeResponse', @@ -17,3 +17,4 @@ export default { 'StopIgnoringUserResponse', ) }; + From 7d8f2aa876c1e9c3154c2faa4fbf3f511b78dfde Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 13 Jun 2017 22:02:54 +0700 Subject: [PATCH 3/4] More robust reactions --- errors.js | 15 +++++++ graph/mutators/action.js | 2 +- locales/en.yml | 1 + plugin-api/beta/client/hocs/withReaction.js | 50 ++++++++++++++++----- plugin-api/beta/server/getReactionConfig.js | 15 ++++++- services/actions.js | 37 ++++++++++----- 6 files changed, 98 insertions(+), 22 deletions(-) diff --git a/errors.js b/errors.js index c203b1d25..5d2593d66 100644 --- a/errors.js +++ b/errors.js @@ -104,6 +104,20 @@ class ErrAuthentication extends APIError { } } +/** + * ErrAlreadyExists is returned when an attempt to create a resource failed due to an existing one. + */ +class ErrAlreadyExists extends APIError { + constructor(existing = null) { + super('authentication error occured', { + translation_key: 'ALREADY_EXISTS', + status: 400 + }, { + existing + }); + } +} + // ErrContainsProfanity is returned in the event that the middleware detects // profanity/wordlisted words in the payload. const ErrContainsProfanity = new APIError('This username contains elements which are not permitted in our community. If you think this is in error, please contact us or try again.', { @@ -171,6 +185,7 @@ const ErrCommentTooShort = new APIError('Comment was too short', { module.exports = { ExtendableError, APIError, + ErrAlreadyExists, ErrPasswordTooShort, ErrSettingsNotInit, ErrMissingEmail, diff --git a/graph/mutators/action.js b/graph/mutators/action.js index de9efea1a..96bf091cf 100644 --- a/graph/mutators/action.js +++ b/graph/mutators/action.js @@ -36,7 +36,7 @@ const createAction = async ({user = {}}, {item_id, item_type, action_type, group * Deletes an action based on the user id if the user owns that action. * @param {Object} user the user performing the request * @param {String} id the id of the action to delete - * @return {Promise} resolves when the action is deleted + * @return {Promise} resolves to the deleted action, or null if not found. */ const deleteAction = ({user}, {id}) => { return ActionModel.findOneAndRemove({ diff --git a/locales/en.yml b/locales/en.yml index ce9abdce5..4f287e36b 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -189,6 +189,7 @@ en: PASSWORD_REQUIRED: "Must input a password" COMMENTING_CLOSED: "Commenting is already closed" NOT_FOUND: "Resource not found" + ALREADY_EXISTS: "Resource already exists" INVALID_ASSET_URL: "Assert URL is invalid" email: "Not a valid E-Mail" confirm_password: "Passwords don't match. Please check again" diff --git a/plugin-api/beta/client/hocs/withReaction.js b/plugin-api/beta/client/hocs/withReaction.js index 42ac79ec5..1ed777996 100644 --- a/plugin-api/beta/client/hocs/withReaction.js +++ b/plugin-api/beta/client/hocs/withReaction.js @@ -149,6 +149,9 @@ export default (reaction) => (WrappedComponent) => { client: PropTypes.object.isRequired, }; + // Whether or not a mutation is currently active. + duringMutation = false; + constructor(props, context) { super(props, context); @@ -208,6 +211,26 @@ export default (reaction) => (WrappedComponent) => { } } + postReaction = () => { + if (this.duringMutation) { + return; + } + this.duringMutation = true; + return this.props.postReaction(this.props.comment) + .then((result) => {this.duringMutation = false; return Promise.resolve(result); }) + .catch((err) => {this.duringMutation = false; throw err; }); + } + + deleteReaction = () => { + if (this.duringMutation) { + return; + } + this.duringMutation = true; + return this.props.deleteReaction(this.props.comment) + .then((result) => {this.duringMutation = false; return Promise.resolve(result); }) + .catch((err) => {this.duringMutation = false; throw err; }); + } + render() { const {comment} = this.props; @@ -223,9 +246,16 @@ export default (reaction) => (WrappedComponent) => { const alreadyReacted = !!reactionSummary; - const withReactionProps = {reactionSummary, count, alreadyReacted}; - - return ; + return ; } } @@ -240,16 +270,16 @@ export default (reaction) => (WrappedComponent) => { } `, { - props: ({mutate, ownProps}) => ({ - deleteReaction: () => { + props: ({mutate}) => ({ + deleteReaction: (comment) => { const reactionSummary = getMyActionSummary( `${Reaction}ActionSummary`, - ownProps.comment + comment ); const id = reactionSummary.current_user.id; - const item_id = ownProps.comment.id; + const item_id = comment.id; const input = {id}; return mutate({ @@ -283,11 +313,11 @@ export default (reaction) => (WrappedComponent) => { } `, { - props: ({mutate, ownProps}) => ({ - postReaction: () => { + props: ({mutate}) => ({ + postReaction: (comment) => { const input = { - item_id: ownProps.comment.id, + item_id: comment.id, }; return mutate({ diff --git a/plugin-api/beta/server/getReactionConfig.js b/plugin-api/beta/server/getReactionConfig.js index 83c45dcb5..78bc45dfd 100644 --- a/plugin-api/beta/server/getReactionConfig.js +++ b/plugin-api/beta/server/getReactionConfig.js @@ -1,5 +1,6 @@ const wrapResponse = require('../../../graph/helpers/response'); const {SEARCH_OTHER_USERS} = require('../../../perms/constants'); +const errors = require('../../../errors'); function getReactionConfig(reaction) { reaction = reaction.toLowerCase(); @@ -134,13 +135,24 @@ function getReactionConfig(reaction) { // The comment is needed to allow better filtering e.g. by asset_id. pubsub.publish(`${reaction}ActionCreated`, {action, comment}); return Promise.resolve(action); - }); + }) + .catch((err) => { + if (err instanceof errors.ErrAlreadyExists) { + return Promise.resolve(err.metadata.existing); + } + throw err; + }); }); return wrapResponse(reaction)(response); }, [`delete${Reaction}Action`]: (_, {input: {id}}, {mutators: {Action}, pubsub, loaders: {Comments}}) => { const response = Action.delete({id}) .then((action) => { + + // Action doesn't exist or was already deleted. + if (!action) { + return Promise.resolve(null); + } return Comments.get.load(action.item_id).then((comment) => { // The comment is needed to allow better filtering e.g. by asset_id. @@ -148,6 +160,7 @@ function getReactionConfig(reaction) { return Promise.resolve(action); }); }); + return wrapResponse(reaction)(response); } }, diff --git a/services/actions.js b/services/actions.js index 40bef65bb..c59b3a3f9 100644 --- a/services/actions.js +++ b/services/actions.js @@ -1,5 +1,6 @@ const ActionModel = require('../models/action'); const _ = require('lodash'); +const errors = require('../errors'); module.exports = class ActionsService { @@ -12,10 +13,10 @@ module.exports = class ActionsService { } /** - * Add an action. - * @param {String} item_id identifier of the comment (uuid) + * Inserts an action. + * @param {String} item_id identifier of the item (uuid) * @param {String} user_id user id of the action (uuid) - * @param {String} action the new action to the comment + * @param {String} action the new action to the item * @return {Promise} */ static insertUserAction(action) { @@ -31,16 +32,32 @@ module.exports = class ActionsService { }; // Create/Update the action. - return ActionModel.findOneAndUpdate(query, action, { + return new Promise((resolve, reject) => { + ActionModel.findOneAndUpdate( + query, { - // Ensure that if it's new, we return the new object created. - new: true, + // Only set when not existing. + $setOnInsert: action, + }, { - // Perform an upsert in the event that this doesn't exist. - upsert: true, + // Ensure that if it's new, we return the new object created. + new: true, - // Set the default values if not provided based on the mongoose models. - setDefaultsOnInsert: true + // Use raw result to get `updatedExisting`. + passRawResult: true, + + // Perform an upsert in the event that this doesn't exist. + upsert: true, + + // Set the default values if not provided based on the mongoose models. + setDefaultsOnInsert: true + }, (err, doc, raw) => { + if (err) { return reject(err); } + if (raw.lastErrorObject.updatedExisting) { + return reject(new errors.ErrAlreadyExists(raw.value)); + } + return resolve(raw.value); + }); }); } From cdfa8379ffe9b2fbb638656da9a861096ce6a973 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Wed, 14 Jun 2017 00:38:34 +0700 Subject: [PATCH 4/4] Change text and status of ErrAlreadyExists --- errors.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/errors.js b/errors.js index 5d2593d66..9b26a9359 100644 --- a/errors.js +++ b/errors.js @@ -109,9 +109,9 @@ class ErrAuthentication extends APIError { */ class ErrAlreadyExists extends APIError { constructor(existing = null) { - super('authentication error occured', { + super('resource already exists', { translation_key: 'ALREADY_EXISTS', - status: 400 + status: 409 }, { existing });