From 2fba89436d50c72d2717d376b9e416dfcb97a8a0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 27 2016 17:39:59 +0000 Subject: [PATCH 1/9] Add a notification flag on comment of tickets --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 2475936..44ac79e 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -699,6 +699,8 @@ class IssueComment(BASE): ), nullable=False, index=True) + + notification = sa.Column(sa.Boolean, default=False, nullable=False) edited_on = sa.Column(sa.DateTime, nullable=True) editor_id = sa.Column( sa.Integer, From 65d1e33e71697dedf714072437b62fc4bea2dfb0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 27 2016 17:39:59 +0000 Subject: [PATCH 2/9] Add an alembic migration script to add the notification field to issue_comments --- diff --git a/alembic/versions/22db0a833d35_add_notifications_to_tickets.py b/alembic/versions/22db0a833d35_add_notifications_to_tickets.py new file mode 100644 index 0000000..20bfad7 --- /dev/null +++ b/alembic/versions/22db0a833d35_add_notifications_to_tickets.py @@ -0,0 +1,33 @@ +"""Add notifications to tickets + +Revision ID: 22db0a833d35 +Revises: 317a285e04a8 +Create Date: 2016-06-27 16:10:33.395495 + +""" + +# revision identifiers, used by Alembic. +revision = '22db0a833d35' +down_revision = '317a285e04a8' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ''' Add the column notification to the table issue_comments. + ''' + op.add_column( + 'issue_comments', + sa.Column('notification', sa.Boolean, default=False, nullable=True) + ) + op.execute('''UPDATE "issue_comments" SET notification=False;''') + op.alter_column( + 'issue_comments', 'notification', + nullable=False, existing_nullable=True) + + +def downgrade(): + ''' Remove the column notification from the table issue_comments. + ''' + op.drop_column('issue_comments', 'notification') From 92563f358d23542974be47796d1139f3380432b7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 27 2016 17:39:59 +0000 Subject: [PATCH 3/9] Adjust docstring in the alembic migration adding notifications to PR comments --- diff --git a/alembic/versions/58e60d869326_add_notification_bool_to_pr.py b/alembic/versions/58e60d869326_add_notification_bool_to_pr.py index 164b730..2c7603b 100644 --- a/alembic/versions/58e60d869326_add_notification_bool_to_pr.py +++ b/alembic/versions/58e60d869326_add_notification_bool_to_pr.py @@ -15,7 +15,7 @@ import sqlalchemy as sa def upgrade(): - ''' Add the column merge_status to the table projects. + ''' Add the column notification to the table pull_request_comments. ''' op.add_column( 'pull_request_comments', @@ -28,6 +28,6 @@ def upgrade(): def downgrade(): - ''' Remove the column merge_status from the table projects. + ''' Remove the column notification from the table pull_request_comments. ''' op.drop_column('pull_request_comments', 'notification') From 63d32cb104cb2ef917ac2fa2288484f71f139c8b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 27 2016 17:39:59 +0000 Subject: [PATCH 4/9] Include the notification status in the JSON and when adding a comment to a ticket --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 85ab87f..bccb055 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -206,7 +206,7 @@ def create_user_ssh_keys_on_disk(user, gitolite_keydir): def add_issue_comment(session, issue, comment, user, ticketfolder, - notify=True, date_created=None): + notify=True, date_created=None, notification=False): ''' Add a comment to an issue. ''' user_obj = __get_user(session, user) @@ -215,6 +215,7 @@ def add_issue_comment(session, issue, comment, user, ticketfolder, comment=comment, user_id=user_obj.id, date_created=date_created, + notification=notification, ) session.add(issue_comment) # Make sure we won't have SQLAlchemy error before we continue @@ -255,6 +256,7 @@ def add_issue_comment(session, issue, comment, user, ticketfolder, issue_comment.user.default_email, size=16), 'comment_date': issue_comment.date_created.strftime( '%Y-%m-%d %H:%M:%S'), + 'notification': notification, })) return 'Comment added' diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 44ac79e..ac973cd 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -749,6 +749,7 @@ class IssueComment(BASE): 'user': self.user.to_json(public=public), 'edited_on': self.edited_on.strftime('%s') if self.edited_on else None, 'editor': self.editor.to_json(public=public) if self.editor_id else None, + 'notification': self.notification, } return output From 1e157cf431d6f6be3990e3e47d1d0c139302fbc8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 27 2016 17:39:59 +0000 Subject: [PATCH 5/9] Display the notifications on tickets as notifications not comments --- diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index 58b9427..288363a 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -41,7 +41,21 @@
{% if issue.comments %} {% for comment in issue.comments %} + {% if comment.notification %} +
+
+
+
+ {{ + comment.date_created | humanize }} +
+ {{ comment.comment | markdown | noJS | safe }} +
+
+
+ {% else %} {{ show_comment(comment, comment.id, repo, username, issueid, form, repo_admin) }} + {% endif %} {% endfor %} {% endif %}
From c077f3f09091cf029e1ea5c750c2f383eadd9fde Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 27 2016 17:39:59 +0000 Subject: [PATCH 6/9] Add a notification to the ticket after updating its status --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index bccb055..728e00a 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1247,6 +1247,17 @@ def edit_issue(session, issue, ticketfolder, user, pagure.lib.git.update_git( issue, repo=issue.project, repofolder=ticketfolder) + if 'status' in edit: + add_issue_comment( + session, + issue, + comment='@%s changed the status to ``%s``' % (user, status), + user=user, + ticketfolder=ticketfolder, + notify=False, + notification=True, + ) + if not issue.private and edit: pagure.lib.notify.log( issue.project, From a15c90a3342ea072e1a280e13af4015865fc6a97 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 27 2016 17:39:59 +0000 Subject: [PATCH 7/9] Hide the notifications when showing the number of comments on a ticket --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index ac973cd..a87f035 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -616,6 +616,12 @@ class Issue(BASE): ''' Return the list of issue this issue blocks on in simple text. ''' return [issue.id for issue in self.parents] + @property + def user_comments(self): + ''' Return user comments only, filter it from notifications + ''' + return [comment for comment in self.comments if not comment.notification] + def to_json(self, public=False, with_comments=True): ''' Returns a dictionary representation of the issue. diff --git a/pagure/templates/issues.html b/pagure/templates/issues.html index d664157..b1c4330 100644 --- a/pagure/templates/issues.html +++ b/pagure/templates/issues.html @@ -125,11 +125,11 @@ {{ issue.title | noJS("img") | safe }}    - {% if issue.comments|count > 0 %} + {% if issue.user_comments|count > 0 %} - {{issue.comments|count}} + {{issue.user_comments|count}} {% endif %} {% for tag in issue.tags%} From bda7f171dd6203eb436afa97a185a6607249b8aa Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 27 2016 17:39:59 +0000 Subject: [PATCH 8/9] Fix the tests for the change in the JSON representation of comment on tickets --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index e668cfe..ad2520c 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -1034,6 +1034,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "date_created": "1435821770", "edited_on": None, "editor": None, + "notification": False, "id": 1, "parent": None, "user": { @@ -1059,6 +1060,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "date_created": "1435821770", "edited_on": None, "editor": None, + "notification": False, "id": 1, "parent": None, "user": { @@ -1104,6 +1106,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "date_created": "1435821770", "edited_on": None, "editor": None, + "notification": False, "id": 2, "parent": None, "user": { diff --git a/tests/test_pagure_lib_git.py b/tests/test_pagure_lib_git.py index 6eb452b..f490f99 100644 --- a/tests/test_pagure_lib_git.py +++ b/tests/test_pagure_lib_git.py @@ -19,7 +19,6 @@ import os import tempfile import pygit2 from mock import patch -from pagure.lib.repo import PagureRepo sys.path.insert(0, os.path.join(os.path.dirname( os.path.abspath(__file__)), '..')) @@ -27,6 +26,8 @@ sys.path.insert(0, os.path.join(os.path.dirname( import pagure.lib.git import tests +from pagure.lib.repo import PagureRepo + class PagureLibGittests(tests.Modeltests): """ Tests for pagure.lib.git """ @@ -549,7 +550,7 @@ diff --git a/123 b/456 index 458821a..77674a8 --- a/123 +++ b/456 -@@ -1,7 +1,24 @@ +@@ -1,7 +1,25 @@ { "assignee": null, "blocks": [], @@ -561,6 +562,7 @@ index 458821a..77674a8 + "edited_on": null, + "editor": null, + "id": 1, ++ "notification": false, + "parent": null, + "user": { + "default_email": "foo@bar.com", From 247fc2e4e493c95d2cd4329f8d309d0db7e50ec3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 27 2016 17:39:59 +0000 Subject: [PATCH 9/9] Add test checking that the notification got added after updating the ticket's status --- diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index e9d51d2..71b7281 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -481,6 +481,7 @@ class PagureFlaskIssuestests(tests.Modeltests): '' in output.data) + # Right status, wrong csrf data['status'] = 'Fixed' output = self.app.post( '/test/issue/1/update', data=data, follow_redirects=True) @@ -496,6 +497,7 @@ class PagureFlaskIssuestests(tests.Modeltests): '' in output.data) + # working status update data['csrf_token'] = csrf_token output = self.app.post( '/test/issue/1/update', data=data, follow_redirects=True) @@ -513,6 +515,10 @@ class PagureFlaskIssuestests(tests.Modeltests): self.assertTrue( '' in output.data) + self.assertIn( + '

@pingou ' + 'changed the status to Fixed

', + output.data) # Add new comment data = {