From 11d0199fbadcca873936285e277b0d8bfb9fce11 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 30 2018 08:09:38 +0000 Subject: Add the possibility to link issues to pull-requests This commit add this linking at the database level as well as in the UI. Fixes https://pagure.io/pagure/issue/2705 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/hooks/files/default_hook.py b/pagure/hooks/files/default_hook.py index 4694560..00e13fd 100644 --- a/pagure/hooks/files/default_hook.py +++ b/pagure/hooks/files/default_hook.py @@ -149,6 +149,10 @@ def inform_pull_request_urls( # Link to existing PRs if there are any seen = len(prs) != 0 for pr in prs: + # Link tickets with pull-requests if the commit mentions it + pagure.lib.tasks.link_pr_to_ticket.delay(pr.uid) + + # Inform the user about the PR print('View pull-request for %s' % refname) print(' %s/%s/pull-request/%s' % ( _config['APP_URL'].rstrip('/'), diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 0d9f1e1..f020ccb 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1773,6 +1773,8 @@ def new_pull_request(session, branch_from, pagure.lib.git.update_git( request, repo=request.project, repofolder=requestfolder) + pagure.lib.tasks.link_pr_to_ticket.delay(request.uid) + log_action(session, 'created', request, user_obj) if notify: @@ -5055,3 +5057,28 @@ def get_project_family(session, project): ) return [parent] + query.all() + + +def link_pr_issue(session, issue, request): + ''' Associate the specified issue with the specified pull-requets. + + :arg session: The SQLAlchemy session to use + :type session: sqlalchemy.orm.session.Session + :arg issue: The issue mentionned in the commits of the pull-requests to + be associated with + :type issue: pagure.lib.model.Issue + :arg request: A pull-request to associate the specified issue with + :type request: pagure.lib.model.PullRequest + + ''' + + + associated_issue = [iss.uid for iss in request.related_issues] + if issue.uid not in associated_issue: + obj = model.PrToIssue( + pull_request_uid=request.uid, + issue_uid=issue.uid + ) + session.add(obj) + session.flush() + session.commit() diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 533e450..b9c1e35 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1312,6 +1312,28 @@ class IssueToIssue(BASE): primary_key=True) +class PrToIssue(BASE): + """ Stores the associations between issues and pull-requests. + + Table -- pr_to_issue + """ + + __tablename__ = 'pr_to_issue' + + pull_request_uid = sa.Column( + sa.String(32), + sa.ForeignKey( + 'pull_requests.uid', ondelete='CASCADE', onupdate='CASCADE', + ), + primary_key=True) + issue_uid = sa.Column( + sa.String(32), + sa.ForeignKey( + 'issues.uid', ondelete='CASCADE', onupdate='CASCADE', + ), + primary_key=True) + + class IssueComment(BASE): """ Stores the comments made on a commit/file. @@ -1765,6 +1787,14 @@ class PullRequest(BASE): viewonly=True ) + related_issues = relation( + "Issue", + secondary="pr_to_issue", + primaryjoin="pull_requests.c.uid==pr_to_issue.c.pull_request_uid", + secondaryjoin="pr_to_issue.c.issue_uid==issues.c.uid", + backref=backref("related_prs", order_by="pull_requests.c.id.desc()") + ) + def __repr__(self): return 'PullRequest(%s, project:%s, user:%s, title:%s)' % ( self.id, self.project.name, self.user.user, self.title diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index 8a329d1..35e132e 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -32,9 +32,11 @@ from sqlalchemy.exc import SQLAlchemyError import pagure.lib import pagure.lib.git import pagure.lib.git_auth +import pagure.lib.link import pagure.lib.repo import pagure.utils from pagure.config import config as pagure_config +from pagure.utils import get_parent_repo_path # logging.config.dictConfig(pagure_config.get('LOGGING') or {'version': 1}) _log = logging.getLogger(__name__) @@ -850,3 +852,65 @@ def commits_history_stats(self, session, repopath): dates[arrow.get(commit.commit_time).date().isoformat()] += 1 return [(key, dates[key]) for key in sorted(dates)] + + +@conn.task(bind=True) +@pagure_task +def link_pr_to_ticket(self, session, pr_uid): + """ Link the specified pull-request against the ticket(s) mentioned in + the commits of the pull-request + + """ + _log.info( + 'LINK_PR_TO_TICKET: Linking ticket(s) to PR for: %s' % pr_uid) + + request = pagure.lib.get_request_by_uid(session, pr_uid) + if not request: + _log.info('LINK_PR_TO_TICKET: Not PR found for: %s' % pr_uid) + return + + if request.remote: + repopath = pagure.utils.get_remote_repo_path( + request.remote_git, request.branch_from) + parentpath = pagure.utils.get_repo_path(request.project) + else: + repo_from = request.project_from + repopath = pagure.utils.get_repo_path(repo_from) + parentpath = get_parent_repo_path(repo_from) + + repo_obj = pygit2.Repository(repopath) + orig_repo = pygit2.Repository(parentpath) + + diff_commits = pagure.lib.git.diff_pull_request( + session, request, repo_obj, orig_repo, + requestfolder=pagure_config['REQUESTS_FOLDER'], with_diff=False) + + _log.info( + 'LINK_PR_TO_TICKET: Found %s commits in that PR' % len(diff_commits)) + + name = request.project.name + namespace = request.project.namespace + user = request.project.user.user \ + if request.project.is_fork else None + branch = request.branch_from + + for line in pagure.lib.git.read_git_lines( + ['log', '--no-walk'] + + [c.oid.hex for c in diff_commits] + + ['--'], repopath): + + line = line.strip() + for issue in pagure.lib.link.get_relation( + session, name, user, namespace, line, 'fixes', + include_prs=False): + _log.info( + 'LINK_PR_TO_TICKET: Link ticket %s to PRs %s' % ( + issue, request)) + pagure.lib.link_pr_issue(session, issue, request) + + for issue in pagure.lib.link.get_relation( + session, name, user, namespace, line, 'relates'): + _log.info( + 'LINK_PR_TO_TICKET: Link ticket %s to PRs %s' % ( + issue, request)) + pagure.lib.link_pr_issue(session, issue, request) diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index aee9f69..e2ab290 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -365,6 +365,32 @@ {% endif %} + {% if issue.related_prs %} +
+
+
+ +
+
    + {% for pr in issue.related_prs %} +
  • + #{{pr.id}} + {{ pr.status if pr.status != 'Open' else 'Last updated' + }} {{ pr.last_updated | humanize }} +
    • + {% endfor %} +
+
+
+
+
+ {% endif %} + {% if repo.issue_keys %}
diff --git a/tests/__init__.py b/tests/__init__.py index 520e6d3..4eec5b0 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -476,31 +476,37 @@ class FakeUser(object): # pylint: disable=too-few-public-methods return self.dic[key] -def create_projects(session): +def create_projects(session, is_fork=False, user_id=1, hook_token_suffix=''): """ Create some projects in the database. """ item = pagure.lib.model.Project( - user_id=1, # pingou + user_id=user_id, # pingou name='test', + is_fork=is_fork, + parent_id=1 if is_fork else None, description='test project #1', - hook_token='aaabbbccc', + hook_token='aaabbbccc' + hook_token_suffix, ) item.close_status = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] session.add(item) item = pagure.lib.model.Project( - user_id=1, # pingou + user_id=user_id, # pingou name='test2', + is_fork=is_fork, + parent_id=2 if is_fork else None, description='test project #2', - hook_token='aaabbbddd', + hook_token='aaabbbddd' + hook_token_suffix, ) item.close_status = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] session.add(item) item = pagure.lib.model.Project( - user_id=1, # pingou + user_id=user_id, # pingou name='test3', + is_fork=is_fork, + parent_id=3 if is_fork else None, description='namespaced test project', - hook_token='aaabbbeee', + hook_token='aaabbbeee' + hook_token_suffix, namespace='somenamespace', ) item.close_status = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] diff --git a/tests/test_pagure_flask_ui_issue_pr_link.py b/tests/test_pagure_flask_ui_issue_pr_link.py new file mode 100644 index 0000000..a82e9a8 --- /dev/null +++ b/tests/test_pagure_flask_ui_issue_pr_link.py @@ -0,0 +1,160 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2018 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources + +import datetime +import json +import unittest +import re +import shutil +import sys +import tempfile +import time +import os + +import pygit2 +from mock import ANY, patch, MagicMock + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure +import pagure.lib +import tests +from pagure.lib.repo import PagureRepo + + +class PagureFlaskPrIssueLinkTest(tests.Modeltests): + """ Tests pagure when linking PRs to tickets """ + + maxDiff = None + + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskPrIssueLinkTest, self).setUp() + + tests.create_projects(self.session) + tests.create_projects( + self.session, is_fork=True, user_id=2, hook_token_suffix='bar') + tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) + tests.create_projects_git(os.path.join( + self.path, 'repos', 'forks', 'foo'), bare=True) + + repo = pagure.lib.get_authorized_project(self.session, 'test') + + # Create issues to play with + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title=u'tést íssüé', + content='We should work on this', + user='pingou', + ticketfolder=None + ) + self.session.commit() + self.assertEqual(msg.title, u'tést íssüé') + + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title=u'tést íssüé #2', + content='We should still work on this', + user='foo', + ticketfolder=None + ) + self.session.commit() + self.assertEqual(msg.title, u'tést íssüé #2') + + # Add a commit to the fork + + newpath = tempfile.mkdtemp(prefix='pagure-fork-test') + repopath = os.path.join(newpath, 'test') + clone_repo = pygit2.clone_repository(os.path.join( + self.path, 'repos', 'forks', 'foo', 'test.git'), repopath) + + # Create a file in that git repo + with open(os.path.join(repopath, 'sources'), 'w') as stream: + stream.write('foo\n bar') + clone_repo.index.add('sources') + clone_repo.index.write() + + try: + com = repo.revparse_single('HEAD') + prev_commit = [com.oid.hex] + except: + prev_commit = [] + + # Commits the files added + tree = clone_repo.index.write_tree() + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + clone_repo.create_commit( + 'refs/heads/master', # the name of the reference to update + author, + committer, + 'Add sources file for testing\n\n Relates to #2', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + prev_commit + ) + refname = 'refs/heads/master:refs/heads/master' + ori_remote = clone_repo.remotes[0] + PagureRepo.push(ori_remote, refname) + + # Create the corresponding PR + + repo = pagure.lib.get_authorized_project(self.session, 'test') + fork_repo = pagure.lib.get_authorized_project( + self.session, 'test', user='foo') + + request = pagure.lib.new_pull_request( + self.session, + branch_from='master', + repo_to=repo, + branch_to='master', + title='test PR', + user='foo', + requestfolder=None, + initial_comment=None, + repo_from=fork_repo, + ) + self.session.commit() + + pagure.lib.tasks.link_pr_to_ticket(request.uid) + self.assertEqual(request.id, 3) + + def test_ticket_no_link(self): + """ Test that no Related PR(s) block is showing in the issue page. + """ + output = self.app.get('/test/issue/1') + self.assertEqual(output.status_code, 200) + self.assertNotIn( + u'Related PR(s)', + output.data.decode('utf-8')) + + def test_ticket_link(self): + """ Test that no Related PR(s) block is showing in the issue page. + """ + time.sleep(1) + output = self.app.get('/test/issue/2') + print output.data.decode('utf-8') + self.assertEqual(output.status_code, 200) + self.assertIn( + u'Related PR(s)', + output.data.decode('utf-8')) + + +if __name__ == '__main__': + unittest.main(verbosity=2)