Repository: VirtusLab/git-machete

Issue:
Is the check for PR existence still needed now that we read the body of 422 response?
`git_machete.github.create_pull_request`:

```
    prs: List[GitHubPullRequest] = derive_pull_requests(org, repo)
    pr_found: Optional[GitHubPullRequest] = find_or_none(lambda pr: pr.base == base and pr.head == head, prs)
    if not pr_found:
        pr = __fire_github_api_request('POST', f'/repos/{org}/{repo}/pulls', token, request_body)
        return __parse_pr_json(pr)
    else:
        raise MacheteException(f'Pull request for branch {head} is already created under link {pr_found.html_url}!\nPR details: {pr_found}')
```

Maybe we can skip one request to the API (`derive_pull_requests`) and rely on our new handling for 422 now?

Discussion:

Patch:
diff --git a/git_machete/exceptions.py b/git_machete/exceptions.py
index a1f9893..ae09ea3 100644
--- a/git_machete/exceptions.py
+++ b/git_machete/exceptions.py
@@ -21,7 +21,7 @@ class UnprocessableEntityHTTPError(MacheteException):
     or creating a pull request for a branch that already has a PR.
     """
     def __init__(self, msg: str) -> None:
-        self.msg: str = 'UnprocessableEntityHTTPError: ' + msg
+        self.msg: str = msg
 
     def __str__(self) -> str:
         return str(self.msg)
diff --git a/git_machete/github.py b/git_machete/github.py
index 031bcb5..8b246d4 100644
--- a/git_machete/github.py
+++ b/git_machete/github.py
@@ -9,9 +9,9 @@ import subprocess
 from pathlib import Path
 from typing import Any, Dict, List, Optional, Tuple
 import urllib.request
-from urllib.error import HTTPError
+import urllib.error
 
-from git_machete.utils import debug, fmt, find_or_none
+from git_machete.utils import debug, fmt
 from git_machete.exceptions import MacheteException, UnprocessableEntityHTTPError
 from git_machete.git_operations import GitContext, LocalBranchShortName
 
@@ -158,7 +158,7 @@ def __fire_github_api_request(method: str, path: str, token: Optional[str], requ
         with urllib.request.urlopen(http_request) as response:
             parsed_response_body: Any = json.loads(response.read().decode())
             return parsed_response_body
-    except HTTPError as err:
+    except urllib.error.HTTPError as err:
         if err.code == http.HTTPStatus.UNPROCESSABLE_ENTITY:
             error_response = json.loads(err.read().decode())
             error_reason: str = __extract_failure_info_from_422(error_response)
@@ -193,13 +193,8 @@ def create_pull_request(org: str, repo: str, head: str, base: str, title: str, d
         'body': description,
         'draft': draft
     }
-    prs: List[GitHubPullRequest] = derive_pull_requests(org, repo)
-    pr_found: Optional[GitHubPullRequest] = find_or_none(lambda pr: pr.base == base and pr.head == head, prs)
-    if not pr_found:
-        pr = __fire_github_api_request('POST', f'/repos/{org}/{repo}/pulls', token, request_body)
-        return __parse_pr_json(pr)
-    else:
-        raise MacheteException(f'Pull request for branch {head} is already created under link {pr_found.html_url}!\nPR details: {pr_found}')
+    pr = __fire_github_api_request('POST', f'/repos/{org}/{repo}/pulls', token, request_body)
+    return __parse_pr_json(pr)
 
 
 def add_assignees_to_pull_request(org: str, repo: str, number: int, assignees: List[str]) -> None:


Test patch:
diff --git a/git_machete/tests/functional/test_machete.py b/git_machete/tests/functional/test_machete.py
index 55749e7..24e5542 100644
--- a/git_machete/tests/functional/test_machete.py
+++ b/git_machete/tests/functional/test_machete.py
@@ -174,6 +174,9 @@ class MockGithubAPIRequest:
     def update_pull_request(self) -> "MockGithubAPIResponse":
         pull_no: str = self.find_number(self.parsed_url.path, 'pulls')
         if not pull_no:
+            if self.is_pull_created():
+                return self.make_response_object(HTTPStatus.UNPROCESSABLE_ENTITY, {'message': 'Validation Failed', 'errors': [
+                    {'message': f'A pull request already exists for test_repo:{json.loads(self.json_data)["head"]}.'}]})
             return self.create_pull_request()
         pull: Dict[str, Any] = self.github_api_state.get_pull(pull_no)
         return self.fill_pull_request_data(json.loads(self.json_data), pull)
@@ -221,6 +224,17 @@ class MockGithubAPIRequest:
             self.github_api_state.issues.append(issue)
         return self.make_response_object(HTTPStatus.CREATED, issue)
 
+    def is_pull_created(self) -> bool:
+        deserialized_json_data = json.loads(self.json_data)
+        head: str = deserialized_json_data['head']
+        base: str = deserialized_json_data['base']
+        for pull in self.github_api_state.pulls:
+            pull_head: str = pull['head']['ref']
+            pull_base: str = pull['base']['ref']
+            if (head, base) == (pull_head, pull_base):
+                return True
+        return False
+
     @staticmethod
     def get_index_or_none(entity: Dict[str, Any], base: List[Dict[str, Any]]) -> Optional[int]:
         try:
@@ -245,6 +259,15 @@ class MockGithubAPIRequest:
         return str(max(numbers) + 1)
 
 
+class MockHTTPError(HTTPError):
+    def __init__(self, url: str, code: int, msg: Any, hdrs: Dict[str, str], fp: Any) -> None:
+        super().__init__(url, code, msg, hdrs, fp)
+        self.msg = msg
+
+    def read(self, n: int = 1) -> bytes:
+        return json.dumps(self.msg).encode()
+
+
 class MockContextManager:
     def __init__(self, obj: MockGithubAPIResponse) -> None:
         self.obj = obj
@@ -252,6 +275,8 @@ class MockContextManager:
     def __enter__(self) -> MockGithubAPIResponse:
         if self.obj.status_code == HTTPStatus.NOT_FOUND:
             raise HTTPError(None, 404, 'Not found', None, None)
+        elif self.obj.status_code == HTTPStatus.UNPROCESSABLE_ENTITY:
+            raise MockHTTPError(None, 422, self.obj.response_data, None, None)
         return self.obj
 
     def __exit__(self, *args: Any) -> None:
@@ -1927,6 +1952,7 @@ class MacheteTester(unittest.TestCase):
     git_api_state_for_test_create_pr = MockGithubAPIState([{'head': {'ref': 'ignore-trailing', 'repo': mock_repository_info}, 'user': {'login': 'github_user'}, 'base': {'ref': 'hotfix/add-trigger'}, 'number': '3', 'html_url': 'www.github.com', 'state': 'open'}],
                                                           issues=[{'number': '4'}, {'number': '5'}, {'number': '6'}])
 
+    @mock.patch('urllib.error.HTTPError', MockHTTPError)  # need to provide read() method, which does not actually reads error from url
     # We need to mock GITHUB_REMOTE_PATTERNS in the tests for `test_github_create_pr` due to `git fetch` executed by `create-pr` subcommand.
     @mock.patch('git_machete.github.GITHUB_REMOTE_PATTERNS', FAKE_GITHUB_REMOTE_PATTERNS)
     @mock.patch('git_machete.utils.run_cmd', mock_run_cmd)  # to hide git outputs in tests
@@ -2062,12 +2088,12 @@ class MacheteTester(unittest.TestCase):
         )
         # check against attempt to create already existing pull request
         machete_client = MacheteClient(git)
-        expected_error_message = "Pull request for branch hotfix/add-trigger is already created under link www.github.com!\nPR details: PR #6 by github_user: hotfix/add-trigger -> master"
+        expected_error_message = "A pull request already exists for test_repo:hotfix/add-trigger."
         machete_client.read_definition_file()
         with self.assertRaises(MacheteException) as e:
             machete_client.create_github_pr(head=LocalBranchShortName.of('hotfix/add-trigger'), opt_draft=False, opt_onto=None)
         if e:
-            self.assertEqual(e.exception.parameter, expected_error_message,
+            self.assertEqual(e.exception.msg, expected_error_message,  # type: ignore
                              'Verify that expected error message has appeared when given pull request to create is already created.')
 
         # check against head branch is ancestor or equal to base branch