diff --git a/alembic/versions/e34e799e4182_add_reactions_to_issue_comments.py b/alembic/versions/e34e799e4182_add_reactions_to_issue_comments.py new file mode 100644 index 0000000..d4d2247 --- /dev/null +++ b/alembic/versions/e34e799e4182_add_reactions_to_issue_comments.py @@ -0,0 +1,29 @@ +"""add_reactions_to_comments + +Revision ID: e34e799e4182 +Revises: 131ad2dc5bbd +Create Date: 2018-05-17 18:44:32.189208 + +""" + +# revision identifiers, used by Alembic. +revision = 'e34e799e4182' +down_revision = '131ad2dc5bbd' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ''' Add the _reactions column to table issue_comments. + ''' + op.add_column( + 'issue_comments', + sa.Column('_reactions', sa.Text, nullable=True) + ) + + +def downgrade(): + ''' Remove the column _reactions from table issue_comments. + ''' + op.drop_column('issue_comments', '_reactions') diff --git a/alembic/versions/ef98dcb838e4_add_reactions_to_request_comments_py.py b/alembic/versions/ef98dcb838e4_add_reactions_to_request_comments_py.py new file mode 100644 index 0000000..d99e8cb --- /dev/null +++ b/alembic/versions/ef98dcb838e4_add_reactions_to_request_comments_py.py @@ -0,0 +1,29 @@ +"""add_reactions_to_request_comments.py + +Revision ID: ef98dcb838e4 +Revises: e34e799e4182 +Create Date: 2018-06-02 19:47:52.975503 + +""" + +# revision identifiers, used by Alembic. +revision = 'ef98dcb838e4' +down_revision = 'e34e799e4182' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ''' Add the _reactions column to table pull_request_comments. + ''' + op.add_column( + 'pull_request_comments', + sa.Column('_reactions', sa.Text, nullable=True) + ) + + +def downgrade(): + ''' Remove the column _reactions from table pull_request_comments. + ''' + op.drop_column('pull_request_comments', '_reactions') diff --git a/pagure/default_config.py b/pagure/default_config.py index 24db249..8701134 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -422,3 +422,14 @@ EXTERNAL_COMMITTER = {} # Allows to require that the users are members of a certain group to be added # to a project (not a fork). REQUIRED_GROUPS = {} + +# Predefined reactions. Selecting others is possible by typing their name. The +# order here will be preserved in the web UI picker for reactions. +REACTIONS = [ + ("Thumbs up", "emojione-1F44D"), # Thumbs up + ("Thumbs down", "emojione-1F44E"), # Thumbs down + ("Confused", "emojione-1F615"), # Confused + ("Heart", "emojione-2764"), # Heart +] +# This is used for faster indexing. Do not change. +_REACTIONS_DICT = dict(REACTIONS) diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 2d35c6c..e6640e4 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1416,6 +1416,8 @@ class IssueComment(BASE): foreign_keys=[editor_id], remote_side=[User.id]) + _reactions = sa.Column(sa.Text, nullable=True) + @property def mail_id(self): ''' Return a unique reprensetation of the issue as string that @@ -1429,6 +1431,20 @@ class IssueComment(BASE): ''' Return the parent, in this case the issue object. ''' return self.issue + @property + def reactions(self): + ''' Return the reactions stored as a string in the database parsed as + an actual dict object. + ''' + if self._reactions: + return json.loads(self._reactions) + return {} + + @reactions.setter + def reactions(self, reactions): + ''' Ensures that reactions are properly saved. ''' + self._reactions = json.dumps(reactions) + def to_json(self, public=False): ''' Returns a dictionary representation of the issue. @@ -1443,6 +1459,7 @@ class IssueComment(BASE): 'editor': self.editor.to_json(public=public) if self.editor_id else None, 'notification': self.notification, + 'reactions': self.reactions, } return output @@ -2000,6 +2017,8 @@ class PullRequestComment(BASE): foreign_keys=[editor_id], remote_side=[User.id]) + _reactions = sa.Column(sa.Text, nullable=True) + @property def mail_id(self): ''' Return a unique representation of the issue as string that @@ -2013,6 +2032,20 @@ class PullRequestComment(BASE): ''' Return the parent, in this case the pull_request object. ''' return self.pull_request + @property + def reactions(self): + ''' Return the reactions stored as a string in the database parsed as + an actual dict object. + ''' + if self._reactions: + return json.loads(self._reactions) + return {} + + @reactions.setter + def reactions(self, reactions): + ''' Ensures that reactions are properly saved. ''' + self._reactions = json.dumps(reactions) + def to_json(self, public=False): ''' Return a dict representation of the pull-request comment. ''' @@ -2030,6 +2063,7 @@ class PullRequestComment(BASE): 'editor': self.editor.to_json(public=public) if self.editor_id else None, 'notification': self.notification, + 'reactions': self.reactions, } diff --git a/pagure/static/reactions.js b/pagure/static/reactions.js new file mode 100644 index 0000000..5411a95 --- /dev/null +++ b/pagure/static/reactions.js @@ -0,0 +1,37 @@ +(function () { + function send_reaction(commentid, reaction, emoji) { + var _url = location.href + '/comment/' + commentid + '/react'; + var csrf_token = $("#csrf_token").val(); + $.ajax({ + url: _url + '?js=1', + type: 'POST', + data: { + 'reaction': reaction, + 'csrf_token': csrf_token, + }, + success: function () { + var reactions = $(".issue_reactions[data-comment-id=" + commentid + "]>.btn-group"); + var selected = reactions.find("span[class=" + emoji + "]"); + if (selected.length > 0) { + var counter = selected.next(); + counter.text(parseInt(counter.text(), 10) + 1); + } else { + reactions.append( + ''); + } + }, + }); + } + + $(document).ready(function () { + $(".reaction-picker button").click(function () { + var commentid = $(this).parents('.reaction-picker').data('comment-id'); + var reaction = $(this).attr('title'); + var emoji = $(this).find('span').attr('class'); + send_reaction(commentid, reaction, emoji); + }); + }); +})(); diff --git a/pagure/templates/_formhelper.html b/pagure/templates/_formhelper.html index 6b8d21d..86f08a7 100644 --- a/pagure/templates/_formhelper.html +++ b/pagure/templates/_formhelper.html @@ -142,9 +142,37 @@ {% if comment.edited_on %} Edited {{ comment.edited_on | humanize }} by {{ comment.editor.username }} {% endif %} +
+
+ {% for r in comment.reactions | sort %} + + {% endfor %} +
+
{% if id != 0 and g.fas_user %} + {% if config.get('REACTIONS') %} +
+ +
+ {% for label, emoji in config.get('REACTIONS') %} + + {% endfor %} +
+
+ {% endif %} diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index d0dec58..190efc6 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -1046,5 +1046,6 @@ $( document ).ready(function() { {% if repo.quick_replies %} {% endif %} + {% endblock %} diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html index 00cf0a0..69de47c 100644 --- a/pagure/templates/pull_request.html +++ b/pagure/templates/pull_request.html @@ -1729,5 +1729,6 @@ sse = false; {% if repo.quick_replies %} {% endif %} + {% endblock %} diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index e725fa1..7d25b3d 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -682,3 +682,15 @@ def flag_to_label(flag): """ For a given flag return the bootstrap label to use """ return pagure_config['FLAG_STATUSES_LABELS'][flag.status.lower()] + + +@UI_NS.app_template_filter('join_prefix') +def join_prefix(values, num): + """Produce a string joining first `num` items in the list and indicate + total number total number of items. + """ + if len(values) <= 1: + return "".join(values) + if len(values) <= num: + return ", ".join(values[:-1]) + " and " + values[-1] + return "%s and %d others" % (", ".join(values[:num]), len(values) - num) diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 38ab14e..13e804a 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -1556,3 +1556,57 @@ def fork_edit_file( return flask.redirect(flask.url_for( 'ui_ns.view_repo', repo=repo.name, username=username, namespace=namespace)) + + +@UI_NS.route( + '//pull-request//comment//react', methods=['POST']) +@UI_NS.route( + '///pull-request//comment//react', + methods=['POST']) +@UI_NS.route( + '/fork///pull-request//comment//react', + methods=['POST']) +@UI_NS.route( + '/fork////pull-request//comment//react', + methods=['POST']) +@login_required +def pull_request_comment_add_reaction(repo, requestid, commentid, + username=None, namespace=None): + repo = flask.g.repo + + form = pagure.forms.ConfirmationForm() + if not form.validate_on_submit(): + flask.abort(400, 'CSRF token not valid') + + request = pagure.lib.search_pull_requests( + flask.g.session, requestid=requestid, project_id=repo.id + ) + + if not request: + flask.abort(404, 'Comment not found') + + comment = pagure.lib.get_request_comment( + flask.g.session, request.uid, commentid) + + if 'reaction' not in flask.request.form: + flask.abort(400, 'Reaction not found') + + reactions = comment.reactions + r = flask.request.form['reaction'] + if not r: + flask.abort(400, 'Empty reaction is not acceptable') + if flask.g.fas_user.username in reactions.get(r, []): + flask.abort(409, 'Already posted this one') + + reactions.setdefault(r, []).append(flask.g.fas_user.username) + comment.reactions = reactions + flask.g.session.add(comment) + + try: + flask.g.session.commit() + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.error(err) + return 'error' + + return 'ok' diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index d440768..a46b60b 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -365,6 +365,79 @@ def update_issue(repo, issueid, username=None, namespace=None): ) +@UI_NS.route( + '//issue//comment//react/', + methods=['POST']) +@UI_NS.route( + '//issue//comment//react', + methods=['POST']) +@UI_NS.route( + '///issue//comment//react/', + methods=['POST']) +@UI_NS.route( + '///issue//comment//react', + methods=['POST']) +@UI_NS.route( + '/fork///issue//comment//react/', + methods=['POST']) +@UI_NS.route( + '/fork///issue//comment//react', + methods=['POST']) +@UI_NS.route( + '/fork////issue//comment//react/', + methods=['POST']) +@UI_NS.route( + '/fork////issue//comment//react', + methods=['POST']) +@login_required +@has_issue_tracker +def issue_comment_add_reaction(repo, issueid, commentid, username=None, + namespace=None): + '''Add a reaction to a comment of an issue''' + repo = flask.g.repo + + issue = pagure.lib.search_issues(flask.g.session, repo, issueid=issueid) + + if not issue or issue.project != repo: + flask.abort(404, 'Comment not found') + + form = pagure.forms.ConfirmationForm() + if not form.validate_on_submit(): + flask.abort(400, 'CSRF token not valid') + + if issue.private and not flask.g.repo_committer \ + and (not authenticated() or + not issue.user.user == flask.g.fas_user.username): + flask.abort( + 404, 'No such issue') + + comment = pagure.lib.get_issue_comment( + flask.g.session, issue.uid, commentid) + + if 'reaction' not in flask.request.form: + flask.abort(400, 'Reaction not found') + + reactions = comment.reactions + r = flask.request.form['reaction'] + if not r: + flask.abort(400, 'Empty reaction is not acceptable') + if flask.g.fas_user.username in reactions.get(r, []): + flask.abort(409, 'Already posted this one') + + reactions.setdefault(r, []).append(flask.g.fas_user.username) + comment.reactions = reactions + flask.g.session.add(comment) + + try: + flask.g.session.commit() + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.error(err) + return 'error' + + return 'ok' + + @UI_NS.route('//tag//edit/', methods=('GET', 'POST')) @UI_NS.route('//tag//edit', methods=('GET', 'POST')) @UI_NS.route('///tag//edit/', methods=('GET', 'POST')) diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index c38b0ae..3f6f685 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -2231,6 +2231,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): "notification": False, "id": 1, "parent": None, + "reactions": {}, "user": { "fullname": "PY C", "name": "pingou" @@ -2257,6 +2258,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): "notification": False, "id": 1, "parent": None, + "reactions": {}, "user": { "fullname": "PY C", "name": "pingou" @@ -2351,6 +2353,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): "notification": False, "id": 1, "parent": None, + "reactions": {}, "user": { "fullname": "foo bar", "name": "foo" diff --git a/tests/test_pagure_flask_api_ui_private_repo.py b/tests/test_pagure_flask_api_ui_private_repo.py index 4d7ee7c..d95d351 100644 --- a/tests/test_pagure_flask_api_ui_private_repo.py +++ b/tests/test_pagure_flask_api_ui_private_repo.py @@ -3093,6 +3093,7 @@ class PagurePrivateRepotest(tests.Modeltests): "editor": None, "id": 1, "parent": None, + "reactions": {}, "user": { "fullname": "PY C", "name": "pingou" @@ -3119,6 +3120,7 @@ class PagurePrivateRepotest(tests.Modeltests): "editor": None, "id": 1, "parent": None, + "reactions": {}, "user": { "fullname": "PY C", "name": "pingou" diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index bce6766..468496e 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -3172,5 +3172,92 @@ index 0000000..2a552bb shutil.rmtree(newpath) + def _set_up_for_reaction_test(self): + self.session.add(pagure.lib.model.User( + user='jdoe', + fullname='John Doe', + password=b'password', + default_email='jdoe@example.com', + )) + self.session.commit() + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + self.set_up_git_repo(new_project=None, branch_from='feature') + pagure.lib.get_authorized_project(self.session, 'test') + request = pagure.lib.search_pull_requests( + self.session, requestid=1, project_id=1, + ) + pagure.lib.add_pull_request_comment( + self.session, + request=request, + commit=None, + tree_id=None, + filename=None, + row=None, + comment='Hello', + user='jdoe', + requestfolder=None, + ) + self.session.commit() + + @patch('pagure.lib.notify.send_email') + def test_add_reaction(self, send_email): + """ Test the request_pull endpoint. """ + send_email.return_value = True + + self._set_up_for_reaction_test() + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + + data = { + 'csrf_token': self.get_csrf(output=output), + 'reaction': 'Thumbs up', + } + + output = self.app.post( + '/test/pull-request/1/comment/1/react', + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 200) + + # Load the page and check reaction is added. + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Thumbs up sent by pingou', + output.get_data(as_text=True) + ) + + @patch('pagure.lib.notify.send_email') + def test_add_reaction_unauthenticated(self, send_email): + """ Test the request_pull endpoint. """ + send_email.return_value = True + + self._set_up_for_reaction_test() + + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + + data = { + 'csrf_token': self.get_csrf(output=output), + 'reaction': 'Thumbs down', + } + + output = self.app.post( + '/test/pull-request/1/comment/1/react', + data=data, + follow_redirects=False, + ) + # Redirect to login page + self.assertEqual(output.status_code, 302) + self.assertIn('/login/', output.headers['Location']) + + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index 6622c5e..c07590c 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -3807,6 +3807,126 @@ class PagureFlaskIssuestests(tests.Modeltests): 'title="Reply to this comment - lose formatting">', ), 1) + def _set_up_for_reaction_test(self, private=False): + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) + + self.session.add(pagure.lib.model.User( + user='naysayer', + fullname='John Doe', + password=b'password', + default_email='jdoe@example.com', + )) + self.session.commit() + repo = pagure.lib.get_authorized_project(self.session, 'test') + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue', + content='Fix me', + user='pingou', + ticketfolder=None, + private=private, + ) + pagure.lib.add_issue_comment( + session=self.session, + issue=msg, + comment='How about no', + user='naysayer', + ticketfolder=None, + ) + self.session.commit() + + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_add_reaction(self, p_send_email, p_ugt): + ''' Test adding a reaction to an issue comment.''' + p_send_email.return_value = True + p_ugt.return_value = True + + self._set_up_for_reaction_test() + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + output = self.app.get('/test/issue/1') + self.assertEqual(output.status_code, 200) + + data = { + 'csrf_token': self.get_csrf(output=output), + 'reaction': 'Thumbs down', + } + + output = self.app.post( + '/test/issue/1/comment/1/react', + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 200) + + # Load the page and check reaction is added. + output = self.app.get('/test/issue/1') + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Thumbs down sent by pingou', + output.get_data(as_text=True) + ) + + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_add_reaction_unauthenticated(self, p_send_email, p_ugt): + ''' + Test adding a reaction to an issue comment without authentication. + ''' + p_send_email.return_value = True + p_ugt.return_value = True + + self._set_up_for_reaction_test() + + output = self.app.get('/test/issue/1') + self.assertEqual(output.status_code, 200) + + data = { + 'csrf_token': self.get_csrf(output=output), + 'reaction': 'Thumbs down', + } + + output = self.app.post( + '/test/issue/1/comment/1/react', + data=data, + follow_redirects=False, + ) + # Redirect to login page + self.assertEqual(output.status_code, 302) + self.assertIn('/login/', output.headers['Location']) + + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_add_reaction_private_issue(self, p_send_email, p_ugt): + '''Test adding a reaction to a private issue comment.''' + p_send_email.return_value = True + p_ugt.return_value = True + + self._set_up_for_reaction_test(private=True) + + user = tests.FakeUser() + user.username = 'naysayer' + with tests.user_set(self.app.application, user): + # Steal CSRF token from new issue page + output = self.app.get('/test/new_issue') + + data = { + 'csrf_token': self.get_csrf(output=output), + 'reaction': 'Thumbs down', + } + + output = self.app.post( + '/test/issue/1/comment/1/react', + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 404) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/tests/test_pagure_lib_git.py b/tests/test_pagure_lib_git.py index 82dffd0..09efc57 100644 --- a/tests/test_pagure_lib_git.py +++ b/tests/test_pagure_lib_git.py @@ -1687,7 +1687,7 @@ diff --git a/123 b/456 index 458821a..77674a8 --- a/123 +++ b/456 -@@ -3,13 +3,31 @@ +@@ -3,13 +3,32 @@ "blocks": [], "close_status": null, "closed_at": null, @@ -1701,6 +1701,7 @@ index 458821a..77674a8 + "id": 1, + "notification": false, + "parent": null, ++ "reactions": {}, + "user": { + "default_email": "foo@bar.com", + "emails": [