From bf07ec1edb7574a73d4b08fcede5c672aade01fe Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 15:37:14 +0000 Subject: [PATCH 1/10] Fix showing inline comments in the main frame --- diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html index 8b9eb05..1123f81 100644 --- a/pagure/templates/pull_request.html +++ b/pagure/templates/pull_request.html @@ -330,8 +330,8 @@ {{ comment.date_created | humanize}} - (Show) -
+ (Show) +
{{ comment.comment }}
From 4b466f1685a94c92c186c93fe27ab4861e9a2485 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 15:37:28 +0000 Subject: [PATCH 2/10] Add a tree_id to the pull_request_comments table --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index e5d7efe..b6e0cb6 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -964,6 +964,9 @@ class PullRequestComment(BASE): line = sa.Column( sa.Integer, nullable=True) + tree_id = sa.Column( + sa.String(40), + nullable=True) comment = sa.Column( sa.Text(), nullable=False) @@ -1014,6 +1017,7 @@ class PullRequestComment(BASE): return { 'id': self.id, 'commit': self.commit_id, + 'tree': self.tree_id, 'filename': self.filename, 'line': self.line, 'comment': self.comment, From d9dde7d70471afd867f9b55df4e9690498b8b2a8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 15:37:50 +0000 Subject: [PATCH 3/10] Add an alembic migration script to add tree_id to pull_request_comments --- diff --git a/alembic/versions/1f3de3853a1a_add_the_tree_id_column_to_pr_inline_.py b/alembic/versions/1f3de3853a1a_add_the_tree_id_column_to_pr_inline_.py new file mode 100644 index 0000000..eb41dcd --- /dev/null +++ b/alembic/versions/1f3de3853a1a_add_the_tree_id_column_to_pr_inline_.py @@ -0,0 +1,29 @@ +"""Add the tree_id column to PR inline comments + +Revision ID: 1f3de3853a1a +Revises: 58e60d869326 +Create Date: 2016-02-22 16:13:59.943083 + +""" + +# revision identifiers, used by Alembic. +revision = '1f3de3853a1a' +down_revision = '58e60d869326' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ''' Add the column tree_id to the table pull_request_comments. + ''' + op.add_column( + 'pull_request_comments', + sa.Column('tree_id', sa.String(40), nullable=True) + ) + + +def downgrade(): + ''' Remove the column tree_id from the table pull_request_comments. + ''' + op.drop_column('pull_request_comments', 'tree_id') From 4fa09b8caae84ac41d954e38ffc52216f84310f8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 15:38:13 +0000 Subject: [PATCH 4/10] Adjust add_pull_request_comment to support tree_id --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index be48a18..f32d3d3 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -789,15 +789,16 @@ def add_group_to_project(session, project, new_group, user): return 'Group added' -def add_pull_request_comment(session, request, commit, filename, row, - comment, user, requestfolder, notify=True, - notification=False): +def add_pull_request_comment(session, request, commit, tree_id, filename, + row, comment, user, requestfolder, + notify=True, notification=False): ''' Add a comment to a pull-request. ''' user_obj = __get_user(session, user) pr_comment = model.PullRequestComment( pull_request_uid=request.uid, commit_id=commit, + tree_id=tree_id, filename=filename, line=row, comment=comment, From c72242aeb49bb2bf291d7486c45931b6f3cd325d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 15:38:40 +0000 Subject: [PATCH 5/10] Specify the tree identifier when building the table of code --- diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index f64cfb6..418f653 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -52,7 +52,8 @@ def format_ts(string): @APP.template_filter('format_loc') -def format_loc(loc, commit=None, filename=None, prequest=None, index=None): +def format_loc(loc, commit=None, filename=None, tree=None, prequest=None, + index=None): """ Template filter putting the provided lines of code into a table """ if loc is None: @@ -88,7 +89,8 @@ def format_loc(loc, commit=None, filename=None, prequest=None, index=None): '' '' '' + ' data-filename="%(filename)s" data-commit="%(commit)s"' + ' data-tree="%(tree)s">' '

' '' '

' @@ -99,6 +101,7 @@ def format_loc(loc, commit=None, filename=None, prequest=None, index=None): 'img': flask.url_for('static', filename='users.png'), 'filename': filename, 'commit': commit, + 'tree': tree, } ) ) From 444b6124240f016e732165a85e69fa7aa132bc56 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 15:39:43 +0000 Subject: [PATCH 6/10] Save the tree_id when adding a comment to a PR --- diff --git a/pagure/forms.py b/pagure/forms.py index 99a3fa9..2bbfe62 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -206,6 +206,7 @@ class AddPullRequestCommentForm(wtf.Form): filename = wtforms.HiddenField('file changed') row = wtforms.HiddenField('row') requestid = wtforms.HiddenField('requestid') + tree_id = wtforms.HiddenField('treeid') comment = wtforms.TextAreaField( 'Comment*', [wtforms.validators.Required()] diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html index 1123f81..49d650f 100644 --- a/pagure/templates/pull_request.html +++ b/pagure/templates/pull_request.html @@ -296,7 +296,8 @@ filename=patch_new_file_path, commit=patch_new_id, prequest=pull_request, - index=loop.index)}} + index=loop.index, + tree=diff_commits[0].tree.id)}} {% endautoescape %} @@ -667,10 +668,14 @@ function setup_reply_btns() { var row = $( this ).attr('data-row'); var commit = $( this ).attr('data-commit'); var filename = $( this ).attr('data-filename'); + var tree_id = $( this ).attr('data-tree'); var url = "{{ url_for( 'pull_request_add_comment', username=username, repo=repo.name, requestid=requestid, commit='', filename='', row='') }}".slice(0, -2); - url = url + commit + '/' + filename + '/' + row; + url = url + commit + '/' + filename + '/' + row + if (tree_id) { + url += '?tree_id=' + tree_id; + } var rowid = $(this).prev().find('a').attr('id'); var table = $( this ).parent().parent(); var nextid = rowid.replace('_' + row, '_' + (Number(row) + 1)); diff --git a/pagure/templates/pull_request_comment.html b/pagure/templates/pull_request_comment.html index 37783f6..882bba8 100644 --- a/pagure/templates/pull_request_comment.html +++ b/pagure/templates/pull_request_comment.html @@ -4,7 +4,8 @@
diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index b1a15e7..b992ff8 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -451,12 +451,14 @@ def pull_request_add_comment( flask.abort(404, 'Pull-request not found') is_js = flask.request.args.get('js', False) + tree_id = flask.request.args.get('tree_id') or None form = pagure.forms.AddPullRequestCommentForm() form.commit.data = commit form.filename.data = filename form.requestid.data = requestid form.row.data = row + form.tree_id.data = tree_id if form.validate_on_submit(): comment = form.comment.data @@ -466,6 +468,7 @@ def pull_request_add_comment( SESSION, request=request, commit=commit, + tree_id=tree_id, filename=filename, row=row, comment=comment, @@ -498,6 +501,7 @@ def pull_request_add_comment( repo=repo, username=username, commit=commit, + tree_id=tree_id, filename=filename, row=row, form=form, From 626dbaefbd149791cd606911e487262ade88800b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 15:55:03 +0000 Subject: [PATCH 7/10] Add support for tree_id in the API endpoint used to add comments to a PR --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 0bd5154..4dc7153 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -434,6 +434,10 @@ def api_pull_request_add_comment(repo, requestid, username=None): | | | | on a specific row | | | | | of a file | +---------------+---------+--------------+-----------------------------+ + | ``tree_id`` | string | Optional | | The identifier of the | + | | | | git tree as it was when | + | | | | the comment was added | + +---------------+---------+--------------+-----------------------------+ Sample response ^^^^^^^^^^^^^^^ @@ -469,6 +473,7 @@ def api_pull_request_add_comment(repo, requestid, username=None): comment = form.comment.data commit = form.commit.data or None filename = form.filename.data or None + tree_id = form.tree_id.data or None row = form.row.data or None try: # New comment @@ -476,6 +481,7 @@ def api_pull_request_add_comment(repo, requestid, username=None): SESSION, request=request, commit=commit, + tree_id=tree_id, filename=filename, row=row, comment=comment, From 0a04458744641b33e79e65f3d8657657dcc591a3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 15:55:30 +0000 Subject: [PATCH 8/10] Adjust all the calls to pull_request_add_comment() in the code for tree_id --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index ea1745d..2ad969b 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -94,6 +94,7 @@ def pull_request_add_comment(): flask.abort(400, 'Invalid request') commit = form.commit.data or None + tree_id = form.tree_id.data or None filename = form.filename.data or None row = form.row.data or None comment = form.comment.data @@ -103,6 +104,7 @@ def pull_request_add_comment(): pagure.SESSION, request=request, commit=commit, + tree_id=tree_id, filename=filename, row=row, comment=comment, diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index f32d3d3..c291849 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1819,7 +1819,7 @@ def close_pull_request(session, request, user, requestfolder, merged=True): pagure.lib.add_pull_request_comment( session, request, - commit=None, filename=None, row=None, + commit=None, tree_id=None, filename=None, row=None, comment='Pull-Request has been %s by %s' % ( request.status.lower(), user), user=user, diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 7a7bdd7..7b55ea4 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -587,6 +587,7 @@ def update_request_from_git( session, request, commit=comment['commit'], + tree_id=comment.get('tree_id') or None, filename=comment['filename'], row=comment['line'], comment=comment['comment'], @@ -1164,7 +1165,7 @@ def diff_pull_request( if request.merge_status is None: pagure.lib.add_pull_request_comment( session, request, - commit=None, filename=None, row=None, + commit=None, tree_id=None, filename=None, row=None, comment='Pull-Request has been %s' % verb, user=request.user.username, requestfolder=requestfolder, From c18b96f3fbbe4d26e6e839e9b685a09903df1ad9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 15:55:47 +0000 Subject: [PATCH 9/10] Adjust all the calls to pull_request_add_comment() in the tests for tree_id --- diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index dfea3c6..39b5140 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -1515,6 +1515,7 @@ index 0000000..fb7093d session=self.session, request=request, commit='commithash', + tree_id=None, filename='file', row=None, comment='This is awesome, I got to remember it!', diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index bba3973..4837c99 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -1331,6 +1331,7 @@ class PagureLibtests(tests.Modeltests): session=self.session, request=request, commit='commithash', + tree_id=None, filename='file', row=None, comment='This is awesome, I got to remember it!', @@ -1816,6 +1817,7 @@ class PagureLibtests(tests.Modeltests): session=self.session, request=request, commit=None, + tree_id=None, filename=None, row=None, comment='This looks great :thumbsup:', @@ -1829,6 +1831,7 @@ class PagureLibtests(tests.Modeltests): session=self.session, request=request, commit=None, + tree_id=None, filename=None, row=None, comment='I disagree -1', @@ -1842,6 +1845,7 @@ class PagureLibtests(tests.Modeltests): session=self.session, request=request, commit=None, + tree_id=None, filename=None, row=None, comment='NM this looks great now +1000', From c2e1f7da8f4be7ec15de4e483a69702671aa3453 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 16:02:29 +0000 Subject: [PATCH 10/10] Rename variable from tree to tree_id for consistency --- diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html index 49d650f..4a31c60 100644 --- a/pagure/templates/pull_request.html +++ b/pagure/templates/pull_request.html @@ -297,7 +297,7 @@ commit=patch_new_id, prequest=pull_request, index=loop.index, - tree=diff_commits[0].tree.id)}} + tree_id=diff_commits[0].tree.id)}} {% endautoescape %} diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index 418f653..1f877db 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -52,7 +52,7 @@ def format_ts(string): @APP.template_filter('format_loc') -def format_loc(loc, commit=None, filename=None, tree=None, prequest=None, +def format_loc(loc, commit=None, filename=None, tree_id=None, prequest=None, index=None): """ Template filter putting the provided lines of code into a table """ @@ -90,7 +90,7 @@ def format_loc(loc, commit=None, filename=None, tree=None, prequest=None, '' '' + ' data-tree="%(tree_id)s">' '

' '' '

' @@ -101,7 +101,7 @@ def format_loc(loc, commit=None, filename=None, tree=None, prequest=None, 'img': flask.url_for('static', filename='users.png'), 'filename': filename, 'commit': commit, - 'tree': tree, + 'tree_id': tree_id, } ) )