From 98b37a4ca8b1bb434a495fb013d5010465dc3b7b Mon Sep 17 00:00:00 2001
From: Lenka Segura <lenka@sepu.cz>
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 <pingou@pingoured.fr>
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 <pingou@pingoured.fr>

---

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 <pingou@pingoured.fr>
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)
 

