diff --git a/alembic/versions/003fcd9e8860_add_an_origin_field_on_pr_to_issue.py b/alembic/versions/003fcd9e8860_add_an_origin_field_on_pr_to_issue.py new file mode 100644 index 0000000..cba9504 --- /dev/null +++ b/alembic/versions/003fcd9e8860_add_an_origin_field_on_pr_to_issue.py @@ -0,0 +1,33 @@ +"""Add an origin field on pr_to_issue + +Revision ID: 003fcd9e8860 +Revises: 2b1743f77436 +Create Date: 2019-03-12 14:55:59.316861 + +""" + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '003fcd9e8860' +down_revision = '2b1743f77436' + + +def upgrade(): + ''' Add a column to record if the custom field should trigger a email + notification. + ''' + op.add_column( + 'pr_to_issue', + sa.Column( + 'origin', sa.String(32), index=True + ) + ) + + +def downgrade(): + ''' Remove the key_notify column. + ''' + op.drop_column('pr_to_issue', 'origin') diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 4de2d35..e676e4d 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -492,6 +492,10 @@ def api_pull_request_update(repo, requestid, username=None, namespace=None): request.initial_comment = form.initial_comment.data.strip() flask.g.session.add(request) try: + # Link the PR to issue(s) if there is such link + pagure.lib.query.link_pr_to_issue_on_description( + flask.g.session, request + ) flask.g.session.commit() except SQLAlchemyError as err: # pragma: no cover flask.g.session.rollback() diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 7dd8b33..97de6d5 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1506,6 +1506,7 @@ class PrToIssue(BASE): sa.ForeignKey("issues.uid", ondelete="CASCADE", onupdate="CASCADE"), primary_key=True, ) + origin = sa.Column(sa.String(32), index=True) class IssueComment(BASE): diff --git a/pagure/lib/query.py b/pagure/lib/query.py index 5313c9e..878aaf3 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -54,6 +54,7 @@ from flask import url_for import pagure.exceptions import pagure.lib.git import pagure.lib.git_auth +import pagure.lib.link import pagure.lib.login import pagure.lib.notify import pagure.lib.plugins @@ -1878,6 +1879,9 @@ def new_pull_request( request.updated_on = datetime.datetime.utcnow() session.add(request) + # Link the PR to issue(s) if there is such link + link_pr_to_issue_on_description(session, request) + # Make sure we won't have SQLAlchemy error before we create the request session.flush() @@ -1926,6 +1930,59 @@ def new_pull_request( return request +def link_pr_to_issue_on_description(session, request): + """ Link the given request to issues it may be referring to in its + description if there is a description and such link in it. + """ + _log.debug("Drop the existing relations") + # Drop the existing initial_comment_pr-based relations + session.query(model.PrToIssue).filter( + model.PrToIssue.pull_request_uid == request.uid + ).filter(model.PrToIssue.origin == "intial_comment_pr").delete( + synchronize_session="fetch" + ) + + # Rebuild the relations from the initial comment + if request.initial_comment: + _log.debug("Checking the initial comment for relations") + for line in request.initial_comment.split("\n"): + for issue in pagure.lib.link.get_relation( + session, + request.project.name, + request.project.user.user if request.project.is_fork else None, + request.project.namespace, + line, + "fixes", + include_prs=False, + ): + _log.debug( + "Link (fix) request %s to issue: %s (project: %s)" + % (request.id, issue.id, request.project.fullname) + ) + pagure.lib.query.link_pr_issue( + session, issue, request, origin="initial_comment_pr" + ) + + for issue in pagure.lib.link.get_relation( + session, + request.project.name, + request.project.user.user if request.project.is_fork else None, + request.project.namespace, + line, + "relates", + include_prs=False, + ): + _log.debug( + "Link (relate) request %s to issue: %s (project: %s)" + % (request.id, issue.id, request.project.fullname) + ) + pagure.lib.query.link_pr_issue( + session, issue, request, origin="initial_comment_pr" + ) + else: + _log.debug("No initial comment, no need to continue") + + def new_tag(session, tag_name, tag_description, tag_color, project_id): """ Return a new tag object """ tagobj = model.TagColored( @@ -5609,7 +5666,7 @@ def get_project_family(session, project): return [parent] + query.all() -def link_pr_issue(session, issue, request): +def link_pr_issue(session, issue, request, origin="commit"): """ Associate the specified issue with the specified pull-requets. :arg session: The SQLAlchemy session to use @@ -5625,7 +5682,7 @@ def link_pr_issue(session, issue, request): associated_issues = [iss.uid for iss in request.related_issues] if issue.uid not in associated_issues: obj = model.PrToIssue( - pull_request_uid=request.uid, issue_uid=issue.uid + pull_request_uid=request.uid, issue_uid=issue.uid, origin=origin ) session.add(obj) session.flush() diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index 27ccb3d..50a9b9a 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -33,6 +33,7 @@ import pagure.lib.git import pagure.lib.git_auth import pagure.lib.link import pagure.lib.query +import pagure.lib.model import pagure.lib.repo import pagure.utils from pagure.lib.tasks_utils import pagure_task @@ -1050,6 +1051,13 @@ def link_pr_to_ticket(self, session, pr_uid): ) return + # Drop the existing commit-based relations + session.query(pagure.lib.model.PrToIssue).filter( + pagure.lib.model.PrToIssue.pull_request_uid == request.uid + ).filter(pagure.lib.model.PrToIssue.origin == "intial_comment_pr").delete( + synchronize_session="fetch" + ) + repo_obj = pygit2.Repository(repopath) orig_repo = pygit2.Repository(parentpath) @@ -1078,7 +1086,9 @@ def link_pr_to_ticket(self, session, pr_uid): "LINK_PR_TO_TICKET: Link ticket %s to PRs %s" % (issue, request) ) - pagure.lib.query.link_pr_issue(session, issue, request) + pagure.lib.query.link_pr_issue( + session, issue, request, origin="commit" + ) for issue in pagure.lib.link.get_relation( session, name, user, namespace, line, "relates" @@ -1087,7 +1097,9 @@ def link_pr_to_ticket(self, session, pr_uid): "LINK_PR_TO_TICKET: Link ticket %s to PRs %s" % (issue, request) ) - pagure.lib.query.link_pr_issue(session, issue, request) + pagure.lib.query.link_pr_issue( + session, issue, request, origin="commit" + ) try: session.commit() diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index e67979f..3ebae01 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -537,6 +537,10 @@ def request_pull_edit(repo, requestid, username=None, namespace=None): request.initial_comment = form.initial_comment.data.strip() flask.g.session.add(request) try: + # Link the PR to issue(s) if there is such link + pagure.lib.query.link_pr_to_issue_on_description( + flask.g.session, request + ) flask.g.session.commit() flask.flash("Pull request edited!") except SQLAlchemyError as err: # pragma: no cover diff --git a/tests/test_pagure_flask_api_fork_update.py b/tests/test_pagure_flask_api_fork_update.py index aa19f08..a574443 100644 --- a/tests/test_pagure_flask_api_fork_update.py +++ b/tests/test_pagure_flask_api_fork_update.py @@ -457,5 +457,155 @@ class PagureFlaskApiForkUpdatetests(tests.SimplePagureTest): } ) + def test_api_pull_request_update_edited_linked(self): + """ Test api_assign_pull_request method when with valid input + """ + project = pagure.lib.query.get_authorized_project( + self.session, 'test') + self.assertEqual(len(project.requests), 1) + self.assertEqual(len(project.requests[0].related_issues), 0) + self.assertEqual(len(project.issues), 0) + + # Create issues to link to + msg = pagure.lib.query.new_issue( + session=self.session, + repo=project, + title='tést íssüé', + content='We should work on this', + user='pingou', + ) + self.session.commit() + self.assertEqual(msg.title, 'tést íssüé') + + headers = {'Authorization': 'token aaabbbcccddd'} + + data = { + 'title': 'edited test PR', + 'initial_comment': 'Edited initial comment\n\n' + 'this PR fixes #2 \n\nThanks', + } + + # Valid request + output = self.app.post( + '/api/0/test/pull-request/1', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + # Hard-code all the values that will change from a test to another + # because either random or time-based + data['date_created'] = '1551276260' + data['last_updated'] = '1551276261' + data['updated_on'] = '1551276260' + data['commit_start'] = '5f5d609db65d447f77ba00e25afd17ba5053344b' + data['commit_stop'] = '5f5d609db65d447f77ba00e25afd17ba5053344b' + data['project']['date_created'] = '1551276259' + data['project']['date_modified'] = '1551276259' + data['repo_from']['date_created'] = '1551276259' + data['repo_from']['date_modified'] = '1551276259' + data['repo_from']['parent']['date_created'] = '1551276259' + data['repo_from']['parent']['date_modified'] = '1551276259' + data['uid'] = 'a2bddecc8ea548e88c22a0df77670092' + self.assertDictEqual( + data, + { + 'assignee': None, + 'branch': 'master', + 'branch_from': 'master', + 'cached_merge_status': 'unknown', + 'closed_at': None, + 'closed_by': None, + 'comments': [], + 'commit_start': '5f5d609db65d447f77ba00e25afd17ba5053344b', + 'commit_stop': '5f5d609db65d447f77ba00e25afd17ba5053344b', + 'date_created': '1551276260', + 'id': 1, + 'initial_comment': 'Edited initial comment\n\nthis PR ' + 'fixes #2 \n\nThanks', + 'last_updated': '1551276261', + 'project': {'access_groups': {'admin': [], 'commit': [], 'ticket': []}, + 'access_users': {'admin': [], + 'commit': [], + 'owner': ['pingou'], + 'ticket': []}, + 'close_status': ['Invalid', + 'Insufficient data', + 'Fixed', + 'Duplicate'], + 'custom_keys': [], + 'date_created': '1551276259', + 'date_modified': '1551276259', + 'description': 'test project #1', + 'fullname': 'test', + 'id': 1, + 'milestones': {}, + 'name': 'test', + 'namespace': None, + 'parent': None, + 'priorities': {}, + 'tags': [], + 'url_path': 'test', + 'user': {'fullname': 'PY C', 'name': 'pingou'}}, + 'remote_git': None, + 'repo_from': {'access_groups': {'admin': [], 'commit': [], 'ticket': []}, + 'access_users': {'admin': [], + 'commit': [], + 'owner': ['pingou'], + 'ticket': []}, + 'close_status': [], + 'custom_keys': [], + 'date_created': '1551276259', + 'date_modified': '1551276259', + 'description': 'test project #1', + 'fullname': 'forks/pingou/test', + 'id': 4, + 'milestones': {}, + 'name': 'test', + 'namespace': None, + 'parent': {'access_groups': {'admin': [], + 'commit': [], + 'ticket': []}, + 'access_users': {'admin': [], + 'commit': [], + 'owner': ['pingou'], + 'ticket': []}, + 'close_status': ['Invalid', + 'Insufficient data', + 'Fixed', + 'Duplicate'], + 'custom_keys': [], + 'date_created': '1551276259', + 'date_modified': '1551276259', + 'description': 'test project #1', + 'fullname': 'test', + 'id': 1, + 'milestones': {}, + 'name': 'test', + 'namespace': None, + 'parent': None, + 'priorities': {}, + 'tags': [], + 'url_path': 'test', + 'user': {'fullname': 'PY C', 'name': 'pingou'}}, + 'priorities': {}, + 'tags': [], + 'url_path': 'fork/pingou/test', + 'user': {'fullname': 'PY C', 'name': 'pingou'}}, + 'status': 'Open', + 'tags': [], + 'threshold_reached': None, + 'title': 'edited test PR', + 'uid': 'a2bddecc8ea548e88c22a0df77670092', + 'updated_on': '1551276260', + 'user': {'fullname': 'PY C', 'name': 'pingou'} + } + ) + + project = pagure.lib.query.get_authorized_project( + self.session, 'test') + self.assertEqual(len(project.requests), 1) + self.assertEqual(len(project.requests[0].related_issues), 1) + self.assertEqual(len(project.issues), 1) + self.assertEqual(len(project.issues[0].related_prs), 1) + + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index e06da68..22832a3 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -2819,6 +2819,125 @@ More information 'class="fa fa-random"> random_branch', output_text) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_new_request_pull_from_fork_fixing_ticket(self): + """ Test creating a fork to fork PR fixing a ticket. """ + # Create main repo with some content + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, "repos"), + bare=True + ) + tests.add_content_git_repo( + os.path.join(self.path, "repos", "test.git")) + + # Create fork repo with more content + tests.create_projects( + self.session, + is_fork=True, + hook_token_suffix='fork') + tests.create_projects_git( + os.path.join(self.path, "repos", "forks", "pingou"), + bare=True + ) + tests.add_content_git_repo( + os.path.join(self.path, "repos", "forks", "pingou", "test.git")) + tests.add_readme_git_repo( + os.path.join(self.path, "repos", "forks", "pingou", "test.git"), + branch='feature') + tests.add_readme_git_repo( + os.path.join(self.path, "repos", "forks", "pingou", "test.git"), + branch='random_branch') + + # Check relations before we create the PR + project = pagure.lib.query.get_authorized_project( + self.session, 'test') + self.assertEqual(len(project.requests), 0) + self.assertEqual(len(project.issues), 0) + + # Create issues to link to + msg = pagure.lib.query.new_issue( + session=self.session, + repo=project, + title='tést íssüé', + content='We should work on this', + user='pingou', + ) + self.session.commit() + self.assertEqual(msg.title, 'tést íssüé') + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + csrf_token = self.get_csrf() + data = { + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/do_fork/test', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + + # Check that pingou's fork do exist + output = self.app.get('/fork/pingou/test') + self.assertEqual(output.status_code, 200) + + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + + fork = pagure.lib.query.get_authorized_project( + self.session, 'test', user='ralph') + + set_up_git_repo( + self.session, self.path, new_project=fork, + branch_from='feature', mtype='FF', prid=2) + + # Try opening a pull-request + output = self.app.get( + '/fork/pingou/test/diff/master..feature') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Create new Pull Request for master - ' + 'fork/pingou/test\n - Pagure', output_text) + self.assertIn( + '\n', + output_text) + self.assertIn( + ' master', + output_text) + self.assertIn( + ' random_branch', + output_text) + + data = { + 'csrf_token': csrf_token, + 'title': 'foo bar PR', + 'initial_comment': 'Test Initial Comment\n\nFixes #1', + } + + output = self.app.post( + '/fork/pingou/test/diff/master..feature', + data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'PR#3: foo bar PR - test\n - Pagure', + output_text) + self.assertIn( + '

Test Initial Comment

\n

Fixes