From 98b37a4ca8b1bb434a495fb013d5010465dc3b7b Mon Sep 17 00:00:00 2001 From: Lenka Segura Date: May 06 2019 09:10:00 +0000 Subject: [PATCH 1/3] Add repo_from argument for API create pull request --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 64eb542..c8b5daa 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -1351,9 +1351,21 @@ def api_pull_request_create(repo, username=None, namespace=None): """ - repo = _get_repo(repo, username, namespace) - _check_pull_request(repo) - _check_token(repo) + repo_to = _get_repo(repo, username, namespace) + repo_from_d = get_request_data().get("repo_from") + try: + repo_from = (_get_repo(repo_from_d['repo'], repo_from_d.get('username'), + repo_from_d.get('namespace'))) + except Exception: + repo_from = None + if not repo_from: + raise pagure.exceptions.APIError( + 400, + error_code=APIERROR.EINVALIDREQ, + errors={"repo_from": ["This field is required."]}, + ) + _check_pull_request(repo_to) + _check_token(repo_from) form = pagure.forms.RequestPullForm(csrf_enabled=False) if not form.validate_on_submit(): @@ -1375,35 +1387,31 @@ def api_pull_request_create(repo, username=None, namespace=None): errors={"branch_from": ["This field is required."]}, ) - parent = repo - if repo.parent: - parent = repo.parent - - if not parent.settings.get("pull_requests", True): + if not repo_to.settings.get("pull_requests", True): raise pagure.exceptions.APIError( 404, error_code=APIERROR.EPULLREQUESTSDISABLED ) - repo_committer = pagure.utils.is_repo_committer(repo) + repo_committer = pagure.utils.is_repo_committer(repo_from) if not repo_committer: raise pagure.exceptions.APIError( 401, error_code=APIERROR.ENOTHIGHENOUGH ) - repo_obj = pygit2.Repository(repo.repopath("main")) - orig_repo = pygit2.Repository(parent.repopath("main")) + git_repo_from = pygit2.Repository(repo_from.repopath("main")) + git_repo_to = pygit2.Repository(repo_to.repopath("main")) try: diff, diff_commits, orig_commit = pagure.lib.git.get_diff_info( - repo_obj, orig_repo, branch_from, branch_to + git_repo_from, git_repo_to, branch_from, branch_to ) except pagure.exceptions.PagureException as err: raise pagure.exceptions.APIError( 400, error_code=APIERROR.EINVALIDREQ, errors=str(err) ) - if parent.settings.get( + if repo_to.settings.get( "Enforce_signed-off_commits_in_pull-request", False ): for commit in diff_commits: @@ -1424,10 +1432,10 @@ def api_pull_request_create(repo, username=None, namespace=None): request = pagure.lib.query.new_pull_request( flask.g.session, - repo_to=parent, + repo_to=repo_to, branch_to=branch_to, branch_from=branch_from, - repo_from=repo, + repo_from=repo_from, title=form.title.data, initial_comment=initial_comment, user=flask.g.fas_user.username, diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 19bc5f9..0ac9f5a 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -470,6 +470,17 @@ class Project(BASE): viewonly=True, ) + def __repr__(self): + return "Project(%s, name:%s, namespace:%s, url:%s, is_fork:%s,\ + parent_id:%s)" % ( + self.id, + self.name, + self.namespace, + self.url, + self.is_fork, + self.parent_id, + ) + @property def isa(self): """ A string to allow finding out that this is a project. """ From 0c23518b051c274d4724ef63f01c8472b0c84440 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: May 06 2019 09:10:00 +0000 Subject: [PATCH 2/3] Code style changes and documentation Fixes the code using python black and flake8. Adjust the logic around detecting if repo_from was provided. Document the ``repo_from`` argument in the API documentation. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index c8b5daa..23e1461 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -1279,25 +1279,45 @@ def api_pull_request_create(repo, username=None, namespace=None): Input ^^^^^ - +--------------------+----------+---------------+----------------------+ - | Key | Type | Optionality | Description | - +====================+==========+===============+======================+ - | ``title`` | string | Mandatory | The title to give to | - | | | | this pull-request | - +--------------------+----------+---------------+----------------------+ - | ``branch_to`` | string | Mandatory | The name of the | - | | | | branch the submitted | - | | | | changes should be | - | | | | merged into. | - +--------------------+----------+---------------+----------------------+ - | ``branch_from`` | string | Mandatory | The name of the | - | | | | branch containing | - | | | | the changes to merge | - +--------------------+----------+---------------+----------------------+ - | ``initial_comment``| string | Optional | The intial comment | - | | | | describing what these| - | | | | changes are about. | - +--------------------+----------+---------------+----------------------+ + +-------------------------+----------+-------------+------------------------+ + | Key | Type | Optionality | Description | + +=========================+==========+=============+========================+ + | ``title`` | string | Mandatory | The title to give to | + | | | | this pull-request | + +-------------------------+----------+-------------+------------------------+ + | ``branch_to`` | string | Mandatory | The name of the branch | + | | | | the submitted changes | + | | | | should be merged into. | + +-------------------------+----------+-------------+------------------------+ + | ``branch_from`` | string | Mandatory | The name of the branch | + | | | | containing the changes | + | | | | to merge | + +-------------------------+----------+-------------+------------------------+ + | ``repo_from`` | string | Optional | The name of the project| + | | | | the changes originate | + | | | | from. | + | | | | If not specified the | + | | | | repo_from is assumed | + | | | | to be the repo_to. | + +-------------------------+----------+-------------+------------------------+ + | ``repo_from_username`` | string | Optional | The username of the | + | | | | project the changes | + | | | | originate from. | + | | | | If not specified the | + | | | | repo_from is assumed | + | | | | to be the repo_to. | + +-------------------------+----------+-------------+------------------------+ + | ``repo_from_namespace`` | string | Optional | The namespace of the | + | | | | project the changes | + | | | | originate from. | + | | | | If not specified the | + | | | | repo_from is assumed | + | | | | to be the repo_to. | + +-------------------------+----------+-------------+------------------------+ + | ``initial_comment`` | string | Optional | The intial comment | + | | | | describing what these | + | | | | changes are about. | + +-------------------------+----------+-------------+------------------------+ Sample response ^^^^^^^^^^^^^^^ @@ -1353,17 +1373,18 @@ def api_pull_request_create(repo, username=None, namespace=None): repo_to = _get_repo(repo, username, namespace) repo_from_d = get_request_data().get("repo_from") - try: - repo_from = (_get_repo(repo_from_d['repo'], repo_from_d.get('username'), - repo_from_d.get('namespace'))) - except Exception: - repo_from = None - if not repo_from: - raise pagure.exceptions.APIError( - 400, - error_code=APIERROR.EINVALIDREQ, - errors={"repo_from": ["This field is required."]}, + + if repo_from_d and repo_from_d.get("repo"): + print(repo_from_d) + print('='*50) + repo_from = _get_repo( + repo_from_d["repo"], + repo_from_d.get("username"), + repo_from_d.get("namespace"), ) + else: + repo_from = repo_to + _check_pull_request(repo_to) _check_token(repo_from) diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 0ac9f5a..e4b660d 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -471,14 +471,17 @@ class Project(BASE): ) def __repr__(self): - return "Project(%s, name:%s, namespace:%s, url:%s, is_fork:%s,\ - parent_id:%s)" % ( - self.id, - self.name, - self.namespace, - self.url, - self.is_fork, - self.parent_id, + return ( + "Project(%s, name:%s, namespace:%s, url:%s, is_fork:%s,\ + parent_id:%s)" + % ( + self.id, + self.name, + self.namespace, + self.url, + self.is_fork, + self.parent_id, + ) ) @property From 6fa4b3d5b9d6db904dc439da3df0540c56eb0eb6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: May 06 2019 09:32:37 +0000 Subject: [PATCH 3/3] Adjust how are passed the arguments about repo_from when opening a new PR Instead of enforcing to pass all the arguments as JSON, split the name, username and namespace into three different arguments. --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 23e1461..0f0ba2d 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -1279,45 +1279,45 @@ def api_pull_request_create(repo, username=None, namespace=None): Input ^^^^^ - +-------------------------+----------+-------------+------------------------+ - | Key | Type | Optionality | Description | - +=========================+==========+=============+========================+ - | ``title`` | string | Mandatory | The title to give to | - | | | | this pull-request | - +-------------------------+----------+-------------+------------------------+ - | ``branch_to`` | string | Mandatory | The name of the branch | - | | | | the submitted changes | - | | | | should be merged into. | - +-------------------------+----------+-------------+------------------------+ - | ``branch_from`` | string | Mandatory | The name of the branch | - | | | | containing the changes | - | | | | to merge | - +-------------------------+----------+-------------+------------------------+ - | ``repo_from`` | string | Optional | The name of the project| - | | | | the changes originate | - | | | | from. | - | | | | If not specified the | - | | | | repo_from is assumed | - | | | | to be the repo_to. | - +-------------------------+----------+-------------+------------------------+ - | ``repo_from_username`` | string | Optional | The username of the | - | | | | project the changes | - | | | | originate from. | - | | | | If not specified the | - | | | | repo_from is assumed | - | | | | to be the repo_to. | - +-------------------------+----------+-------------+------------------------+ - | ``repo_from_namespace`` | string | Optional | The namespace of the | - | | | | project the changes | - | | | | originate from. | - | | | | If not specified the | - | | | | repo_from is assumed | - | | | | to be the repo_to. | - +-------------------------+----------+-------------+------------------------+ - | ``initial_comment`` | string | Optional | The intial comment | - | | | | describing what these | - | | | | changes are about. | - +-------------------------+----------+-------------+------------------------+ + +-----------------------+----------+-------------+------------------------+ + | Key | Type | Optionality | Description | + +=======================+==========+=============+========================+ + | ``title`` | string | Mandatory | The title to give to | + | | | | this pull-request | + +-----------------------+----------+-------------+------------------------+ + | ``branch_to`` | string | Mandatory | The name of the branch | + | | | | the submitted changes | + | | | | should be merged into. | + +-----------------------+----------+-------------+------------------------+ + | ``branch_from`` | string | Mandatory | The name of the branch | + | | | | containing the changes | + | | | | to merge | + +-----------------------+----------+-------------+------------------------+ + | ``repo_from`` | string | Optional | The name of the project| + | | | | the changes originate | + | | | | from. | + | | | | If not specified the | + | | | | repo_from is assumed | + | | | | to be the repo_to. | + +-----------------------+----------+-------------+------------------------+ + | ``repo_from_username``| string | Optional | The username of the | + | | | | project the changes | + | | | | originate from. | + | | | | If not specified the | + | | | | repo_from is assumed | + | | | | to be the repo_to. | + +-----------------------+----------+-------------+------------------------+ + |``repo_from_namespace``| string | Optional | The namespace of the | + | | | | project the changes | + | | | | originate from. | + | | | | If not specified the | + | | | | repo_from is assumed | + | | | | to be the repo_to. | + +-----------------------+----------+-------------+------------------------+ + | ``initial_comment`` | string | Optional | The intial comment | + | | | | describing what these | + | | | | changes are about. | + +-----------------------+----------+-------------+------------------------+ Sample response ^^^^^^^^^^^^^^^ @@ -1369,18 +1369,20 @@ def api_pull_request_create(repo, username=None, namespace=None): } } - """ + """ # noqa repo_to = _get_repo(repo, username, namespace) - repo_from_d = get_request_data().get("repo_from") - if repo_from_d and repo_from_d.get("repo"): - print(repo_from_d) - print('='*50) + req_data = get_request_data() + repo_from = req_data.get("repo_from") + repo_from_username = req_data.get("repo_from_username") + repo_from_namespace = req_data.get("repo_from_namespace") + + if repo_from: repo_from = _get_repo( - repo_from_d["repo"], - repo_from_d.get("username"), - repo_from_d.get("namespace"), + repo_from, + username=repo_from_username, + namespace=repo_from_namespace, ) else: repo_from = repo_to @@ -1606,7 +1608,7 @@ def api_pull_request_diffstats(repo, requestid, username=None, namespace=None): for patch in diff: stats = pagure.lib.git.get_stats_patch(patch) new_path = stats["new_path"] - del (stats["new_path"]) + del stats["new_path"] output[new_path] = stats else: raise pagure.exceptions.APIError(400, error_code=APIERROR.ENOPRSTATS) diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index b53759c..90c0cfc 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -2810,12 +2810,12 @@ class PagureFlaskApiForktests(tests.Modeltests): "initial_comment": "the manifest", "branch_to": "master", "branch_from": "branch", + "repo_from": "test", + "repo_from_username": "pingou", } output = self.app.post( - "/api/0/fork/pingou/test/pull-request/new", - headers=headers, - data=data, + "/api/0/test/pull-request/new", headers=headers, data=data ) self.assertEqual(output.status_code, 200) diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 96965fc..31dcda6 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -6757,9 +6757,7 @@ class PagureFlaskRepoTestHooktests(tests.Modeltests): """ Test the test_hook endpoint when the user is logged in. """ user = tests.FakeUser(username="pingou") with tests.user_set(self.app.application, user): - data = { - "csrf_token": self.get_csrf() - } + data = {"csrf_token": self.get_csrf()} output = self.app.post("/test/settings/test_hook", data=data) self.assertEqual(output.status_code, 302)