From 6da2c1464cdd0158f315c06d99f149c3ad9391d4 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Sat, 30 May 2026 10:54:57 -0400 Subject: [PATCH 1/3] Bug 2043733 - Live Github Status for Pull Requests --- extensions/GitHubPullRequests/Config.pm | 16 ++ extensions/GitHubPullRequests/Extension.pm | 52 ++++ .../GitHubPullRequests/lib/WebService.pm | 189 ++++++++++++++ .../en/default/github/header.html.tmpl | 10 + .../en/default/github/table.html.tmpl | 38 +++ .../hook/bug_modal/edit-module.html.tmpl | 24 ++ .../hook/bug_modal/header-end.html.tmpl | 13 + .../web/js/github_pull_requests.js | 237 ++++++++++++++++++ .../web/style/github_pull_requests.css | 94 +++++++ 9 files changed, 673 insertions(+) create mode 100644 extensions/GitHubPullRequests/Config.pm create mode 100644 extensions/GitHubPullRequests/Extension.pm create mode 100644 extensions/GitHubPullRequests/lib/WebService.pm create mode 100644 extensions/GitHubPullRequests/template/en/default/github/header.html.tmpl create mode 100644 extensions/GitHubPullRequests/template/en/default/github/table.html.tmpl create mode 100644 extensions/GitHubPullRequests/template/en/default/hook/bug_modal/edit-module.html.tmpl create mode 100644 extensions/GitHubPullRequests/template/en/default/hook/bug_modal/header-end.html.tmpl create mode 100644 extensions/GitHubPullRequests/web/js/github_pull_requests.js create mode 100644 extensions/GitHubPullRequests/web/style/github_pull_requests.css diff --git a/extensions/GitHubPullRequests/Config.pm b/extensions/GitHubPullRequests/Config.pm new file mode 100644 index 0000000000..e86abc9348 --- /dev/null +++ b/extensions/GitHubPullRequests/Config.pm @@ -0,0 +1,16 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::GitHubPullRequests; + +use 5.10.1; +use strict; +use warnings; + +use constant NAME => 'GitHubPullRequests'; + +__PACKAGE__->NAME; diff --git a/extensions/GitHubPullRequests/Extension.pm b/extensions/GitHubPullRequests/Extension.pm new file mode 100644 index 0000000000..36cb281851 --- /dev/null +++ b/extensions/GitHubPullRequests/Extension.pm @@ -0,0 +1,52 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::GitHubPullRequests; + +use 5.10.1; +use strict; +use warnings; + +use parent qw(Bugzilla::Extension); + +use Bugzilla; + +use constant GITHUB_CONTENT_TYPE => 'text/x-github-pull-request'; + +our $VERSION = '0.01'; + +sub template_before_process { + my ($self, $args) = @_; + my $file = $args->{'file'}; + my $vars = $args->{'vars'}; + + return unless Bugzilla->user->id; + return unless $file =~ /bug_modal\/(header|edit)\.html\.tmpl$/; + + my $bug = exists $vars->{'bugs'} ? $vars->{'bugs'}[0] : $vars->{'bug'}; + return unless $bug; + + my $has_prs = 0; + my $open_pr_count = 0; + foreach my $attachment (@{$bug->attachments}) { + next if $attachment->contenttype ne GITHUB_CONTENT_TYPE; + next if $attachment->isobsolete; + $has_prs = 1; + $open_pr_count++; + } + + $vars->{github_pull_requests} = $has_prs; + $vars->{github_open_pr_count} = $open_pr_count; +} + +sub webservice { + my ($self, $args) = @_; + $args->{dispatch}->{GitHubPullRequests} + = 'Bugzilla::Extension::GitHubPullRequests::WebService'; +} + +__PACKAGE__->NAME; diff --git a/extensions/GitHubPullRequests/lib/WebService.pm b/extensions/GitHubPullRequests/lib/WebService.pm new file mode 100644 index 0000000000..7cf9d48182 --- /dev/null +++ b/extensions/GitHubPullRequests/lib/WebService.pm @@ -0,0 +1,189 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::GitHubPullRequests::WebService; + +use 5.10.1; +use strict; +use warnings; + +use base qw(Bugzilla::WebService); + +use Bugzilla::Bug; +use Bugzilla::Constants; +use Bugzilla::Error; +use Bugzilla::Logging; +use Bugzilla::WebService::Constants; +use Types::Standard qw(-types); +use Type::Params qw(compile); + +use JSON qw(decode_json); +use LWP::UserAgent; +use List::Util qw(uniq); + +use constant GITHUB_CONTENT_TYPE => 'text/x-github-pull-request'; +use constant GITHUB_PR_REGEX => qr{^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)/?$}; +use constant GITHUB_API_BASE => 'https://api.github.com'; +use constant GITHUB_API_TIMEOUT => 10; + +use constant READ_ONLY => qw( + bug_pull_requests +); + +use constant PUBLIC_METHODS => qw( + bug_pull_requests +); + +sub bug_pull_requests { + state $check = compile(Object, Dict [bug_id => Int]); + my ($self, $params) = $check->(@_); + + my $user = Bugzilla->login(LOGIN_REQUIRED); + + ThrowUserError('invalid_parameter', {name => 'bug_id', err => 'required'}) + unless $params->{bug_id}; + + my $bug = Bugzilla::Bug->check({id => $params->{bug_id}, cache => 1}); + + my $ua = LWP::UserAgent->new(timeout => GITHUB_API_TIMEOUT); + $ua->agent('BMO-Bugzilla/1.0'); + if (Bugzilla->params->{proxy_url}) { + $ua->proxy('https', Bugzilla->params->{proxy_url}); + } + + my @pull_requests; + foreach my $attachment (@{$bug->attachments}) { + next if $attachment->contenttype ne GITHUB_CONTENT_TYPE; + next if $attachment->isobsolete; + + my $url = $attachment->data; + $url =~ s/\s+$//; + + my ($owner, $repo, $pr_number) = ($url =~ GITHUB_PR_REGEX); + unless ($owner && $repo && $pr_number) { + WARN("GitHub: could not parse PR URL: $url"); + next; + } + + my $pr_data = _fetch_pull_request($ua, $owner, $repo, $pr_number); + push @pull_requests, $pr_data; + } + + return {pull_requests => \@pull_requests}; +} + +sub _fetch_pull_request { + my ($ua, $owner, $repo, $pr_number) = @_; + + my $url = "https://github.com/$owner/$repo/pull/$pr_number"; + my $api_url = GITHUB_API_BASE . "/repos/$owner/$repo/pulls/$pr_number"; + + my $base = { + url => $url, + number => int($pr_number), + repo => "$owner/$repo", + sortkey => int($pr_number), + }; + + my $pr_response = _github_get($ua, $api_url); + unless ($pr_response->{ok}) { + WARN("GitHub: failed to fetch PR $url: " . $pr_response->{errmsg}); + return {%$base, inaccessible => 1}; + } + + my $pr = $pr_response->{data}; + + my $state; + if ($pr->{draft}) { + $state = 'draft'; + } + elsif ($pr->{merged_at}) { + $state = 'merged'; + } + elsif ($pr->{state} eq 'closed') { + $state = 'closed'; + } + else { + $state = 'open'; + } + + my @labels = map { $_->{name} } @{$pr->{labels} // []}; + + my $reviews_response = _github_get($ua, $api_url . '/reviews'); + my @reviews; + if ($reviews_response->{ok}) { + @reviews = _summarize_reviews($reviews_response->{data}); + } + + return { + %$base, + title => $pr->{title}, + state => $state, + author => $pr->{user}{login}, + reviews => \@reviews, + labels => \@labels, + inaccessible => 0, + }; +} + +sub _github_get { + my ($ua, $url) = @_; + + my $response = $ua->get( + $url, + 'Accept' => 'application/vnd.github+json', + 'X-GitHub-Api-Version' => '2022-11-28', + ); + + unless ($response->is_success) { + return {ok => 0, errmsg => $response->status_line}; + } + + my $data = eval { decode_json($response->decoded_content) }; + if ($@) { + return {ok => 0, errmsg => "JSON parse error: $@"}; + } + + return {ok => 1, data => $data}; +} + +sub _summarize_reviews { + my ($reviews) = @_; + + # Keep only the latest review state per reviewer. + # Reviews are returned in chronological order so we can just overwrite. + my %latest; + my @order; + for my $review (@{$reviews}) { + my $login = $review->{user}{login}; + my $state = $review->{state}; + + # COMMENTED is not a conclusive review state; skip it + next if $state eq 'COMMENTED'; + + if (!exists $latest{$login}) { + push @order, $login; + } + $latest{$login} = $state; + } + + return map { {user => $_, state => $latest{$_}} } @order; +} + +sub rest_resources { + return [ + qr{^/githubpr/bug_pull_requests/(\d+)$}, + { + GET => { + method => 'bug_pull_requests', + params => sub { return {bug_id => $_[0]} }, + }, + }, + ]; +} + +1; diff --git a/extensions/GitHubPullRequests/template/en/default/github/header.html.tmpl b/extensions/GitHubPullRequests/template/en/default/github/header.html.tmpl new file mode 100644 index 0000000000..d7ec048d65 --- /dev/null +++ b/extensions/GitHubPullRequests/template/en/default/github/header.html.tmpl @@ -0,0 +1,10 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% style_urls.push('extensions/GitHubPullRequests/web/style/github_pull_requests.css') %] +[% javascript_urls.push('extensions/GitHubPullRequests/web/js/github_pull_requests.js') %] diff --git a/extensions/GitHubPullRequests/template/en/default/github/table.html.tmpl b/extensions/GitHubPullRequests/template/en/default/github/table.html.tmpl new file mode 100644 index 0000000000..1f989685e2 --- /dev/null +++ b/extensions/GitHubPullRequests/template/en/default/github/table.html.tmpl @@ -0,0 +1,38 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + + + + + + + + + + + + + + + + + + + + + + + + + + +
PRStatusAuthorReviewersRepositoryLabelsTitle
Loading...
Error loading GitHub pull requests: +
+ + +
diff --git a/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/edit-module.html.tmpl b/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/edit-module.html.tmpl new file mode 100644 index 0000000000..28b5141e81 --- /dev/null +++ b/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/edit-module.html.tmpl @@ -0,0 +1,24 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% RETURN UNLESS github_pull_requests %] + +[% + gh_subtitle = []; + IF github_open_pr_count; + gh_subtitle.push(github_open_pr_count _ " open pull request" _ (github_open_pr_count == 1 ? "" : "s")); + END; +%] + +[% WRAPPER bug_modal/module.html.tmpl + title = "GitHub Pull Requests" + subtitle = gh_subtitle + collapsed = 0 +%] + [% INCLUDE github/table.html.tmpl %] +[% END %] diff --git a/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/header-end.html.tmpl b/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/header-end.html.tmpl new file mode 100644 index 0000000000..a37dc98972 --- /dev/null +++ b/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/header-end.html.tmpl @@ -0,0 +1,13 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% + IF github_pull_requests; + PROCESS github/header.html.tmpl; + END; +%] diff --git a/extensions/GitHubPullRequests/web/js/github_pull_requests.js b/extensions/GitHubPullRequests/web/js/github_pull_requests.js new file mode 100644 index 0000000000..0745ee756a --- /dev/null +++ b/extensions/GitHubPullRequests/web/js/github_pull_requests.js @@ -0,0 +1,237 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +"use strict"; + +const GitHubPullRequests = { + showClosed: false, + pullRequests: [], + + // Maps PR state to display label and CSS class + STATE_LABELS: { + open: { label: "Open", cls: "gh-state-open" }, + draft: { label: "Draft", cls: "gh-state-draft" }, + merged: { label: "Merged", cls: "gh-state-merged" }, + closed: { label: "Closed", cls: "gh-state-closed" }, + }, + + // Maps GitHub review state to a symbol shown next to reviewer name + REVIEW_SYMBOLS: { + APPROVED: { symbol: "✓", cls: "gh-review-approved", title: "Approved" }, + CHANGES_REQUESTED: { symbol: "✗", cls: "gh-review-changes", title: "Changes Requested" }, + DISMISSED: { symbol: "–", cls: "gh-review-dismissed", title: "Dismissed" }, + PENDING: { symbol: "…", cls: "gh-review-pending", title: "Pending" }, + }, + + isClosedState(state) { + return state === "closed" || state === "merged"; + }, + + buildRow(pr) { + const tr = document.createElement("tr"); + tr.dataset.prUrl = pr.url; + tr.dataset.prState = pr.state || ""; + + if (pr.inaccessible) { + tr.classList.add("github-pr-inaccessible"); + } + + if (pr.state && this.isClosedState(pr.state)) { + tr.classList.add("github-pr-closed"); + if (!this.showClosed) { + tr.classList.add("bz_default_hidden"); + } + } + + // PR number cell + const tdPr = document.createElement("td"); + tdPr.className = "gh-col-pr"; + const prLink = document.createElement("a"); + prLink.href = pr.url; + prLink.target = "_blank"; + prLink.rel = "noopener noreferrer"; + prLink.textContent = `#${pr.number}`; + tdPr.appendChild(prLink); + tr.appendChild(tdPr); + + // Status cell + const tdStatus = document.createElement("td"); + tdStatus.className = "gh-col-status"; + if (pr.inaccessible) { + tdStatus.textContent = "—"; + } else { + const stateInfo = this.STATE_LABELS[pr.state] || {label: pr.state, cls: ""}; + const badge = document.createElement("span"); + badge.className = `gh-state-badge ${stateInfo.cls}`; + badge.textContent = stateInfo.label; + tdStatus.appendChild(badge); + } + tr.appendChild(tdStatus); + + // Author cell + const tdAuthor = document.createElement("td"); + tdAuthor.className = "gh-col-author"; + if (pr.inaccessible || !pr.author) { + tdAuthor.textContent = "—"; + } else { + const authorLink = document.createElement("a"); + authorLink.href = `https://github.com/${pr.author}`; + authorLink.target = "_blank"; + authorLink.rel = "noopener noreferrer"; + authorLink.textContent = pr.author; + tdAuthor.appendChild(authorLink); + } + tr.appendChild(tdAuthor); + + // Reviewers cell + const tdReviewers = document.createElement("td"); + tdReviewers.className = "gh-col-reviewers"; + if (pr.inaccessible || !pr.reviews || pr.reviews.length === 0) { + tdReviewers.textContent = "—"; + } else { + const reviewerList = document.createElement("ul"); + reviewerList.className = "gh-reviewer-list"; + for (const review of pr.reviews) { + const li = document.createElement("li"); + const symbol = this.REVIEW_SYMBOLS[review.state] || {symbol: "?", cls: "", title: review.state}; + const sym = document.createElement("span"); + sym.className = `gh-review-symbol ${symbol.cls}`; + sym.title = symbol.title; + sym.textContent = symbol.symbol; + li.appendChild(sym); + li.appendChild(document.createTextNode(` ${review.user}`)); + reviewerList.appendChild(li); + } + tdReviewers.appendChild(reviewerList); + } + tr.appendChild(tdReviewers); + + // Repository cell + const tdRepo = document.createElement("td"); + tdRepo.className = "gh-col-repo"; + if (pr.inaccessible || !pr.repo) { + tdRepo.textContent = pr.repo || "—"; + } else { + const repoLink = document.createElement("a"); + repoLink.href = `https://github.com/${pr.repo}`; + repoLink.target = "_blank"; + repoLink.rel = "noopener noreferrer"; + repoLink.textContent = pr.repo; + tdRepo.appendChild(repoLink); + } + tr.appendChild(tdRepo); + + // Labels cell + const tdLabels = document.createElement("td"); + tdLabels.className = "gh-col-labels"; + if (pr.inaccessible || !pr.labels || pr.labels.length === 0) { + tdLabels.textContent = "—"; + } else { + const labelList = document.createElement("span"); + labelList.className = "gh-label-list"; + for (const label of pr.labels) { + const span = document.createElement("span"); + span.className = "gh-label"; + span.textContent = label; + labelList.appendChild(span); + } + tdLabels.appendChild(labelList); + } + tr.appendChild(tdLabels); + + // Title cell + const tdTitle = document.createElement("td"); + tdTitle.className = "gh-col-title"; + if (pr.inaccessible) { + const titleLink = document.createElement("a"); + titleLink.href = pr.url; + titleLink.target = "_blank"; + titleLink.rel = "noopener noreferrer"; + titleLink.textContent = `PR #${pr.number}`; + tdTitle.appendChild(titleLink); + const note = document.createElement("span"); + note.className = "gh-inaccessible-note"; + note.textContent = " (details unavailable)"; + tdTitle.appendChild(note); + } else { + const titleLink = document.createElement("a"); + titleLink.href = pr.url; + titleLink.target = "_blank"; + titleLink.rel = "noopener noreferrer"; + titleLink.textContent = pr.title; + tdTitle.appendChild(titleLink); + } + tr.appendChild(tdTitle); + + return tr; + }, + + updateVisibility() { + for (const pr of this.pullRequests) { + const tr = document.querySelector(`tr[data-pr-url="${CSS.escape(pr.url)}"]`); + if (!tr) continue; + if (this.isClosedState(pr.state || "")) { + tr.classList.toggle("bz_default_hidden", !this.showClosed); + } + } + }, + + async onLoad() { + const showClosedCheckbox = document.querySelector("#github-show-closed"); + if (!showClosedCheckbox) return; + + this.showClosed = showClosedCheckbox.checked; + + const tbody = document.querySelector("tbody.github-prs-body"); + if (!tbody) return; + + const displayLoadError = (errStr) => { + const errRow = tbody.querySelector(".github-loading-error-row"); + errRow.querySelector(".github-load-error-string").replaceChildren(errStr); + errRow.classList.remove("bz_default_hidden"); + }; + + try { + const { pull_requests } = await Bugzilla.API.get( + `githubpr/bug_pull_requests/${BUGZILLA.bug_id}` + ); + + const loadingRow = tbody.querySelector(".github-loading-row"); + + if (pull_requests.length === 0) { + displayLoadError("no pull requests returned"); + } else { + this.pullRequests = pull_requests; + + for (const pr of pull_requests) { + tbody.insertBefore(this.buildRow(pr), loadingRow); + } + + // Show the closed toggle if any PRs are closed/merged + const hasClosed = pull_requests.some(pr => this.isClosedState(pr.state || "")); + if (hasClosed) { + const showClosedTbody = document.querySelector("tbody.github-show-closed"); + if (showClosedTbody) { + showClosedTbody.classList.remove("bz_default_hidden"); + } + } + } + + loadingRow.classList.add("bz_default_hidden"); + } catch (e) { + console.error(e); + displayLoadError(e.message); + tbody.querySelector(".github-loading-row").classList.add("bz_default_hidden"); + } + + showClosedCheckbox.addEventListener("click", () => { + this.showClosed = showClosedCheckbox.checked; + this.updateVisibility(); + }); + }, +}; + +window.addEventListener("DOMContentLoaded", () => { + GitHubPullRequests.onLoad(); +}); diff --git a/extensions/GitHubPullRequests/web/style/github_pull_requests.css b/extensions/GitHubPullRequests/web/style/github_pull_requests.css new file mode 100644 index 0000000000..4373757eff --- /dev/null +++ b/extensions/GitHubPullRequests/web/style/github_pull_requests.css @@ -0,0 +1,94 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +.github-pull-requests { + width: 100%; + border-collapse: collapse; + font-size: 13px; +} + +.github-pull-requests th, +.github-pull-requests td { + padding: 4px 8px; + text-align: left; + vertical-align: top; + border-bottom: 1px solid #e0e0e0; +} + +.github-pull-requests thead th { + background-color: #f5f5f5; + font-weight: bold; + white-space: nowrap; +} + +/* State badge */ +.gh-state-badge { + display: inline-block; + padding: 2px 6px; + border-radius: 3px; + font-size: 11px; + font-weight: bold; + white-space: nowrap; +} + +.gh-state-open { background-color: #2da44e; color: #fff; } +.gh-state-draft { background-color: #6e7781; color: #fff; } +.gh-state-merged { background-color: #8250df; color: #fff; } +.gh-state-closed { background-color: #cf222e; color: #fff; } + +/* Reviewer list */ +.gh-reviewer-list { + list-style: none; + margin: 0; + padding: 0; +} + +.gh-reviewer-list li { + white-space: nowrap; +} + +.gh-review-symbol { + font-weight: bold; + font-family: monospace; +} + +.gh-review-approved { color: #2da44e; } +.gh-review-changes { color: #cf222e; } +.gh-review-dismissed { color: #6e7781; } +.gh-review-pending { color: #6e7781; } + +/* Labels */ +.gh-label-list { + display: flex; + flex-wrap: wrap; + gap: 4px; +} + +.gh-label { + display: inline-block; + padding: 2px 6px; + border-radius: 3px; + font-size: 11px; + background-color: #ddf4ff; + color: #0550ae; + white-space: nowrap; +} + +/* Inaccessible row */ +.github-pr-inaccessible td { + color: #6e7781; +} + +.gh-inaccessible-note { + font-style: italic; + color: #6e7781; +} + +/* Column sizing hints */ +.gh-col-pr { white-space: nowrap; width: 4em; } +.gh-col-status { white-space: nowrap; width: 6em; } +.gh-col-author { white-space: nowrap; } +.gh-col-repo { white-space: nowrap; } +.gh-col-labels { } +.gh-col-title { width: 40%; } From b675dd2c9845677fa9e6bafc1aaedcd7e55f8959 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Wed, 10 Jun 2026 14:35:53 -0400 Subject: [PATCH 2/3] Copilot suggested fixes --- Bugzilla/Config/Github.pm | 1 + extensions/GitHubPullRequests/Extension.pm | 9 +++-- .../GitHubPullRequests/lib/WebService.pm | 40 +++++++++++++++++-- .../hook/bug_modal/edit-module.html.tmpl | 4 +- .../web/js/github_pull_requests.js | 24 ++++++----- .../en/default/admin/params/github.html.tmpl | 6 +++ 6 files changed, 65 insertions(+), 19 deletions(-) diff --git a/Bugzilla/Config/Github.pm b/Bugzilla/Config/Github.pm index baf5867195..100b4319c2 100644 --- a/Bugzilla/Config/Github.pm +++ b/Bugzilla/Config/Github.pm @@ -17,6 +17,7 @@ sub get_param_list { {name => 'github_pr_linking_enabled', type => 'b', default => 0}, {name => 'github_pr_signature_secret', type => 't', default => ''}, {name => 'github_push_comment_enabled', type => 'b', default => 0}, + {name => 'github_api_token', type => 't', default => ''}, ); return @param_list; } diff --git a/extensions/GitHubPullRequests/Extension.pm b/extensions/GitHubPullRequests/Extension.pm index 36cb281851..23783c71e2 100644 --- a/extensions/GitHubPullRequests/Extension.pm +++ b/extensions/GitHubPullRequests/Extension.pm @@ -30,17 +30,20 @@ sub template_before_process { my $bug = exists $vars->{'bugs'} ? $vars->{'bugs'}[0] : $vars->{'bug'}; return unless $bug; + # Note: this only counts linked (non-obsolete) PR attachments. The actual + # open/closed/merged state isn't known until the WebService queries GitHub, + # so we can't report an "open" count at template-processing time. my $has_prs = 0; - my $open_pr_count = 0; + my $linked_pr_count = 0; foreach my $attachment (@{$bug->attachments}) { next if $attachment->contenttype ne GITHUB_CONTENT_TYPE; next if $attachment->isobsolete; $has_prs = 1; - $open_pr_count++; + $linked_pr_count++; } $vars->{github_pull_requests} = $has_prs; - $vars->{github_open_pr_count} = $open_pr_count; + $vars->{github_linked_pr_count} = $linked_pr_count; } sub webservice { diff --git a/extensions/GitHubPullRequests/lib/WebService.pm b/extensions/GitHubPullRequests/lib/WebService.pm index 7cf9d48182..5da210947b 100644 --- a/extensions/GitHubPullRequests/lib/WebService.pm +++ b/extensions/GitHubPullRequests/lib/WebService.pm @@ -23,13 +23,17 @@ use Type::Params qw(compile); use JSON qw(decode_json); use LWP::UserAgent; -use List::Util qw(uniq); use constant GITHUB_CONTENT_TYPE => 'text/x-github-pull-request'; use constant GITHUB_PR_REGEX => qr{^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)/?$}; use constant GITHUB_API_BASE => 'https://api.github.com'; use constant GITHUB_API_TIMEOUT => 10; +# How long (in seconds) to cache a PR's summary in memcached. GitHub's +# unauthenticated rate limit is low (60 req/hr per IP) and authenticated is +# 5000/hr, so caching avoids re-fetching the same PR on every bug view. +use constant GITHUB_CACHE_SECONDS => 300; + use constant READ_ONLY => qw( bug_pull_requests ); @@ -55,11 +59,22 @@ sub bug_pull_requests { $ua->proxy('https', Bugzilla->params->{proxy_url}); } + # Authenticate when a token is configured. This raises GitHub's API rate + # limit from 60 to 5000 requests/hour, avoiding HTTP 403 failures under load. + my $token = Bugzilla->params->{github_api_token}; + if ($token) { + $ua->default_header('Authorization' => "Bearer $token"); + } + my @pull_requests; foreach my $attachment (@{$bug->attachments}) { next if $attachment->contenttype ne GITHUB_CONTENT_TYPE; next if $attachment->isobsolete; + # Don't expose private attachments (and their PR details) to users who + # aren't permitted to see them. + next if $attachment->isprivate && !$user->is_insider; + my $url = $attachment->data; $url =~ s/\s+$//; @@ -89,6 +104,11 @@ sub _fetch_pull_request { sortkey => int($pr_number), }; + # Return a cached summary if we have a fresh one. + my $cache_key = "github_pr." . $url; + my $cached = Bugzilla->memcached->get_data({key => $cache_key}); + return $cached if defined $cached; + my $pr_response = _github_get($ua, $api_url); unless ($pr_response->{ok}) { WARN("GitHub: failed to fetch PR $url: " . $pr_response->{errmsg}); @@ -97,6 +117,13 @@ sub _fetch_pull_request { my $pr = $pr_response->{data}; + # GitHub should return a JSON object; anything else (an error object, a + # list, etc.) means we can't trust the structure, so fall back gracefully. + unless (ref($pr) eq 'HASH') { + WARN("GitHub: unexpected response shape for PR $url"); + return {%$base, inaccessible => 1}; + } + my $state; if ($pr->{draft}) { $state = 'draft'; @@ -119,15 +146,20 @@ sub _fetch_pull_request { @reviews = _summarize_reviews($reviews_response->{data}); } - return { + my $pr_data = { %$base, title => $pr->{title}, state => $state, - author => $pr->{user}{login}, + author => ref($pr->{user}) eq 'HASH' ? $pr->{user}{login} : undef, reviews => \@reviews, labels => \@labels, inaccessible => 0, }; + + Bugzilla->memcached->set_data( + {key => $cache_key, value => $pr_data, expires_in => GITHUB_CACHE_SECONDS}); + + return $pr_data; } sub _github_get { @@ -154,6 +186,8 @@ sub _github_get { sub _summarize_reviews { my ($reviews) = @_; + return () unless ref($reviews) eq 'ARRAY'; + # Keep only the latest review state per reviewer. # Reviews are returned in chronological order so we can just overwrite. my %latest; diff --git a/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/edit-module.html.tmpl b/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/edit-module.html.tmpl index 28b5141e81..81974f9e09 100644 --- a/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/edit-module.html.tmpl +++ b/extensions/GitHubPullRequests/template/en/default/hook/bug_modal/edit-module.html.tmpl @@ -10,8 +10,8 @@ [% gh_subtitle = []; - IF github_open_pr_count; - gh_subtitle.push(github_open_pr_count _ " open pull request" _ (github_open_pr_count == 1 ? "" : "s")); + IF github_linked_pr_count; + gh_subtitle.push(github_linked_pr_count _ " pull request" _ (github_linked_pr_count == 1 ? "" : "s")); END; %] diff --git a/extensions/GitHubPullRequests/web/js/github_pull_requests.js b/extensions/GitHubPullRequests/web/js/github_pull_requests.js index 0745ee756a..9a1598b4aa 100644 --- a/extensions/GitHubPullRequests/web/js/github_pull_requests.js +++ b/extensions/GitHubPullRequests/web/js/github_pull_requests.js @@ -186,9 +186,12 @@ const GitHubPullRequests = { const tbody = document.querySelector("tbody.github-prs-body"); if (!tbody) return; + const loadingRow = tbody.querySelector(".github-loading-row"); + const displayLoadError = (errStr) => { const errRow = tbody.querySelector(".github-loading-error-row"); - errRow.querySelector(".github-load-error-string").replaceChildren(errStr); + if (!errRow) return; + errRow.querySelector(".github-load-error-string")?.replaceChildren(errStr); errRow.classList.remove("bz_default_hidden"); }; @@ -197,19 +200,20 @@ const GitHubPullRequests = { `githubpr/bug_pull_requests/${BUGZILLA.bug_id}` ); - const loadingRow = tbody.querySelector(".github-loading-row"); + this.pullRequests = pull_requests || []; - if (pull_requests.length === 0) { - displayLoadError("no pull requests returned"); + if (this.pullRequests.length === 0) { + // Zero results is a normal outcome (e.g. all attachments were obsolete + // or unparseable), not an error - show a neutral message in place. + loadingRow.querySelector("td").textContent = "No pull requests found."; } else { - this.pullRequests = pull_requests; - - for (const pr of pull_requests) { + for (const pr of this.pullRequests) { tbody.insertBefore(this.buildRow(pr), loadingRow); } + loadingRow.classList.add("bz_default_hidden"); // Show the closed toggle if any PRs are closed/merged - const hasClosed = pull_requests.some(pr => this.isClosedState(pr.state || "")); + const hasClosed = this.pullRequests.some(pr => this.isClosedState(pr.state || "")); if (hasClosed) { const showClosedTbody = document.querySelector("tbody.github-show-closed"); if (showClosedTbody) { @@ -217,12 +221,10 @@ const GitHubPullRequests = { } } } - - loadingRow.classList.add("bz_default_hidden"); } catch (e) { console.error(e); displayLoadError(e.message); - tbody.querySelector(".github-loading-row").classList.add("bz_default_hidden"); + loadingRow.classList.add("bz_default_hidden"); } showClosedCheckbox.addEventListener("click", () => { diff --git a/template/en/default/admin/params/github.html.tmpl b/template/en/default/admin/params/github.html.tmpl index c21699b392..077bcb0fed 100644 --- a/template/en/default/admin/params/github.html.tmpl +++ b/template/en/default/admin/params/github.html.tmpl @@ -40,5 +40,11 @@ github_push_comment_enabled => "Enable the ability for Github pushes to add a comment to a related bug report." + + github_api_token => + "Optional GitHub personal access token used when fetching live pull request " _ + "status for attached pull requests. Providing a token raises GitHub's API " _ + "rate limit from 60 to 5000 requests per hour. A token with read-only " _ + "'public_repo' scope is sufficient for public repositories." } %] From 680378e5564cd6394385ff8411132fa52eaa4f59 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Wed, 10 Jun 2026 16:26:19 -0400 Subject: [PATCH 3/3] Copilot suggested fixes --- Bugzilla/Config/Github.pm | 2 +- extensions/GitHubPullRequests/Extension.pm | 4 ++++ .../GitHubPullRequests/lib/WebService.pm | 22 +++++++++++++++++-- .../en/default/admin/params/github.html.tmpl | 5 +++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Bugzilla/Config/Github.pm b/Bugzilla/Config/Github.pm index 100b4319c2..db6559e26d 100644 --- a/Bugzilla/Config/Github.pm +++ b/Bugzilla/Config/Github.pm @@ -17,7 +17,7 @@ sub get_param_list { {name => 'github_pr_linking_enabled', type => 'b', default => 0}, {name => 'github_pr_signature_secret', type => 't', default => ''}, {name => 'github_push_comment_enabled', type => 'b', default => 0}, - {name => 'github_api_token', type => 't', default => ''}, + {name => 'github_api_token', type => 'p', default => ''}, ); return @param_list; } diff --git a/extensions/GitHubPullRequests/Extension.pm b/extensions/GitHubPullRequests/Extension.pm index 23783c71e2..c1db5a2a09 100644 --- a/extensions/GitHubPullRequests/Extension.pm +++ b/extensions/GitHubPullRequests/Extension.pm @@ -38,6 +38,10 @@ sub template_before_process { foreach my $attachment (@{$bug->attachments}) { next if $attachment->contenttype ne GITHUB_CONTENT_TYPE; next if $attachment->isobsolete; + + # Don't reveal that a private PR attachment exists to users who aren't + # permitted to see it; the WebService applies the same check. + next if $attachment->isprivate && !Bugzilla->user->is_insider; $has_prs = 1; $linked_pr_count++; } diff --git a/extensions/GitHubPullRequests/lib/WebService.pm b/extensions/GitHubPullRequests/lib/WebService.pm index 5da210947b..87a282f329 100644 --- a/extensions/GitHubPullRequests/lib/WebService.pm +++ b/extensions/GitHubPullRequests/lib/WebService.pm @@ -34,6 +34,11 @@ use constant GITHUB_API_TIMEOUT => 10; # 5000/hr, so caching avoids re-fetching the same PR on every bug view. use constant GITHUB_CACHE_SECONDS => 300; +# Cache inaccessible/failed lookups for a shorter period so that persistent +# failures (rate limiting, outages, private repos) don't re-hit GitHub on every +# bug view, while still recovering quickly once the PR becomes reachable again. +use constant GITHUB_ERROR_CACHE_SECONDS => 60; + use constant READ_ONLY => qw( bug_pull_requests ); @@ -112,7 +117,7 @@ sub _fetch_pull_request { my $pr_response = _github_get($ua, $api_url); unless ($pr_response->{ok}) { WARN("GitHub: failed to fetch PR $url: " . $pr_response->{errmsg}); - return {%$base, inaccessible => 1}; + return _cache_inaccessible($cache_key, $base); } my $pr = $pr_response->{data}; @@ -121,7 +126,7 @@ sub _fetch_pull_request { # list, etc.) means we can't trust the structure, so fall back gracefully. unless (ref($pr) eq 'HASH') { WARN("GitHub: unexpected response shape for PR $url"); - return {%$base, inaccessible => 1}; + return _cache_inaccessible($cache_key, $base); } my $state; @@ -162,6 +167,19 @@ sub _fetch_pull_request { return $pr_data; } +sub _cache_inaccessible { + my ($cache_key, $base) = @_; + + my $error_data = {%$base, inaccessible => 1}; + Bugzilla->memcached->set_data({ + key => $cache_key, + value => $error_data, + expires_in => GITHUB_ERROR_CACHE_SECONDS, + }); + + return $error_data; +} + sub _github_get { my ($ua, $url) = @_; diff --git a/template/en/default/admin/params/github.html.tmpl b/template/en/default/admin/params/github.html.tmpl index 077bcb0fed..6a925990c6 100644 --- a/template/en/default/admin/params/github.html.tmpl +++ b/template/en/default/admin/params/github.html.tmpl @@ -44,7 +44,8 @@ github_api_token => "Optional GitHub personal access token used when fetching live pull request " _ "status for attached pull requests. Providing a token raises GitHub's API " _ - "rate limit from 60 to 5000 requests per hour. A token with read-only " _ - "'public_repo' scope is sufficient for public repositories." + "rate limit from 60 to 5000 requests per hour. A fine-grained token with " _ + "read-only 'Pull requests' access (or a classic token with 'public_repo' " _ + "scope) is sufficient for public repositories." } %]