From af5258039cee3e138ec2a6339b21f4ce5e6ec584 Mon Sep 17 00:00:00 2001 From: Zeid Date: Fri, 19 Sep 2025 15:49:50 -0400 Subject: [PATCH 01/26] github: add GitHubAPI, GitHubAPIClient classes (bug 1989637) - add GitHubAPI class - add GitHubAPIClient class - move "get token" functionality to GitHubAPI class - fix failing test_GitSCM_push_get_github_token --- src/lando/main/models/repo.py | 5 ++ src/lando/main/scm/git.py | 34 ++---------- src/lando/main/tests/test_git.py | 19 +++++-- src/lando/utils/github.py | 91 ++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 34 deletions(-) create mode 100644 src/lando/utils/github.py diff --git a/src/lando/main/models/repo.py b/src/lando/main/models/repo.py index 824678951..a4e4dd2d9 100644 --- a/src/lando/main/models/repo.py +++ b/src/lando/main/models/repo.py @@ -275,6 +275,11 @@ def _github_repo_url(self) -> str | None: if self.is_github: return self.url.removesuffix(".git") + @property + def _github_repo_org(self) -> str | None: + if self.is_github: + return self._github_repo_url.split("/")[-2] + @property def git_repo_name(self) -> str: """Provide the bare name of the Git repo.""" diff --git a/src/lando/main/scm/git.py b/src/lando/main/scm/git.py index c35d1e7a4..5c99cef1a 100644 --- a/src/lando/main/scm/git.py +++ b/src/lando/main/scm/git.py @@ -1,4 +1,3 @@ -import asyncio import io import logging import os @@ -11,8 +10,6 @@ from pathlib import Path from typing import Any -from django.conf import settings -from simple_github import AppAuth, AppInstallationAuth from typing_extensions import override from lando.main.scm.commit import CommitData @@ -112,6 +109,8 @@ def push( tags: list[str] | None = None, ): """Push local code to the remote repository.""" + from lando.utils.github import GitHubAPI + push_command = ["push"] if force_push: @@ -134,7 +133,8 @@ def push( repo = match["repo"] repo_name = repo.removesuffix(".git") - token = self._get_github_token(owner, repo_name) + token = GitHubAPI._get_token(owner, repo_name) + if token: push_path = f"https://git:{token}@github.com/{owner}/{repo}" @@ -152,32 +152,6 @@ def push( self._git_run(*push_command, cwd=self.path) - @staticmethod - def _get_github_token(repo_owner: str, repo_name: str) -> str | None: - """Obtain a fresh GitHub token to push to the specified repo. - - This relies on GITHUB_APP_ID and GITHUB_APP_PRIVKEY to be set in the - settings. Returns None if those are missing. - - The app with ID GITHUB_APP_ID needs to be enabled for the target repo. - - """ - app_id = settings.GITHUB_APP_ID - private_key = settings.GITHUB_APP_PRIVKEY - - if not app_id or not private_key: - logger.warning( - f"Missing GITHUB_APP_ID or GITHUB_APP_PRIVKEY to authenticate against GitHub repo {repo_owner}/{repo_name}", - ) - return None - - app_auth = AppAuth( - app_id, - private_key, - ) - session = AppInstallationAuth(app_auth, repo_owner, repositories=[repo_name]) - return asyncio.run(session.get_token()) - def last_commit_for_path(self, path: str) -> str: """Find last commit to touch a path.""" command = ["log", "--max-count=1", "--format=%H", "--", path] diff --git a/src/lando/main/tests/test_git.py b/src/lando/main/tests/test_git.py index 16dbba200..9c413047b 100644 --- a/src/lando/main/tests/test_git.py +++ b/src/lando/main/tests/test_git.py @@ -7,6 +7,7 @@ from collections.abc import Callable from pathlib import Path from textwrap import dedent +from unittest import mock from unittest.mock import MagicMock import pytest @@ -668,17 +669,27 @@ def test_GitSCM_push( ) -def test_GitSCM_push_get_github_token(git_repo: Path): +@pytest.fixture +def mock_github_api_get_token(monkeypatch: pytest.MonkeyPatch): + mock_get_token = MagicMock() + mock_get_token.side_effect = ["ghs_yolo"] + + monkeypatch.setattr("lando.utils.github.GitHubAPI._get_token", mock_get_token) + + return mock_get_token + + +def test_GitSCM_push_get_github_token( + git_repo: Path, mock_github_api_get_token: mock.Mock +): scm = GitSCM(str(git_repo)) scm._git_run = MagicMock() - scm._get_github_token = MagicMock() - scm._get_github_token.side_effect = ["ghs_yolo"] scm.push("https://github.com/some/repo") assert scm._git_run.call_count == 1, "_git_run wasn't called when pushing" assert ( - scm._get_github_token.call_count == 1 + mock_github_api_get_token.call_count == 1 ), "_get_github_token wasn't called when pushing to a github-like URL" assert ( "git:ghs_yolo@github.com" in scm._git_run.call_args[0][1] diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py new file mode 100644 index 000000000..277ff5217 --- /dev/null +++ b/src/lando/utils/github.py @@ -0,0 +1,91 @@ +import asyncio +import logging + +import requests +from django.conf import settings +from simple_github import AppAuth, AppInstallationAuth + +from lando.main.models.repo import Repo + +logger = logging.getLogger(__name__) + + +class GitHubAPI: + """A simple wrapper that authenticates with and communicates with the GitHub API.""" + + GITHUB_BASE_URL = "https://api.github.com" + + def __init__(self, repo: Repo): + repo_owner = repo._github_repo_org + repo_name = repo.git_repo_name + + self.session = requests.Session() + self.session.headers.update( + { + "Authorization": f"Bearer {self._get_token(repo_owner, repo_name)}", + "User-Agent": settings.HTTP_USER_AGENT, + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + ) + + @staticmethod + def _get_token(repo_owner: str, repo_name: str) -> str | None: + """Obtain a fresh GitHub token to push to the specified repo. + + This relies on GITHUB_APP_ID and GITHUB_APP_PRIVKEY to be set in the + environment. Returns None if those are missing. + + The app with ID GITHUB_APP_ID needs to be enabled for the target repo. + + """ + app_id = settings.GITHUB_APP_ID + private_key = settings.GITHUB_APP_PRIVKEY + + if not app_id or not private_key: + logger.warning( + f"Missing GITHUB_APP_ID or GITHUB_APP_PRIVKEY to authenticate against GitHub repo {repo_owner}/{repo_name}", + ) + return None + + app_auth = AppAuth( + app_id, + private_key, + ) + session = AppInstallationAuth(app_auth, repo_owner, repositories=[repo_name]) + return asyncio.run(session.get_token()) + + def get(self, path: str, *args, **kwargs) -> dict: + """Send a GET request to the GitHub API with given args and kwargs.""" + url = f"{self.GITHUB_BASE_URL}/{path}" + return self.session.get(url, *args, **kwargs) + + def post(self, path: str, *args, **kwargs) -> dict: + """Send a POST request to the GitHub API with given args and kwargs.""" + url = f"self.GITHUB_BASE_URL/{path}" + return self.session.post(url, *args, **kwargs) + + +class GitHubAPIClient: + """A convenience client that provides various methods to interact with the GitHub API.""" + + client = None + + def __init__(self, repo: Repo): + self.client = GitHubAPI(repo) + self.repo = repo + self.repo_base_url = ( + f"repos/{self.repo._github_repo_org}/{self.repo.git_repo_name}" + ) + + def _get(self, path: str, *args, **kwargs) -> dict: + result = self.client.get(path, *args, **kwargs) + return result.json() + + def list_pull_requests(self) -> list: + """List all pull requests in the repo.""" + return self._get(f"{self.repo_base_url}/pulls") + + def get_pull_request(self, pull_number: int) -> dict: + """Get a specific pull request from the repo.""" + return self._get(f"{self.repo_base_url}/pulls/{pull_number}") From 456ba02047854094ce47dfa4865749d3a09b7ccf Mon Sep 17 00:00:00 2001 From: Zeid Date: Fri, 26 Sep 2025 10:50:52 -0400 Subject: [PATCH 02/26] github: add pull request view, url, template (bug 1989963) --- src/lando/ui/jinja2/stack/pull_request.html | 40 +++++++++++++++ src/lando/ui/pull_requests.py | 34 ++++++++++++ src/lando/urls.py | 7 ++- src/lando/utils/github.py | 57 +++++++++++++++++++++ 4 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 src/lando/ui/jinja2/stack/pull_request.html create mode 100644 src/lando/ui/pull_requests.py diff --git a/src/lando/ui/jinja2/stack/pull_request.html b/src/lando/ui/jinja2/stack/pull_request.html new file mode 100644 index 000000000..d647308f4 --- /dev/null +++ b/src/lando/ui/jinja2/stack/pull_request.html @@ -0,0 +1,40 @@ +{% extends "partials/layout.html" %} +{% block page_title %}{{pull_request}}{% endblock %} + +{% block main %} +
+

Pull request {{ pull_request.number }}

+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Pull request{{ pull_request.number }}
Target Lando Repo{{ target_repo }}
GitHub repo{{ pull_request.head_repo_git_url }}
Author{{ pull_request.user_login }}
State{{ pull_request.state}}
Title{{ pull_request.title }}
Description{{ pull_request.body }}
+
+
+{% endblock %} diff --git a/src/lando/ui/pull_requests.py b/src/lando/ui/pull_requests.py new file mode 100644 index 000000000..acc3f8683 --- /dev/null +++ b/src/lando/ui/pull_requests.py @@ -0,0 +1,34 @@ +import logging + +from django.core.handlers.wsgi import WSGIRequest +from django.template.response import TemplateResponse + +from lando.main.models import Repo +from lando.ui.views import LandoView +from lando.utils.github import GitHubAPIClient, PullRequest + +logger = logging.getLogger(__name__) + + +class PullRequestView(LandoView): + """A class-based view to handle pull requests in the Lando UI.""" + + def get( + self, request: WSGIRequest, repo_name: str, number: int, *args, **kwargs + ) -> TemplateResponse: + """Handle the GET request for the pull request view.""" + target_repo = Repo.objects.get(name=repo_name) + client = GitHubAPIClient(target_repo) + pull_request = PullRequest(client.get_pull_request(number)) + + context = { + "dryrun": None, + "target_repo": target_repo, + "pull_request": pull_request, + } + + return TemplateResponse( + request=request, + template="stack/pull_request.html", + context=context, + ) diff --git a/src/lando/urls.py b/src/lando/urls.py index 24a56314e..c3e2f08b9 100644 --- a/src/lando/urls.py +++ b/src/lando/urls.py @@ -38,7 +38,7 @@ from lando.try_api.api import ( api as try_api, ) -from lando.ui import jobs +from lando.ui import jobs, pull_requests from lando.ui.legacy import pages, revisions, user_settings urlpatterns = [ @@ -52,6 +52,11 @@ path( "D/", revisions.RevisionView.as_view(), name="revisions-page" ), + path( + "pulls///", + pull_requests.PullRequestView.as_view(), + name="pull-request", + ), path("manage_api_key/", user_settings.manage_api_key, name="user-settings"), path("uplift/", revisions.UpliftRequestView.as_view(), name="uplift-page"), path( diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index 277ff5217..ac45931e7 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -89,3 +89,60 @@ def list_pull_requests(self) -> list: def get_pull_request(self, pull_number: int) -> dict: """Get a specific pull request from the repo.""" return self._get(f"{self.repo_base_url}/pulls/{pull_number}") + + def get_diff(self, url: str) -> str: + pass + + +class PullRequest: + """A class that parses data returned from the GitHub API for pull requests.""" + + def __repr__(self) -> str: + return f"Pull request #{self.number} ({self.head_repo_git_url})" + + def __init__(self, data: dict): + self.url = data["url"] + self.base_ref = data["base"]["ref"] # "source" branch name + self.base_sha = data["base"]["sha"] # "source" branch sha + self.base_user_login = data["base"]["user"]["login"] + self.base_user_id = data["base"]["user"]["id"] + self.created_at = data["created_at"] + self.updated_at = data["updated_at"] + self.closed_at = data["closed_at"] + self.merged_at = data["merged_at"] + self.diff_url = data["diff_url"] + self.patch_url = data["patch_url"] + self.body = data["body"] # description + self.is_draft = data["draft"] + self.comments_url = data["comments_url"] + self.commits_url = data["commits_url"] + + self.head_ref = data["head"]["ref"] # "destination" branch name + self.head_sha = data["head"]["sha"] + self.head_repo_git_url = data["head"]["repo"][ + "git_url" + ] # e.g., git://github.com/mozilla-conduit/test-repo.git + self.html_url = data["html_url"] + self.id = data["id"] + self.number = data["number"] + self.requested_reviewers = [ + {"id": r["id"], "html_url": r["html_url"], "login": r["login"]} + for r in data["requested_reviewers"] + ] + self.requested_teams = [ + { + "id": r["id"], + "html_url": r["html_url"], + "name": r["name"], + "slug": r["slug"], + "description": r["description"], + } + for r in data["requested_teams"] + ] + + self.state = data["state"] # e.g., "open" + self.title = data["title"] + + self.user_id = data["user"]["id"] + self.user_html_url = data["user"]["html_url"] + self.user_login = data["user"]["login"] From 9e7692019b4cfd129a6d38150b4c34b38577346f Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 6 Oct 2025 14:19:19 -0400 Subject: [PATCH 03/26] github: add pull request endpoint (bug 1991125) --- src/lando/api/views.py | 28 +++++++++++++++++++++++++++- src/lando/urls.py | 9 +++++++++ src/lando/utils/github.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 4b15aa3d9..14cb91878 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -9,15 +9,22 @@ from django.views import View from django.views.decorators.csrf import csrf_exempt -from lando.main.models import CommitMap +from lando.main.models import CommitMap, Repo from lando.main.models.revision import DiffWarning, DiffWarningStatus from lando.main.scm import ( SCM_TYPE_GIT, SCM_TYPE_HG, ) +from lando.utils.github import GitHubAPIClient, PullRequest from lando.utils.phabricator import get_phabricator_client +class APIView(View): + """A base class for API views.""" + + pass + + def phabricator_api_key_required(func: callable) -> Callable: """A simple wrapper that checks for a valid Phabricator API token.""" @@ -148,3 +155,22 @@ class hg2gitCommitMapView(CommitMapBaseView): """Return corresponding CommitMap given an hg hash.""" scm = SCM_TYPE_HG + + +class PullRequestAPIView(APIView): + """Handle pull requests in the API.""" + + def get(self, request: WSGIRequest, repo_name: str, number: int) -> JsonResponse: + """Return a serialized JSON representation of a pull request.""" + target_repo = Repo.objects.get(name=repo_name) + client = GitHubAPIClient(target_repo) + pull_request = PullRequest(client.get_pull_request(number)) + return JsonResponse(pull_request.serialize(), status=200) + + +class LandingJobAPIView(View): + """Handle landing jobs in the API.""" + + def post(self, request: WSGIRequest, *args, **kwargs): + """Placeholder for creating new landing jobs.""" + pass diff --git a/src/lando/urls.py b/src/lando/urls.py index c3e2f08b9..62c06fec1 100644 --- a/src/lando/urls.py +++ b/src/lando/urls.py @@ -21,6 +21,7 @@ from lando.api.legacy.api import landing_jobs from lando.api.views import ( LegacyDiffWarningView, + PullRequestAPIView, git2hgCommitMapView, hg2gitCommitMapView, ) @@ -85,6 +86,14 @@ ), ] +urlpatterns += [ + path( + "api/pulls//", + PullRequestAPIView.as_view(), + name="api-pull-request", + ), +] + # "API" endpoints ported from legacy API app. urlpatterns += [ path( diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index ac45931e7..5eb0f4c8e 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -146,3 +146,36 @@ def __init__(self, data: dict): self.user_id = data["user"]["id"] self.user_html_url = data["user"]["html_url"] self.user_login = data["user"]["login"] + + def serialize(self) -> dict[str, str]: + """Return a dictionary with various pull request data.""" + return { + "url": self.url, + "base_ref": self.base_ref, + "base_sha": self.base_sha, + "base_user_login": self.base_user_login, + "base_user_id": self.base_user_id, + "created_at": self.created_at, + "updated_at": self.updated_at, + "closed_at": self.closed_at, + "merged_at": self.merged_at, + "diff_url": self.diff_url, + "patch_url": self.patch_url, + "body": self.body, + "is_draft": self.is_draft, + "comments_url": self.comments_url, + "commits_url": self.commits_url, + "head_ref": self.head_ref, + "head_sha": self.head_sha, + "head_repo_git_url": self.head_repo_git_url, + "html_url": self.html_url, + "id": self.id, + "number": self.number, + "requested_reviewers": self.requested_reviewers, + "requested_teams": self.requested_teams, + "state": self.state, + "title": self.title, + "user_id": self.user_id, + "user_html_url": self.user_html_url, + "user_login": self.user_login, + } From 1c63a7a08057a37e07f54b60a60f1e88ff356669 Mon Sep 17 00:00:00 2001 From: Zeid Date: Tue, 14 Oct 2025 14:30:56 -0400 Subject: [PATCH 04/26] pull_request: add more functionality to end points and ui (bug 1991125) --- src/lando/api/views.py | 102 ++++++++++++++++-- .../migrations/0032_revision_pull_number.py | 18 ++++ src/lando/main/models/revision.py | 3 + .../static_src/legacy/js/components/Stack.js | 66 ++++++++++++ src/lando/ui/jinja2/stack/pull_request.html | 39 +++++-- src/lando/ui/pull_requests.py | 2 +- src/lando/urls.py | 6 ++ src/lando/utils/github.py | 62 +++++++++-- 8 files changed, 275 insertions(+), 23 deletions(-) create mode 100644 src/lando/main/migrations/0032_revision_pull_number.py diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 14cb91878..67695d364 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -1,4 +1,6 @@ import json +from collections import defaultdict +from datetime import datetime from functools import wraps from typing import Callable @@ -9,7 +11,14 @@ from django.views import View from django.views.decorators.csrf import csrf_exempt -from lando.main.models import CommitMap, Repo +from lando.main.models import ( + CommitMap, + JobStatus, + LandingJob, + Repo, + Revision, + add_revisions_to_job, +) from lando.main.models.revision import DiffWarning, DiffWarningStatus from lando.main.scm import ( SCM_TYPE_GIT, @@ -157,6 +166,7 @@ class hg2gitCommitMapView(CommitMapBaseView): scm = SCM_TYPE_HG +# TODO: move all these to `lando.api.views.pull_requests`. class PullRequestAPIView(APIView): """Handle pull requests in the API.""" @@ -164,13 +174,91 @@ def get(self, request: WSGIRequest, repo_name: str, number: int) -> JsonResponse """Return a serialized JSON representation of a pull request.""" target_repo = Repo.objects.get(name=repo_name) client = GitHubAPIClient(target_repo) - pull_request = PullRequest(client.get_pull_request(number)) + pull_request = PullRequest(client.get_pull_request(number), target_repo) + patch = client.get_patch(number) + diff = client.get_diff(number) + return JsonResponse({"diff": diff, "patch": patch}, status=200) return JsonResponse(pull_request.serialize(), status=200) -class LandingJobAPIView(View): - """Handle landing jobs in the API.""" +class LandingJobPullRequestAPIView(View): + """Handle pull request landing jobs in the API.""" + + def get( + self, request: WSGIRequest, repo_name: int, pull_number: int + ) -> JsonResponse: + """Return list of landing job ids by status.""" + + target_repo = Repo.objects.get(name=repo_name) + client = GitHubAPIClient(target_repo) + pull_request = PullRequest(client.get_pull_request(pull_number), target_repo) + landing_jobs = defaultdict(list) + for landing_job in pull_request.landing_jobs: + landing_jobs[landing_job.status].append(landing_job.id) + + if len(landing_jobs[JobStatus.LANDED]): + status = "landed" + elif len(landing_jobs[JobStatus.CREATED]): + status = "created" + elif len(landing_jobs[JobStatus.SUBMITTED]): + status = "submitted" + elif len(landing_jobs[JobStatus.IN_PROGRESS]): + status = "in progress" + elif len(landing_jobs[JobStatus.FAILED]): + status = "failed" + else: + status = "unknown" + return JsonResponse({"status": status}, status=200) + + def post( + self, request: WSGIRequest, repo_name: int, pull_number: int + ) -> JsonResponse: + """Create a new landing job for a pull request.""" + + class Form(forms.Form): + """Simple form to get clean some fields.""" + + head_sha = forms.CharField() + # base_ref = forms.CharField() + + # Create a new landing job for a GitHub pull request. + # To do this, verify that the given hash matches the most recent hash + # in the pull request. So we first refetch the pull request. + + target_repo = Repo.objects.get(name=repo_name) + client = GitHubAPIClient(target_repo) + ldap_username = request.user.email + pull_request = PullRequest(client.get_pull_request(pull_number), target_repo) + form = Form(json.loads(request.body)) - def post(self, request: WSGIRequest, *args, **kwargs): - """Placeholder for creating new landing jobs.""" - pass + if not form.is_valid(): + return JsonResponse(form.errors, 400) + + # TODO, use these for verification. + # target_repo = form.cleaned_data["target_repo"] + # base_ref = form.cleaned_data["base_ref"] + + # TODO: validate that the target repo and base_ref match what is in the PR. + # For now, we will just fetch the patch and apply it as-is. + + # TODO: this does not work with binary data, must use patch instead. + diff = client.get_diff(pull_number) + + job = LandingJob(target_repo=target_repo, requester_email=ldap_username) + job.save() + + revision = Revision.objects.create(pull_number=pull_request.number) + patch_data = { + # These should be parsed from the patch, but for now, use logged in user. + "author_name": "Author Name", + "author_email": "Author Email ", + "commit_message": pull_request.title, + "timestamp": int(datetime.now().timestamp()), + } + revision.set_patch(diff, patch_data) + revision.save() + add_revisions_to_job([revision], job) + job.status = JobStatus.SUBMITTED + job.save() + + return JsonResponse({"id": job.id}, status=201) diff --git a/src/lando/main/migrations/0032_revision_pull_number.py b/src/lando/main/migrations/0032_revision_pull_number.py new file mode 100644 index 000000000..a0b03f68e --- /dev/null +++ b/src/lando/main/migrations/0032_revision_pull_number.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.5 on 2025-10-09 18:55 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("main", "0031_alter_landingjob_requester_email"), + ] + + operations = [ + migrations.AddField( + model_name="revision", + name="pull_number", + field=models.IntegerField(blank=True, null=True), + ), + ] diff --git a/src/lando/main/models/revision.py b/src/lando/main/models/revision.py index 3037da0a1..a1c4e4945 100644 --- a/src/lando/main/models/revision.py +++ b/src/lando/main/models/revision.py @@ -46,6 +46,9 @@ class Revision(BaseModel): # does not track all diffs. diff_id = models.IntegerField(blank=True, null=True) + # GitHub pull request number, if this is a pull request. + pull_number = models.IntegerField(blank=True, null=True) + # The actual patch with Mercurial metadata format. patch = models.TextField(blank=True, default="") diff --git a/src/lando/static_src/legacy/js/components/Stack.js b/src/lando/static_src/legacy/js/components/Stack.js index 80b86668d..6e8cf9066 100644 --- a/src/lando/static_src/legacy/js/components/Stack.js +++ b/src/lando/static_src/legacy/js/components/Stack.js @@ -25,5 +25,71 @@ $.fn.stack = function() { $('.edit-assessment-close').on("click", function () { $('.uplift-assessment-edit-modal').removeClass("is-active"); }); + + + var landing_button = $('button.post-landing-job'); + var pull_number = landing_button.data("pull-number"); + var head_sha = landing_button.data("head-sha"); + var repo_name = landing_button.data("repo-name"); + var csrf_token = landing_button.data("csrf-token"); + + fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { + method: 'GET', + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + 'X-CSRFToken': csrf_token + }, + }).then(async response => { + if (response.status == 200) { + var result = await response.json(); + if (result.status == "landed") { + landing_button.prop("disabled", true); + landing_button.removeClass("is-loading").addClass("is-danger"); + landing_button.html("Pull request landed"); + } else if (["created", "submitted", "in progress"].includes(result.status)) { + landing_button.prop("disabled", true); + landing_button.removeClass("is-loading"); + // TODO: allow cancelling job in this case. + landing_button.html("Landing job submitted"); + } else { + landing_button.prop("disabled", false); + landing_button.removeClass("is-loading").addClass("is-success");; + landing_button.html("Request landing"); + } + } else { + // TODO: handle this case. + } + }); + + + landing_button.on('click', function(e) { + landing_button.addClass("is-loading"); + fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { + method: 'POST', + body: JSON.stringify({"head_sha": head_sha}), + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + 'X-CSRFToken': csrf_token + }, + }).then(response => { + if (response.status == 201) { + window.location.reload(); + } else if (response.status == 400) { + landing_button.prop("disabled", true); + landing_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); + landing_button.html("Could not create landing job"); + } else { + landing_button.prop("disabled", true); + landing_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); + landing_button.html("An unknown error occurred"); + } + }); + }); + + + + }); }; diff --git a/src/lando/ui/jinja2/stack/pull_request.html b/src/lando/ui/jinja2/stack/pull_request.html index d647308f4..d45a40481 100644 --- a/src/lando/ui/jinja2/stack/pull_request.html +++ b/src/lando/ui/jinja2/stack/pull_request.html @@ -3,24 +3,40 @@ {% block main %}
-

Pull request {{ pull_request.number }}

+

{{ pull_request.title }}

+ + + + - - - - - + + + + + + + + @@ -28,12 +44,21 @@

Pull request {{ pull_request.number }}

- + + + + + +
Actions + +
Pull request {{ pull_request.number }}
Target Lando Repo{{ target_repo }}
GitHub repo{{ pull_request.head_repo_git_url }}{{ target_repo }} ({{ pull_request.head_repo_git_url }})
Author {{ pull_request.user_login }}
Working branch{{ pull_request.head_ref }} ({{ pull_request.head_sha }})
Target branch{{ pull_request.base_ref }} ({{ pull_request.base_sha }})
State
Title{{ pull_request.title }}{{ pull_request.title }}
Description {{ pull_request.body }}
Landing attempts + {% for landing_job in pull_request.landing_jobs %} + {{ landing_job }}
+ {% endfor %} +
diff --git a/src/lando/ui/pull_requests.py b/src/lando/ui/pull_requests.py index acc3f8683..8704d1165 100644 --- a/src/lando/ui/pull_requests.py +++ b/src/lando/ui/pull_requests.py @@ -19,7 +19,7 @@ def get( """Handle the GET request for the pull request view.""" target_repo = Repo.objects.get(name=repo_name) client = GitHubAPIClient(target_repo) - pull_request = PullRequest(client.get_pull_request(number)) + pull_request = PullRequest(client.get_pull_request(number), target_repo) context = { "dryrun": None, diff --git a/src/lando/urls.py b/src/lando/urls.py index 62c06fec1..1ad7621c9 100644 --- a/src/lando/urls.py +++ b/src/lando/urls.py @@ -20,6 +20,7 @@ from lando.api.legacy.api import landing_jobs from lando.api.views import ( + LandingJobPullRequestAPIView, LegacyDiffWarningView, PullRequestAPIView, git2hgCommitMapView, @@ -92,6 +93,11 @@ PullRequestAPIView.as_view(), name="api-pull-request", ), + path( + "api/pulls///landing_jobs", + LandingJobPullRequestAPIView.as_view(), + name="api-pull-request-landing-job", + ), ] # "API" endpoints ported from legacy API app. diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index 5eb0f4c8e..21d108183 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -3,8 +3,11 @@ import requests from django.conf import settings +from django.db.models.query import QuerySet from simple_github import AppAuth, AppInstallationAuth +from lando.main.models import LandingJob, Revision +from lando.main.models.jobs import JobStatus from lando.main.models.repo import Repo logger = logging.getLogger(__name__) @@ -80,7 +83,13 @@ def __init__(self, repo: Repo): def _get(self, path: str, *args, **kwargs) -> dict: result = self.client.get(path, *args, **kwargs) - return result.json() + content_type = result.headers["content-type"] + if content_type == "application/json; charset=utf-8": + return result.json() + elif content_type == "application/vnd.github.patch; charset=utf-8": + return result.text + elif content_type == "application/vnd.github.diff; charset=utf-8": + return result.text def list_pull_requests(self) -> list: """List all pull requests in the repo.""" @@ -90,8 +99,19 @@ def get_pull_request(self, pull_number: int) -> dict: """Get a specific pull request from the repo.""" return self._get(f"{self.repo_base_url}/pulls/{pull_number}") - def get_diff(self, url: str) -> str: - pass + def get_diff(self, pull_number: int) -> str: + """Fetch a diff, given a pull request number.""" + return self._get( + f"{self.repo_base_url}/pulls/{pull_number}", + headers={"Accept": "application/vnd.github.diff"}, + ) + + def get_patch(self, pull_number: int) -> str: + """Fetch a patch, given a pull request number.""" + return self._get( + f"{self.repo_base_url}/pulls/{pull_number}", + headers={"Accept": "application/vnd.github.patch"}, + ) class PullRequest: @@ -100,10 +120,38 @@ class PullRequest: def __repr__(self) -> str: return f"Pull request #{self.number} ({self.head_repo_git_url})" - def __init__(self, data: dict): + @property + def is_landing(self) -> bool: + """Return True if there are any active landing jobs for this pull request.""" + return self.landing_jobs.filter( + status__in=(JobStatus.CREATED, JobStatus.SUBMITTED, JobStatus.IN_PROGRESS) + ).exists() + + @property + def is_landed(self) -> bool: + """Return True if there are any landed jobs for this pull request.""" + return self.landing_jobs.filter(status=JobStatus.LANDED).exists() + + @property + def landing_jobs(self) -> QuerySet: + """Return a queryset of landing jobs for this pull request.""" + revisions = Revision.objects.filter( + landing_jobs__target_repo__id=self.repo.id, pull_number=self.number + ) + landing_jobs = LandingJob.objects.filter( + unsorted_revisions__in=revisions + ).order_by("-created_at") + + return landing_jobs + + def __init__(self, data: dict, repo: Repo): + self.repo = repo self.url = data["url"] - self.base_ref = data["base"]["ref"] # "source" branch name - self.base_sha = data["base"]["sha"] # "source" branch sha + self.base_ref = data["base"]["ref"] # "target" branch name + self.base_sha = data["base"]["sha"] # "target" branch sha + self.head_ref = data["head"]["ref"] # "working" branch name + self.head_sha = data["head"]["sha"] # "working" branch sha + self.base_user_login = data["base"]["user"]["login"] self.base_user_id = data["base"]["user"]["id"] self.created_at = data["created_at"] @@ -117,8 +165,6 @@ def __init__(self, data: dict): self.comments_url = data["comments_url"] self.commits_url = data["commits_url"] - self.head_ref = data["head"]["ref"] # "destination" branch name - self.head_sha = data["head"]["sha"] self.head_repo_git_url = data["head"]["repo"][ "git_url" ] # e.g., git://github.com/mozilla-conduit/test-repo.git From dea1b83982c7fc6ae6d0c8c30249fb739a6bf4cd Mon Sep 17 00:00:00 2001 From: Zeid Date: Wed, 15 Oct 2025 15:49:11 -0400 Subject: [PATCH 05/26] github: add write methods (bug 1989960) --- src/lando/utils/github.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index 21d108183..3ff858c07 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -65,7 +65,7 @@ def get(self, path: str, *args, **kwargs) -> dict: def post(self, path: str, *args, **kwargs) -> dict: """Send a POST request to the GitHub API with given args and kwargs.""" - url = f"self.GITHUB_BASE_URL/{path}" + url = f"{self.GITHUB_BASE_URL}/{path}" return self.session.post(url, *args, **kwargs) @@ -91,6 +91,10 @@ def _get(self, path: str, *args, **kwargs) -> dict: elif content_type == "application/vnd.github.diff; charset=utf-8": return result.text + def _post(self, path: str, *args, **kwargs): + result = self.client.post(path, *args, **kwargs) + return result.json() + def list_pull_requests(self) -> list: """List all pull requests in the repo.""" return self._get(f"{self.repo_base_url}/pulls") @@ -113,6 +117,25 @@ def get_patch(self, pull_number: int) -> str: headers={"Accept": "application/vnd.github.patch"}, ) + def open_pull_request(self, pull_number: int) -> dict: + """Open the given pull request.""" + return self._post( + f"{self.repo_base_url}/pulls/{pull_number}", json={"state": "open"} + ) + + def close_pull_request(self, pull_number: int) -> dict: + """Close the given pull request.""" + return self._post( + f"{self.repo_base_url}/pulls/{pull_number}", json={"state": "closed"} + ) + + def add_comment_to_pull_request(self, pull_number: int, comment: str) -> dict: + """Add a comment to the given pull request.""" + return self._post( + f"{self.repo_base_url}/issues/{pull_number}/comments", + json={"body": comment}, + ) + class PullRequest: """A class that parses data returned from the GitHub API for pull requests.""" From 5f9c40d7e8cd683d1c99a52203794e8f224cec58 Mon Sep 17 00:00:00 2001 From: Zeid Date: Fri, 17 Oct 2025 10:18:44 -0400 Subject: [PATCH 06/26] landing_worker: add comment + close pr functionality (bug 1994736) --- src/lando/api/legacy/workers/landing_worker.py | 10 ++++++++++ src/lando/main/models/landing_job.py | 8 ++++++++ 2 files changed, 18 insertions(+) diff --git a/src/lando/api/legacy/workers/landing_worker.py b/src/lando/api/legacy/workers/landing_worker.py index 969e02b2c..12aaf24c6 100644 --- a/src/lando/api/legacy/workers/landing_worker.py +++ b/src/lando/api/legacy/workers/landing_worker.py @@ -50,6 +50,7 @@ ) from lando.pushlog.pushlog import PushLog, PushLogForRepo from lando.utils.config import read_lando_config +from lando.utils.github import GitHubAPIClient from lando.utils.tasks import phab_trigger_repo_update logger = logging.getLogger(__name__) @@ -143,6 +144,15 @@ def run_job(self, job: LandingJob) -> bool: job.transition_status(JobAction.LAND, commit_id=commit_id) + if job.is_pull_request_job: + # TODO: move this to different method, and retry if needed. + # NOTE: This may need to happen on the revision-level when stack support is added. + pull_number = job.revisions.first().pull_number + message = f"Pull request closed by commit {commit_id}" + client = GitHubAPIClient(job.target_repo) + client.add_comment_to_pull_request(pull_number, message) + client.close_pull_request(pull_number) + mots_path = Path(repo.path) / "mots.yaml" if mots_path.exists(): logger.info(f"{mots_path} found, setting reviewer data.") diff --git a/src/lando/main/models/landing_job.py b/src/lando/main/models/landing_job.py index db5278861..4406d8a12 100644 --- a/src/lando/main/models/landing_job.py +++ b/src/lando/main/models/landing_job.py @@ -47,6 +47,14 @@ class LandingJob(BaseJob): Revision, through="RevisionLandingJob", related_name="landing_jobs" ) + def is_pull_request_job(self) -> bool: + """Return True if the landing job has one revision that has a pull_number set.""" + # TODO: RE: stack support, this will need to be modified. + return ( + self.revisions.count() == 1 + and self.revisions.first().pull_number is not None + ) + @property def landed_phabricator_revisions(self) -> dict: """Return a mapping associating Phabricator revision IDs with the ID of the landed Diff.""" From a54f5267ccb95a5c698f8478cdeef9d4ebbc7303 Mon Sep 17 00:00:00 2001 From: Zeid <2043828+zzzeid@users.noreply.github.com> Date: Wed, 22 Oct 2025 09:04:29 -0400 Subject: [PATCH 07/26] github: add GitHubAPI, GitHubAPIClient classes (bug 1989637) (#536) - add GitHubAPI class - add GitHubAPIClient class - move "get token" functionality to GitHubAPI class - fix failing test_GitSCM_push_get_github_token --- src/lando/main/models/repo.py | 5 ++ src/lando/main/scm/git.py | 34 ++---------- src/lando/main/tests/test_git.py | 19 +++++-- src/lando/utils/github.py | 91 ++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 34 deletions(-) create mode 100644 src/lando/utils/github.py diff --git a/src/lando/main/models/repo.py b/src/lando/main/models/repo.py index 824678951..a4e4dd2d9 100644 --- a/src/lando/main/models/repo.py +++ b/src/lando/main/models/repo.py @@ -275,6 +275,11 @@ def _github_repo_url(self) -> str | None: if self.is_github: return self.url.removesuffix(".git") + @property + def _github_repo_org(self) -> str | None: + if self.is_github: + return self._github_repo_url.split("/")[-2] + @property def git_repo_name(self) -> str: """Provide the bare name of the Git repo.""" diff --git a/src/lando/main/scm/git.py b/src/lando/main/scm/git.py index c35d1e7a4..5c99cef1a 100644 --- a/src/lando/main/scm/git.py +++ b/src/lando/main/scm/git.py @@ -1,4 +1,3 @@ -import asyncio import io import logging import os @@ -11,8 +10,6 @@ from pathlib import Path from typing import Any -from django.conf import settings -from simple_github import AppAuth, AppInstallationAuth from typing_extensions import override from lando.main.scm.commit import CommitData @@ -112,6 +109,8 @@ def push( tags: list[str] | None = None, ): """Push local code to the remote repository.""" + from lando.utils.github import GitHubAPI + push_command = ["push"] if force_push: @@ -134,7 +133,8 @@ def push( repo = match["repo"] repo_name = repo.removesuffix(".git") - token = self._get_github_token(owner, repo_name) + token = GitHubAPI._get_token(owner, repo_name) + if token: push_path = f"https://git:{token}@github.com/{owner}/{repo}" @@ -152,32 +152,6 @@ def push( self._git_run(*push_command, cwd=self.path) - @staticmethod - def _get_github_token(repo_owner: str, repo_name: str) -> str | None: - """Obtain a fresh GitHub token to push to the specified repo. - - This relies on GITHUB_APP_ID and GITHUB_APP_PRIVKEY to be set in the - settings. Returns None if those are missing. - - The app with ID GITHUB_APP_ID needs to be enabled for the target repo. - - """ - app_id = settings.GITHUB_APP_ID - private_key = settings.GITHUB_APP_PRIVKEY - - if not app_id or not private_key: - logger.warning( - f"Missing GITHUB_APP_ID or GITHUB_APP_PRIVKEY to authenticate against GitHub repo {repo_owner}/{repo_name}", - ) - return None - - app_auth = AppAuth( - app_id, - private_key, - ) - session = AppInstallationAuth(app_auth, repo_owner, repositories=[repo_name]) - return asyncio.run(session.get_token()) - def last_commit_for_path(self, path: str) -> str: """Find last commit to touch a path.""" command = ["log", "--max-count=1", "--format=%H", "--", path] diff --git a/src/lando/main/tests/test_git.py b/src/lando/main/tests/test_git.py index 16dbba200..9c413047b 100644 --- a/src/lando/main/tests/test_git.py +++ b/src/lando/main/tests/test_git.py @@ -7,6 +7,7 @@ from collections.abc import Callable from pathlib import Path from textwrap import dedent +from unittest import mock from unittest.mock import MagicMock import pytest @@ -668,17 +669,27 @@ def test_GitSCM_push( ) -def test_GitSCM_push_get_github_token(git_repo: Path): +@pytest.fixture +def mock_github_api_get_token(monkeypatch: pytest.MonkeyPatch): + mock_get_token = MagicMock() + mock_get_token.side_effect = ["ghs_yolo"] + + monkeypatch.setattr("lando.utils.github.GitHubAPI._get_token", mock_get_token) + + return mock_get_token + + +def test_GitSCM_push_get_github_token( + git_repo: Path, mock_github_api_get_token: mock.Mock +): scm = GitSCM(str(git_repo)) scm._git_run = MagicMock() - scm._get_github_token = MagicMock() - scm._get_github_token.side_effect = ["ghs_yolo"] scm.push("https://github.com/some/repo") assert scm._git_run.call_count == 1, "_git_run wasn't called when pushing" assert ( - scm._get_github_token.call_count == 1 + mock_github_api_get_token.call_count == 1 ), "_get_github_token wasn't called when pushing to a github-like URL" assert ( "git:ghs_yolo@github.com" in scm._git_run.call_args[0][1] diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py new file mode 100644 index 000000000..277ff5217 --- /dev/null +++ b/src/lando/utils/github.py @@ -0,0 +1,91 @@ +import asyncio +import logging + +import requests +from django.conf import settings +from simple_github import AppAuth, AppInstallationAuth + +from lando.main.models.repo import Repo + +logger = logging.getLogger(__name__) + + +class GitHubAPI: + """A simple wrapper that authenticates with and communicates with the GitHub API.""" + + GITHUB_BASE_URL = "https://api.github.com" + + def __init__(self, repo: Repo): + repo_owner = repo._github_repo_org + repo_name = repo.git_repo_name + + self.session = requests.Session() + self.session.headers.update( + { + "Authorization": f"Bearer {self._get_token(repo_owner, repo_name)}", + "User-Agent": settings.HTTP_USER_AGENT, + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + ) + + @staticmethod + def _get_token(repo_owner: str, repo_name: str) -> str | None: + """Obtain a fresh GitHub token to push to the specified repo. + + This relies on GITHUB_APP_ID and GITHUB_APP_PRIVKEY to be set in the + environment. Returns None if those are missing. + + The app with ID GITHUB_APP_ID needs to be enabled for the target repo. + + """ + app_id = settings.GITHUB_APP_ID + private_key = settings.GITHUB_APP_PRIVKEY + + if not app_id or not private_key: + logger.warning( + f"Missing GITHUB_APP_ID or GITHUB_APP_PRIVKEY to authenticate against GitHub repo {repo_owner}/{repo_name}", + ) + return None + + app_auth = AppAuth( + app_id, + private_key, + ) + session = AppInstallationAuth(app_auth, repo_owner, repositories=[repo_name]) + return asyncio.run(session.get_token()) + + def get(self, path: str, *args, **kwargs) -> dict: + """Send a GET request to the GitHub API with given args and kwargs.""" + url = f"{self.GITHUB_BASE_URL}/{path}" + return self.session.get(url, *args, **kwargs) + + def post(self, path: str, *args, **kwargs) -> dict: + """Send a POST request to the GitHub API with given args and kwargs.""" + url = f"self.GITHUB_BASE_URL/{path}" + return self.session.post(url, *args, **kwargs) + + +class GitHubAPIClient: + """A convenience client that provides various methods to interact with the GitHub API.""" + + client = None + + def __init__(self, repo: Repo): + self.client = GitHubAPI(repo) + self.repo = repo + self.repo_base_url = ( + f"repos/{self.repo._github_repo_org}/{self.repo.git_repo_name}" + ) + + def _get(self, path: str, *args, **kwargs) -> dict: + result = self.client.get(path, *args, **kwargs) + return result.json() + + def list_pull_requests(self) -> list: + """List all pull requests in the repo.""" + return self._get(f"{self.repo_base_url}/pulls") + + def get_pull_request(self, pull_number: int) -> dict: + """Get a specific pull request from the repo.""" + return self._get(f"{self.repo_base_url}/pulls/{pull_number}") From f9cc520f57420b1821428a0eac1b940513563cd7 Mon Sep 17 00:00:00 2001 From: Zeid <2043828+zzzeid@users.noreply.github.com> Date: Wed, 22 Oct 2025 09:29:25 -0400 Subject: [PATCH 08/26] github: add pull request view, url, template (bug 1989963) (#553) - add pull request template - add pull request view - add pull request helper class - add pulls url --- src/lando/ui/jinja2/stack/pull_request.html | 40 +++++++++++++++ src/lando/ui/pull_requests.py | 33 ++++++++++++ src/lando/urls.py | 7 ++- src/lando/utils/github.py | 57 +++++++++++++++++++++ 4 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 src/lando/ui/jinja2/stack/pull_request.html create mode 100644 src/lando/ui/pull_requests.py diff --git a/src/lando/ui/jinja2/stack/pull_request.html b/src/lando/ui/jinja2/stack/pull_request.html new file mode 100644 index 000000000..d647308f4 --- /dev/null +++ b/src/lando/ui/jinja2/stack/pull_request.html @@ -0,0 +1,40 @@ +{% extends "partials/layout.html" %} +{% block page_title %}{{pull_request}}{% endblock %} + +{% block main %} +
+

Pull request {{ pull_request.number }}

+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Pull request{{ pull_request.number }}
Target Lando Repo{{ target_repo }}
GitHub repo{{ pull_request.head_repo_git_url }}
Author{{ pull_request.user_login }}
State{{ pull_request.state}}
Title{{ pull_request.title }}
Description{{ pull_request.body }}
+
+
+{% endblock %} diff --git a/src/lando/ui/pull_requests.py b/src/lando/ui/pull_requests.py new file mode 100644 index 000000000..60f31e8d4 --- /dev/null +++ b/src/lando/ui/pull_requests.py @@ -0,0 +1,33 @@ +import logging + +from django.core.handlers.wsgi import WSGIRequest +from django.template.response import TemplateResponse + +from lando.main.models import Repo +from lando.ui.views import LandoView +from lando.utils.github import GitHubAPIClient, PullRequest + +logger = logging.getLogger(__name__) + + +class PullRequestView(LandoView): + """A class-based view to handle pull requests in the Lando UI.""" + + def get( + self, request: WSGIRequest, repo_name: str, number: int, *args, **kwargs + ) -> TemplateResponse: + """Handle the GET request for the pull request view.""" + target_repo = Repo.objects.get(name=repo_name) + client = GitHubAPIClient(target_repo) + pull_request = PullRequest(client.get_pull_request(number)) + + context = { + "target_repo": target_repo, + "pull_request": pull_request, + } + + return TemplateResponse( + request=request, + template="stack/pull_request.html", + context=context, + ) diff --git a/src/lando/urls.py b/src/lando/urls.py index 24a56314e..c3e2f08b9 100644 --- a/src/lando/urls.py +++ b/src/lando/urls.py @@ -38,7 +38,7 @@ from lando.try_api.api import ( api as try_api, ) -from lando.ui import jobs +from lando.ui import jobs, pull_requests from lando.ui.legacy import pages, revisions, user_settings urlpatterns = [ @@ -52,6 +52,11 @@ path( "D/", revisions.RevisionView.as_view(), name="revisions-page" ), + path( + "pulls///", + pull_requests.PullRequestView.as_view(), + name="pull-request", + ), path("manage_api_key/", user_settings.manage_api_key, name="user-settings"), path("uplift/", revisions.UpliftRequestView.as_view(), name="uplift-page"), path( diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index 277ff5217..ac45931e7 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -89,3 +89,60 @@ def list_pull_requests(self) -> list: def get_pull_request(self, pull_number: int) -> dict: """Get a specific pull request from the repo.""" return self._get(f"{self.repo_base_url}/pulls/{pull_number}") + + def get_diff(self, url: str) -> str: + pass + + +class PullRequest: + """A class that parses data returned from the GitHub API for pull requests.""" + + def __repr__(self) -> str: + return f"Pull request #{self.number} ({self.head_repo_git_url})" + + def __init__(self, data: dict): + self.url = data["url"] + self.base_ref = data["base"]["ref"] # "source" branch name + self.base_sha = data["base"]["sha"] # "source" branch sha + self.base_user_login = data["base"]["user"]["login"] + self.base_user_id = data["base"]["user"]["id"] + self.created_at = data["created_at"] + self.updated_at = data["updated_at"] + self.closed_at = data["closed_at"] + self.merged_at = data["merged_at"] + self.diff_url = data["diff_url"] + self.patch_url = data["patch_url"] + self.body = data["body"] # description + self.is_draft = data["draft"] + self.comments_url = data["comments_url"] + self.commits_url = data["commits_url"] + + self.head_ref = data["head"]["ref"] # "destination" branch name + self.head_sha = data["head"]["sha"] + self.head_repo_git_url = data["head"]["repo"][ + "git_url" + ] # e.g., git://github.com/mozilla-conduit/test-repo.git + self.html_url = data["html_url"] + self.id = data["id"] + self.number = data["number"] + self.requested_reviewers = [ + {"id": r["id"], "html_url": r["html_url"], "login": r["login"]} + for r in data["requested_reviewers"] + ] + self.requested_teams = [ + { + "id": r["id"], + "html_url": r["html_url"], + "name": r["name"], + "slug": r["slug"], + "description": r["description"], + } + for r in data["requested_teams"] + ] + + self.state = data["state"] # e.g., "open" + self.title = data["title"] + + self.user_id = data["user"]["id"] + self.user_html_url = data["user"]["html_url"] + self.user_login = data["user"]["login"] From 46cc0bef00564625112472db542176ee63e933b0 Mon Sep 17 00:00:00 2001 From: Zeid <2043828+zzzeid@users.noreply.github.com> Date: Wed, 22 Oct 2025 09:42:02 -0400 Subject: [PATCH 09/26] github: add pull request endpoint (bug 1991125) (#582) - add `PullRequestAPIView.get` endpoint - add placeholder for `LandingJobAPIView.post` endpoint - add `serialize` method to `PullRequest` class - update urls with above --- src/lando/api/views.py | 28 +++++++++++++++++++++++++++- src/lando/urls.py | 9 +++++++++ src/lando/utils/github.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 4b15aa3d9..14cb91878 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -9,15 +9,22 @@ from django.views import View from django.views.decorators.csrf import csrf_exempt -from lando.main.models import CommitMap +from lando.main.models import CommitMap, Repo from lando.main.models.revision import DiffWarning, DiffWarningStatus from lando.main.scm import ( SCM_TYPE_GIT, SCM_TYPE_HG, ) +from lando.utils.github import GitHubAPIClient, PullRequest from lando.utils.phabricator import get_phabricator_client +class APIView(View): + """A base class for API views.""" + + pass + + def phabricator_api_key_required(func: callable) -> Callable: """A simple wrapper that checks for a valid Phabricator API token.""" @@ -148,3 +155,22 @@ class hg2gitCommitMapView(CommitMapBaseView): """Return corresponding CommitMap given an hg hash.""" scm = SCM_TYPE_HG + + +class PullRequestAPIView(APIView): + """Handle pull requests in the API.""" + + def get(self, request: WSGIRequest, repo_name: str, number: int) -> JsonResponse: + """Return a serialized JSON representation of a pull request.""" + target_repo = Repo.objects.get(name=repo_name) + client = GitHubAPIClient(target_repo) + pull_request = PullRequest(client.get_pull_request(number)) + return JsonResponse(pull_request.serialize(), status=200) + + +class LandingJobAPIView(View): + """Handle landing jobs in the API.""" + + def post(self, request: WSGIRequest, *args, **kwargs): + """Placeholder for creating new landing jobs.""" + pass diff --git a/src/lando/urls.py b/src/lando/urls.py index c3e2f08b9..62c06fec1 100644 --- a/src/lando/urls.py +++ b/src/lando/urls.py @@ -21,6 +21,7 @@ from lando.api.legacy.api import landing_jobs from lando.api.views import ( LegacyDiffWarningView, + PullRequestAPIView, git2hgCommitMapView, hg2gitCommitMapView, ) @@ -85,6 +86,14 @@ ), ] +urlpatterns += [ + path( + "api/pulls//", + PullRequestAPIView.as_view(), + name="api-pull-request", + ), +] + # "API" endpoints ported from legacy API app. urlpatterns += [ path( diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index ac45931e7..5eb0f4c8e 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -146,3 +146,36 @@ def __init__(self, data: dict): self.user_id = data["user"]["id"] self.user_html_url = data["user"]["html_url"] self.user_login = data["user"]["login"] + + def serialize(self) -> dict[str, str]: + """Return a dictionary with various pull request data.""" + return { + "url": self.url, + "base_ref": self.base_ref, + "base_sha": self.base_sha, + "base_user_login": self.base_user_login, + "base_user_id": self.base_user_id, + "created_at": self.created_at, + "updated_at": self.updated_at, + "closed_at": self.closed_at, + "merged_at": self.merged_at, + "diff_url": self.diff_url, + "patch_url": self.patch_url, + "body": self.body, + "is_draft": self.is_draft, + "comments_url": self.comments_url, + "commits_url": self.commits_url, + "head_ref": self.head_ref, + "head_sha": self.head_sha, + "head_repo_git_url": self.head_repo_git_url, + "html_url": self.html_url, + "id": self.id, + "number": self.number, + "requested_reviewers": self.requested_reviewers, + "requested_teams": self.requested_teams, + "state": self.state, + "title": self.title, + "user_id": self.user_id, + "user_html_url": self.user_html_url, + "user_login": self.user_login, + } From d5de18f83e23508f34359b29b2fccf2e3d057395 Mon Sep 17 00:00:00 2001 From: Zeid Date: Tue, 14 Oct 2025 14:30:56 -0400 Subject: [PATCH 10/26] pull_request: add more functionality to end points and ui (bug 1991125) --- src/lando/api/views.py | 102 ++++++++++++++++-- .../migrations/0032_revision_pull_number.py | 18 ++++ src/lando/main/models/revision.py | 3 + .../static_src/legacy/js/components/Stack.js | 66 ++++++++++++ src/lando/ui/jinja2/stack/pull_request.html | 39 +++++-- src/lando/ui/pull_requests.py | 2 +- src/lando/urls.py | 6 ++ src/lando/utils/github.py | 62 +++++++++-- 8 files changed, 275 insertions(+), 23 deletions(-) create mode 100644 src/lando/main/migrations/0032_revision_pull_number.py diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 14cb91878..67695d364 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -1,4 +1,6 @@ import json +from collections import defaultdict +from datetime import datetime from functools import wraps from typing import Callable @@ -9,7 +11,14 @@ from django.views import View from django.views.decorators.csrf import csrf_exempt -from lando.main.models import CommitMap, Repo +from lando.main.models import ( + CommitMap, + JobStatus, + LandingJob, + Repo, + Revision, + add_revisions_to_job, +) from lando.main.models.revision import DiffWarning, DiffWarningStatus from lando.main.scm import ( SCM_TYPE_GIT, @@ -157,6 +166,7 @@ class hg2gitCommitMapView(CommitMapBaseView): scm = SCM_TYPE_HG +# TODO: move all these to `lando.api.views.pull_requests`. class PullRequestAPIView(APIView): """Handle pull requests in the API.""" @@ -164,13 +174,91 @@ def get(self, request: WSGIRequest, repo_name: str, number: int) -> JsonResponse """Return a serialized JSON representation of a pull request.""" target_repo = Repo.objects.get(name=repo_name) client = GitHubAPIClient(target_repo) - pull_request = PullRequest(client.get_pull_request(number)) + pull_request = PullRequest(client.get_pull_request(number), target_repo) + patch = client.get_patch(number) + diff = client.get_diff(number) + return JsonResponse({"diff": diff, "patch": patch}, status=200) return JsonResponse(pull_request.serialize(), status=200) -class LandingJobAPIView(View): - """Handle landing jobs in the API.""" +class LandingJobPullRequestAPIView(View): + """Handle pull request landing jobs in the API.""" + + def get( + self, request: WSGIRequest, repo_name: int, pull_number: int + ) -> JsonResponse: + """Return list of landing job ids by status.""" + + target_repo = Repo.objects.get(name=repo_name) + client = GitHubAPIClient(target_repo) + pull_request = PullRequest(client.get_pull_request(pull_number), target_repo) + landing_jobs = defaultdict(list) + for landing_job in pull_request.landing_jobs: + landing_jobs[landing_job.status].append(landing_job.id) + + if len(landing_jobs[JobStatus.LANDED]): + status = "landed" + elif len(landing_jobs[JobStatus.CREATED]): + status = "created" + elif len(landing_jobs[JobStatus.SUBMITTED]): + status = "submitted" + elif len(landing_jobs[JobStatus.IN_PROGRESS]): + status = "in progress" + elif len(landing_jobs[JobStatus.FAILED]): + status = "failed" + else: + status = "unknown" + return JsonResponse({"status": status}, status=200) + + def post( + self, request: WSGIRequest, repo_name: int, pull_number: int + ) -> JsonResponse: + """Create a new landing job for a pull request.""" + + class Form(forms.Form): + """Simple form to get clean some fields.""" + + head_sha = forms.CharField() + # base_ref = forms.CharField() + + # Create a new landing job for a GitHub pull request. + # To do this, verify that the given hash matches the most recent hash + # in the pull request. So we first refetch the pull request. + + target_repo = Repo.objects.get(name=repo_name) + client = GitHubAPIClient(target_repo) + ldap_username = request.user.email + pull_request = PullRequest(client.get_pull_request(pull_number), target_repo) + form = Form(json.loads(request.body)) - def post(self, request: WSGIRequest, *args, **kwargs): - """Placeholder for creating new landing jobs.""" - pass + if not form.is_valid(): + return JsonResponse(form.errors, 400) + + # TODO, use these for verification. + # target_repo = form.cleaned_data["target_repo"] + # base_ref = form.cleaned_data["base_ref"] + + # TODO: validate that the target repo and base_ref match what is in the PR. + # For now, we will just fetch the patch and apply it as-is. + + # TODO: this does not work with binary data, must use patch instead. + diff = client.get_diff(pull_number) + + job = LandingJob(target_repo=target_repo, requester_email=ldap_username) + job.save() + + revision = Revision.objects.create(pull_number=pull_request.number) + patch_data = { + # These should be parsed from the patch, but for now, use logged in user. + "author_name": "Author Name", + "author_email": "Author Email ", + "commit_message": pull_request.title, + "timestamp": int(datetime.now().timestamp()), + } + revision.set_patch(diff, patch_data) + revision.save() + add_revisions_to_job([revision], job) + job.status = JobStatus.SUBMITTED + job.save() + + return JsonResponse({"id": job.id}, status=201) diff --git a/src/lando/main/migrations/0032_revision_pull_number.py b/src/lando/main/migrations/0032_revision_pull_number.py new file mode 100644 index 000000000..a0b03f68e --- /dev/null +++ b/src/lando/main/migrations/0032_revision_pull_number.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.5 on 2025-10-09 18:55 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("main", "0031_alter_landingjob_requester_email"), + ] + + operations = [ + migrations.AddField( + model_name="revision", + name="pull_number", + field=models.IntegerField(blank=True, null=True), + ), + ] diff --git a/src/lando/main/models/revision.py b/src/lando/main/models/revision.py index 3037da0a1..a1c4e4945 100644 --- a/src/lando/main/models/revision.py +++ b/src/lando/main/models/revision.py @@ -46,6 +46,9 @@ class Revision(BaseModel): # does not track all diffs. diff_id = models.IntegerField(blank=True, null=True) + # GitHub pull request number, if this is a pull request. + pull_number = models.IntegerField(blank=True, null=True) + # The actual patch with Mercurial metadata format. patch = models.TextField(blank=True, default="") diff --git a/src/lando/static_src/legacy/js/components/Stack.js b/src/lando/static_src/legacy/js/components/Stack.js index 80b86668d..6e8cf9066 100644 --- a/src/lando/static_src/legacy/js/components/Stack.js +++ b/src/lando/static_src/legacy/js/components/Stack.js @@ -25,5 +25,71 @@ $.fn.stack = function() { $('.edit-assessment-close').on("click", function () { $('.uplift-assessment-edit-modal').removeClass("is-active"); }); + + + var landing_button = $('button.post-landing-job'); + var pull_number = landing_button.data("pull-number"); + var head_sha = landing_button.data("head-sha"); + var repo_name = landing_button.data("repo-name"); + var csrf_token = landing_button.data("csrf-token"); + + fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { + method: 'GET', + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + 'X-CSRFToken': csrf_token + }, + }).then(async response => { + if (response.status == 200) { + var result = await response.json(); + if (result.status == "landed") { + landing_button.prop("disabled", true); + landing_button.removeClass("is-loading").addClass("is-danger"); + landing_button.html("Pull request landed"); + } else if (["created", "submitted", "in progress"].includes(result.status)) { + landing_button.prop("disabled", true); + landing_button.removeClass("is-loading"); + // TODO: allow cancelling job in this case. + landing_button.html("Landing job submitted"); + } else { + landing_button.prop("disabled", false); + landing_button.removeClass("is-loading").addClass("is-success");; + landing_button.html("Request landing"); + } + } else { + // TODO: handle this case. + } + }); + + + landing_button.on('click', function(e) { + landing_button.addClass("is-loading"); + fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { + method: 'POST', + body: JSON.stringify({"head_sha": head_sha}), + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + 'X-CSRFToken': csrf_token + }, + }).then(response => { + if (response.status == 201) { + window.location.reload(); + } else if (response.status == 400) { + landing_button.prop("disabled", true); + landing_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); + landing_button.html("Could not create landing job"); + } else { + landing_button.prop("disabled", true); + landing_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); + landing_button.html("An unknown error occurred"); + } + }); + }); + + + + }); }; diff --git a/src/lando/ui/jinja2/stack/pull_request.html b/src/lando/ui/jinja2/stack/pull_request.html index d647308f4..d45a40481 100644 --- a/src/lando/ui/jinja2/stack/pull_request.html +++ b/src/lando/ui/jinja2/stack/pull_request.html @@ -3,24 +3,40 @@ {% block main %}
-

Pull request {{ pull_request.number }}

+

{{ pull_request.title }}

+ + + + - - - - - + + + + + + + + @@ -28,12 +44,21 @@

Pull request {{ pull_request.number }}

- + + + + + +
Actions + +
Pull request {{ pull_request.number }}
Target Lando Repo{{ target_repo }}
GitHub repo{{ pull_request.head_repo_git_url }}{{ target_repo }} ({{ pull_request.head_repo_git_url }})
Author {{ pull_request.user_login }}
Working branch{{ pull_request.head_ref }} ({{ pull_request.head_sha }})
Target branch{{ pull_request.base_ref }} ({{ pull_request.base_sha }})
State
Title{{ pull_request.title }}{{ pull_request.title }}
Description {{ pull_request.body }}
Landing attempts + {% for landing_job in pull_request.landing_jobs %} + {{ landing_job }}
+ {% endfor %} +
diff --git a/src/lando/ui/pull_requests.py b/src/lando/ui/pull_requests.py index 60f31e8d4..709251f7e 100644 --- a/src/lando/ui/pull_requests.py +++ b/src/lando/ui/pull_requests.py @@ -19,7 +19,7 @@ def get( """Handle the GET request for the pull request view.""" target_repo = Repo.objects.get(name=repo_name) client = GitHubAPIClient(target_repo) - pull_request = PullRequest(client.get_pull_request(number)) + pull_request = PullRequest(client.get_pull_request(number), target_repo) context = { "target_repo": target_repo, diff --git a/src/lando/urls.py b/src/lando/urls.py index 62c06fec1..1ad7621c9 100644 --- a/src/lando/urls.py +++ b/src/lando/urls.py @@ -20,6 +20,7 @@ from lando.api.legacy.api import landing_jobs from lando.api.views import ( + LandingJobPullRequestAPIView, LegacyDiffWarningView, PullRequestAPIView, git2hgCommitMapView, @@ -92,6 +93,11 @@ PullRequestAPIView.as_view(), name="api-pull-request", ), + path( + "api/pulls///landing_jobs", + LandingJobPullRequestAPIView.as_view(), + name="api-pull-request-landing-job", + ), ] # "API" endpoints ported from legacy API app. diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index 5eb0f4c8e..21d108183 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -3,8 +3,11 @@ import requests from django.conf import settings +from django.db.models.query import QuerySet from simple_github import AppAuth, AppInstallationAuth +from lando.main.models import LandingJob, Revision +from lando.main.models.jobs import JobStatus from lando.main.models.repo import Repo logger = logging.getLogger(__name__) @@ -80,7 +83,13 @@ def __init__(self, repo: Repo): def _get(self, path: str, *args, **kwargs) -> dict: result = self.client.get(path, *args, **kwargs) - return result.json() + content_type = result.headers["content-type"] + if content_type == "application/json; charset=utf-8": + return result.json() + elif content_type == "application/vnd.github.patch; charset=utf-8": + return result.text + elif content_type == "application/vnd.github.diff; charset=utf-8": + return result.text def list_pull_requests(self) -> list: """List all pull requests in the repo.""" @@ -90,8 +99,19 @@ def get_pull_request(self, pull_number: int) -> dict: """Get a specific pull request from the repo.""" return self._get(f"{self.repo_base_url}/pulls/{pull_number}") - def get_diff(self, url: str) -> str: - pass + def get_diff(self, pull_number: int) -> str: + """Fetch a diff, given a pull request number.""" + return self._get( + f"{self.repo_base_url}/pulls/{pull_number}", + headers={"Accept": "application/vnd.github.diff"}, + ) + + def get_patch(self, pull_number: int) -> str: + """Fetch a patch, given a pull request number.""" + return self._get( + f"{self.repo_base_url}/pulls/{pull_number}", + headers={"Accept": "application/vnd.github.patch"}, + ) class PullRequest: @@ -100,10 +120,38 @@ class PullRequest: def __repr__(self) -> str: return f"Pull request #{self.number} ({self.head_repo_git_url})" - def __init__(self, data: dict): + @property + def is_landing(self) -> bool: + """Return True if there are any active landing jobs for this pull request.""" + return self.landing_jobs.filter( + status__in=(JobStatus.CREATED, JobStatus.SUBMITTED, JobStatus.IN_PROGRESS) + ).exists() + + @property + def is_landed(self) -> bool: + """Return True if there are any landed jobs for this pull request.""" + return self.landing_jobs.filter(status=JobStatus.LANDED).exists() + + @property + def landing_jobs(self) -> QuerySet: + """Return a queryset of landing jobs for this pull request.""" + revisions = Revision.objects.filter( + landing_jobs__target_repo__id=self.repo.id, pull_number=self.number + ) + landing_jobs = LandingJob.objects.filter( + unsorted_revisions__in=revisions + ).order_by("-created_at") + + return landing_jobs + + def __init__(self, data: dict, repo: Repo): + self.repo = repo self.url = data["url"] - self.base_ref = data["base"]["ref"] # "source" branch name - self.base_sha = data["base"]["sha"] # "source" branch sha + self.base_ref = data["base"]["ref"] # "target" branch name + self.base_sha = data["base"]["sha"] # "target" branch sha + self.head_ref = data["head"]["ref"] # "working" branch name + self.head_sha = data["head"]["sha"] # "working" branch sha + self.base_user_login = data["base"]["user"]["login"] self.base_user_id = data["base"]["user"]["id"] self.created_at = data["created_at"] @@ -117,8 +165,6 @@ def __init__(self, data: dict): self.comments_url = data["comments_url"] self.commits_url = data["commits_url"] - self.head_ref = data["head"]["ref"] # "destination" branch name - self.head_sha = data["head"]["sha"] self.head_repo_git_url = data["head"]["repo"][ "git_url" ] # e.g., git://github.com/mozilla-conduit/test-repo.git From 1446cb06c9bc90a69314c3c8e9be022eb5115fa9 Mon Sep 17 00:00:00 2001 From: Zeid Date: Wed, 22 Oct 2025 14:06:36 -0400 Subject: [PATCH 11/26] code review feedback --- src/lando/api/views.py | 12 +- .../static_src/legacy/js/components/Stack.js | 123 +++++++++--------- src/lando/ui/jinja2/stack/pull_request.html | 9 -- 3 files changed, 68 insertions(+), 76 deletions(-) diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 67695d364..d11018aa7 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -196,15 +196,15 @@ def get( for landing_job in pull_request.landing_jobs: landing_jobs[landing_job.status].append(landing_job.id) - if len(landing_jobs[JobStatus.LANDED]): + if landing_jobs[JobStatus.LANDED]: status = "landed" - elif len(landing_jobs[JobStatus.CREATED]): + elif landing_jobs[JobStatus.CREATED]: status = "created" - elif len(landing_jobs[JobStatus.SUBMITTED]): + elif landing_jobs[JobStatus.SUBMITTED]: status = "submitted" - elif len(landing_jobs[JobStatus.IN_PROGRESS]): + elif landing_jobs[JobStatus.IN_PROGRESS]: status = "in progress" - elif len(landing_jobs[JobStatus.FAILED]): + elif landing_jobs[JobStatus.FAILED]: status = "failed" else: status = "unknown" @@ -249,7 +249,7 @@ class Form(forms.Form): revision = Revision.objects.create(pull_number=pull_request.number) patch_data = { - # These should be parsed from the patch, but for now, use logged in user. + # These should be parsed from the patch, but for now, use a placeholder. "author_name": "Author Name", "author_email": "Author Email ", "commit_message": pull_request.title, diff --git a/src/lando/static_src/legacy/js/components/Stack.js b/src/lando/static_src/legacy/js/components/Stack.js index 6e8cf9066..877ec39ce 100644 --- a/src/lando/static_src/legacy/js/components/Stack.js +++ b/src/lando/static_src/legacy/js/components/Stack.js @@ -27,69 +27,70 @@ $.fn.stack = function() { }); - var landing_button = $('button.post-landing-job'); - var pull_number = landing_button.data("pull-number"); - var head_sha = landing_button.data("head-sha"); - var repo_name = landing_button.data("repo-name"); - var csrf_token = landing_button.data("csrf-token"); - - fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { - method: 'GET', - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/json', - 'X-CSRFToken': csrf_token - }, - }).then(async response => { - if (response.status == 200) { - var result = await response.json(); - if (result.status == "landed") { - landing_button.prop("disabled", true); - landing_button.removeClass("is-loading").addClass("is-danger"); - landing_button.html("Pull request landed"); - } else if (["created", "submitted", "in progress"].includes(result.status)) { - landing_button.prop("disabled", true); - landing_button.removeClass("is-loading"); - // TODO: allow cancelling job in this case. - landing_button.html("Landing job submitted"); - } else { - landing_button.prop("disabled", false); - landing_button.removeClass("is-loading").addClass("is-success");; - landing_button.html("Request landing"); - } - } else { - // TODO: handle this case. - } - }); - - - landing_button.on('click', function(e) { - landing_button.addClass("is-loading"); - fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { - method: 'POST', - body: JSON.stringify({"head_sha": head_sha}), - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/json', - 'X-CSRFToken': csrf_token - }, - }).then(response => { - if (response.status == 201) { - window.location.reload(); - } else if (response.status == 400) { - landing_button.prop("disabled", true); - landing_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); - landing_button.html("Could not create landing job"); - } else { - landing_button.prop("disabled", true); - landing_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); - landing_button.html("An unknown error occurred"); - } - }); - }); - + // Simple check for time being. If the button exists, assume this is a pull request page. + // This should be cleaned up as part of bug 1995754. + var is_pull_request_page = Boolean($('button.post-landing-job').length); + if (is_pull_request_page) { + var pull_request_button = $('button.post-landing-job'); + var pull_number = pull_request_button.data("pull-number"); + var head_sha = pull_request_button.data("head-sha"); + var repo_name = pull_request_button.data("repo-name"); + var csrf_token = pull_request_button.data("csrf-token"); + fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { + method: 'GET', + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + 'X-CSRFToken': csrf_token + }, + }).then(async response => { + if (response.status == 200) { + var result = await response.json(); + if (result.status == "landed") { + pull_request_button.prop("disabled", true); + pull_request_button.removeClass("is-loading").addClass("is-danger"); + pull_request_button.html("Pull request landed"); + } else if (["created", "submitted", "in progress"].includes(result.status)) { + pull_request_button.prop("disabled", true); + pull_request_button.removeClass("is-loading"); + // TODO: allow cancelling job in this case. + pull_request_button.html("Landing job submitted"); + } else { + pull_request_button.prop("disabled", false); + pull_request_button.removeClass("is-loading").addClass("is-success");; + pull_request_button.html("Request landing"); + } + } else { + // TODO: handle this case. + } + }); + pull_request_button.on('click', function(e) { + pull_request_button.addClass("is-loading"); + fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { + method: 'POST', + body: JSON.stringify({"head_sha": head_sha}), + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + 'X-CSRFToken': csrf_token + }, + }).then(response => { + if (response.status == 201) { + window.location.reload(); + } else if (response.status == 400) { + pull_request_button.prop("disabled", true); + pull_request_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); + pull_request_button.html("Could not create landing job"); + } else { + pull_request_button.prop("disabled", true); + pull_request_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); + pull_request_button.html("An unknown error occurred"); + } + }); + }); + }; }); }; diff --git a/src/lando/ui/jinja2/stack/pull_request.html b/src/lando/ui/jinja2/stack/pull_request.html index d45a40481..5b6fb370a 100644 --- a/src/lando/ui/jinja2/stack/pull_request.html +++ b/src/lando/ui/jinja2/stack/pull_request.html @@ -50,15 +50,6 @@

{{ pull_request.title }}

Description {{ pull_request.body }} - - Landing attempts - - {% for landing_job in pull_request.landing_jobs %} - {{ landing_job }}
- {% endfor %} - - - From 5fc249b5f26751f853be5d57d122886fe7938b22 Mon Sep 17 00:00:00 2001 From: Zeid Date: Fri, 24 Oct 2025 09:45:36 -0400 Subject: [PATCH 12/26] code review feedback --- src/lando/main/models/landing_job.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/lando/main/models/landing_job.py b/src/lando/main/models/landing_job.py index 4406d8a12..78797eab5 100644 --- a/src/lando/main/models/landing_job.py +++ b/src/lando/main/models/landing_job.py @@ -48,12 +48,8 @@ class LandingJob(BaseJob): ) def is_pull_request_job(self) -> bool: - """Return True if the landing job has one revision that has a pull_number set.""" - # TODO: RE: stack support, this will need to be modified. - return ( - self.revisions.count() == 1 - and self.revisions.first().pull_number is not None - ) + """Return True if all revisions in the landing job have a pull_number set.""" + return not self.revisions.filter(pull_number__isnull=True).exists() @property def landed_phabricator_revisions(self) -> dict: From c5303a7cdc49519d17519bc35182f838e58b469a Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 09:38:34 -0400 Subject: [PATCH 13/26] clean up --- src/lando/urls.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/lando/urls.py b/src/lando/urls.py index 1ad7621c9..247504e46 100644 --- a/src/lando/urls.py +++ b/src/lando/urls.py @@ -22,7 +22,6 @@ from lando.api.views import ( LandingJobPullRequestAPIView, LegacyDiffWarningView, - PullRequestAPIView, git2hgCommitMapView, hg2gitCommitMapView, ) @@ -88,15 +87,10 @@ ] urlpatterns += [ - path( - "api/pulls//", - PullRequestAPIView.as_view(), - name="api-pull-request", - ), path( "api/pulls///landing_jobs", LandingJobPullRequestAPIView.as_view(), - name="api-pull-request-landing-job", + name="api-landing-job-pull-request", ), ] From 18b2b884a8c15a4f8f1d1cf4a2ba5eacb2f1df29 Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 09:44:03 -0400 Subject: [PATCH 14/26] more clean up --- src/lando/api/views.py | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/lando/api/views.py b/src/lando/api/views.py index d11018aa7..132812378 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -166,21 +166,6 @@ class hg2gitCommitMapView(CommitMapBaseView): scm = SCM_TYPE_HG -# TODO: move all these to `lando.api.views.pull_requests`. -class PullRequestAPIView(APIView): - """Handle pull requests in the API.""" - - def get(self, request: WSGIRequest, repo_name: str, number: int) -> JsonResponse: - """Return a serialized JSON representation of a pull request.""" - target_repo = Repo.objects.get(name=repo_name) - client = GitHubAPIClient(target_repo) - pull_request = PullRequest(client.get_pull_request(number), target_repo) - patch = client.get_patch(number) - diff = client.get_diff(number) - return JsonResponse({"diff": diff, "patch": patch}, status=200) - return JsonResponse(pull_request.serialize(), status=200) - - class LandingJobPullRequestAPIView(View): """Handle pull request landing jobs in the API.""" @@ -234,14 +219,12 @@ class Form(forms.Form): if not form.is_valid(): return JsonResponse(form.errors, 400) - # TODO, use these for verification. + # TODO, use these for verification. See bug 1996571. # target_repo = form.cleaned_data["target_repo"] # base_ref = form.cleaned_data["base_ref"] - # TODO: validate that the target repo and base_ref match what is in the PR. - # For now, we will just fetch the patch and apply it as-is. - # TODO: this does not work with binary data, must use patch instead. + # See bug 1993047. diff = client.get_diff(pull_number) job = LandingJob(target_repo=target_repo, requester_email=ldap_username) @@ -250,6 +233,7 @@ class Form(forms.Form): revision = Revision.objects.create(pull_number=pull_request.number) patch_data = { # These should be parsed from the patch, but for now, use a placeholder. + # See bug 1995006. "author_name": "Author Name", "author_email": "Author Email ", "commit_message": pull_request.title, From f0dc67a598b3c0f06aa116f1e4961880645f7cee Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 09:47:30 -0400 Subject: [PATCH 15/26] minor change --- src/lando/api/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 132812378..99611edcf 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -172,7 +172,7 @@ class LandingJobPullRequestAPIView(View): def get( self, request: WSGIRequest, repo_name: int, pull_number: int ) -> JsonResponse: - """Return list of landing job ids by status.""" + """Return the status of a pull request based on landing job counts.""" target_repo = Repo.objects.get(name=repo_name) client = GitHubAPIClient(target_repo) @@ -193,6 +193,7 @@ def get( status = "failed" else: status = "unknown" + return JsonResponse({"status": status}, status=200) def post( From 1652ee65c019c41ff642400b855061e530c75843 Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 09:49:06 -0400 Subject: [PATCH 16/26] minor change --- src/lando/static_src/legacy/js/components/Stack.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lando/static_src/legacy/js/components/Stack.js b/src/lando/static_src/legacy/js/components/Stack.js index 877ec39ce..09055530c 100644 --- a/src/lando/static_src/legacy/js/components/Stack.js +++ b/src/lando/static_src/legacy/js/components/Stack.js @@ -55,7 +55,6 @@ $.fn.stack = function() { } else if (["created", "submitted", "in progress"].includes(result.status)) { pull_request_button.prop("disabled", true); pull_request_button.removeClass("is-loading"); - // TODO: allow cancelling job in this case. pull_request_button.html("Landing job submitted"); } else { pull_request_button.prop("disabled", false); @@ -63,7 +62,7 @@ $.fn.stack = function() { pull_request_button.html("Request landing"); } } else { - // TODO: handle this case. + // TODO: handle this case. See bug 1996000. } }); From 850827cf06c16808063f78b5c6a06c9194f7c6b1 Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 09:58:41 -0400 Subject: [PATCH 17/26] clean up --- src/lando/utils/github.py | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index 21d108183..502497d3c 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -3,11 +3,8 @@ import requests from django.conf import settings -from django.db.models.query import QuerySet from simple_github import AppAuth, AppInstallationAuth -from lando.main.models import LandingJob, Revision -from lando.main.models.jobs import JobStatus from lando.main.models.repo import Repo logger = logging.getLogger(__name__) @@ -120,30 +117,6 @@ class PullRequest: def __repr__(self) -> str: return f"Pull request #{self.number} ({self.head_repo_git_url})" - @property - def is_landing(self) -> bool: - """Return True if there are any active landing jobs for this pull request.""" - return self.landing_jobs.filter( - status__in=(JobStatus.CREATED, JobStatus.SUBMITTED, JobStatus.IN_PROGRESS) - ).exists() - - @property - def is_landed(self) -> bool: - """Return True if there are any landed jobs for this pull request.""" - return self.landing_jobs.filter(status=JobStatus.LANDED).exists() - - @property - def landing_jobs(self) -> QuerySet: - """Return a queryset of landing jobs for this pull request.""" - revisions = Revision.objects.filter( - landing_jobs__target_repo__id=self.repo.id, pull_number=self.number - ) - landing_jobs = LandingJob.objects.filter( - unsorted_revisions__in=revisions - ).order_by("-created_at") - - return landing_jobs - def __init__(self, data: dict, repo: Repo): self.repo = repo self.url = data["url"] From e22d561ce6029d1c264d6c6891482c6294298ad5 Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 10:10:12 -0400 Subject: [PATCH 18/26] small refactor --- src/lando/api/views.py | 43 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 99611edcf..56d6046dc 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -181,18 +181,18 @@ def get( for landing_job in pull_request.landing_jobs: landing_jobs[landing_job.status].append(landing_job.id) - if landing_jobs[JobStatus.LANDED]: - status = "landed" - elif landing_jobs[JobStatus.CREATED]: - status = "created" - elif landing_jobs[JobStatus.SUBMITTED]: - status = "submitted" - elif landing_jobs[JobStatus.IN_PROGRESS]: - status = "in progress" - elif landing_jobs[JobStatus.FAILED]: - status = "failed" - else: - status = "unknown" + status = None + # Return the first encountered status in this list. + for _status in [ + JobStatus.LANDED, + JobStatus.CREATED, + JobStatus.SUBMITTED, + JobStatus.IN_PROGRESS, + JobStatus.FAILED, + ]: + if landing_jobs[_status]: + status = str(_status) + break return JsonResponse({"status": status}, status=200) @@ -205,12 +205,9 @@ class Form(forms.Form): """Simple form to get clean some fields.""" head_sha = forms.CharField() + # TODO: use this for verification later, see bug 1996571. # base_ref = forms.CharField() - # Create a new landing job for a GitHub pull request. - # To do this, verify that the given hash matches the most recent hash - # in the pull request. So we first refetch the pull request. - target_repo = Repo.objects.get(name=repo_name) client = GitHubAPIClient(target_repo) ldap_username = request.user.email @@ -220,21 +217,15 @@ class Form(forms.Form): if not form.is_valid(): return JsonResponse(form.errors, 400) - # TODO, use these for verification. See bug 1996571. - # target_repo = form.cleaned_data["target_repo"] - # base_ref = form.cleaned_data["base_ref"] - # TODO: this does not work with binary data, must use patch instead. # See bug 1993047. diff = client.get_diff(pull_number) - - job = LandingJob(target_repo=target_repo, requester_email=ldap_username) - job.save() - + job = LandingJob.objects.create( + target_repo=target_repo, requester_email=ldap_username + ) revision = Revision.objects.create(pull_number=pull_request.number) patch_data = { - # These should be parsed from the patch, but for now, use a placeholder. - # See bug 1995006. + # See bug 1995006 (to actually parse authorship info). Use placeholder for now. "author_name": "Author Name", "author_email": "Author Email ", "commit_message": pull_request.title, From e957288a0f5acb67129599080792c5516f902c2f Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 10:17:02 -0400 Subject: [PATCH 19/26] fixes --- src/lando/api/views.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 56d6046dc..6e2e50276 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -175,11 +175,17 @@ def get( """Return the status of a pull request based on landing job counts.""" target_repo = Repo.objects.get(name=repo_name) - client = GitHubAPIClient(target_repo) - pull_request = PullRequest(client.get_pull_request(pull_number), target_repo) - landing_jobs = defaultdict(list) - for landing_job in pull_request.landing_jobs: - landing_jobs[landing_job.status].append(landing_job.id) + landing_jobs_by_status = defaultdict(list) + + revisions = Revision.objects.filter( + landing_jobs__target_repo=target_repo, pull_number=pull_number + ) + landing_jobs = LandingJob.objects.filter( + unsorted_revisions__in=revisions + ).order_by("-created_at") + + for landing_job in landing_jobs: + landing_jobs_by_status[landing_job.status].append(landing_job.id) status = None # Return the first encountered status in this list. @@ -190,7 +196,7 @@ def get( JobStatus.IN_PROGRESS, JobStatus.FAILED, ]: - if landing_jobs[_status]: + if landing_jobs_by_status[_status]: status = str(_status) break From 138326cb07e35cd1d63e42fa324f0d9b278db432 Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 10:20:50 -0400 Subject: [PATCH 20/26] small fix --- src/lando/api/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 6e2e50276..3f96a1aef 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -197,7 +197,7 @@ def get( JobStatus.FAILED, ]: if landing_jobs_by_status[_status]: - status = str(_status) + status = str(_status).lower() break return JsonResponse({"status": status}, status=200) From 4daf7ddf54cfa1c6799f470cdc1384a18d05a5dd Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 10:46:02 -0400 Subject: [PATCH 21/26] fix error --- src/lando/main/models/landing_job.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lando/main/models/landing_job.py b/src/lando/main/models/landing_job.py index 78797eab5..66ade9217 100644 --- a/src/lando/main/models/landing_job.py +++ b/src/lando/main/models/landing_job.py @@ -47,6 +47,7 @@ class LandingJob(BaseJob): Revision, through="RevisionLandingJob", related_name="landing_jobs" ) + @property def is_pull_request_job(self) -> bool: """Return True if all revisions in the landing job have a pull_number set.""" return not self.revisions.filter(pull_number__isnull=True).exists() From d2cf00261a82b363f800fd64f89a3cdef276a9be Mon Sep 17 00:00:00 2001 From: Zeid <2043828+zzzeid@users.noreply.github.com> Date: Tue, 28 Oct 2025 08:53:00 -0400 Subject: [PATCH 22/26] pull_request: add more functionality to end points and ui (bug 1991125) (#607) - add and update pull request API views - add Revision.pull_number field - add pull request functionality to Stack.js - update pull request template - add helper methods and properties in github utils --- src/lando/api/views.py | 96 ++++++++++++++++--- .../migrations/0032_revision_pull_number.py | 18 ++++ src/lando/main/models/revision.py | 3 + .../static_src/legacy/js/components/Stack.js | 66 +++++++++++++ src/lando/ui/jinja2/stack/pull_request.html | 30 ++++-- src/lando/urls.py | 8 +- src/lando/utils/github.py | 32 +++++-- 7 files changed, 222 insertions(+), 31 deletions(-) create mode 100644 src/lando/main/migrations/0032_revision_pull_number.py diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 14cb91878..534952b93 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -1,4 +1,6 @@ import json +from collections import defaultdict +from datetime import datetime from functools import wraps from typing import Callable @@ -9,7 +11,14 @@ from django.views import View from django.views.decorators.csrf import csrf_exempt -from lando.main.models import CommitMap, Repo +from lando.main.models import ( + CommitMap, + JobStatus, + LandingJob, + Repo, + Revision, + add_revisions_to_job, +) from lando.main.models.revision import DiffWarning, DiffWarningStatus from lando.main.scm import ( SCM_TYPE_GIT, @@ -157,20 +166,81 @@ class hg2gitCommitMapView(CommitMapBaseView): scm = SCM_TYPE_HG -class PullRequestAPIView(APIView): - """Handle pull requests in the API.""" +class LandingJobPullRequestAPIView(View): + """Handle pull request landing jobs in the API.""" + + def get( + self, request: WSGIRequest, repo_name: int, pull_number: int + ) -> JsonResponse: + """Return the status of a pull request based on landing job counts.""" - def get(self, request: WSGIRequest, repo_name: str, number: int) -> JsonResponse: - """Return a serialized JSON representation of a pull request.""" target_repo = Repo.objects.get(name=repo_name) - client = GitHubAPIClient(target_repo) - pull_request = PullRequest(client.get_pull_request(number)) - return JsonResponse(pull_request.serialize(), status=200) + landing_jobs_by_status = defaultdict(list) + + revisions = Revision.objects.filter( + landing_jobs__target_repo=target_repo, pull_number=pull_number + ) + landing_jobs = LandingJob.objects.filter( + unsorted_revisions__in=revisions + ).order_by("-created_at") + + for landing_job in landing_jobs: + landing_jobs_by_status[landing_job.status].append(landing_job.id) + + status = None + # Return the first encountered status in this list. + for _status in [ + JobStatus.LANDED, + JobStatus.CREATED, + JobStatus.SUBMITTED, + JobStatus.IN_PROGRESS, + JobStatus.FAILED, + ]: + if landing_jobs_by_status[_status]: + status = str(_status).lower() + break + + return JsonResponse({"status": status}, status=200) + + def post( + self, request: WSGIRequest, repo_name: int, pull_number: int + ) -> JsonResponse: + """Create a new landing job for a pull request.""" + class Form(forms.Form): + """Simple form to get clean some fields.""" + + head_sha = forms.CharField() + # TODO: use this for verification later, see bug 1996571. + # base_ref = forms.CharField() -class LandingJobAPIView(View): - """Handle landing jobs in the API.""" + target_repo = Repo.objects.get(name=repo_name) + client = GitHubAPIClient(target_repo) + ldap_username = request.user.email + pull_request = PullRequest(client.get_pull_request(pull_number)) + form = Form(json.loads(request.body)) - def post(self, request: WSGIRequest, *args, **kwargs): - """Placeholder for creating new landing jobs.""" - pass + if not form.is_valid(): + return JsonResponse(form.errors, 400) + + # TODO: this does not work with binary data, must use patch instead. + # See bug 1993047. + diff = client.get_diff(pull_number) + job = LandingJob.objects.create( + target_repo=target_repo, requester_email=ldap_username + ) + revision = Revision.objects.create(pull_number=pull_request.number) + patch_data = { + # See bug 1995006 (to actually parse authorship info). Use placeholder for now. + "author_name": "Author Name", + "author_email": "Author Email ", + "commit_message": pull_request.title, + "timestamp": int(datetime.now().timestamp()), + } + revision.set_patch(diff, patch_data) + revision.save() + add_revisions_to_job([revision], job) + job.status = JobStatus.SUBMITTED + job.save() + + return JsonResponse({"id": job.id}, status=201) diff --git a/src/lando/main/migrations/0032_revision_pull_number.py b/src/lando/main/migrations/0032_revision_pull_number.py new file mode 100644 index 000000000..a0b03f68e --- /dev/null +++ b/src/lando/main/migrations/0032_revision_pull_number.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.5 on 2025-10-09 18:55 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("main", "0031_alter_landingjob_requester_email"), + ] + + operations = [ + migrations.AddField( + model_name="revision", + name="pull_number", + field=models.IntegerField(blank=True, null=True), + ), + ] diff --git a/src/lando/main/models/revision.py b/src/lando/main/models/revision.py index 3037da0a1..a1c4e4945 100644 --- a/src/lando/main/models/revision.py +++ b/src/lando/main/models/revision.py @@ -46,6 +46,9 @@ class Revision(BaseModel): # does not track all diffs. diff_id = models.IntegerField(blank=True, null=True) + # GitHub pull request number, if this is a pull request. + pull_number = models.IntegerField(blank=True, null=True) + # The actual patch with Mercurial metadata format. patch = models.TextField(blank=True, default="") diff --git a/src/lando/static_src/legacy/js/components/Stack.js b/src/lando/static_src/legacy/js/components/Stack.js index 80b86668d..09055530c 100644 --- a/src/lando/static_src/legacy/js/components/Stack.js +++ b/src/lando/static_src/legacy/js/components/Stack.js @@ -25,5 +25,71 @@ $.fn.stack = function() { $('.edit-assessment-close').on("click", function () { $('.uplift-assessment-edit-modal').removeClass("is-active"); }); + + + // Simple check for time being. If the button exists, assume this is a pull request page. + // This should be cleaned up as part of bug 1995754. + var is_pull_request_page = Boolean($('button.post-landing-job').length); + if (is_pull_request_page) { + var pull_request_button = $('button.post-landing-job'); + + var pull_number = pull_request_button.data("pull-number"); + var head_sha = pull_request_button.data("head-sha"); + var repo_name = pull_request_button.data("repo-name"); + var csrf_token = pull_request_button.data("csrf-token"); + + fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { + method: 'GET', + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + 'X-CSRFToken': csrf_token + }, + }).then(async response => { + if (response.status == 200) { + var result = await response.json(); + if (result.status == "landed") { + pull_request_button.prop("disabled", true); + pull_request_button.removeClass("is-loading").addClass("is-danger"); + pull_request_button.html("Pull request landed"); + } else if (["created", "submitted", "in progress"].includes(result.status)) { + pull_request_button.prop("disabled", true); + pull_request_button.removeClass("is-loading"); + pull_request_button.html("Landing job submitted"); + } else { + pull_request_button.prop("disabled", false); + pull_request_button.removeClass("is-loading").addClass("is-success");; + pull_request_button.html("Request landing"); + } + } else { + // TODO: handle this case. See bug 1996000. + } + }); + + pull_request_button.on('click', function(e) { + pull_request_button.addClass("is-loading"); + fetch(`/api/pulls/${repo_name}/${pull_number}/landing_jobs`, { + method: 'POST', + body: JSON.stringify({"head_sha": head_sha}), + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + 'X-CSRFToken': csrf_token + }, + }).then(response => { + if (response.status == 201) { + window.location.reload(); + } else if (response.status == 400) { + pull_request_button.prop("disabled", true); + pull_request_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); + pull_request_button.html("Could not create landing job"); + } else { + pull_request_button.prop("disabled", true); + pull_request_button.removeClass("is-danger").removeClass("is-loading").addClass("is-warning"); + pull_request_button.html("An unknown error occurred"); + } + }); + }); + }; }); }; diff --git a/src/lando/ui/jinja2/stack/pull_request.html b/src/lando/ui/jinja2/stack/pull_request.html index d647308f4..5b6fb370a 100644 --- a/src/lando/ui/jinja2/stack/pull_request.html +++ b/src/lando/ui/jinja2/stack/pull_request.html @@ -3,24 +3,40 @@ {% block main %}
-

Pull request {{ pull_request.number }}

+

{{ pull_request.title }}

+ + + + - - - - - + + + + + + + + @@ -28,7 +44,7 @@

Pull request {{ pull_request.number }}

- + diff --git a/src/lando/urls.py b/src/lando/urls.py index 62c06fec1..247504e46 100644 --- a/src/lando/urls.py +++ b/src/lando/urls.py @@ -20,8 +20,8 @@ from lando.api.legacy.api import landing_jobs from lando.api.views import ( + LandingJobPullRequestAPIView, LegacyDiffWarningView, - PullRequestAPIView, git2hgCommitMapView, hg2gitCommitMapView, ) @@ -88,9 +88,9 @@ urlpatterns += [ path( - "api/pulls//", - PullRequestAPIView.as_view(), - name="api-pull-request", + "api/pulls///landing_jobs", + LandingJobPullRequestAPIView.as_view(), + name="api-landing-job-pull-request", ), ] diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index 5eb0f4c8e..1e1a4d129 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -80,7 +80,13 @@ def __init__(self, repo: Repo): def _get(self, path: str, *args, **kwargs) -> dict: result = self.client.get(path, *args, **kwargs) - return result.json() + content_type = result.headers["content-type"] + if content_type == "application/json; charset=utf-8": + return result.json() + elif content_type == "application/vnd.github.patch; charset=utf-8": + return result.text + elif content_type == "application/vnd.github.diff; charset=utf-8": + return result.text def list_pull_requests(self) -> list: """List all pull requests in the repo.""" @@ -90,8 +96,19 @@ def get_pull_request(self, pull_number: int) -> dict: """Get a specific pull request from the repo.""" return self._get(f"{self.repo_base_url}/pulls/{pull_number}") - def get_diff(self, url: str) -> str: - pass + def get_diff(self, pull_number: int) -> str: + """Fetch a diff, given a pull request number.""" + return self._get( + f"{self.repo_base_url}/pulls/{pull_number}", + headers={"Accept": "application/vnd.github.diff"}, + ) + + def get_patch(self, pull_number: int) -> str: + """Fetch a patch, given a pull request number.""" + return self._get( + f"{self.repo_base_url}/pulls/{pull_number}", + headers={"Accept": "application/vnd.github.patch"}, + ) class PullRequest: @@ -102,8 +119,11 @@ def __repr__(self) -> str: def __init__(self, data: dict): self.url = data["url"] - self.base_ref = data["base"]["ref"] # "source" branch name - self.base_sha = data["base"]["sha"] # "source" branch sha + self.base_ref = data["base"]["ref"] # "target" branch name + self.base_sha = data["base"]["sha"] # "target" branch sha + self.head_ref = data["head"]["ref"] # "working" branch name + self.head_sha = data["head"]["sha"] # "working" branch sha + self.base_user_login = data["base"]["user"]["login"] self.base_user_id = data["base"]["user"]["id"] self.created_at = data["created_at"] @@ -117,8 +137,6 @@ def __init__(self, data: dict): self.comments_url = data["comments_url"] self.commits_url = data["commits_url"] - self.head_ref = data["head"]["ref"] # "destination" branch name - self.head_sha = data["head"]["sha"] self.head_repo_git_url = data["head"]["repo"][ "git_url" ] # e.g., git://github.com/mozilla-conduit/test-repo.git From a9a01e9acd0f8d9a2727b7b5728cfa004f79335a Mon Sep 17 00:00:00 2001 From: Zeid <2043828+zzzeid@users.noreply.github.com> Date: Tue, 28 Oct 2025 09:16:12 -0400 Subject: [PATCH 23/26] github: add write methods (bug 1989960) (#611) - add methods to open, close, and comment on pull requests --- src/lando/utils/github.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index 1e1a4d129..298ed9dc2 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -62,7 +62,7 @@ def get(self, path: str, *args, **kwargs) -> dict: def post(self, path: str, *args, **kwargs) -> dict: """Send a POST request to the GitHub API with given args and kwargs.""" - url = f"self.GITHUB_BASE_URL/{path}" + url = f"{self.GITHUB_BASE_URL}/{path}" return self.session.post(url, *args, **kwargs) @@ -88,6 +88,10 @@ def _get(self, path: str, *args, **kwargs) -> dict: elif content_type == "application/vnd.github.diff; charset=utf-8": return result.text + def _post(self, path: str, *args, **kwargs): + result = self.client.post(path, *args, **kwargs) + return result.json() + def list_pull_requests(self) -> list: """List all pull requests in the repo.""" return self._get(f"{self.repo_base_url}/pulls") @@ -110,6 +114,25 @@ def get_patch(self, pull_number: int) -> str: headers={"Accept": "application/vnd.github.patch"}, ) + def open_pull_request(self, pull_number: int) -> dict: + """Open the given pull request.""" + return self._post( + f"{self.repo_base_url}/pulls/{pull_number}", json={"state": "open"} + ) + + def close_pull_request(self, pull_number: int) -> dict: + """Close the given pull request.""" + return self._post( + f"{self.repo_base_url}/pulls/{pull_number}", json={"state": "closed"} + ) + + def add_comment_to_pull_request(self, pull_number: int, comment: str) -> dict: + """Add a comment to the given pull request.""" + return self._post( + f"{self.repo_base_url}/issues/{pull_number}/comments", + json={"body": comment}, + ) + class PullRequest: """A class that parses data returned from the GitHub API for pull requests.""" From 34074e5641eb3da48e3d9abda3787ea41e0e1282 Mon Sep 17 00:00:00 2001 From: Zeid Date: Tue, 28 Oct 2025 09:18:36 -0400 Subject: [PATCH 24/26] merge fixups --- src/lando/ui/pull_requests.py | 2 +- src/lando/utils/github.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lando/ui/pull_requests.py b/src/lando/ui/pull_requests.py index 709251f7e..60f31e8d4 100644 --- a/src/lando/ui/pull_requests.py +++ b/src/lando/ui/pull_requests.py @@ -19,7 +19,7 @@ def get( """Handle the GET request for the pull request view.""" target_repo = Repo.objects.get(name=repo_name) client = GitHubAPIClient(target_repo) - pull_request = PullRequest(client.get_pull_request(number), target_repo) + pull_request = PullRequest(client.get_pull_request(number)) context = { "target_repo": target_repo, diff --git a/src/lando/utils/github.py b/src/lando/utils/github.py index 01771ef7a..298ed9dc2 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -140,8 +140,7 @@ class PullRequest: def __repr__(self) -> str: return f"Pull request #{self.number} ({self.head_repo_git_url})" - def __init__(self, data: dict, repo: Repo): - self.repo = repo + def __init__(self, data: dict): self.url = data["url"] self.base_ref = data["base"]["ref"] # "target" branch name self.base_sha = data["base"]["sha"] # "target" branch sha From 5b15cc70eb4903e00bc39d459f4020866be03daf Mon Sep 17 00:00:00 2001 From: Zeid Date: Tue, 28 Oct 2025 09:47:17 -0400 Subject: [PATCH 25/26] merge fixes --- .../migrations/0032_revision_pull_number.py | 18 ------------------ src/lando/main/models/landing_job.py | 5 ----- 2 files changed, 23 deletions(-) delete mode 100644 src/lando/main/migrations/0032_revision_pull_number.py diff --git a/src/lando/main/migrations/0032_revision_pull_number.py b/src/lando/main/migrations/0032_revision_pull_number.py deleted file mode 100644 index a0b03f68e..000000000 --- a/src/lando/main/migrations/0032_revision_pull_number.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 5.2.5 on 2025-10-09 18:55 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("main", "0031_alter_landingjob_requester_email"), - ] - - operations = [ - migrations.AddField( - model_name="revision", - name="pull_number", - field=models.IntegerField(blank=True, null=True), - ), - ] diff --git a/src/lando/main/models/landing_job.py b/src/lando/main/models/landing_job.py index 2ae4be170..4b28b4428 100644 --- a/src/lando/main/models/landing_job.py +++ b/src/lando/main/models/landing_job.py @@ -47,11 +47,6 @@ class LandingJob(BaseJob): Revision, through="RevisionLandingJob", related_name="landing_jobs" ) - @property - def is_pull_request_job(self) -> bool: - """Return True if all revisions in the landing job have a pull_number set.""" - return not self.revisions.filter(pull_number__isnull=True).exists() - @property def landed_phabricator_revisions(self) -> dict: """Return a mapping associating Phabricator revision IDs with the ID of the landed Diff.""" From 7bb963d778da6f84516bfa5b7925cec275f5f160 Mon Sep 17 00:00:00 2001 From: Zeid Date: Tue, 28 Oct 2025 10:00:52 -0400 Subject: [PATCH 26/26] Revert "merge fixes" This reverts commit cfd18486e854a0089df24785ddb4ba3d85bdb2e7. --- src/lando/main/models/landing_job.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lando/main/models/landing_job.py b/src/lando/main/models/landing_job.py index 4b28b4428..2ae4be170 100644 --- a/src/lando/main/models/landing_job.py +++ b/src/lando/main/models/landing_job.py @@ -47,6 +47,11 @@ class LandingJob(BaseJob): Revision, through="RevisionLandingJob", related_name="landing_jobs" ) + @property + def is_pull_request_job(self) -> bool: + """Return True if all revisions in the landing job have a pull_number set.""" + return not self.revisions.filter(pull_number__isnull=True).exists() + @property def landed_phabricator_revisions(self) -> dict: """Return a mapping associating Phabricator revision IDs with the ID of the landed Diff."""
Actions + +
Pull request {{ pull_request.number }}
Target Lando Repo{{ target_repo }}
GitHub repo{{ pull_request.head_repo_git_url }}{{ target_repo }} ({{ pull_request.head_repo_git_url }})
Author {{ pull_request.user_login }}
Working branch{{ pull_request.head_ref }} ({{ pull_request.head_sha }})
Target branch{{ pull_request.base_ref }} ({{ pull_request.base_sha }})
State
Title{{ pull_request.title }}{{ pull_request.title }}
Description