From 4f5301b3676aab9378ded64e702661ae28d5315e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 21 2017 10:45:56 +0000 Subject: Add a button and an API endpoint to subscribe to PR's notifications Fixes https://pagure.io/pagure/issue/1866 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index ebd01f3..1ec439d 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -16,7 +16,7 @@ import pagure import pagure.exceptions import pagure.lib import pagure.lib.tasks -from pagure import APP, SESSION, is_repo_committer +from pagure import APP, SESSION, is_repo_committer, api_authenticated from pagure.api import (API, api_method, api_login_required, APIERROR, get_authorized_api_project) @@ -718,3 +718,107 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): jsonout = flask.jsonify(output) return jsonout + + +@API.route( + '//pull-request//subscribe', + methods=['POST']) +@API.route( + '///pull-request//subscribe', + methods=['POST']) +@API.route( + '/fork///pull-request//subscribe', + methods=['POST']) +@API.route( + '/fork////pull-request//subscribe', + methods=['POST']) +@api_login_required(acls=['issue_subscribe']) +@api_method +def api_subscribe_pull_request(repo, requestid, username=None, namespace=None): + """ + Subscribe to an pull-request + ---------------------------- + Allows someone to subscribe to or unsubscribe from the notifications + related to a pull-request. + + :: + + POST /api/0//pull-request//subscribe + POST /api/0///pull-request//subscribe + + :: + + POST /api/0/fork///pull-request//subscribe + POST /api/0/fork////pull-request//subscribe + + Input + ^^^^^ + + +--------------+----------+---------------+---------------------------+ + | Key | Type | Optionality | Description | + +==============+==========+===============+===========================+ + | ``status`` | boolean | Mandatory | The intended subscription | + | | | | status. ``true`` for | + | | | | subscribing, ``false`` | + | | | | for unsubscribing. | + +--------------+----------+---------------+---------------------------+ + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "message": "User subscribed" + } + + """ # noqa + + repo = get_authorized_api_project( + SESSION, repo, user=username, namespace=namespace) + + output = {} + + if repo is None: + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.ENOPROJECT) + + if not repo.settings.get('pull_requests', True): + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.EPULLREQUESTSDISABLED) + + if api_authenticated(): + if flask.g.token.project and repo != flask.g.token.project: + raise pagure.exceptions.APIError( + 401, error_code=APIERROR.EINVALIDTOK) + + request = pagure.lib.search_pull_requests( + SESSION, project_id=repo.id, requestid=requestid) + + if not request: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOREQ) + + form = pagure.forms.SubscribtionForm(csrf_enabled=False) + if form.validate_on_submit(): + status = str(form.status.data).strip().lower() in ['1', 'true'] + try: + # Toggle subscribtion + message = pagure.lib.set_watch_obj( + SESSION, + user=flask.g.fas_user.username, + obj=request, + watch_status=status + ) + SESSION.commit() + output['message'] = message + except SQLAlchemyError as err: # pragma: no cover + SESSION.rollback() + APP.logger.exception(err) + raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) + + else: + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) + + jsonout = flask.jsonify(output) + return jsonout diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html index 7a3059b..800b175 100644 --- a/pagure/templates/pull_request.html +++ b/pagure/templates/pull_request.html @@ -719,7 +719,8 @@ - {% if authenticated and subscribers %} + {% if authenticated %} + {% if subscribers %}
@@ -736,6 +737,16 @@
+ {% endif %} +
+ +
{% endif %} @@ -1286,6 +1297,41 @@ function updateHighlight(onload) { } $(document).ready(function () { + {% if authenticated and pull_request %} + function set_up_subcribed() { + $("#subcribe-btn").click(function(){ + var _url = "{{ url_for( + 'api_ns.api_subscribe_pull_request', + repo=repo.name, + username=username, + namespace=repo.namespace, + requestid=pull_request.id + ) }}"; + var _btn = $("#subcribe-btn"); + var _data = {}; + if (_btn.text() == 'Unsubscribe'){ + _data.status = false; + } else { + _data.status = true; + } + console.log(_data); + $.post( _url, _data ).done( + function(data) { + var _btn = $("#subcribe-btn"); + if (_btn.text() == 'Subscribe'){ + _btn.text('Unsubscribe'); + } else { + _btn.text('Subscribe'); + } + return false; + } + ) + return false; + }); + }; + set_up_subcribed(); + {% endif %} + updateHighlight(true) {% if form %} $( "#previewinmarkdown" ).click( diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index 5cf42ca..4d13c7e 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -1249,6 +1249,173 @@ class PagureFlaskApiForktests(tests.Modeltests): self.assertEqual(request.flags[0].comment, 'Tests passed') self.assertEqual(request.flags[0].percent, 100) + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_api_subscribe_pull_request(self, p_send_email, p_ugt): + """ Test the api_subscribe_pull_request method of the flask api. """ + p_send_email.return_value = True + p_ugt.return_value = True + + item = pagure.lib.model.User( + user='bar', + fullname='bar foo', + password='foo', + default_email='bar@bar.com', + ) + self.session.add(item) + item = pagure.lib.model.UserEmail( + user_id=3, + email='bar@bar.com') + self.session.add(item) + + self.session.commit() + + tests.create_projects(self.session) + tests.create_tokens(self.session, user_id=3) + tests.create_tokens_acl(self.session) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid project + output = self.app.post( + '/api/0/foo/pull-request/1/subscribe', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Project not found", + "error_code": "ENOPROJECT", + } + ) + + # Valid token, wrong project + output = self.app.post( + '/api/0/test2/pull-request/1/subscribe', headers=headers) + self.assertEqual(output.status_code, 401) + data = json.loads(output.data) + self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.name, + data['error_code']) + self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.value, data['error']) + + # No input + output = self.app.post( + '/api/0/test/pull-request/1/subscribe', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + u'error': u'Pull-Request not found', + u'error_code': u'ENOREQ' + } + ) + + # Create pull-request + repo = pagure.get_authorized_project(self.session, 'test') + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=repo, + branch_from='feature', + repo_to=repo, + branch_to='master', + title='test pull-request', + user='pingou', + requestfolder=None, + ) + self.session.commit() + self.assertEqual(req.id, 1) + self.assertEqual(req.title, 'test pull-request') + + # Check subscribtion before + repo = pagure.get_authorized_project(self.session, 'test') + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual( + pagure.lib.get_watch_list(self.session, request), + set(['pingou'])) + + # Unsubscribe - no changes + data = {} + output = self.app.post( + '/api/0/test/pull-request/1/subscribe', + data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'You are no longer watching this pull-request'} + ) + + data = {} + output = self.app.post( + '/api/0/test/pull-request/1/subscribe', + data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'You are no longer watching this pull-request'} + ) + + # No change + repo = pagure.get_authorized_project(self.session, 'test') + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual( + pagure.lib.get_watch_list(self.session, request), + set(['pingou'])) + + # Subscribe + data = {'status': True} + output = self.app.post( + '/api/0/test/pull-request/1/subscribe', + data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'You are now watching this pull-request'} + ) + + # Subscribe - no changes + data = {'status': True} + output = self.app.post( + '/api/0/test/pull-request/1/subscribe', + data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'You are now watching this pull-request'} + ) + + repo = pagure.get_authorized_project(self.session, 'test') + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual( + pagure.lib.get_watch_list(self.session, request), + set(['pingou', 'bar'])) + + # Unsubscribe + data = {} + output = self.app.post( + '/api/0/test/pull-request/1/subscribe', + data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'You are no longer watching this pull-request'} + ) + + repo = pagure.get_authorized_project(self.session, 'test') + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual( + pagure.lib.get_watch_list(self.session, request), + set(['pingou'])) + if __name__ == '__main__': unittest.main(verbosity=2)