From 71b551f5c11ce421ee4eade10017dd68df084875 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 08:39:52 +0000 Subject: [PATCH 1/31] Add the issue_watchers and pull_request_watchers tables in the model --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 02054da..0d82991 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1670,6 +1670,88 @@ class PagureLog(BASE): return desc % arg + +class IssueWatcher(BASE): + """ Stores the users watching issues. + + Table -- issue_watchers + """ + + __tablename__ = 'issue_watchers' + __table_args__ = ( + sa.UniqueConstraint('issue_uid', 'user_id'), + ) + + id = sa.Column(sa.Integer, primary_key=True) + issue_uid = sa.Column( + sa.Integer, + sa.ForeignKey('issues.uid', onupdate='CASCADE', ondelete='CASCADE'), + nullable=False) + user_id = sa.Column( + sa.Integer, + sa.ForeignKey('users.id', onupdate='CASCADE', ondelete='CASCADE'), + nullable=False, + index=True) + watch = sa.Column( + sa.Boolean, + nullable=False) + + user = relation( + 'User', foreign_keys=[user_id], remote_side=[User.id], + backref=backref( + 'issue_watched', cascade="delete, delete-orphan" + ), + ) + + issue = relation( + 'Issue', foreign_keys=[issue_uid], remote_side=[Issue.uid], + backref=backref( + 'watchers', cascade="delete, delete-orphan", + ), + ) + + +class PullRequestWatcher(BASE): + """ Stores the users watching issues. + + Table -- pull_request_watchers + """ + + __tablename__ = 'pull_request_watchers' + __table_args__ = ( + sa.UniqueConstraint('pull_request_uid', 'user_id'), + ) + + id = sa.Column(sa.Integer, primary_key=True) + pull_request_uid = sa.Column( + sa.Integer, + sa.ForeignKey( + 'pull_requests.uid', onupdate='CASCADE', ondelete='CASCADE'), + nullable=False) + user_id = sa.Column( + sa.Integer, + sa.ForeignKey('users.id', onupdate='CASCADE', ondelete='CASCADE'), + nullable=False, + index=True) + watch = sa.Column( + sa.Boolean, + nullable=False) + + user = relation( + 'User', foreign_keys=[user_id], remote_side=[User.id], + backref=backref( + 'pr_watched', cascade="delete, delete-orphan" + ), + ) + + pull_request = relation( + 'PullRequest', foreign_keys=[pull_request_uid], remote_side=[PullRequest.uid], + backref=backref( + 'watchers', cascade="delete, delete-orphan", + ), + ) + + # # Class and tables specific for the API/token access # From 5ee1be5869804ae307d16bdd191146124c49d193 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 08:39:52 +0000 Subject: [PATCH 2/31] Add method watching_obj and is_watching_obj to pagure.lib These two methods allow one to set a watch status on an issue or a pull-request and to retrieve whether someone is watching a given issue or pull-request. --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index bbe9323..7362a81 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3199,6 +3199,76 @@ def user_watch_list(session, user): return sorted(list(watch), key=lambda proj: proj.name) +def watching_obj(session, user, obj, watch_status): + ''' Set the watch status of the user on the specified object. + + Objects can be either an issue or a pull-request + ''' + + user_obj = get_user(session, user.username) + + if obj.isa == "issue": + dbobj = model.IssueWatcher( + user_id=user.id, + issue_uid=obj.uid, + watch=watch_status, + ) + elif obj.isa == "pull-request": + dbobj = model.PullRequestWatcher( + user_id=user.id, + pull_request_uid=obj.uid, + watch=watch_status, + ) + + session.add(dbobj) + + output = 'You are no longer watching this %s' % obj.isa + if watch_status: + output = 'You are now watching this %s' % obj.isa + return output + + +def is_watching_obj(session, user, obj): + ''' Check if the user is watching the specified object. + + Objects can be either an issue or a pull-request + ''' + + if not user: + return False + + if obj.user.user == user.user: + return True + + for comment in obj.comments: + if comment.user.user == user.user: + return True + + if obj.isa == "issue": + query = session.query( + model.IssueWatcher + ).filter( + model.IssueWatcher.user_id == user.id + ).filter( + model.IssueWatcher.issue_uid == obj.uid + ) + elif obj.isa == "pull-request": + query = session.query( + model.PullRequestWatcher + ).filter( + model.PullRequestWatcher.user_id == user.id + ).filter( + model.PullRequestWatcher.pull_request_uid == obj.uid + ) + + watcher = query.first() + + if watcher: + return watcher.watch + + return False + + def save_report(session, repo, name, url, username): """ Save the report of issues based on the given URL of the project. """ From ad2b94cbaa4bc330842d011c0ea84ecb4070bd87 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 08:39:52 +0000 Subject: [PATCH 3/31] Add unit-tests for pagure.lib.is_watching_obj --- diff --git a/tests/__init__.py b/tests/__init__.py index 64718a5..ad24c94 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -226,14 +226,16 @@ class FakeGroup(object): class FakeUser(object): """ Fake user used to test the fedocallib library. """ - def __init__(self, groups=[], username='username', cla_done=True): + def __init__(self, groups=[], username='username', cla_done=True, id=1): """ Constructor. :arg groups: list of the groups in which this fake user is supposed to be. """ if isinstance(groups, basestring): groups = [groups] + self.id = id self.groups = groups + self.user = username self.username = username self.name = username self.email = 'foo@bar.com' diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 77cddbe..b844216 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2677,6 +2677,88 @@ class PagureLibtests(tests.Modeltests): html = pagure.lib.text2markdown(text) self.assertEqual(html, expected[idx]) + def test_is_watching_obj(self): + """ Test the is_watching_obj method in pagure.lib """ + # Create the project ns/test + item = pagure.lib.model.Project( + user_id=1, # pingou + name='test3', + namespace='ns', + description='test project #1', + hook_token='aaabbbcccdd', + ) + item.close_status = ['Invalid', 'Insufficient data', 'Fixed'] + self.session.add(item) + self.session.commit() + + # Create the ticket + iss = pagure.lib.new_issue( + issue_id=4, + session=self.session, + repo=item, + title='test issue', + content='content test issue', + user='pingou', + ticketfolder=None, + ) + self.session.commit() + self.assertEqual(iss.id, 4) + self.assertEqual(iss.title, 'test issue') + + # Created the ticket + user = tests.FakeUser(username='pingou') + self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) + user = tests.FakeUser(username='foo') + self.assertFalse(pagure.lib.is_watching_obj(self.session, user, iss)) + user = tests.FakeUser(username='bar') + self.assertFalse(pagure.lib.is_watching_obj(self.session, user, iss)) + + # Comment on the ticket + out = pagure.lib.add_issue_comment( + self.session, + issue=iss, + comment='This is a comment', + user='foo', + ticketfolder=None, + notify=False) + self.assertEqual(out, 'Comment added') + + # Commented on the ticket + user = tests.FakeUser(username='pingou') + self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) + user = tests.FakeUser(username='foo') + self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) + user = tests.FakeUser(username='bar') + self.assertFalse(pagure.lib.is_watching_obj(self.session, user, iss)) + + # Add user `bar` + item = pagure.lib.model.User( + user='bar', + fullname='bar name', + password='bar', + 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() + + # Watch the ticket + user = tests.FakeUser(username='bar') + out = pagure.lib.watching_obj(self.session, user, iss, True) + self.assertEqual(out, 'You are now watching this issue') + + # Is watching the ticket + user = tests.FakeUser(username='pingou') + self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) + user = tests.FakeUser(username='foo') + self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) + user = tests.FakeUser(username='bar') + self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) + + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase(PagureLibtests) From f8cc4e5e458959f488c5a7e96215a124eab2601d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 08:39:52 +0000 Subject: [PATCH 4/31] Add unit-tests for pagure.lib.watching_obj --- diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index b844216..5d2c3c0 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2758,6 +2758,51 @@ class PagureLibtests(tests.Modeltests): user = tests.FakeUser(username='bar') self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) + def test_watching_obj(self): + """ Test the watching_obj method in pagure.lib """ + # Create the project ns/test + item = pagure.lib.model.Project( + user_id=1, # pingou + name='test3', + namespace='ns', + description='test project #1', + hook_token='aaabbbcccdd', + ) + item.close_status = ['Invalid', 'Insufficient data', 'Fixed'] + self.session.add(item) + self.session.commit() + + # Create the ticket + iss = pagure.lib.new_issue( + issue_id=4, + session=self.session, + repo=item, + title='test issue', + content='content test issue', + user='pingou', + ticketfolder=None, + ) + self.session.commit() + self.assertEqual(iss.id, 4) + self.assertEqual(iss.title, 'test issue') + + # Unknown user + user = tests.FakeUser(username='bar') + self.assertRaises( + pagure.exceptions.PagureException, + pagure.lib.watching_obj, + self.session, user, iss, True + ) + + # Watch the ticket + user = tests.FakeUser(username='foo') + out = pagure.lib.watching_obj(self.session, user, iss, True) + self.assertEqual(out, 'You are now watching this issue') + + # Un-watch the ticket + user = tests.FakeUser(username='foo') + out = pagure.lib.watching_obj(self.session, user, iss, False) + self.assertEqual(out, 'You are no longer watching this issue') if __name__ == '__main__': From c26db652e6631696922977ea83c5215f8de546bc Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 08:39:52 +0000 Subject: [PATCH 5/31] Rename pagure.lib.watching_obj to pagure.lib.set_watch_obj --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 7362a81..da2f3e9 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3199,7 +3199,7 @@ def user_watch_list(session, user): return sorted(list(watch), key=lambda proj: proj.name) -def watching_obj(session, user, obj, watch_status): +def set_watch_obj(session, user, obj, watch_status): ''' Set the watch status of the user on the specified object. Objects can be either an issue or a pull-request diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 5d2c3c0..f62ab86 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2747,7 +2747,7 @@ class PagureLibtests(tests.Modeltests): # Watch the ticket user = tests.FakeUser(username='bar') - out = pagure.lib.watching_obj(self.session, user, iss, True) + out = pagure.lib.set_watch_obj(self.session, user, iss, True) self.assertEqual(out, 'You are now watching this issue') # Is watching the ticket @@ -2758,8 +2758,8 @@ class PagureLibtests(tests.Modeltests): user = tests.FakeUser(username='bar') self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) - def test_watching_obj(self): - """ Test the watching_obj method in pagure.lib """ + def test_set_watch_obj(self): + """ Test the set_watch_obj method in pagure.lib """ # Create the project ns/test item = pagure.lib.model.Project( user_id=1, # pingou @@ -2790,18 +2790,18 @@ class PagureLibtests(tests.Modeltests): user = tests.FakeUser(username='bar') self.assertRaises( pagure.exceptions.PagureException, - pagure.lib.watching_obj, + pagure.lib.set_watch_obj, self.session, user, iss, True ) # Watch the ticket user = tests.FakeUser(username='foo') - out = pagure.lib.watching_obj(self.session, user, iss, True) + out = pagure.lib.set_watch_obj(self.session, user, iss, True) self.assertEqual(out, 'You are now watching this issue') # Un-watch the ticket user = tests.FakeUser(username='foo') - out = pagure.lib.watching_obj(self.session, user, iss, False) + out = pagure.lib.set_watch_obj(self.session, user, iss, False) self.assertEqual(out, 'You are no longer watching this issue') From 7ed29e60cbde0bd29decc68316c709d6025cf6d8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 08:39:52 +0000 Subject: [PATCH 6/31] Let's raise a dedicated exception if we end up with an odd object to watch --- diff --git a/pagure/exceptions.py b/pagure/exceptions.py index 8764baf..e02abd6 100644 --- a/pagure/exceptions.py +++ b/pagure/exceptions.py @@ -68,3 +68,8 @@ class NoCorrespondingPR(PagureException): ''' Exception raised when no pull-request is found with the given information. ''' pass + + +class InvalidObjetException(PagureException): + ''' Exception raised when a given object is not what was expected. ''' + pass diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index da2f3e9..1c9cd62 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3219,6 +3219,10 @@ def set_watch_obj(session, user, obj, watch_status): pull_request_uid=obj.uid, watch=watch_status, ) + else: + raise pagure.exceptions.InvalidObjetException( + 'Unknow watch target: "%s"' % obj + ) session.add(dbobj) From dc7cb851907705cbc6a8a772fc433e5ece65a6b8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 08:39:52 +0000 Subject: [PATCH 7/31] More tests around pagure.lib.set_watch_obj --- diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index f62ab86..9fb8831 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2794,6 +2794,22 @@ class PagureLibtests(tests.Modeltests): self.session, user, iss, True ) + # Invalid object to watch - project + user = tests.FakeUser(username='foo') + self.assertRaises( + pagure.exceptions.InvalidObjetException, + pagure.lib.set_watch_obj, + self.session, user, iss.project, True + ) + + # Invalid object to watch - string + user = tests.FakeUser(username='foo') + self.assertRaises( + AttributeError, + pagure.lib.set_watch_obj, + self.session, user, 'foo', True + ) + # Watch the ticket user = tests.FakeUser(username='foo') out = pagure.lib.set_watch_obj(self.session, user, iss, True) From cb035c518eec22ee2ae84dbe9f2e1198789d248e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 10:56:58 +0000 Subject: [PATCH 8/31] Fix the foreign key on the watchers tables --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 0d82991..34fda86 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1684,7 +1684,7 @@ class IssueWatcher(BASE): id = sa.Column(sa.Integer, primary_key=True) issue_uid = sa.Column( - sa.Integer, + sa.String(32), sa.ForeignKey('issues.uid', onupdate='CASCADE', ondelete='CASCADE'), nullable=False) user_id = sa.Column( @@ -1724,7 +1724,7 @@ class PullRequestWatcher(BASE): id = sa.Column(sa.Integer, primary_key=True) pull_request_uid = sa.Column( - sa.Integer, + sa.String(32), sa.ForeignKey( 'pull_requests.uid', onupdate='CASCADE', ondelete='CASCADE'), nullable=False) From c121a3c3b298539a8a7168f0350e200d41bf7d25 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 10:57:38 +0000 Subject: [PATCH 9/31] Fix properly setting the watch status on an issue or a PR --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 1c9cd62..857d6bf 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3205,24 +3205,42 @@ def set_watch_obj(session, user, obj, watch_status): Objects can be either an issue or a pull-request ''' - user_obj = get_user(session, user.username) + user_obj = get_user(session, user) + if obj.isa == "issue": - dbobj = model.IssueWatcher( - user_id=user.id, - issue_uid=obj.uid, - watch=watch_status, + query = session.query( + model.IssueWatcher + ).filter( + model.IssueWatcher.user_id == user_obj.id + ).filter( + model.IssueWatcher.issue_uid == obj.uid ) elif obj.isa == "pull-request": - dbobj = model.PullRequestWatcher( - user_id=user.id, - pull_request_uid=obj.uid, - watch=watch_status, + query = session.query( + model.PullRequestWatcher + ).filter( + model.PullRequestWatcher.user_id == user_obj.id + ).filter( + model.PullRequestWatcher.pull_request_uid == obj.uid ) + dbobj = query.first() + + if not dbobj: + if obj.isa == "issue": + dbobj = model.IssueWatcher( + user_id=user_obj.id, + issue_uid=obj.uid, + watch=watch_status, + ) + elif obj.isa == "pull-request": + dbobj = model.PullRequestWatcher( + user_id=user_obj.id, + pull_request_uid=obj.uid, + watch=watch_status, + ) else: - raise pagure.exceptions.InvalidObjetException( - 'Unknow watch target: "%s"' % obj - ) + dbobj.watch = watch_status session.add(dbobj) From 93b01b137df8ea45b0b1bb933fa3904910a543d5 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 10:59:50 +0000 Subject: [PATCH 10/31] Fix the logic in is_watching_obj This is an important fix as the watcher tables allow to turn on or off watching a PR/issue, so we need to check first if there was any explicit instructions before checking if the user is in one of the default recipients. --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 857d6bf..2b3eb65 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3259,13 +3259,7 @@ def is_watching_obj(session, user, obj): if not user: return False - if obj.user.user == user.user: - return True - - for comment in obj.comments: - if comment.user.user == user.user: - return True - + # First check if the user explicitely turned on/off notifications if obj.isa == "issue": query = session.query( model.IssueWatcher @@ -3288,6 +3282,14 @@ def is_watching_obj(session, user, obj): if watcher: return watcher.watch + # Otherwise, just check if they are in the default group + if obj.user.user == user.user: + return True + + for comment in obj.comments: + if comment.user.user == user.user: + return True + return False From f0a4f5c8248182779c88aff13c4cb89de8223658 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:00:13 +0000 Subject: [PATCH 11/31] Add a new API endpoint to toggle subscription on an issue --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 4f0c64a..f598265 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -798,3 +798,107 @@ def api_assign_issue(repo, issueid, username=None, namespace=None): jsonout = flask.jsonify(output) return jsonout + + +@API.route('//issue//subscribe', methods=['POST']) +@API.route('///issue//subscribe', methods=['POST']) +@API.route( + '/fork///issue//subscribe', methods=['POST']) +@API.route( + '/fork////issue//subscribe', + methods=['POST']) +@api_login_required(acls=['issue_subscribe']) +@api_method +def api_subscribe_issue(repo, issueid, username=None, namespace=None): + """ + Subscribe to an issue + --------------------- + Allows someone to subscribe or unscribe to the notifications related to + an issue. + + :: + + POST /api/0//issue//subscribe + POST /api/0///issue//subscribe + + :: + + POST /api/0/fork///issue//subscribe + POST /api/0/fork////issue//subscribe + + Input + ^^^^^ + + +--------------+----------+---------------+---------------------------+ + | Key | Type | Optionality | Description | + +==============+==========+===============+===========================+ + | ``username`` | string | Mandatory | | The username of the user| + | | | | to (un)subscribe to the | + | | | | issue. | + +--------------+----------+---------------+---------------------------+ + | ``status`` | boolean | Mandatory | | The subscription status | + | | | | to subscribe or | + | | | | unsubscribe to the. | + | | | | issue. | + +--------------+----------+---------------+---------------------------+ + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "message": "User subscribed" + } + + """ + repo = pagure.lib.get_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('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) + + issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid) + + if issue is None or issue.project != repo: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE) + + if issue.private and not is_repo_admin(repo) \ + and (not api_authenticated() or + not issue.user.user == flask.g.fas_user.username): + raise pagure.exceptions.APIError( + 403, error_code=APIERROR.EISSUENOTALLOWED) + + 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=issue, + 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) + + jsonout = flask.jsonify(output) + return jsonout diff --git a/pagure/forms.py b/pagure/forms.py index 7bfdaac..a2d7316 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -599,3 +599,11 @@ class PublicNotificationForm(FlaskForm): 'Public PR notification*', [wtforms.validators.optional(), MultipleEmail()] ) + + +class SubscribtionForm(FlaskForm): + ''' Form to subscribe or unsubscribe to an issue or a PR. ''' + status = wtforms.BooleanField( + 'Subscription status', + [wtforms.validators.optional()], + ) From a375e42724cad4f757e4d187d95ea1225102a073 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:01:03 +0000 Subject: [PATCH 12/31] Add the subscribe/unsubscribe button on the issue page --- diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index 576c1ff..3850c5c 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -302,6 +302,18 @@ + {% if authenticated %} +
+ +
+ {% endif %} + {% if repo.issue_keys %}
@@ -592,7 +604,7 @@ function set_ui_for_comment(setting){ } } function try_async_comment(form) { - console.log(form) + console.log(form); set_ui_for_comment(true); var _data = $(form).serialize(); var btn = $(document.activeElement); @@ -840,6 +852,41 @@ $( document ).ready(function() { }); {% endif %} + {% if authenticated %} + function set_up_subcribed() { + $("#subcribe-btn").click(function(){ + console.log('click'); + var _url = "{{ url_for( + 'api_ns.api_subscribe_issue', + repo=repo.name, + username=username, + namespace=repo.namespace, + issueid=issueid + ) }}"; + var _btn = $("#subcribe-btn"); + var _data = {}; + if (_btn.text() == 'Subscribe'){ + _data.status = false; + } else { + _data.status = true; + } + $.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 %} + }); {% endblock %} From 5a4832c9449c6588db1a31b3c8175afe66b49420 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:14:31 +0000 Subject: [PATCH 13/31] Let is_watch_obj accept username or email as well as an user object --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 2b3eb65..576b2ee 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3256,6 +3256,9 @@ def is_watching_obj(session, user, obj): Objects can be either an issue or a pull-request ''' + if not isinstance(user, model.User): + user = get_user(session, user) + if not user: return False From 184c1ceea989588b82940dcf18cb34733caa1a33 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:15:07 +0000 Subject: [PATCH 14/31] Correctly present the subscribe and unsubscribe buttons in the UI --- diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index 3850c5c..243ff53 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -306,9 +306,9 @@
@@ -865,11 +865,12 @@ $( document ).ready(function() { ) }}"; var _btn = $("#subcribe-btn"); var _data = {}; - if (_btn.text() == 'Subscribe'){ + if (_btn.text() == 'Unsubscribe'){ _data.status = false; } else { _data.status = true; } + console.log(_data); $.post( _url, _data ).done( function(data) { var _btn = $("#subcribe-btn"); diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index dafab72..24ebf97 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -817,6 +817,11 @@ def view_issue(repo, issueid, username=None, namespace=None): for key in issue.other_fields: knowns_keys[key.key.name] = key + subscribed = False + if authenticated(): + subscribed = pagure.lib.is_watching_obj( + SESSION, flask.g.fas_user.username, issue) + return flask.render_template( 'issue.html', select='issues', @@ -827,6 +832,7 @@ def view_issue(repo, issueid, username=None, namespace=None): issueid=issueid, form=form, knowns_keys=knowns_keys, + subscribed=subscribed, ) From 4bd1cba4345919495c02de8b81c560f7e8dc09e3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:22:40 +0000 Subject: [PATCH 15/31] Rename _get_emails_for_issue to _get_emails_for_obj This makes sense since this method is called for issues as well as pull-requests --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index ada2453..b364e1b 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -92,56 +92,56 @@ def _clean_emails(emails, user): return emails -def _get_emails_for_issue(issue): +def _get_emails_for_obj(obj): ''' Return the list of emails to send notification to when notifying - about the specified issue. + about the specified issue or pull-request. ''' emails = set() # Add project creator/owner - if issue.project.user.default_email: - emails.add(issue.project.user.default_email) + if obj.project.user.default_email: + emails.add(obj.project.user.default_email) # Add project maintainers - for user in issue.project.users: + for user in obj.project.users: if user.default_email: emails.add(user.default_email) # Add people in groups with commits access to the project: - for group in issue.project.groups: + for group in obj.project.groups: if group.creator.default_email: emails.add(group.creator.default_email) for user in group.users: if user.default_email: emails.add(user.default_email) - # Add people that commented on the ticket - for comment in issue.comments: + # Add people that commented on the issue/PR + for comment in obj.comments: if comment.user.default_email: emails.add(comment.user.default_email) - # Add the person that opened the issue - if issue.user.default_email: - emails.add(issue.user.default_email) + # Add the person that opened the issue/PR + if obj.user.default_email: + emails.add(obj.user.default_email) - # Add the person assigned to the ticket - if issue.assignee and issue.assignee.default_email: - emails.add(issue.assignee.default_email) + # Add the person assigned to the issue/PR + if obj.assignee and obj.assignee.default_email: + emails.add(obj.assignee.default_email) # Add the person watching this project, if the issue is public - if issue.isa == 'issue' and not issue.private: - for watcher in issue.project.watchers: + if obj.isa == 'issue' and not obj.private: + for watcher in obj.project.watchers: emails.add(watcher.user.default_email) # Add public notifications to lists/users set project-wide - if issue.isa == 'issue' and not issue.private: - for notifs in issue.project.notifications.get('issues', []): + if obj.isa == 'issue' and not obj.private: + for notifs in obj.project.notifications.get('issues', []): emails.add(notifs) - elif issue.isa == 'pull-request': - for notifs in issue.project.notifications.get('requests', []): + elif obj.isa == 'pull-request': + for notifs in obj.project.notifications.get('requests', []): emails.add(notifs) # Remove the person list in unwatch - for unwatcher in issue.project.unwatchers: + for unwatcher in obj.project.unwatchers: if unwatcher.user.default_email in emails: emails.remove(unwatcher.user.default_email) @@ -279,7 +279,7 @@ def notify_new_comment(comment, user=None): comment.issue.project.name, 'issue', comment.issue.id)) - mail_to = _get_emails_for_issue(comment.issue) + mail_to = _get_emails_for_obj(comment.issue) if comment.user and comment.user.default_email: mail_to.add(comment.user.default_email) @@ -318,7 +318,7 @@ def notify_new_issue(issue, user=None): issue.project.name, 'issue', issue.id)) - mail_to = _get_emails_for_issue(issue) + mail_to = _get_emails_for_obj(issue) mail_to = _add_mentioned_users(mail_to, issue.content) mail_to = _clean_emails(mail_to, user) @@ -351,7 +351,7 @@ The issue: `%s` of project: `%s` has been %s by %s. issue.project.name, 'issue', issue.id)) - mail_to = _get_emails_for_issue(issue) + mail_to = _get_emails_for_obj(issue) if new_assignee and new_assignee.default_email: mail_to.add(new_assignee.default_email) @@ -388,7 +388,7 @@ The pull-request: `%s` of project: `%s` has been %s by %s. request.project.name, 'pull-request', request.id)) - mail_to = _get_emails_for_issue(request) + mail_to = _get_emails_for_obj(request) if new_assignee and new_assignee.default_email: mail_to.add(new_assignee.default_email) @@ -427,7 +427,7 @@ def notify_new_pull_request(request): request.project.name, 'pull-request', request.id)) - mail_to = _get_emails_for_issue(request) + mail_to = _get_emails_for_obj(request) send_email( text, @@ -461,7 +461,7 @@ Merged pull-request: request.project.name, 'pull-request', request.id)) - mail_to = _get_emails_for_issue(request) + mail_to = _get_emails_for_obj(request) uid = time.mktime(datetime.datetime.now().timetuple()) send_email( @@ -497,7 +497,7 @@ Cancelled pull-request: request.project.name, 'pull-request', request.id)) - mail_to = _get_emails_for_issue(request) + mail_to = _get_emails_for_obj(request) uid = time.mktime(datetime.datetime.now().timetuple()) send_email( @@ -532,7 +532,7 @@ def notify_pull_request_comment(comment, user): comment.pull_request.project.name, 'pull-request', comment.pull_request.id)) - mail_to = _get_emails_for_issue(comment.pull_request) + mail_to = _get_emails_for_obj(comment.pull_request) mail_to = _add_mentioned_users(mail_to, comment.comment) mail_to = _clean_emails(mail_to, user) From 7e3cd265ddab366189b800a5502470ddb0704bd1 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:26:18 +0000 Subject: [PATCH 16/31] Add/Remove people who explicitly asked to be/not be notified --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index b364e1b..262446d 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -145,6 +145,13 @@ def _get_emails_for_obj(obj): if unwatcher.user.default_email in emails: emails.remove(unwatcher.user.default_email) + # Add/Remove people who explicitly asked to be added/removed + for watcher in obj.watchers: + if not watcher.watch and watcher.user.default_email in emails: + emails.remove(watcher.user.default_email) + elif watcher.watch: + emails.add(watcher.user.default_email) + # Drop the email used by pagure when sending emails = _clean_emails( emails, pagure.APP.config.get(pagure.APP.config.get( From 2cd91562d878d211732d74d68be5f617aa9b3a8d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:49:39 +0000 Subject: [PATCH 17/31] Make set_watch_obj and is_watching_obj raise an exception for odd objects Also return False if we cannot find the specified user in the DB --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 576b2ee..9f6deb1 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3207,7 +3207,6 @@ def set_watch_obj(session, user, obj, watch_status): user_obj = get_user(session, user) - if obj.isa == "issue": query = session.query( model.IssueWatcher @@ -3224,6 +3223,11 @@ def set_watch_obj(session, user, obj, watch_status): ).filter( model.PullRequestWatcher.pull_request_uid == obj.uid ) + else: + raise pagure.exceptions.InvalidObjetException( + 'Unsupported object found: "%s"' % obj + ) + dbobj = query.first() if not dbobj: @@ -3257,7 +3261,10 @@ def is_watching_obj(session, user, obj): ''' if not isinstance(user, model.User): - user = get_user(session, user) + try: + user = get_user(session, user) + except pagure.exceptions.PagureException: + return False if not user: return False @@ -3279,6 +3286,10 @@ def is_watching_obj(session, user, obj): ).filter( model.PullRequestWatcher.pull_request_uid == obj.uid ) + else: + raise pagure.exceptions.InvalidObjetException( + 'Unsupported object found: "%s"' % obj + ) watcher = query.first() From 737aa4fc749fb03fd87e3ad7e722bab6bf9fd760 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:49:44 +0000 Subject: [PATCH 18/31] Fix running unit-tests for the change in the methods User object vs username --- diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 9fb8831..271b346 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2706,12 +2706,12 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(iss.title, 'test issue') # Created the ticket - user = tests.FakeUser(username='pingou') - self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) - user = tests.FakeUser(username='foo') - self.assertFalse(pagure.lib.is_watching_obj(self.session, user, iss)) - user = tests.FakeUser(username='bar') - self.assertFalse(pagure.lib.is_watching_obj(self.session, user, iss)) + self.assertTrue(pagure.lib.is_watching_obj( + self.session, 'pingou', iss)) + self.assertFalse(pagure.lib.is_watching_obj( + self.session, 'foo', iss)) + self.assertFalse(pagure.lib.is_watching_obj( + self.session, 'bar', iss)) # Comment on the ticket out = pagure.lib.add_issue_comment( @@ -2724,12 +2724,12 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(out, 'Comment added') # Commented on the ticket - user = tests.FakeUser(username='pingou') - self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) - user = tests.FakeUser(username='foo') - self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) - user = tests.FakeUser(username='bar') - self.assertFalse(pagure.lib.is_watching_obj(self.session, user, iss)) + self.assertTrue(pagure.lib.is_watching_obj( + self.session, 'pingou', iss)) + self.assertTrue(pagure.lib.is_watching_obj( + self.session, 'foo', iss)) + self.assertFalse(pagure.lib.is_watching_obj( + self.session, 'bar', iss)) # Add user `bar` item = pagure.lib.model.User( @@ -2746,17 +2746,16 @@ class PagureLibtests(tests.Modeltests): self.session.commit() # Watch the ticket - user = tests.FakeUser(username='bar') - out = pagure.lib.set_watch_obj(self.session, user, iss, True) + out = pagure.lib.set_watch_obj(self.session, 'bar', iss, True) self.assertEqual(out, 'You are now watching this issue') # Is watching the ticket - user = tests.FakeUser(username='pingou') - self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) - user = tests.FakeUser(username='foo') - self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) - user = tests.FakeUser(username='bar') - self.assertTrue(pagure.lib.is_watching_obj(self.session, user, iss)) + self.assertTrue(pagure.lib.is_watching_obj( + self.session, 'pingou', iss)) + self.assertTrue(pagure.lib.is_watching_obj( + self.session, 'foo', iss)) + self.assertTrue(pagure.lib.is_watching_obj( + self.session, 'bar', iss)) def test_set_watch_obj(self): """ Test the set_watch_obj method in pagure.lib """ @@ -2787,37 +2786,32 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(iss.title, 'test issue') # Unknown user - user = tests.FakeUser(username='bar') self.assertRaises( pagure.exceptions.PagureException, pagure.lib.set_watch_obj, - self.session, user, iss, True + self.session, 'unknown', iss, True ) # Invalid object to watch - project - user = tests.FakeUser(username='foo') self.assertRaises( pagure.exceptions.InvalidObjetException, pagure.lib.set_watch_obj, - self.session, user, iss.project, True + self.session, 'foo', iss.project, True ) # Invalid object to watch - string - user = tests.FakeUser(username='foo') self.assertRaises( AttributeError, pagure.lib.set_watch_obj, - self.session, user, 'foo', True + self.session, 'foo', 'ticket', True ) # Watch the ticket - user = tests.FakeUser(username='foo') - out = pagure.lib.set_watch_obj(self.session, user, iss, True) + out = pagure.lib.set_watch_obj(self.session, 'foo', iss, True) self.assertEqual(out, 'You are now watching this issue') # Un-watch the ticket - user = tests.FakeUser(username='foo') - out = pagure.lib.set_watch_obj(self.session, user, iss, False) + out = pagure.lib.set_watch_obj(self.session, 'foo', iss, False) self.assertEqual(out, 'You are no longer watching this issue') From fd2f9e5ceee4873f60ca45dedaaf72e529cce3ea Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:50:04 +0000 Subject: [PATCH 19/31] Add first unit-tests for pagure.lib.notify --- diff --git a/tests/test_pagure_lib_notify.py b/tests/test_pagure_lib_notify.py new file mode 100644 index 0000000..16316c9 --- /dev/null +++ b/tests/test_pagure_lib_notify.py @@ -0,0 +1,182 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2016 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources + +import unittest +import shutil +import sys +import os + +from mock import patch + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure.lib +import pagure.lib.model +import pagure.lib.notify +import tests + + +class PagureLibNotifytests(tests.Modeltests): + """ Tests for pagure.lib.notify """ + + def test_get_emails_for_obj_issue(self): + """ Test the _get_emails_for_obj method from pagure.lib.notify. """ + + # Create the project ns/test + item = pagure.lib.model.Project( + user_id=1, # pingou + name='test3', + namespace='ns', + description='test project #1', + hook_token='aaabbbcccdd', + ) + item.close_status = ['Invalid', 'Insufficient data', 'Fixed'] + self.session.add(item) + self.session.commit() + + # Create the ticket + iss = pagure.lib.new_issue( + issue_id=4, + session=self.session, + repo=item, + title='test issue', + content='content test issue', + user='pingou', + ticketfolder=None, + ) + self.session.commit() + self.assertEqual(iss.id, 4) + self.assertEqual(iss.title, 'test issue') + + exp = set(['bar@pingou.com']) + out = pagure.lib.notify._get_emails_for_obj(iss) + self.assertEqual(out, exp) + + # Comment on the ticket + out = pagure.lib.add_issue_comment( + self.session, + issue=iss, + comment='This is a comment', + user='foo', + ticketfolder=None, + notify=False) + self.assertEqual(out, 'Comment added') + + exp = set(['bar@pingou.com', 'foo@bar.com']) + out = pagure.lib.notify._get_emails_for_obj(iss) + self.assertEqual(out, exp) + + # Create user `bar` + item = pagure.lib.model.User( + user='bar', + fullname='bar name', + password='bar', + 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() + + # Watch the ticket + out = pagure.lib.set_watch_obj(self.session, 'bar', iss, True) + self.assertEqual(out, 'You are now watching this issue') + + exp = set(['bar@pingou.com', 'foo@bar.com', 'bar@bar.com']) + out = pagure.lib.notify._get_emails_for_obj(iss) + self.assertEqual(out, exp) + + def test_get_emails_for_obj_pr(self): + """ Test the _get_emails_for_obj method from pagure.lib.notify. """ + tests.create_projects(self.session) + + # Create the project ns/test + item = pagure.lib.model.Project( + user_id=1, # pingou + name='test3', + namespace='ns', + description='test project #1', + hook_token='aaabbbcccdd', + ) + item.close_status = ['Invalid', 'Insufficient data', 'Fixed'] + self.session.add(item) + self.session.commit() + + # Create the PR + repo = pagure.lib.get_project(self.session, 'test') + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=repo, + branch_from='master', + 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') + self.assertEqual(repo.open_requests, 1) + + exp = set(['bar@pingou.com']) + out = pagure.lib.notify._get_emails_for_obj(req) + self.assertEqual(out, exp) + + # Comment on the ticket + out = pagure.lib.add_pull_request_comment( + self.session, + request=req, + commit=None, + tree_id=None, + filename=None, + row=None, + comment='This is a comment', + user='foo', + requestfolder=None, + notify=False) + self.assertEqual(out, 'Comment added') + + exp = set(['bar@pingou.com', 'foo@bar.com']) + out = pagure.lib.notify._get_emails_for_obj(req) + self.assertEqual(out, exp) + + # Create user `bar` + item = pagure.lib.model.User( + user='bar', + fullname='bar name', + password='bar', + 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() + + # Watch the ticket + out = pagure.lib.set_watch_obj(self.session, 'bar', req, True) + self.assertEqual(out, 'You are now watching this pull-request') + + exp = set(['bar@pingou.com', 'foo@bar.com', 'bar@bar.com']) + out = pagure.lib.notify._get_emails_for_obj(req) + self.assertEqual(out, exp) + + +if __name__ == '__main__': + SUITE = unittest.TestLoader().loadTestsFromTestCase(PagureLibNotifytests) + unittest.TextTestRunner(verbosity=2).run(SUITE) From f1bbc36a50a5fdd589f66d6ac1d2495e8ff419ce Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 11:58:34 +0000 Subject: [PATCH 20/31] Attempt to fix running the tests on jenkins --- diff --git a/tests/test_pagure_flask_api_user.py b/tests/test_pagure_flask_api_user.py index de25a4c..7ee7843 100644 --- a/tests/test_pagure_flask_api_user.py +++ b/tests/test_pagure_flask_api_user.py @@ -137,7 +137,12 @@ class PagureFlaskApiUSertests(tests.Modeltests): self.assertEqual(output.status_code, 200) data = json.loads(output.data) date = datetime.datetime.utcnow().date().strftime('%Y-%m-%d') - self.assertEqual(data, {date: 4}) + # There seems to be a difference in the JSON generated between + # flask-0.10.1 (F23) and 0.11.1 (jenkins) + data == {date: 4} + or + data == [[date, 4]] + ) @patch('pagure.lib.notify.send_email') def test_api_view_user_activity_date(self, mockemail): From 0325cb995eb9b1e00001cc7f1e9f4c56dae7276e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 15:53:54 +0000 Subject: [PATCH 21/31] Add the issue_subscribe ACL to the list of ACLs --- diff --git a/pagure/default_config.py b/pagure/default_config.py index 6b03967..3edab09 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -224,6 +224,7 @@ ACLS = { 'pull_request_comment': 'Comment on a pull-request of this project', 'pull_request_flag': 'Flag a pull-request of this project', 'pull_request_merge': 'Merge a pull-request of this project', + 'issue_subscribe': 'Subscribe the user with this token to an issue', } # Bootstrap URLS From be459f9f719b44128bd373940c5726ffb20f441d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 15:53:58 +0000 Subject: [PATCH 22/31] Add unit-tests for api_subscribe_issue --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index be86391..4832be7 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -1319,6 +1319,146 @@ class PagureFlaskApiIssuetests(tests.Modeltests): {'message': 'Issue assigned'} ) + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_api_subscribe_issue(self, p_send_email, p_ugt): + """ Test the api_subscribe_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_tokens_acl(self.session) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid project + output = self.app.post( + '/api/0/foo/issue/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/issue/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/issue/1/subscribe', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Issue not found", + "error_code": "ENOISSUE", + } + ) + + # Create normal issue + repo = pagure.lib.get_project(self.session, 'test') + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue #1', + content='We should work on this', + user='foo', + ticketfolder=None, + private=False, + issue_uid='aaabbbccc#1', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #1') + + # Check subscribtion before + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertFalse( + pagure.lib.is_watching_obj(self.session, 'pingou', issue)) + + + # Unsubscribe - no changes + data = {} + output = self.app.post( + '/api/0/test/issue/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 issue'} + ) + + data = {} + output = self.app.post( + '/api/0/test/issue/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 issue'} + ) + + # No change + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertFalse( + pagure.lib.is_watching_obj(self.session, 'pingou', issue)) + + # Subscribe + data = {'status': True} + output = self.app.post( + '/api/0/test/issue/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 issue'} + ) + + # Subscribe - no changes + data = {'status': True} + output = self.app.post( + '/api/0/test/issue/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 issue'} + ) + + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertTrue( + pagure.lib.is_watching_obj(self.session, 'pingou', issue)) + + # Unsubscribe + data = {} + output = self.app.post( + '/api/0/test/issue/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 issue'} + ) + + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertFalse( + pagure.lib.is_watching_obj(self.session, 'pingou', issue)) + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase( From b11d54ded35f46c0ccd072bc37997a34e9fcb5db Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 16:01:51 +0000 Subject: [PATCH 23/31] Let's find the acl.id automatically instead of hard-coding it --- diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index 916a6a6..9e0ddcd 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -368,9 +368,14 @@ class PagureFlaskApiForktests(tests.Modeltests): self.session.commit() # Allow the token to close PR + acls = pagure.lib.get_acls(self.session) + acl = None + for acl in acls: + if acl.name == 'pull_request_close': + break item = pagure.lib.model.TokenAcl( token_id='foobar_token', - acl_id=7, + acl_id=acl.id, ) self.session.add(item) self.session.commit() @@ -477,9 +482,14 @@ class PagureFlaskApiForktests(tests.Modeltests): self.session.commit() # Allow the token to merge PR + acls = pagure.lib.get_acls(self.session) + acl = None + for acl in acls: + if acl.name == 'pull_request_merge': + break item = pagure.lib.model.TokenAcl( token_id='foobar_token', - acl_id=10, + acl_id=acl.id, ) self.session.add(item) self.session.commit() From 7507e127a13126b98d86dd14e3b085f42ee3f7ee Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 16:02:09 +0000 Subject: [PATCH 24/31] Fix running the test --- diff --git a/tests/test_pagure_flask_api_user.py b/tests/test_pagure_flask_api_user.py index 7ee7843..3b171a9 100644 --- a/tests/test_pagure_flask_api_user.py +++ b/tests/test_pagure_flask_api_user.py @@ -139,6 +139,7 @@ class PagureFlaskApiUSertests(tests.Modeltests): date = datetime.datetime.utcnow().date().strftime('%Y-%m-%d') # There seems to be a difference in the JSON generated between # flask-0.10.1 (F23) and 0.11.1 (jenkins) + self.assertTrue( data == {date: 4} or data == [[date, 4]] From 58803135a3ce8101b1aca0f6a78b844e1253df7b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 04 2016 16:16:45 +0000 Subject: [PATCH 25/31] Rename InvalidObjetException to InvalidObjectException as found by @tibbs --- diff --git a/pagure/exceptions.py b/pagure/exceptions.py index e02abd6..fed319d 100644 --- a/pagure/exceptions.py +++ b/pagure/exceptions.py @@ -70,6 +70,6 @@ class NoCorrespondingPR(PagureException): pass -class InvalidObjetException(PagureException): +class InvalidObjectException(PagureException): ''' Exception raised when a given object is not what was expected. ''' pass diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 9f6deb1..2d0f1fc 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3224,7 +3224,7 @@ def set_watch_obj(session, user, obj, watch_status): model.PullRequestWatcher.pull_request_uid == obj.uid ) else: - raise pagure.exceptions.InvalidObjetException( + raise pagure.exceptions.InvalidObjectException( 'Unsupported object found: "%s"' % obj ) @@ -3287,7 +3287,7 @@ def is_watching_obj(session, user, obj): model.PullRequestWatcher.pull_request_uid == obj.uid ) else: - raise pagure.exceptions.InvalidObjetException( + raise pagure.exceptions.InvalidObjectException( 'Unsupported object found: "%s"' % obj ) @@ -3418,7 +3418,7 @@ def log_action(session, action, obj): elif obj.isa == 'project': project_id = obj.id else: - raise pagure.exceptions.PagureException( + raise pagure.exceptions.InvalidObjectException( 'Unsupported object found: "%s"' % obj ) From 953cbbc591503f6b8d697d0930faddbf6fd99974 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 05 2016 18:34:29 +0000 Subject: [PATCH 26/31] Order the activity entries by their id rather than relying on dates --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 2d0f1fc..3e72920 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3403,7 +3403,7 @@ def get_user_activity_day(session, user, date): ).filter( model.PagureLog.user_id == user.id ).order_by( - model.PagureLog.date_created + model.PagureLog.id.asc() ) return query.all() From ede575c8b8288c693565b60d796fbac0d5eaaf88 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 05 2016 18:34:49 +0000 Subject: [PATCH 27/31] Fix typo in the exception raised --- diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 271b346..4eff467 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2794,7 +2794,7 @@ class PagureLibtests(tests.Modeltests): # Invalid object to watch - project self.assertRaises( - pagure.exceptions.InvalidObjetException, + pagure.exceptions.InvalidObjectException, pagure.lib.set_watch_obj, self.session, 'foo', iss.project, True ) From abfbd85b9c9bae1977237ff122ff47ba0b0cb3e1 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 05 2016 18:41:02 +0000 Subject: [PATCH 28/31] Drop the username from the documentation of api_subscribe_issue That username is not used as was described since we rely on the user authenticated. --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index f598265..ad0ddfc 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -832,10 +832,6 @@ def api_subscribe_issue(repo, issueid, username=None, namespace=None): +--------------+----------+---------------+---------------------------+ | Key | Type | Optionality | Description | +==============+==========+===============+===========================+ - | ``username`` | string | Mandatory | | The username of the user| - | | | | to (un)subscribe to the | - | | | | issue. | - +--------------+----------+---------------+---------------------------+ | ``status`` | boolean | Mandatory | | The subscription status | | | | | to subscribe or | | | | | unsubscribe to the. | From af7b855152c9ae66436e73d763bc6d6a244a9cf0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 05 2016 18:41:50 +0000 Subject: [PATCH 29/31] Expand the doc on api_view_user_activity_date --- diff --git a/pagure/api/user.py b/pagure/api/user.py index 3625f87..7786aaa 100644 --- a/pagure/api/user.py +++ b/pagure/api/user.py @@ -232,6 +232,9 @@ def api_view_user_activity_date(username, date): GET /api/0/user/ralph/activity/2016-01-02 + GET /api/0/user/ralph/activity/2016-01-02?grouped=true + + Parameters ^^^^^^^^^^ From 92d74d99853c734344fab02f38674e46ccbd4be7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 05 2016 18:42:03 +0000 Subject: [PATCH 30/31] Remove remaining debugging code --- diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index 243ff53..d95bf38 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -855,7 +855,6 @@ $( document ).ready(function() { {% if authenticated %} function set_up_subcribed() { $("#subcribe-btn").click(function(){ - console.log('click'); var _url = "{{ url_for( 'api_ns.api_subscribe_issue', repo=repo.name, From fa91a252c23b186d7dd472069d495cec76e2298f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 05 2016 19:05:49 +0000 Subject: [PATCH 31/31] Small pep8 fix --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 3e72920..253bed4 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -2318,7 +2318,6 @@ def add_email_to_user(session, user, user_email): update_log_email_user(session, user_email, user) - def update_user_ssh(session, user, ssh_key, keydir): ''' Set up a new user into the database or update its information. ''' if isinstance(user, basestring):