From e7eebb1400d986394aa404b4c3385ea452f9e92d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 09 2017 11:20:47 +0000 Subject: [PATCH 1/5] If no assignee is provided, do not try updating the assignee field The method pagure.lib.add_issue_assignee() always returns a message, so if we call this method while there are no assignee provided, it returns a 'Nothing to change' message that we display to our user, while in fact things did change, just not the assignee. With this change, we no longer have this un-justified message. --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 2a12cb4..e91573d 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -241,16 +241,17 @@ def update_issue(repo, issueid, username=None, namespace=None): # other fields will be missing for non-admin and thus reset if we let them if repo_admin: # Assign or update assignee of the ticket - message = pagure.lib.add_issue_assignee( - SESSION, - issue=issue, - assignee=assignee or None, - user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'], - ) - SESSION.commit() - if message: - messages.add(message) + if assignee: + message = pagure.lib.add_issue_assignee( + SESSION, + issue=issue, + assignee=assignee or None, + user=flask.g.fas_user.username, + ticketfolder=APP.config['TICKETS_FOLDER'], + ) + SESSION.commit() + if message: + messages.add(message) # Update priority if str(new_priority) in repo.priorities: From e5f996f65a25892c48194fd003ae8c40f6f166ab Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 09 2017 11:20:47 +0000 Subject: [PATCH 2/5] Add a method to notify user of status changes to issues --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index 384fc90..3fb0e69 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -397,6 +397,39 @@ The issue: `%s` of project: `%s` has been %s by %s. ) +def notify_status_change_issue(issue, user): + ''' Notify the people following a project that an issue changed status. + ''' + status = issue.status + if status.lower() != 'open' and issue.close_status: + status = '%s as %s' % (status, issue.close_status) + text = u""" +The status of the issue: `%s` of project: `%s` has been updated to: %s by %s. + +%s +""" % (issue.title, + issue.project.fullname, + status, + user.username, + _build_url( + pagure.APP.config['APP_URL'], + _fullname_to_url(issue.project.fullname), + 'issue', + issue.id)) + mail_to = _get_emails_for_obj(issue) + + uid = time.mktime(datetime.datetime.now().timetuple()) + send_email( + text, + 'Issue #%s `%s`' % (issue.id, issue.title), + ','.join(mail_to), + mail_id='%s/close/%s' % (issue.mail_id, uid), + in_reply_to=issue.mail_id, + project_name=issue.project.fullname, + user_from=user.fullname or user.user, + ) + + def notify_assigned_request(request, new_assignee, user): ''' Notify the people following a pull-request that the assignee changed. ''' From b32a293802d25037caa9c49575981a00682d29bd Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 09 2017 11:20:47 +0000 Subject: [PATCH 3/5] Notify the users of status changes on ticket Fixes https://pagure.io/pagure/issue/1643 --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 222c334..f0bae8b 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1439,6 +1439,7 @@ def edit_issue(session, issue, ticketfolder, user, notify=False, notification=True, ) + pagure.lib.notify.notify_status_change_issue(issue, user_obj) if not issue.private and edit: pagure.lib.notify.log( From d5262a34fec0a41f25694a1d502d9de349a832b8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 09 2017 11:20:47 +0000 Subject: [PATCH 4/5] Fix re-setting the close_status to None or reset it when re-opening a ticket This commit allows to re-set the close_status to nothing (its default value when the ticket is open), it also does so automatically when the status of a ticket is set to 'Open' (ie: the ticket is re-opened). Fixes : https://pagure.io/pagure/issue/1526 --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index f0bae8b..abcbb94 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1381,7 +1381,7 @@ def new_tag(session, tag_name, tag_color, project_id): def edit_issue(session, issue, ticketfolder, user, - title=None, content=None, status=None, close_status=None, + title=None, content=None, status=None, close_status=-1, priority=None, milestone=None, private=False): ''' Edit the specified issue. ''' @@ -1405,8 +1405,11 @@ def edit_issue(session, issue, ticketfolder, user, issue.status = status if status.lower() != 'open': issue.closed_at = datetime.datetime.utcnow() + elif issue.close_status: + issue.close_status = None + edit.append('close_status') edit.append('status') - if close_status and close_status != issue.close_status: + if close_status != -1 and close_status != issue.close_status: issue.close_status = close_status edit.append('close_status') if priority: @@ -1424,6 +1427,8 @@ def edit_issue(session, issue, ticketfolder, user, issue.milestone = milestone edit.append('milestone') issue.last_updated = datetime.datetime.utcnow() + # uniquify the list of edited fields + edit = list(set(edit)) pagure.lib.git.update_git( issue, repo=issue.project, repofolder=ticketfolder) @@ -1448,7 +1453,7 @@ def edit_issue(session, issue, ticketfolder, user, msg=dict( issue=issue.to_json(public=True), project=issue.project.to_json(public=True), - fields=edit, + fields=list(set(edit)), agent=user_obj.username, ), redis=REDIS, diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 3ea6706..6aa9fa4 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -266,8 +266,50 @@ class PagureLibtests(tests.Modeltests): self.session.commit() self.assertEqual(msg, 'Successfully edited issue #2') + repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual(repo.open_tickets, 1) + self.assertEqual(repo.open_tickets_public, 1) + self.assertEqual(repo.issues[1].status, 'Closed') + self.assertEqual(repo.issues[1].close_status, 'Invalid') + + # Edit the status: re-open the ticket + msg = pagure.lib.edit_issue( + session=self.session, + issue=issue, + user='pingou', + status='Open', + ticketfolder=None, + private=True, + ) + self.session.commit() + self.assertEqual(msg, 'Successfully edited issue #2') + + repo = pagure.lib.get_project(self.session, 'test') + for issue in repo.issues: + self.assertEqual(issue.status, 'Open') + self.assertEqual(issue.close_status, None) + # 2 open but one of them is private + self.assertEqual(repo.open_tickets, 2) + self.assertEqual(repo.open_tickets_public, 1) + + # Edit the status: re-close the ticket + msg = pagure.lib.edit_issue( + session=self.session, + issue=issue, + user='pingou', + status='Closed', + close_status='Invalid', + ticketfolder=None, + private=True, + ) + self.session.commit() + self.assertEqual(msg, 'Successfully edited issue #2') + + repo = pagure.lib.get_project(self.session, 'test') self.assertEqual(repo.open_tickets, 1) self.assertEqual(repo.open_tickets_public, 1) + self.assertEqual(repo.issues[1].status, 'Closed') + self.assertEqual(repo.issues[1].close_status, 'Invalid') @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') From b873ef03bb7457ab0f576ff159fac803cf0eb89a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 09 2017 11:20:47 +0000 Subject: [PATCH 5/5] Allow setting the assignee to None but just discard any un-interested messages As mentioned before, the 'Nothing changed' message is sent just by this method, so if another method changed something, we end up with two messages one saying 'Nothing changed' and the other saying that something did change. Not nice. This fixes it. --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index e91573d..4ce8d5d 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -241,17 +241,16 @@ def update_issue(repo, issueid, username=None, namespace=None): # other fields will be missing for non-admin and thus reset if we let them if repo_admin: # Assign or update assignee of the ticket - if assignee: - message = pagure.lib.add_issue_assignee( - SESSION, - issue=issue, - assignee=assignee or None, - user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'], - ) - SESSION.commit() - if message: - messages.add(message) + message = pagure.lib.add_issue_assignee( + SESSION, + issue=issue, + assignee=assignee or None, + user=flask.g.fas_user.username, + ticketfolder=APP.config['TICKETS_FOLDER'], + ) + SESSION.commit() + if message and message != 'Nothing to change': + messages.add(message) # Update priority if str(new_priority) in repo.priorities: