refactoring

Author: jpchauvel

src/edit_python_pe/main.py

@@ -22,8 +22,17 @@
 class MemberApp(App):
     """Single app that toggles between a file list and a form while connected to a GitHub fork+push flow."""
 
-    def __init__(self, repo_path: str) -> None:
+    def __init__(
+        self,
+        original_repo: Repository,
+        forked_repo: Repository,
+        token: str,
+        repo_path: str,
+    ) -> None:
         super().__init__()
+        self.original_repo = original_repo
+        self.forked_repo = forked_repo
+        self.token = token
         self.repo_path = repo_path
 
     def compose(self) -> ComposeResult:
@@ -476,112 +485,18 @@ def save_member(self) -> None:
 
         md_content = "\n".join(md_lines)
 
-        if not self.current_file:
-            # compute name_file
-            if aliases:
-                alias_for_name = aliases[0].lower().replace(" ", "_")
-            else:
-                alias_for_name = name.lower().replace(" ", "_")
-
-            sha_hash = hashlib.sha1(
-                (alias_for_name + email + datetime.now().isoformat()).encode(
-                    "utf-8"
-                )
-            ).hexdigest()[:8]
-            name_file = f"{alias_for_name}-{sha_hash}"
-
-            # Write file
-            file_path = os.path.join(
-                self.repo_path, "blog", "members", f"{name_file}.md"
-            )
-        else:
-            name_file = self.current_file
-            file_path = os.path.join(
-                self.repo_path, "blog", "members", f"{name_file}"
-            )
-        os.makedirs(os.path.dirname(file_path), exist_ok=True)
-        with open(file_path, "w", encoding="utf-8") as f:
-            f.write(md_content)
-
-        # commit & push
-        repo = pygit2.Repository(self.repo_path)
-        rel_path = os.path.relpath(file_path, self.repo_path)
-        rel_path = pathlib.Path(
-            rel_path
-        ).as_posix()  # Force path to POSIX format so Windows backslashes (\) don't break pygit2
-        repo.index.add(rel_path)
-        repo.index.write()
-        author_sig = pygit2.Signature(
-            name or "Unknown", email or "unknown@email"
-        )
-        tree_id = repo.index.write_tree()
-        parents = [] if repo.head_is_unborn else [repo.head.target]
-        commit_msg = (
-            f"Changed {self.current_file}"
-            if self.current_file
-            else f"Added {name_file}.md"
-        )
-        repo.create_commit(
-            "HEAD", author_sig, author_sig, commit_msg, tree_id, parents
+        message = create_pr(
+            md_content,
+            self.current_file,
+            self.repo_path,
+            self.original_repo,
+            self.forked_repo,
+            self.token,
+            aliases,
+            name,
+            email,
         )
-
-        callbacks = pygit2.callbacks.RemoteCallbacks(
-            credentials=pygit2.UserPass(self.token, "x-oauth-basic")
-        )
-        remote = repo.remotes["origin"]
-        remote.push([repo.head.name], callbacks=callbacks)
-
-        # PR logic
-        pr_title = commit_msg
-        first_alias = aliases[0] if aliases else ""
-        pr_body = (
-            f"Changing an entry to `blog/members` for {name} (alias: {first_alias})."
-            if self.current_file
-            else f"Creating a new entry to `blog/members` for {name} (alias: {first_alias})."
-        )
-        fork_owner = self.forked_repo.owner.login
-        head_branch = f"{fork_owner}:main"
-        base_branch = "main"
-
-        # If editing, retrieve PR by title and push to its branch
-        if self.current_file:
-            # Try to find an open PR with matching title
-            prs = self.original_repo.get_pulls(
-                state="open", sort="created", base=base_branch
-            )
-            pr_found = None
-            for pr in prs:
-                if self.current_file in pr.title:
-                    pr_found = pr
-                    break
-            if pr_found:
-                # Push to the PR branch (simulate, as actual branch logic may differ)
-                remote.push([repo.head.name], callbacks=callbacks)
-                self.exit(
-                    message=f"Archivo {self.current_file} editado, commit y cambios enviados al PR existente."
-                )
-            else:
-                # Otherwise, create a new PR
-                self.original_repo.create_pull(
-                    title=pr_title,
-                    body=pr_body,
-                    head=head_branch,
-                    base=base_branch,
-                )
-                self.exit(
-                    message=f"Archivo {self.current_file} guardado, commit y PR listo."
-                )
-        else:
-            # Otherwise, create a new PR
-            self.original_repo.create_pull(
-                title=pr_title,
-                body=pr_body,
-                head=head_branch,
-                base=base_branch,
-            )
-            self.exit(
-                message=f"Archivo {name_file}.md guardado, commit y PR listo."
-            )
+        self.exit(message=message)
 
     async def on_event(self, event: Event) -> None:
         # catch listview selection
@@ -609,7 +524,7 @@ def get_repo() -> tuple[str, Repository]:
         exit(1)
 
 
-def fork_repo(token: str, original_repo: Repository) -> str:
+def fork_repo(token: str, original_repo: Repository) -> tuple[str, Repository]:
     forked_repo = original_repo.create_fork()
     forked_repo_url = forked_repo.clone_url
     repo_path = user_data_dir(appname="edit-python-pe", appauthor="python.pe")
@@ -622,13 +537,151 @@ def fork_repo(token: str, original_repo: Repository) -> str:
         pygit2.clone_repository(
             forked_repo_url, repo_path, callbacks=callbacks
         )
-    return repo_path
+    return repo_path, forked_repo
+
+
+def compute_file_name(aliases: list[str], name: str, email: str) -> str:
+    # compute name_file
+    if aliases:
+        alias_for_name = aliases[0].lower().replace(" ", "_")
+    else:
+        alias_for_name = name.lower().replace(" ", "_")
+
+    sha_hash = hashlib.sha1(
+        (alias_for_name + email + datetime.now().isoformat()).encode("utf-8")
+    ).hexdigest()[:8]
+    return f"{alias_for_name}-{sha_hash}.md"
+
+
+def write_file(file_content: str, file_path: str) -> None:
+    os.makedirs(os.path.dirname(file_path), exist_ok=True)
+    with open(file_path, "w", encoding="utf-8") as f:
+        f.write(file_content)
+
+
+def commit_and_push(
+    repo_path: str,
+    token: str,
+    was_changed: bool,
+    name_file: str,
+    file_path: str,
+    name: str,
+    email: str,
+) -> tuple[
+    str,
+    pygit2.repository.Repository,
+    pygit2.remotes.Remote,
+    pygit2.callbacks.RemoteCallbacks,
+]:
+    repo = pygit2.repository.Repository(repo_path)
+    rel_path = os.path.relpath(file_path, repo_path)
+    rel_path = pathlib.Path(
+        rel_path
+    ).as_posix()  # Force path to POSIX format so Windows backslashes (\) don't break pygit2
+    repo.index.add(rel_path)
+    repo.index.write()
+    author_sig = pygit2.Signature(name or "Unknown", email or "unknown@email")
+    tree_id = repo.index.write_tree()
+    parents = [] if repo.head_is_unborn else [repo.head.target]
+    commit_msg = (
+        f"Changed {name_file}" if was_changed else f"Added {name_file}"
+    )
+    repo.create_commit(
+        "HEAD", author_sig, author_sig, commit_msg, tree_id, parents
+    )
+
+    callbacks = pygit2.callbacks.RemoteCallbacks(
+        credentials=pygit2.UserPass(token, "x-oauth-basic")
+    )
+    remote = repo.remotes["origin"]
+    remote.push([repo.head.name], callbacks=callbacks)
+    return commit_msg, repo, remote, callbacks
+
+
+def create_pr(
+    file_content: str,
+    current_file: str | None,
+    repo_path: str,
+    original_repo: Repository,
+    forked_repo: Repository,
+    token: str,
+    aliases: list[str],
+    name: str,
+    email: str,
+) -> str:
+    # Name the file
+    name_file = (
+        current_file
+        if current_file is not None
+        else compute_file_name(aliases, name, email)
+    )
+
+    # Write file
+    file_path = os.path.join(repo_path, "blog", "members", name_file)
+    write_file(file_content, file_path)
+
+    # commit & push
+    commit_msg, repo, remote, callbacks = commit_and_push(
+        repo_path,
+        token,
+        current_file is not None,
+        name_file,
+        file_path,
+        name,
+        email,
+    )
+
+    # PR logic
+    pr_title = commit_msg
+    first_alias = aliases[0] if aliases else ""
+    pr_body = (
+        f"Changing an entry to `blog/members` for {name} (alias: {first_alias})."
+        if current_file
+        else f"Creating a new entry to `blog/members` for {name} (alias: {first_alias})."
+    )
+    fork_owner = forked_repo.owner.login
+    head_branch = f"{fork_owner}:main"
+    base_branch = "main"
+
+    # If editing, retrieve PR by title and push to its branch
+    if current_file:
+        # Try to find an open PR with matching title
+        prs = original_repo.get_pulls(
+            state="open", sort="created", base=base_branch
+        )
+        pr_found = None
+        for pr in prs:
+            if current_file in pr.title:
+                pr_found = pr
+                break
+        if pr_found:
+            # Push to the PR branch (simulate, as actual branch logic may differ)
+            remote.push([repo.head.name], callbacks=callbacks)
+            return f"Archivo {name_file} editado, commit y cambios enviados al PR existente."
+        else:
+            # Otherwise, create a new PR
+            original_repo.create_pull(
+                title=pr_title,
+                body=pr_body,
+                head=head_branch,
+                base=base_branch,
+            )
+            return f"Archivo {name_file} guardado, commit y PR listo."
+    else:
+        # Otherwise, create a new PR
+        original_repo.create_pull(
+            title=pr_title,
+            body=pr_body,
+            head=head_branch,
+            base=base_branch,
+        )
+        return f"Archivo {name_file} guardado, commit y PR listo."
 
 
 def main() -> None:
     token, original_repo = get_repo()
-    repo_path = fork_repo(token, original_repo)
-    app = MemberApp(repo_path)
+    repo_path, forked_repo = fork_repo(token, original_repo)
+    app = MemberApp(original_repo, forked_repo, token, repo_path)
     app.run()
 
 

tests/test_member_app.py

@@ -16,7 +16,14 @@ def setUp(self):
         # Patch Github and Repository for testing
         self.token = "fake-token"
         self.repo = MagicMock()
-        self.app = MemberApp(repo_path="test_repo")
+        self.original_repo = MagicMock()
+        self.forked_repo = MagicMock()
+        self.app = MemberApp(
+            repo_path="test_repo",
+            original_repo=self.original_repo,
+            forked_repo=self.forked_repo,
+            token=self.token,
+        )
         self.app.social_container = MagicMock()
         self.app.alias_container = MagicMock()
         self.app.list_container = MagicMock()
@@ -164,7 +171,7 @@ def test_save_member_edit_no_pr(self):
         with (
             patch("os.makedirs") as makedirs,
             patch("builtins.open", MagicMock()),
-            patch("pygit2.Repository") as RepoMock,
+            patch("pygit2.repository.Repository") as RepoMock,
         ):
             repo_instance = RepoMock.return_value
             repo_instance.index.add = MagicMock()
@@ -221,7 +228,7 @@ def test_save_member_edit(self):
         with (
             patch("os.makedirs") as makedirs,
             patch("builtins.open", MagicMock()),
-            patch("pygit2.Repository") as RepoMock,
+            patch("pygit2.repository.Repository") as RepoMock,
         ):
             repo_instance = RepoMock.return_value
             repo_instance.index.add = MagicMock()
@@ -277,7 +284,7 @@ def test_save_member_new(self):
         with (
             patch("os.makedirs") as makedirs,
             patch("builtins.open", MagicMock()),
-            patch("pygit2.Repository") as RepoMock,
+            patch("pygit2.repository.Repository") as RepoMock,
         ):
             repo_instance = RepoMock.return_value
             repo_instance.index.add = MagicMock()
@@ -329,7 +336,7 @@ def test_save_member_error_handling(self):
             patch.object(app, "exit") as exit_mock,
             patch("os.makedirs"),
             patch("builtins.open", MagicMock()),
-            patch("pygit2.Repository"),
+            patch("pygit2.repository.Repository"),
         ):
             # Leave name and email blank to trigger error
             app.name_input.value = ""
@@ -499,11 +506,12 @@ def test_fork_repo_clones_if_not_exists(
         mock_original_repo = MagicMock()
         mock_original_repo.create_fork.return_value = mock_forked_repo
         token = "fake-token"
-        repo_path = fork_repo(token, mock_original_repo)
+        repo_path = fork_repo(token, mock_original_repo)[0]
         mock_original_repo.create_fork.assert_called_once()
-        mock_clone.assert_called_once_with(
-            mock_forked_repo.clone_url, repo_path, callbacks=unittest.mock.ANY
-        )
+        mock_clone.assert_called_once()
+        call_args = mock_clone.call_args
+        self.assertEqual(call_args[0][0], mock_forked_repo.clone_url)
+        self.assertEqual(call_args[0][1], repo_path)
         self.assertEqual(repo_path, "/tmp/testrepo")
 
     @patch("edit_python_pe.main.user_data_dir", return_value="/tmp/testrepo")
@@ -523,11 +531,16 @@ def test_main_runs_app(
         self, mock_member_app, mock_fork_repo, mock_get_repo
     ):
         mock_get_repo.return_value = ("token", MagicMock())
-        mock_fork_repo.return_value = "/tmp/testrepo"
+        mock_fork_repo.return_value = ("/tmp/testrepo", MagicMock())
         mock_app_instance = MagicMock()
         mock_member_app.return_value = mock_app_instance
         main()
         mock_get_repo.assert_called_once()
         mock_fork_repo.assert_called_once()
-        mock_member_app.assert_called_once_with("/tmp/testrepo")
+        mock_member_app.assert_called_once_with(
+            unittest.mock.ANY,
+            unittest.mock.ANY,
+            unittest.mock.ANY,
+            unittest.mock.ANY,
+        )
         mock_app_instance.run.assert_called_once()