diff --git a/script/queries/github_reaction_add.graphql b/script/queries/github_reaction_add.graphql new file mode 100644 index 00000000..9b2fa95f --- /dev/null +++ b/script/queries/github_reaction_add.graphql @@ -0,0 +1,10 @@ +mutation($id: ID!, $content: ReactionContent!) { + addReaction(input:{subjectId:$id, content: $content}) { + reaction { + content + } + subject { + id + } + } +} diff --git a/script/queries/github_reaction_remove.graphql b/script/queries/github_reaction_remove.graphql new file mode 100644 index 00000000..4cf580fd --- /dev/null +++ b/script/queries/github_reaction_remove.graphql @@ -0,0 +1,10 @@ +mutation($id: ID!, $content: ReactionContent!) { + removeReaction(input:{subjectId:$id, content: $content}) { + reaction { + content + } + subject { + id + } + } +} diff --git a/script/queries/index.js b/script/queries/index.js index f95d3544..19add780 100644 --- a/script/queries/index.js +++ b/script/queries/index.js @@ -2,9 +2,13 @@ import GITHUB_ISSUE_INFO_QUERY from './github_issue_info.graphql'; import GITHUB_PR_INFO_QUERY from './github_pr_info.graphql'; import GITHUB_LABEL_INFO_QUERY from './github_label_info.graphql'; import GITHUB_REACTION_INFO_QUERY from './github_reaction_info.graphql'; +import GITHUB_REACTION_ADD_MUTATION from './github_reaction_add.graphql'; +import GITHUB_REACTION_REMOVE_MUTATION from './github_reaction_remove.graphql'; export { GITHUB_ISSUE_INFO_QUERY, GITHUB_PR_INFO_QUERY, GITHUB_LABEL_INFO_QUERY, - GITHUB_REACTION_INFO_QUERY + GITHUB_REACTION_INFO_QUERY, + GITHUB_REACTION_ADD_MUTATION, + GITHUB_REACTION_REMOVE_MUTATION, }; diff --git a/src/components/login-auth.jsx b/src/components/login-auth.jsx new file mode 100644 index 00000000..2d5b4378 --- /dev/null +++ b/src/components/login-auth.jsx @@ -0,0 +1,34 @@ +import {Component} from 'react'; + +import CurrentUserStore from '../user-store'; +import Client from '../github-client'; + +function withAuth(WrappedComponent) { + return class extends Component { + state = {loginInfo: null}; + + componentDidMount() { + Client.on('changeToken', this.onChangeToken); + this.onChangeToken(); + } + + componentWillUnmount() { + Client.off('changeToken', this.onChangeToken); + } + + onChangeToken = () => { + CurrentUserStore.fetchUser() + .then((loginInfo) => { + this.setState({loginInfo}); + }).catch(() => { + this.setState({loginInfo: null}); + }); + }; + + render() { + return ; + } + }; +} + +export default withAuth; diff --git a/src/components/reactions.jsx b/src/components/reactions.jsx index 47938ca3..03dd8287 100644 --- a/src/components/reactions.jsx +++ b/src/components/reactions.jsx @@ -1,45 +1,186 @@ +import {Component} from 'react'; import * as BS from 'react-bootstrap'; -function Reactions({stat}) { - // use null when count is zero because we don't want to display - // number zero on frontend - const reactions = [ - { - emoji: '👍', - count: stat.THUMBS_UP || null, - name: 'THUMBS_UP' - }, - { - emoji: '👎', - count: stat.THUMBS_DOWN || null, - name: 'THUMBS_DOWN' - }, - { - emoji: '😄', - count: stat.LAUGH || null, - name: 'LAUGH' - }, - { - emoji: '🎉', - count: stat.HOORAY || null, - name: 'HOORAY' - }, - { - emoji: '😕', - count: stat.CONFUSED || null, - name: 'CONFUSED' - }, - { - emoji: '❤️', - count: stat.HEART || null, - name: 'HEART' +import Client from '../github-client'; + +class Reactions extends Component { + constructor(props) { + super(props); + this.state = { + canAdd: {}, + // use cache to reflect reaction count on frontend + // if we fetch up-to-date reaction count after mutation, + // we have to refetch the whole pull request which wastes + // a lot of API hits (there is no way to fetch single review + // comment at the moment) + cacheCount: { + THUMBS_UP: 0, + THUMBS_DOWN: 0, + LAUGH: 0, + HOORAY: 0, + CONFUSED: 0, + HEART: 0 + } + }; + } + + onClick = async (id, content) => { + const canAdd = this.state.canAdd[content]; + const saveToDatabase = this.props.saveCallBack; + let result, msg; + if (canAdd) { + ({ result, msg } = await Client.getGraphQLClient().addReaction( + {id, content} + )); + } else { + ({ result, msg } = await Client.getGraphQLClient().removeReaction( + {id, content} + )); + } + if (result) { + if (canAdd) { + // reaction creation succeeds + + // Note that if it is already meta-reviewed by the user but not via gh-board, + // action (add reaction) will fail, but GitHub won't return any error/warning. + // The good news is that user won't be annoyed because the frontend behavior + // is they add reactions successfully. + + // A side note is that gh-board will not update accordingly if user does + // meta-review directly on GitHub web page instead of on gh-board. This is + // because the `updatedBy` attribute of the pull request won't get changed + // due to meta-review. + + this.setState((prevState) => { + let newState = prevState; + newState.canAdd[content] = false; + // update cache + newState.cacheCount[content] += 1; + return newState; + }); + + saveToDatabase(content, true); + } else { + // reaction removal succeeds + this.setState((prevState) => { + let newState = prevState; + newState.canAdd[content] = true; + // update cache + newState.cacheCount[content] -= 1; + return newState; + }); + + saveToDatabase(content, false); + } + } else { + if (canAdd) { + // reaction creation fails + console.log('add', content, 'to comment id', id, 'failed.', + 'message: ', msg); + } else { + console.log('remove', content, 'from comment id', id, 'failed', + 'message:', msg); + // reaction removal fails + if (msg && msg.length && msg[0].type === 'FORBIDDEN') { + console.log('reaction removal failed due to permission error.', + 'This is probably because user has done meta-review somewhere out', + 'of gh-board.'); + this.setState((prevState) => { + let newState = prevState; + newState.canAdd[content] = true; + // clean cache + newState.cacheCount[content] = 0; + return newState; + }); + this.syncReview(); + } + } + } + } + + render() { + // id is the global identifier for the corresponding review comment + const {id, stat, hasLogin, noReactionByMe} = this.props; + + if (noReactionByMe && !Object.keys(this.state.canAdd).length) { + // use deep copy for canAdd instead of reference so that we can + // we deliberately only copy them once + this.state.canAdd = { + THUMBS_UP: noReactionByMe.THUMBS_UP, + THUMBS_DOWN: noReactionByMe.THUMBS_DOWN, + LAUGH: noReactionByMe.LAUGH, + HOORAY: noReactionByMe.HOORAY, + CONFUSED: noReactionByMe.CONFUSED, + HEART: noReactionByMe.HEART + }; } - ]; - return reactions.map(reaction => ( - - {reaction.emoji} {reaction.count} - - )); + + // props reflect real status of reactions, but may be out of date + // we need to update cached information (this.state) accordingly + if (noReactionByMe && this.state.canAdd) { + const contents = ['THUMBS_UP', 'THUMBS_DOWN', 'LAUGH', 'HOORAY', 'CONFUSED', 'HEART']; + for (const content of contents) { + if (!noReactionByMe[content] && !this.state.canAdd[content] + && this.state.cacheCount[content] === 1) { + // our action (reaction creation) is now correctly reflected by props + // need to flush cache, otherwise reaction count would be wrong + console.log('flush creation cache of content', content); + this.state.cacheCount[content] = 0; + } + if (noReactionByMe[content] && this.state.canAdd[content] + && this.state.cacheCount[content] === -1) { + // our action (reaction removal) is now correctly reflected by props + // need to flush cache, otherwise reaction count would be wrong + console.log('flush removal cache of content', content); + this.state.cacheCount[content] = 0; + } + } + } + + // use null when count is zero because we don't want to display + // number zero on frontend + const reactions = [ + { + emoji: '👍', + count: stat.THUMBS_UP + this.state.cacheCount.THUMBS_UP || null, + name: 'THUMBS_UP' + }, + { + emoji: '👎', + count: stat.THUMBS_DOWN + this.state.cacheCount.THUMBS_DOWN || null, + name: 'THUMBS_DOWN' + }, + { + emoji: '😄', + count: stat.LAUGH + this.state.cacheCount.LAUGH || null, + name: 'LAUGH' + }, + { + emoji: '🎉', + count: stat.HOORAY + this.state.cacheCount.HOORAY || null, + name: 'HOORAY' + }, + { + emoji: '😕', + count: stat.CONFUSED + this.state.cacheCount.CONFUSED || null, + name: 'CONFUSED' + }, + { + emoji: '❤️', + count: stat.HEART + this.state.cacheCount.HEART || null, + name: 'HEART' + } + ]; + return reactions.map(reaction => ( + this.onClick(id, reaction.name)} + disabled={!hasLogin}> + {reaction.emoji} {reaction.count} + + )); + } } export default Reactions; diff --git a/src/components/review.jsx b/src/components/review.jsx index 0ecc5b8b..ce084e5c 100644 --- a/src/components/review.jsx +++ b/src/components/review.jsx @@ -1,125 +1,186 @@ +import {Component} from 'react'; import * as BS from 'react-bootstrap'; import classnames from 'classnames'; import {Link} from 'react-router'; import {getFilters} from '../route-utils'; import IssueStore from '../issue-store'; +import Database from '../database'; import GithubFlavoredMarkdown from './gfm'; import Time from './time'; import ReviewBlurb from './review-blurb'; import IssueOrPullRequestBlurb from './issue-blurb'; import Reactions from './reactions'; +import withAuth from './login-auth'; -function ReviewCard(props) { - const {card, primaryRepoName} = props; - const {repoOwner, repoName, number, id, bodyText, reactions, url} = card; - - const key = `${repoOwner}/${repoName}#${number}-${id}`; - - // comment updatedAt is updated when comment content is edited. - // Note that the default `updatedAt` field of review comment - // provided by GraphQL API is inaccurate. Thus, we use our custom - // updatedAt, defined by `lastEditedAt` and `createdAt` time if never edited. - const updatedAt = card.updatedAt; - - const user = card.author; - const assignedAvatar = ( - - - - ); - // stop highlighting after 5min - const isUpdated = Date.now() - Date.parse(updatedAt) < 2 * 60 * 1000; - - // put the corresponding pull request as related card - const issueCard = IssueStore.issueNumberToCard(repoOwner, repoName, number); - const relatedCards = [issueCard].map((issueCard) => { - let title; - if (issueCard.issue) { - title = ( - {issueCard.issue.title} +class ReviewCard extends Component { + saveToDatabase = (content, isAdd) => { + // reviewCard is just part of issueCard + const {card, loginInfo} = this.props; + const {repoOwner, repoName, number} = card; + const {login} = loginInfo; + if (isAdd) { + // add a new reaction + if (!card.reactions) card.reactions = []; + card.reactions.push({ + content, + user: { + login, + }, + }); + } else { + // remove an existing reaction + card.reactions = card.reactions.filter((reaction) => { + return !(reaction.user.login === login && reaction.content === content); + }); + } + // find the corresponding issueCard + const issueCard = IssueStore.issueNumberToCard(repoOwner, repoName, number); + // update corresponding issueCard + issueCard.issue.pullRequest.comments.forEach(reviewCard => { + if (reviewCard.id === card.id) { + reviewCard = card; + } + }); + Database.putCards([issueCard]); + } + + render() { + const {card, primaryRepoName, loginInfo} = this.props; + const {repoOwner, repoName, number, id, bodyText, reactions, url} = card; + const key = `${repoOwner}/${repoName}#${number}-${id}`; + + // comment updatedAt is updated when comment content is edited. + // Note that the default `updatedAt` field of review comment + // provided by GraphQL API is inaccurate. Thus, we use our custom + // updatedAt, defined by `lastEditedAt` and `createdAt` time if never edited. + const updatedAt = card.updatedAt; + + const user = card.author; + const assignedAvatar = ( + + + + ); + // stop highlighting after 5min + const isUpdated = Date.now() - Date.parse(updatedAt) < 2 * 60 * 1000; + + // put the corresponding pull request as related card + const issueCard = IssueStore.issueNumberToCard(repoOwner, repoName, number); + const relatedCards = [issueCard].map((issueCard) => { + let title; + if (issueCard.issue) { + title = ( + {issueCard.issue.title} + ); + } + return ( +
+ + {title} +
); + }); + + const classes = { + 'review': true, + 'is-updated': isUpdated, + }; + + const header = [ + , + ]; + + let reactionsStat = { + THUMBS_UP: 0, + THUMBS_DOWN: 0, + LAUGH: 0, + HOORAY: 0, + HEART: 0, + CONFUSED: 0 + }; + if (reactions) { + reactions.forEach(reaction => reactionsStat[reaction.content]++); } + + let noReactionByMe; + let hasLogin = false; + if (loginInfo) { + hasLogin = true; + noReactionByMe = { + THUMBS_UP: true, + THUMBS_DOWN: true, + LAUGH: true, + HOORAY: true, + HEART: true, + CONFUSED: true + }; + if (reactions) { + reactions.forEach(reaction => { + if (reaction.user && reaction.user.login === loginInfo.login) { + noReactionByMe[reaction.content] = false; + } + }); + } + } + return ( -
- - {title} -
- ); - }); - - const classes = { - 'review': true, - 'is-updated': isUpdated, - }; - - const header = [ - , - ]; - - let reactionsStat = { - THUMBS_UP: 0, - THUMBS_DOWN: 0, - LAUGH: 0, - HOORAY: 0, - HEART: 0, - CONFUSED: 0 - }; - if (reactions) { - reactions.forEach(reaction => reactionsStat[reaction.content]++); - } +
+ - return ( -
- - - - - - - - - - - + + + + - - - -
- {relatedCards} + +
+ {relatedCards} +
-
- ); + ); + } } -function Review({review}) { +function Review({review, loginInfo}) { return ( - + ); } -export default Review; +export default withAuth(Review); diff --git a/src/github-graphql.js b/src/github-graphql.js index e8305617..45afacb0 100644 --- a/src/github-graphql.js +++ b/src/github-graphql.js @@ -6,6 +6,8 @@ import { GITHUB_PR_INFO_QUERY, GITHUB_LABEL_INFO_QUERY, GITHUB_REACTION_INFO_QUERY, + GITHUB_REACTION_ADD_MUTATION, + GITHUB_REACTION_REMOVE_MUTATION, } from '../script/queries'; const DEBUG = process.env.NODE_ENV === 'development'; @@ -148,6 +150,48 @@ class GraphQLClient { return this; } + // first type of mutations: add reaction + // return boolean value indicating result of action + async addReaction({id, content}) { + if (DEBUG) { + console.log('add reaction for id', id, 'with content', content); + } + let data, errors; + try { + ({ data, errors } = await this.client.query( + GITHUB_REACTION_ADD_MUTATION, + {id, content} + )); + } catch (error) { + return {result: false, msg: error}; + } + if (!data || errors) { + return {result: false, msg: errors}; + } + return {result: true, msg: data}; + } + + // second type of mutations: remove reaction + // return boolean value indicating result of action + async removeReaction({id, content}) { + if (DEBUG) { + console.log('remove reaction for id', id, 'with content', content); + } + let data, errors; + try { + ({ data, errors } = await this.client.query( + GITHUB_REACTION_REMOVE_MUTATION, + {id, content} + )); + } catch (error) { + return {result: false, msg: error}; + } + if (!data || errors) { + return {result: false, msg: errors}; + } + return {result: true, msg: data}; + } + async fetchAll(config) { const { per_page } = config || {}; this.perPage = per_page || 100; diff --git a/style/review.less b/style/review.less index efc0c6c7..650ba5f4 100644 --- a/style/review.less +++ b/style/review.less @@ -33,6 +33,10 @@ border-radius: 2px; line-height: 18px; } + .reaction-btn[disabled] { + cursor: not-allowed; + opacity: 0.65; + } .review-btn { padding: 4px 7px 3px; border-radius: 2px; diff --git a/test/src/components/__snapshots__/reactions.test.js.snap b/test/src/components/__snapshots__/reactions.test.js.snap index 6b81eb06..7dd6de8e 100644 --- a/test/src/components/__snapshots__/reactions.test.js.snap +++ b/test/src/components/__snapshots__/reactions.test.js.snap @@ -4,7 +4,8 @@ exports[`does not display number zero 1`] = ` Array [ , , , , ,