From af5258039cee3e138ec2a6339b21f4ce5e6ec584 Mon Sep 17 00:00:00 2001 From: Zeid Date: Fri, 19 Sep 2025 15:49:50 -0400 Subject: [PATCH 01/15] 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/15] 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/15] 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/15] 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 d5de18f83e23508f34359b29b2fccf2e3d057395 Mon Sep 17 00:00:00 2001 From: Zeid Date: Tue, 14 Oct 2025 14:30:56 -0400 Subject: [PATCH 05/15] 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 06/15] 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 c5303a7cdc49519d17519bc35182f838e58b469a Mon Sep 17 00:00:00 2001 From: Zeid Date: Mon, 27 Oct 2025 09:38:34 -0400 Subject: [PATCH 07/15] 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 08/15] 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 09/15] 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 10/15] 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 11/15] 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 12/15] 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 13/15] 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 14/15] 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 665600a34e512598f84e36a86c601e25cc238ff1 Mon Sep 17 00:00:00 2001 From: Zeid Date: Tue, 28 Oct 2025 08:36:51 -0400 Subject: [PATCH 15/15] code review feedback --- src/lando/api/views.py | 2 +- src/lando/ui/pull_requests.py | 2 +- src/lando/utils/github.py | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lando/api/views.py b/src/lando/api/views.py index 3f96a1aef..534952b93 100644 --- a/src/lando/api/views.py +++ b/src/lando/api/views.py @@ -217,7 +217,7 @@ class Form(forms.Form): 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) + pull_request = PullRequest(client.get_pull_request(pull_number)) form = Form(json.loads(request.body)) if not form.is_valid(): 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 502497d3c..1e1a4d129 100644 --- a/src/lando/utils/github.py +++ b/src/lando/utils/github.py @@ -117,8 +117,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