From 8ef7f22166dc55232960ac1c62c446e50fea507c Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 14:20:36 +0000 Subject: [PATCH 1/23] Always specify the private flag otherwise we end-up changing it --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 29de606..a20712b 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -172,6 +172,7 @@ def update_issue(repo, issueid, username=None): SESSION, issue=issue, status=new_status, + private=issue.private, user=flask.g.fas_user.username, ticketfolder=APP.config['TICKETS_FOLDER'], redis=REDIS, From b7e379dd965764aaa7d3bfbc18f1e078790bd42d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 14:22:23 +0000 Subject: [PATCH 2/23] Send a specific message when someone updates a private ticket --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 57b238a..e38d75d 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -185,13 +185,17 @@ def add_issue_comment(session, issue, comment, user, ticketfolder, ) if redis: - redis.publish(issue.uid, json.dumps({ - 'comment_id': len(issue.comments), - 'comment_added': text2markdown(issue_comment.comment), - 'comment_user': issue_comment.user.user, - 'avatar_url': avatar_url(issue_comment.user.user, size=16), - 'comment_date': issue_comment.date_created.strftime('%Y-%m-%d %H:%M'), - })) + if issue.private: + redis.publish(issue.uid, json.dumps({'issue': 'private'})) + else: + redis.publish(issue.uid, json.dumps({ + 'comment_id': len(issue.comments), + 'comment_added': text2markdown(issue_comment.comment), + 'comment_user': issue_comment.user.user, + 'avatar_url': avatar_url(issue_comment.user.user, size=16), + 'comment_date': issue_comment.date_created.strftime( + '%Y-%m-%d %H:%M'), + })) return 'Comment added' @@ -1020,10 +1024,13 @@ def edit_issue(session, issue, ticketfolder, user, ) if redis and edit: - redis.publish(issue.uid, json.dumps({ - 'fields': edit, - 'issue': issue.to_json(public=True, with_comments=False), - })) + if issue.private: + redis.publish(issue.uid, json.dumps({'issue': 'private'})) + else: + redis.publish(issue.uid, json.dumps({ + 'fields': edit, + 'issue': issue.to_json(public=True, with_comments=False), + })) if edit: session.add(issue) From 0d9165063c05430095db53ec1b54aab1fd640584 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:13:30 +0000 Subject: [PATCH 3/23] Specify the identifier of the comment added to the ticket in the redis message --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index e38d75d..351df66 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -186,7 +186,10 @@ def add_issue_comment(session, issue, comment, user, ticketfolder, if redis: if issue.private: - redis.publish(issue.uid, json.dumps({'issue': 'private'})) + redis.publish(issue.uid, json.dumps({ + 'issue': 'private', + 'comment_id': issue_comment.id, + })) else: redis.publish(issue.uid, json.dumps({ 'comment_id': len(issue.comments), From 887974a74ba2059ee4ac7134e60a4ab05814cae8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:14:28 +0000 Subject: [PATCH 4/23] Add a new API endpoint returning a specific comment of a specific ticket This is used by the JS logic handling the EV messages as a way to update private tickets. Private changes are not broadcasted via the EV, redis just says, something changed and JS will call this endpoint and if you have the proper authorization it will refresh the page. --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 2ac5ddd..4c82135 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -354,6 +354,73 @@ def api_view_issue(repo, issueid, username=None): return jsonout +@API.route('//issue//comment/') +@API.route('/fork///issue//comment/') +@api_login_optional() +@api_method +def api_view_issue_comment(repo, issue_uid, commentid, username=None): + """ + Comment of a ticket + ------------------- + Retrieve a specific comment of a ticket. + + :: + + GET /api/0//issue//comment/ + + :: + + GET /api/0/fork///issue//comment/ + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "avatar_url": "https://seccdn.libravatar.org/avatar/...?s=16&d=retro", + "comment": "9", + "comment_date": "2015-07-01 15:08", + "date_created": "1435756127", + "id": 464, + "parent": null, + "user": { + "fullname": "P.-Y.C.", + "name": "pingou" + } + } + + """ + + comment = pagure.lib.get_issue_comment(SESSION, issue_uid, commentid) + + if comment is None: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) + + if not comment.issue.project.settings.get('issue_tracker', True): + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.ETRACKERDISABLED) + + if api_authenticated(): + if repo != flask.g.token.project: + raise pagure.exceptions.APIError( + 401, error_code=APIERROR.EINVALIDTOK) + + if comment.issue.private and not is_repo_admin(comment.issue.project) \ + and (not api_authenticated() or + not comment.issue.user.user == flask.g.fas_user.username): + raise pagure.exceptions.APIError( + 403, error_code=APIERROR.EISSUENOTALLOWED) + + + output = comment.to_json(public=True) + output['avatar_url'] = pagure.lib.avatar_url(comment.user.user, size=16) + output['comment_date'] = comment.date_created.strftime('%Y-%m-%d %H:%M') + jsonout = flask.jsonify(output) + return jsonout + + + @API.route('//issue//status', methods=['POST']) @API.route('/fork////status', methods=['POST']) @api_login_required(acls=['issue_change_status']) From e814b5a4e7414ceaba2f7427037871dd1a74998a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:17:16 +0000 Subject: [PATCH 5/23] Handle adding new comments to a private ticket using SSE and the API --- diff --git a/pagure/static/issue_ev.js b/pagure/static/issue_ev.js index 52f7f0f..867c44d 100644 --- a/pagure/static/issue_ev.js +++ b/pagure/static/issue_ev.js @@ -162,7 +162,29 @@ update_issue = function(data) { } } -process_event = function(data, issue_uid, _issue_url, _issues_url){ +private_issue = function(data, _api_issue_url, issue_uid) { + if (data.comment_id){ + var _url = _api_issue_url.replace('-1', issue_uid) + + '/comment/' + data.comment_id; + console.log(_url); + + $.get( _url ) + .done(function(data) { + add_comment({ + comment_added: data.comment, + comment_id: data.id, + comment_user: data.user.name, + comment_date: data.comment_date, + avatar_url: data.avatar_url, + }); + }) + } + +} + +process_event = function( + data, issue_uid, _issue_url, _issues_url, _api_issue_url) +{ console.log(data); if (data.added_tags){ add_tags(data, _issues_url); @@ -188,4 +210,8 @@ process_event = function(data, issue_uid, _issue_url, _issues_url){ else if (data.fields){ update_issue(data); } + else if (data.issue == 'private'){ + console.log('private issue'); + private_issue(data, _api_issue_url, issue_uid) + } } diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index 892210d..a08d85b 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -349,9 +349,13 @@ source.addEventListener('message', function(e) { var data = $.parseJSON(e.data); var _issues_url =''; - process_event(data, "{{ issue.uid }}", _issue_url, _issues_url); + process_event(data, "{{ issue.uid }}", _issue_url, + _issues_url, _api_issues_url); }, false); From 59df716e372a7f7fce337452deac242353f6679c Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:17:22 +0000 Subject: [PATCH 6/23] Use the commit identifier in the anchors in the html instead of their place in the loop --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 351df66..49adaa8 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -192,7 +192,7 @@ def add_issue_comment(session, issue, comment, user, ticketfolder, })) else: redis.publish(issue.uid, json.dumps({ - 'comment_id': len(issue.comments), + 'comment_id': issue_comment.id, 'comment_added': text2markdown(issue_comment.comment), 'comment_user': issue_comment.user.user, 'avatar_url': avatar_url(issue_comment.user.user, size=16), diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index a08d85b..717230c 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -126,7 +126,7 @@
{% if issue.comments %} {% for comment in issue.comments %} - {{ show_comment(comment, loop.index, repo, username, issueid, form, repo_admin) }} + {{ show_comment(comment, comment.id, repo, username, issueid, form, repo_admin) }} {% endfor %} {% endif %}
From b585e1d4bc1c628a52f105d129db251b578a94fb Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:33:24 +0000 Subject: [PATCH 7/23] Adjust pagure.lib.search_issues to support searching issue by their uid --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 49adaa8..e84afa0 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1268,8 +1268,9 @@ def get_project(session, name, user=None): def search_issues( - session, repo, issueid=None, status=None, closed=False, tags=None, - assignee=None, author=None, private=None, count=False): + session, repo, issueid=None, issueuid=None, status=None, + closed=False, tags=None, assignee=None, author=None, private=None, + count=False): ''' Retrieve one or more issues associated to a project with the given criterias. @@ -1287,6 +1288,8 @@ def search_issues( :type repo: pagure.lib.model.Project :kwarg issueid: the identifier of the issue to look for :type issueid: int or None + :kwarg issueuid: the unique identifier of the issue to look for + :type issueuid: str or None :kwarg status: the status of the issue to look for (incompatible with the `closed` argument). :type status: str or None @@ -1326,6 +1329,11 @@ def search_issues( model.Issue.id == issueid ) + if issueuid is not None: + query = query.filter( + model.Issue.uid == issueuid + ) + if status is not None and not closed: query = query.filter( model.Issue.status == status @@ -1438,7 +1446,7 @@ def search_issues( model.Issue.id ) - if issueid is not None: + if issueid is not None or issueuid is not None: output = query.first() elif count: output = query.count() From f85bfc992427c1eeaa05fc3e73d5f9269893e071 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:33:55 +0000 Subject: [PATCH 8/23] Allow the api_view_issue endpoint to work for both regular id and unique id --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 4c82135..dca5fb5 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -282,8 +282,8 @@ def api_view_issues(repo, username=None): return jsonout -@API.route('//issue/') -@API.route('/fork///issue/') +@API.route('//issue/') +@API.route('/fork///issue/') @api_login_optional() @api_method def api_view_issue(repo, issueid, username=None): @@ -300,6 +300,10 @@ def api_view_issue(repo, issueid, username=None): GET /api/0/fork///issue/ + The identifier provided can be either the unique identifier or the + regular identifier used in the UI (for example ``24`` in + ``/forks/user/test/issue/24``) + Sample response ^^^^^^^^^^^^^^^ @@ -324,6 +328,9 @@ def api_view_issue(repo, issueid, username=None): } """ + comments = flask.request.args.get('comments', True) + if str(comments).lower() in ['0', 'False']: + comments = False repo = pagure.lib.get_project(SESSION, repo, user=username) @@ -334,7 +341,14 @@ def api_view_issue(repo, issueid, username=None): raise pagure.exceptions.APIError( 404, error_code=APIERROR.ETRACKERDISABLED) - issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid) + issue_id = issue_uid = None + try: + issue_id = int(issueid) + except: + issue_uid = issueid + + issue = pagure.lib.search_issues( + SESSION, repo, issueid=issue_id, issueuid=issue_uid) if issue is None or issue.project != repo: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE) @@ -350,7 +364,8 @@ def api_view_issue(repo, issueid, username=None): raise pagure.exceptions.APIError( 403, error_code=APIERROR.EISSUENOTALLOWED) - jsonout = flask.jsonify(issue.to_json(public=True)) + jsonout = flask.jsonify( + issue.to_json(public=True, with_comments=comments)) return jsonout From ff274a6fbd16408c48915919c241bc804d0f6679 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:34:50 +0000 Subject: [PATCH 9/23] On private ticket, publish which fields changed (but not how) --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index e84afa0..4cd76eb 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1028,7 +1028,10 @@ def edit_issue(session, issue, ticketfolder, user, if redis and edit: if issue.private: - redis.publish(issue.uid, json.dumps({'issue': 'private'})) + redis.publish(issue.uid, json.dumps({ + 'issue': 'private', + 'fields': edit, + })) else: redis.publish(issue.uid, json.dumps({ 'fields': edit, From 05eb63fc21a48c4b9085facf0464eb4de57b9dc3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:35:30 +0000 Subject: [PATCH 10/23] Check if the update if for a private ticket before doing anything --- diff --git a/pagure/static/issue_ev.js b/pagure/static/issue_ev.js index 867c44d..f5af91a 100644 --- a/pagure/static/issue_ev.js +++ b/pagure/static/issue_ev.js @@ -186,7 +186,11 @@ process_event = function( data, issue_uid, _issue_url, _issues_url, _api_issue_url) { console.log(data); - if (data.added_tags){ + if (data.issue == 'private'){ + console.log('private issue'); + private_issue(data, _api_issue_url, issue_uid) + } + else if (data.added_tags){ add_tags(data, _issues_url); } else if (data.removed_tags){ @@ -210,8 +214,4 @@ process_event = function( else if (data.fields){ update_issue(data); } - else if (data.issue == 'private'){ - console.log('private issue'); - private_issue(data, _api_issue_url, issue_uid) - } } From 5f5a8ce04f41b224151f07edb1e5b4ec08f2e0a7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:35:57 +0000 Subject: [PATCH 11/23] Fix indentation --- diff --git a/pagure/static/issue_ev.js b/pagure/static/issue_ev.js index f5af91a..7c73ee1 100644 --- a/pagure/static/issue_ev.js +++ b/pagure/static/issue_ev.js @@ -171,12 +171,12 @@ private_issue = function(data, _api_issue_url, issue_uid) { $.get( _url ) .done(function(data) { add_comment({ - comment_added: data.comment, - comment_id: data.id, - comment_user: data.user.name, - comment_date: data.comment_date, - avatar_url: data.avatar_url, - }); + comment_added: data.comment, + comment_id: data.id, + comment_user: data.user.name, + comment_date: data.comment_date, + avatar_url: data.avatar_url, + }); }) } From e26b087ab876ce871319e9371b4406845e1ff877 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:36:06 +0000 Subject: [PATCH 12/23] Add logic to refresh a private ticket after an edit --- diff --git a/pagure/static/issue_ev.js b/pagure/static/issue_ev.js index 7c73ee1..04007ed 100644 --- a/pagure/static/issue_ev.js +++ b/pagure/static/issue_ev.js @@ -178,6 +178,20 @@ private_issue = function(data, _api_issue_url, issue_uid) { avatar_url: data.avatar_url, }); }) + } else if (data.fields) { + var _url = _api_issue_url.replace('-1', issue_uid) + '?comments=0'; + console.log(_url); + $.get( _url ) + .done(function(ndata) { + update_issue({ + fields: data.fields, + issue: { + status: ndata.status, + title: ndata.title, + content: ndata.content, + } + }); + }) } } From d79ff7f777165b3768fcc397f8fb9938e5187d6b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 15:43:42 +0000 Subject: [PATCH 13/23] Reduce the spam level in the logs --- diff --git a/pagure/static/issue_ev.js b/pagure/static/issue_ev.js index 04007ed..ef9b032 100644 --- a/pagure/static/issue_ev.js +++ b/pagure/static/issue_ev.js @@ -166,8 +166,6 @@ private_issue = function(data, _api_issue_url, issue_uid) { if (data.comment_id){ var _url = _api_issue_url.replace('-1', issue_uid) + '/comment/' + data.comment_id; - console.log(_url); - $.get( _url ) .done(function(data) { add_comment({ @@ -180,7 +178,6 @@ private_issue = function(data, _api_issue_url, issue_uid) { }) } else if (data.fields) { var _url = _api_issue_url.replace('-1', issue_uid) + '?comments=0'; - console.log(_url); $.get( _url ) .done(function(ndata) { update_issue({ From b686558b3faac5d6ed3695dce0a2d74df7391b9b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 01 2015 16:02:05 +0000 Subject: [PATCH 14/23] Make api_view_issue_comment more flexible by allowing it to use regular and unique identifiers --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index dca5fb5..33abaff 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -369,11 +369,11 @@ def api_view_issue(repo, issueid, username=None): return jsonout -@API.route('//issue//comment/') -@API.route('/fork///issue//comment/') +@API.route('//issue//comment/') +@API.route('/fork///issue//comment/') @api_login_optional() @api_method -def api_view_issue_comment(repo, issue_uid, commentid, username=None): +def api_view_issue_comment(repo, issueid, commentid, username=None): """ Comment of a ticket ------------------- @@ -381,11 +381,15 @@ def api_view_issue_comment(repo, issue_uid, commentid, username=None): :: - GET /api/0//issue//comment/ + GET /api/0//issue//comment/ :: - GET /api/0/fork///issue//comment/ + GET /api/0/fork///issue//comment/ + + The identifier provided can be either the unique identifier or the + regular identifier used in the UI (for example ``24`` in + ``/forks/user/test/issue/24``) Sample response ^^^^^^^^^^^^^^^ @@ -407,26 +411,39 @@ def api_view_issue_comment(repo, issue_uid, commentid, username=None): """ - comment = pagure.lib.get_issue_comment(SESSION, issue_uid, commentid) + repo = pagure.lib.get_project(SESSION, repo, user=username) - if comment is None: + if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) - if not comment.issue.project.settings.get('issue_tracker', True): + if not repo.settings.get('issue_tracker', True): raise pagure.exceptions.APIError( 404, error_code=APIERROR.ETRACKERDISABLED) + issue_id = issue_uid = None + try: + issue_id = int(issueid) + except: + issue_uid = issueid + + issue = pagure.lib.search_issues( + SESSION, repo, issueid=issue_id, issueuid=issue_uid) + + if issue is None or issue.project != repo: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE) + if api_authenticated(): if repo != flask.g.token.project: raise pagure.exceptions.APIError( 401, error_code=APIERROR.EINVALIDTOK) - if comment.issue.private and not is_repo_admin(comment.issue.project) \ + if issue.private and not is_repo_admin(issue.project) \ and (not api_authenticated() or - not comment.issue.user.user == flask.g.fas_user.username): + not issue.user.user == flask.g.fas_user.username): raise pagure.exceptions.APIError( 403, error_code=APIERROR.EISSUENOTALLOWED) + comment = pagure.lib.get_issue_comment(SESSION, issue.uid, commentid) output = comment.to_json(public=True) output['avatar_url'] = pagure.lib.avatar_url(comment.user.user, size=16) @@ -435,7 +452,6 @@ def api_view_issue_comment(repo, issue_uid, commentid, username=None): return jsonout - @API.route('//issue//status', methods=['POST']) @API.route('/fork////status', methods=['POST']) @api_login_required(acls=['issue_change_status']) From 23b769dfdb5c97ef3456ea415d8c5a63938b412b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 02 2015 08:23:28 +0000 Subject: [PATCH 15/23] Adjust docstring on api_view_issue_comment --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 33abaff..4c63659 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -375,9 +375,9 @@ def api_view_issue(repo, issueid, username=None): @api_method def api_view_issue_comment(repo, issueid, commentid, username=None): """ - Comment of a ticket - ------------------- - Retrieve a specific comment of a ticket. + Comment of an issue + -------------------- + Retrieve a specific comment of an issue. :: From 9f0f93022887d8f3084fc27aff13ce3c267859f4 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 02 2015 08:23:36 +0000 Subject: [PATCH 16/23] Document the new API endpoint api_view_issue_comment --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index ec0ba76..020e01c 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -390,6 +390,7 @@ def api(): api_new_issue_doc = load_doc(issue.api_new_issue) api_view_issue_doc = load_doc(issue.api_view_issue) + api_view_issue_comment_doc = load_doc(issue.api_view_issue_comment) api_view_issues_doc = load_doc(issue.api_view_issues) api_issue_add_comment_doc = load_doc(issue.api_comment_issue) @@ -417,6 +418,7 @@ def api(): api_new_issue_doc, api_view_issues_doc, api_view_issue_doc, + api_view_issue_comment_doc, api_issue_add_comment_doc, ], requests=[ From 7b4f84dec792da854603d5a2ac0c1a75e6b58351 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 02 2015 09:09:17 +0000 Subject: [PATCH 17/23] Adjust the unit-tests to test accessing an issue via it's uid --- diff --git a/tests/test_progit_flask_api_issue.py b/tests/test_progit_flask_api_issue.py index 81347e4..0aec6db 100644 --- a/tests/test_progit_flask_api_issue.py +++ b/tests/test_progit_flask_api_issue.py @@ -433,6 +433,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): user='pingou', ticketfolder=None, private=True, + issue_uid='aaabbbccc', ) self.session.commit() self.assertEqual(msg.title, 'Test issue') @@ -515,6 +516,32 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) + # Access private issue authenticated correctly using the issue's uid + output = self.app.get('/api/0/test/issue/aaabbbccc', headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['date_created'] = '1431414800' + self.assertDictEqual( + data, + { + "assignee": None, + "blocks": [], + "comments": [], + "content": "We should work on this", + "date_created": "1431414800", + "depends": [], + "id": 2, + "private": True, + "status": "Open", + "tags": [], + "title": "Test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + } + ) + def test_api_change_status_issue(self): """ Test the api_change_status_issue method of the flask api. """ tests.create_projects(self.session) From 060662218f87d6c20b6b3729dd071fa399ce6b65 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 02 2015 11:06:08 +0000 Subject: [PATCH 18/23] Add and raise an API error code if there was no comment found for the given info --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index 020e01c..4c2e2e1 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -48,6 +48,7 @@ class APIERROR(enum.Enum): ENOTASSIGNEE = 'Only the assignee can merge this review' ENOTASSIGNED = 'This request must be assigned to be merged' ENOUSER = 'No such user found' + ENOCOMMENT = 'Comment not found' def check_api_acls(acls, optional=False): diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 4c63659..32eba50 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -444,6 +444,9 @@ def api_view_issue_comment(repo, issueid, commentid, username=None): 403, error_code=APIERROR.EISSUENOTALLOWED) comment = pagure.lib.get_issue_comment(SESSION, issue.uid, commentid) + if not comment: + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.ENOCOMMENT) output = comment.to_json(public=True) output['avatar_url'] = pagure.lib.avatar_url(comment.user.user, size=16) From b20a301481b4d20be1d8a31f17b94af6c012ae9c Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 02 2015 11:06:25 +0000 Subject: [PATCH 19/23] Fix typo --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index c936185..882f8f5 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -207,7 +207,7 @@ def send_email(text, subject, to_mail, msg['In-Reply-To'] = '<%s>' % in_reply_to msg['X-pagure'] = pagure.APP.config['APP_URL'] - if procject_name is not None: + if project_name is not None: msg['X-pagure-project'] = project_name # Send the message via our own SMTP server, but don't include the From 631c331f863c785a60d2af5fe2f0edbf85cf4fd6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 02 2015 11:06:35 +0000 Subject: [PATCH 20/23] Make the token_id a keyword argument in create_tokens_acl --- diff --git a/tests/__init__.py b/tests/__init__.py index 6c41a5a..44b2720 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -272,11 +272,11 @@ def create_acls(session): session.commit() -def create_tokens_acl(session): +def create_tokens_acl(session, token_id='aaabbbcccddd'): """ Create some acls for the tokens. """ for aclid in range(7): item = pagure.lib.model.TokenAcl( - token_id='aaabbbcccddd', + token_id=token_id, acl_id=aclid + 1, ) session.add(item) From 5aca937873809d4062c7bd5cdeed6b912cc7b0b5 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 02 2015 11:07:08 +0000 Subject: [PATCH 21/23] Adjust test_api_comment_issue to not send emails and set the issue uid --- diff --git a/tests/test_progit_flask_api_issue.py b/tests/test_progit_flask_api_issue.py index 0aec6db..7c9ea1e 100644 --- a/tests/test_progit_flask_api_issue.py +++ b/tests/test_progit_flask_api_issue.py @@ -721,8 +721,13 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) - def test_api_comment_issue(self): + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_api_comment_issue(self, p_send_email, p_ugt): """ Test the api_comment_issue method of the flask api. """ + p_send_email.return_value = True + p_ugt.return_value = True + tests.create_projects(self.session) tests.create_tokens(self.session) tests.create_acls(self.session) @@ -777,6 +782,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): user='pingou', ticketfolder=None, private=False, + issue_uid='aaabbbccc#1', ) self.session.commit() self.assertEqual(msg.title, 'Test issue #1') @@ -866,6 +872,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): user='foo', ticketfolder=None, private=True, + issue_uid='aaabbbccc#2', ) self.session.commit() self.assertEqual(msg.title, 'Test issue') From 5711a1d2ed5426da651ed6cd81a7d89185639287 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 02 2015 11:10:06 +0000 Subject: [PATCH 22/23] Expand the unit-tests to check adding a comment to a private repo --- diff --git a/tests/test_progit_flask_api_issue.py b/tests/test_progit_flask_api_issue.py index 7c9ea1e..9e679ec 100644 --- a/tests/test_progit_flask_api_issue.py +++ b/tests/test_progit_flask_api_issue.py @@ -905,6 +905,32 @@ class PagureFlaskApiIssuetests(tests.Modeltests): issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.comments), 0) + # Create token for user foo + item = pagure.lib.model.Token( + id='foo_token2', + user_id=2, + project_id=3, + expiration=datetime.datetime.utcnow() + datetime.timedelta(days=30) + ) + self.session.add(item) + self.session.commit() + tests.create_tokens_acl(self.session, token_id='foo_token2') + + data = { + 'comment': 'This is a very interesting question', + } + headers = {'Authorization': 'token foo_token2'} + + # Valid request and authorized + output = self.app.post( + '/api/0/foo/issue/1/comment', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Comment added'} + ) + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase( From 896cc0479704bbe19160daa20aeb0aae320f8cca Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 02 2015 11:10:35 +0000 Subject: [PATCH 23/23] Add unit-tests for the api_view_issue_comment endpoint --- diff --git a/tests/test_progit_flask_api_issue.py b/tests/test_progit_flask_api_issue.py index 9e679ec..ebd6748 100644 --- a/tests/test_progit_flask_api_issue.py +++ b/tests/test_progit_flask_api_issue.py @@ -931,6 +931,112 @@ class PagureFlaskApiIssuetests(tests.Modeltests): {'message': 'Comment added'} ) + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_api_view_issue_comment(self, p_send_email, p_ugt): + """ Test the api_view_issue_comment endpoint. """ + p_send_email.return_value = True + p_ugt.return_value = True + + self.test_api_comment_issue() + + # View a comment that does not exist + output = self.app.get('/api/0/foo/issue/100/comment/2') + self.assertEqual(output.status_code, 404) + + # Issue exists but not the comment + output = self.app.get('/api/0/test/issue/1/comment/2') + self.assertEqual(output.status_code, 404) + + # Issue and comment exists + output = self.app.get('/api/0/test/issue/1/comment/1') + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['date_created'] = '1435821770' + data["comment_date"] = "2015-07-02 09:22" + data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..." + self.assertDictEqual( + data, + { + "avatar_url": "https://seccdn.libravatar.org/avatar/...", + "comment": "This is a very interesting question", + "comment_date": "2015-07-02 09:22", + "date_created": "1435821770", + "id": 1, + "parent": None, + "user": { + "fullname": "PY C", + "name": "pingou" + } + } + ) + + # Issue and comment exists, using UID + output = self.app.get('/api/0/test/issue/aaabbbccc#1/comment/1') + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['date_created'] = '1435821770' + data["comment_date"] = "2015-07-02 09:22" + data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..." + self.assertDictEqual( + data, + { + "avatar_url": "https://seccdn.libravatar.org/avatar/...", + "comment": "This is a very interesting question", + "comment_date": "2015-07-02 09:22", + "date_created": "1435821770", + "id": 1, + "parent": None, + "user": { + "fullname": "PY C", + "name": "pingou" + } + } + ) + + # Private issue + output = self.app.get('/api/0/foo/issue/1/comment/2') + self.assertEqual(output.status_code, 403) + + # Private issue - Auth - wrong token + headers = {'Authorization': 'token pingou_foo'} + output = self.app.get('/api/0/foo/issue/1/comment/2', headers=headers) + self.assertEqual(output.status_code, 403) + + # Private issue - Auth - Invalid token + headers = {'Authorization': 'token aaabbbcccddd'} + output = self.app.get('/api/0/foo/issue/1/comment/2', headers=headers) + self.assertEqual(output.status_code, 401) + + # Private issue - Auth - valid token - unknown comment + headers = {'Authorization': 'token foo_token2'} + output = self.app.get('/api/0/foo/issue/1/comment/3', headers=headers) + self.assertEqual(output.status_code, 404) + + # Private issue - Auth - valid token - known comment + headers = {'Authorization': 'token foo_token2'} + output = self.app.get('/api/0/foo/issue/1/comment/2', headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['date_created'] = '1435821770' + data["comment_date"] = "2015-07-02 09:22" + data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..." + self.assertDictEqual( + data, + { + "avatar_url": "https://seccdn.libravatar.org/avatar/...", + "comment": "This is a very interesting question", + "comment_date": "2015-07-02 09:22", + "date_created": "1435821770", + "id": 2, + "parent": None, + "user": { + "fullname": "foo bar", + "name": "foo" + } + } + ) + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase(