FEATURE: Allow oneboxing private GitHub repo URLs and add private indicator to HTML (#27947)

Followup 560e8aff75 The linked commit allowed oneboxing private GitHub PRs, issues, commits, and so on, but it didn't actually allow oneboxing the root repo e.g https://github.com/discourse/discourse-reactions We didn't have an engine for this, we were relying on OpenGraph tags on the HTML rendering of the page like we do with other oneboxes. To fix this, we needed a new github engine for repos specifically. Also, this commit adds a `data-github-private-repo` attribute to PR, issue, and repo onebox HTML so we have an indicator of whether the repo was private, which can be used for theme components and so on.
2024-07-19 12:21:45 +10:00 · 2024-07-19 12:21:45 +10:00 · f5cbc3e3b8
parent 803877748d
commit f5cbc3e3b8
12 changed files with 386 additions and 17 deletions
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@ -5468,6 +5468,7 @@ en:
      comment_by: "Comment by"
      review_by: "Review by"
      pr_summary: "%{commits} commits changed %{changed_files} files with %{additions} additions and %{deletions} deletions"
+      no_description: "Contribute to %{repo} development by creating an account on GitHub."

  discourse_push_notifications:
    popup:
--- a/lib/onebox/engine/github_gist_onebox.rb
+++ b/lib/onebox/engine/github_gist_onebox.rb
@ -16,6 +16,10 @@ module Onebox
        "https://api.github.com/gists/#{match[:sha]}"
      end

+      def self.priority
+        110 # overlaps with GithubRepoOnebox
+      end
+
      private

      def data
--- a/lib/onebox/engine/github_issue_onebox.rb
+++ b/lib/onebox/engine/github_issue_onebox.rb
@ -38,6 +38,7 @@ module Onebox

      def data
        result = raw(github_auth_header(match[:org])).clone
+        repo = load_repo
        created_at = Time.parse(result["created_at"])
        closed_at = Time.parse(result["closed_at"]) if result["closed_at"]
        body, excerpt = compute_body(result["body"])
@ -65,8 +66,21 @@ module Onebox
          avatar: "https://avatars1.githubusercontent.com/u/#{result["user"]["id"]}?v=2&s=96",
          domain: "#{ulink.host}/#{ulink.path.split("/")[1]}/#{ulink.path.split("/")[2]}",
          i18n: i18n,
+          is_private: repo["private"],
        }
      end
+
+      private
+
+      def load_repo
+        load_json("https://api.github.com/repos/#{match[:org]}/#{match[:repo]}")
+      end
+
+      def load_json(url)
+        ::MultiJson.load(
+          URI.parse(url).open({ read_timeout: timeout }.merge(github_auth_header(match[:org]))),
+        )
+      end
    end
  end
 end
--- a/lib/onebox/engine/github_pull_request_onebox.rb
+++ b/lib/onebox/engine/github_pull_request_onebox.rb
@ -62,6 +62,7 @@ module Onebox
            deletions: result["deletions"],
          },
        )
+        result["is_private"] = result.dig("base", "repo", "private")

        result
      end
@ -100,7 +101,9 @@ module Onebox
      end

      def load_json(url)
-        ::MultiJson.load(URI.parse(url).open(read_timeout: timeout))
+        ::MultiJson.load(
+          URI.parse(url).open({ read_timeout: timeout }.merge(github_auth_header(match[:org]))),
+        )
      end
    end
  end
--- a/lib/onebox/engine/github_repo_onebox.rb
+++ b/lib/onebox/engine/github_repo_onebox.rb
@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require_relative "../mixins/github_body"
+require_relative "../mixins/github_auth_header"
+
+module Onebox
+  module Engine
+    class GithubRepoOnebox
+      include Engine
+      include LayoutSupport
+      include JSON
+      include Onebox::Mixins::GithubAuthHeader
+
+      GITHUB_COMMENT_REGEX = /(<!--.*?-->\r\n)/
+
+      matches_regexp(%r{^https?:\/\/(?:www\.)?(?!gist\.)[^\/]*github\.com\/[^\/]+\/[^\/]+\/?$})
+      always_https
+
+      def url
+        "https://api.github.com/repos/#{match[:org]}/#{match[:repository]}"
+      end
+
+      private
+
+      def match
+        @match ||= @url.match(%r{github\.com/(?<org>[^/]+)/(?<repository>[^/]+)})
+      end
+
+      def data
+        result = raw(github_auth_header(match[:org])).clone
+        result["link"] = link
+        description = result["description"]
+        title = "GitHub - #{result["full_name"]}"
+
+        if description.blank?
+          description = I18n.t("onebox.github.no_description", repo: result["full_name"])
+        else
+          title += ": #{Onebox::Helpers.truncate(description)}"
+        end
+
+        result["description"] = description
+        result["title"] = title
+        result["is_private"] = result["private"]
+
+        # The SecureRandom part of this doesn't matter, it's just used for caching the
+        # repo thumbnail which is generated on the fly by GitHub. There isn't detail
+        # in https://github.blog/2021-06-22-framework-building-open-graph-images/,
+        # but this SO answer https://stackoverflow.com/a/69043743 suggests this is
+        # how it works and testing confirms it.
+        result[
+          "thumbnail"
+        ] = "https://opengraph.githubassets.com/#{SecureRandom.hex}/#{match[:org]}/#{match[:repository]}"
+        result
+      end
+    end
+  end
+end
--- a/lib/onebox/templates/githubissue.mustache
+++ b/lib/onebox/templates/githubissue.mustache
@ -1,5 +1,5 @@
 <div class="github-row">
-  <div class="github-icon-container" title="Issue">
+  <div class="github-icon-container" title="Issue" data-github-private-repo="{{is_private}}">
 	  <svg width="60" height="60" class="github-icon" viewBox="0 0 14 16" version="1.1" aria-hidden="true"><path fill-rule="evenodd" d="M7 2.3c3.14 0 5.7 2.56 5.7 5.7s-2.56 5.7-5.7 5.7A5.71 5.71 0 0 1 1.3 8c0-3.14 2.56-5.7 5.7-5.7zM7 1C3.14 1 0 4.14 0 8s3.14 7 7 7 7-3.14 7-7-3.14-7-7-7zm1 3H6v5h2V4zm0 6H6v2h2v-2z"></path></svg>
  </div>

--- a/lib/onebox/templates/githubpullrequest.mustache
+++ b/lib/onebox/templates/githubpullrequest.mustache
@ -1,4 +1,4 @@
-<div class="github-row">
+<div class="github-row" data-github-private-repo="{{is_private}}">
  {{#commit}}
    <div class="github-icon-container" title="Commit">
      <svg width="60" height="60" class="github-icon" viewBox="0 0 16 16" version="1.1" aria-hidden="true"><path fill-rule="evenodd" d="M10.5 7.75a2.5 2.5 0 11-5 0 2.5 2.5 0 015 0zm1.43.75a4.002 4.002 0 01-7.86 0H.75a.75.75 0 110-1.5h3.32a4.001 4.001 0 017.86 0h3.32a.75.75 0 110 1.5h-3.32z"></path></svg>
--- a/lib/onebox/templates/githubrepo.mustache
+++ b/lib/onebox/templates/githubrepo.mustache
@ -0,0 +1,9 @@
+<div class="github-row" data-github-private-repo="{{is_private}}">
+  <img width="695" height="347" src="{{thumbnail}}" class="thumbnail">
+
+  <h3><a href="{{link}}" target="_blank" rel="noopener ugc noopener">{{title}}</a></h3>
+
+  {{#description}}
+    <p><span class="github-repo-description">{{description}}</span></p>
+  {{/description}}
+</div>
--- a/spec/fixtures/onebox/githubrepo.response
+++ b/spec/fixtures/onebox/githubrepo.response
@ -0,0 +1,146 @@
+{
+  "id": 7569578,
+  "node_id": "MDEwOlJlcG9zaXRvcnk3NTY5NTc4",
+  "name": "discourse",
+  "full_name": "discourse/discourse",
+  "private": false,
+  "owner": {
+    "login": "discourse",
+    "id": 3220138,
+    "node_id": "MDEyOk9yZ2FuaXphdGlvbjMyMjAxMzg=",
+    "avatar_url": "https://avatars.githubusercontent.com/u/3220138?v=4",
+    "gravatar_id": "",
+    "url": "https://api.github.com/users/discourse",
+    "html_url": "https://github.com/discourse",
+    "followers_url": "https://api.github.com/users/discourse/followers",
+    "following_url": "https://api.github.com/users/discourse/following{/other_user}",
+    "gists_url": "https://api.github.com/users/discourse/gists{/gist_id}",
+    "starred_url": "https://api.github.com/users/discourse/starred{/owner}{/repo}",
+    "subscriptions_url": "https://api.github.com/users/discourse/subscriptions",
+    "organizations_url": "https://api.github.com/users/discourse/orgs",
+    "repos_url": "https://api.github.com/users/discourse/repos",
+    "events_url": "https://api.github.com/users/discourse/events{/privacy}",
+    "received_events_url": "https://api.github.com/users/discourse/received_events",
+    "type": "Organization",
+    "site_admin": false
+  },
+  "html_url": "https://github.com/discourse/discourse",
+  "description": "A platform for community discussion. Free, open, simple.",
+  "fork": false,
+  "url": "https://api.github.com/repos/discourse/discourse",
+  "forks_url": "https://api.github.com/repos/discourse/discourse/forks",
+  "keys_url": "https://api.github.com/repos/discourse/discourse/keys{/key_id}",
+  "collaborators_url": "https://api.github.com/repos/discourse/discourse/collaborators{/collaborator}",
+  "teams_url": "https://api.github.com/repos/discourse/discourse/teams",
+  "hooks_url": "https://api.github.com/repos/discourse/discourse/hooks",
+  "issue_events_url": "https://api.github.com/repos/discourse/discourse/issues/events{/number}",
+  "events_url": "https://api.github.com/repos/discourse/discourse/events",
+  "assignees_url": "https://api.github.com/repos/discourse/discourse/assignees{/user}",
+  "branches_url": "https://api.github.com/repos/discourse/discourse/branches{/branch}",
+  "tags_url": "https://api.github.com/repos/discourse/discourse/tags",
+  "blobs_url": "https://api.github.com/repos/discourse/discourse/git/blobs{/sha}",
+  "git_tags_url": "https://api.github.com/repos/discourse/discourse/git/tags{/sha}",
+  "git_refs_url": "https://api.github.com/repos/discourse/discourse/git/refs{/sha}",
+  "trees_url": "https://api.github.com/repos/discourse/discourse/git/trees{/sha}",
+  "statuses_url": "https://api.github.com/repos/discourse/discourse/statuses/{sha}",
+  "languages_url": "https://api.github.com/repos/discourse/discourse/languages",
+  "stargazers_url": "https://api.github.com/repos/discourse/discourse/stargazers",
+  "contributors_url": "https://api.github.com/repos/discourse/discourse/contributors",
+  "subscribers_url": "https://api.github.com/repos/discourse/discourse/subscribers",
+  "subscription_url": "https://api.github.com/repos/discourse/discourse/subscription",
+  "commits_url": "https://api.github.com/repos/discourse/discourse/commits{/sha}",
+  "git_commits_url": "https://api.github.com/repos/discourse/discourse/git/commits{/sha}",
+  "comments_url": "https://api.github.com/repos/discourse/discourse/comments{/number}",
+  "issue_comment_url": "https://api.github.com/repos/discourse/discourse/issues/comments{/number}",
+  "contents_url": "https://api.github.com/repos/discourse/discourse/contents/{+path}",
+  "compare_url": "https://api.github.com/repos/discourse/discourse/compare/{base}...{head}",
+  "merges_url": "https://api.github.com/repos/discourse/discourse/merges",
+  "archive_url": "https://api.github.com/repos/discourse/discourse/{archive_format}{/ref}",
+  "downloads_url": "https://api.github.com/repos/discourse/discourse/downloads",
+  "issues_url": "https://api.github.com/repos/discourse/discourse/issues{/number}",
+  "pulls_url": "https://api.github.com/repos/discourse/discourse/pulls{/number}",
+  "milestones_url": "https://api.github.com/repos/discourse/discourse/milestones{/number}",
+  "notifications_url": "https://api.github.com/repos/discourse/discourse/notifications{?since,all,participating}",
+  "labels_url": "https://api.github.com/repos/discourse/discourse/labels{/name}",
+  "releases_url": "https://api.github.com/repos/discourse/discourse/releases{/id}",
+  "deployments_url": "https://api.github.com/repos/discourse/discourse/deployments",
+  "created_at": "2013-01-12T00:25:55Z",
+  "updated_at": "2024-07-17T05:24:55Z",
+  "pushed_at": "2024-07-17T06:18:53Z",
+  "git_url": "git://github.com/discourse/discourse.git",
+  "ssh_url": "git@github.com:discourse/discourse.git",
+  "clone_url": "https://github.com/discourse/discourse.git",
+  "svn_url": "https://github.com/discourse/discourse",
+  "homepage": "https://www.discourse.org",
+  "size": 654594,
+  "stargazers_count": 41215,
+  "watchers_count": 41215,
+  "language": "Ruby",
+  "has_issues": false,
+  "has_projects": false,
+  "has_downloads": true,
+  "has_wiki": false,
+  "has_pages": false,
+  "has_discussions": false,
+  "forks_count": 8210,
+  "mirror_url": null,
+  "archived": false,
+  "disabled": false,
+  "open_issues_count": 59,
+  "license": {
+    "key": "gpl-2.0",
+    "name": "GNU General Public License v2.0",
+    "spdx_id": "GPL-2.0",
+    "url": "https://api.github.com/licenses/gpl-2.0",
+    "node_id": "MDc6TGljZW5zZTg="
+  },
+  "allow_forking": true,
+  "is_template": false,
+  "web_commit_signoff_required": false,
+  "topics": [
+    "discourse",
+    "ember",
+    "forum",
+    "javascript",
+    "postgresql",
+    "rails",
+    "ruby"
+  ],
+  "visibility": "public",
+  "forks": 8210,
+  "open_issues": 59,
+  "watchers": 41215,
+  "default_branch": "main",
+  "permissions": {
+    "admin": false,
+    "maintain": true,
+    "push": true,
+    "triage": true,
+    "pull": true
+  },
+  "custom_properties": {
+
+  },
+  "organization": {
+    "login": "discourse",
+    "id": 3220138,
+    "node_id": "MDEyOk9yZ2FuaXphdGlvbjMyMjAxMzg=",
+    "avatar_url": "https://avatars.githubusercontent.com/u/3220138?v=4",
+    "gravatar_id": "",
+    "url": "https://api.github.com/users/discourse",
+    "html_url": "https://github.com/discourse",
+    "followers_url": "https://api.github.com/users/discourse/followers",
+    "following_url": "https://api.github.com/users/discourse/following{/other_user}",
+    "gists_url": "https://api.github.com/users/discourse/gists{/gist_id}",
+    "starred_url": "https://api.github.com/users/discourse/starred{/owner}{/repo}",
+    "subscriptions_url": "https://api.github.com/users/discourse/subscriptions",
+    "organizations_url": "https://api.github.com/users/discourse/orgs",
+    "repos_url": "https://api.github.com/users/discourse/repos",
+    "events_url": "https://api.github.com/users/discourse/events{/privacy}",
+    "received_events_url": "https://api.github.com/users/discourse/received_events",
+    "type": "Organization",
+    "site_admin": false
+  },
+  "network_count": 8210,
+  "subscribers_count": 901
+}
--- a/spec/lib/onebox/engine/github_issue_onebox_spec.rb
+++ b/spec/lib/onebox/engine/github_issue_onebox_spec.rb
@ -2,12 +2,15 @@

 RSpec.describe Onebox::Engine::GithubIssueOnebox do
  let(:issue_uri) { "https://api.github.com/repos/discourse/discourse/issues/1" }
+  let(:repo_uri) { "https://api.github.com/repos/discourse/discourse" }
+  let(:repo_response) { onebox_response("githubrepo") }

  before do
    stub_request(:get, issue_uri).to_return(
      status: 200,
      body: onebox_response("github_issue_onebox"),
    )
+    stub_request(:get, repo_uri).to_return(status: 200, body: repo_response)
  end

  include_context "with engines" do
@ -23,6 +26,22 @@ RSpec.describe Onebox::Engine::GithubIssueOnebox do
      expect(html).to include(sanitized_label)
    end

+    it "sets the data-github-private-repo attr to false" do
+      expect(html).to include("data-github-private-repo=\"false\"")
+    end
+
+    context "when the PR is in a private repo" do
+      let(:repo_response) do
+        resp = MultiJson.load(onebox_response("githubrepo"))
+        resp["private"] = true
+        MultiJson.dump(resp)
+      end
+
+      it "sets the data-github-private-repo attr to true" do
+        expect(html).to include("data-github-private-repo=\"true\"")
+      end
+    end
+
    context "when github_onebox_access_token is configured" do
      before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }

--- a/spec/lib/onebox/engine/github_pull_request_onebox_spec.rb
+++ b/spec/lib/onebox/engine/github_pull_request_onebox_spec.rb
@ -3,13 +3,9 @@
 RSpec.describe Onebox::Engine::GithubPullRequestOnebox do
  let(:gh_link) { "https://github.com/discourse/discourse/pull/1253/" }
  let(:api_uri) { "https://api.github.com/repos/discourse/discourse/pulls/1253" }
+  let(:response) { onebox_response(described_class.onebox_name) }

-  before do
-    stub_request(:get, api_uri).to_return(
-      status: 200,
-      body: onebox_response(described_class.onebox_name),
-    )
-  end
+  before { stub_request(:get, api_uri).to_return(status: 200, body: response) }

  include_context "with engines" do
    let(:link) { gh_link }
@ -53,18 +49,37 @@ RSpec.describe Onebox::Engine::GithubPullRequestOnebox do
      expect(html).to include("http://meta.discourse.org/t/audio-html5-tag/8168")
      expect(html).not_to include("test comment")
    end
+
+    it "sets the data-github-private-repo attr to false" do
+      expect(html).to include("data-github-private-repo=\"false\"")
+    end
+
+    context "when the PR is in a private repo" do
+      let(:response) do
+        resp = MultiJson.load(onebox_response(described_class.onebox_name))
+        resp["base"]["repo"]["private"] = true
+        MultiJson.dump(resp)
+      end
+
+      it "sets the data-github-private-repo attr to true" do
+        expect(html).to include("data-github-private-repo=\"true\"")
+      end
+    end
  end

  context "with commit links" do
    let(:gh_link) do
      "https://github.com/discourse/discourse/pull/1253/commits/d7d3be1130c665cc7fab9f05dbf32335229137a6"
    end
+    let(:pr_commit_url) do
+      "https://api.github.com/repos/discourse/discourse/commits/d7d3be1130c665cc7fab9f05dbf32335229137a6"
+    end

    before do
-      stub_request(
-        :get,
-        "https://api.github.com/repos/discourse/discourse/commits/d7d3be1130c665cc7fab9f05dbf32335229137a6",
-      ).to_return(status: 200, body: onebox_response(described_class.onebox_name + "_commit"))
+      stub_request(:get, pr_commit_url).to_return(
+        status: 200,
+        body: onebox_response(described_class.onebox_name + "_commit"),
+      )
    end

    it "includes commit name" do
@ -74,21 +89,50 @@ RSpec.describe Onebox::Engine::GithubPullRequestOnebox do
        "http://meta.discourse.org/t/audio-html5-tag/8168",
      )
    end
+
+    context "when github_onebox_access_token is configured" do
+      before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
+
+      it "sends it as part of the request" do
+        html
+        expect(WebMock).to have_requested(:get, pr_commit_url).with(
+          headers: {
+            "Authorization" => "Bearer github_pat_1234",
+          },
+        )
+      end
+    end
  end

  context "with comment links" do
    let(:gh_link) { "https://github.com/discourse/discourse/pull/1253/#issuecomment-21597425" }
+    let(:comment_api_url) do
+      "https://api.github.com/repos/discourse/discourse/issues/comments/21597425"
+    end

    before do
-      stub_request(
-        :get,
-        "https://api.github.com/repos/discourse/discourse/issues/comments/21597425",
-      ).to_return(status: 200, body: onebox_response(described_class.onebox_name + "_comment"))
+      stub_request(:get, comment_api_url).to_return(
+        status: 200,
+        body: onebox_response(described_class.onebox_name + "_comment"),
+      )
    end

    it "includes comment" do
      expect(html).to include("You&#39;ve signed the CLA")
    end
+
+    context "when github_onebox_access_token is configured" do
+      before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
+
+      it "sends it as part of the request" do
+        html
+        expect(WebMock).to have_requested(:get, api_uri).with(
+          headers: {
+            "Authorization" => "Bearer github_pat_1234",
+          },
+        )
+      end
+    end
  end

  context "when github_onebox_access_token is configured" do
--- a/spec/lib/onebox/engine/github_repo_onebox_spec.rb
+++ b/spec/lib/onebox/engine/github_repo_onebox_spec.rb
@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+RSpec.describe Onebox::Engine::GithubRepoOnebox do
+  let(:gh_link) { "https://github.com/discourse/discourse" }
+  let(:api_uri) { "https://api.github.com/repos/discourse/discourse" }
+  let(:response) { onebox_response(described_class.onebox_name) }
+
+  before { stub_request(:get, api_uri).to_return(status: 200, body: response) }
+
+  include_context "with engines" do
+    let(:link) { gh_link }
+  end
+  it_behaves_like "an engine"
+
+  describe "#to_html" do
+    it "includes the description of the repo" do
+      expect(html).to include("A platform for community discussion. Free, open, simple.")
+    end
+
+    it "includes the name of the repo and truncated description for the title" do
+      expect(html).to include(
+        "GitHub - discourse/discourse: A platform for community discussion. Free, open,...",
+      )
+    end
+
+    it "includes a thumbnail url" do
+      SecureRandom.stubs(:hex).returns("1234")
+      expect(html).to include("https://opengraph.githubassets.com/1234/discourse/discourse")
+    end
+
+    it "sets the data-github-private-repo attr to false" do
+      expect(html).to include("data-github-private-repo=\"false\"")
+    end
+
+    context "when the PR is in a private repo" do
+      let(:response) do
+        resp = MultiJson.load(onebox_response(described_class.onebox_name))
+        resp["private"] = true
+        MultiJson.dump(resp)
+      end
+
+      it "sets the data-github-private-repo attr to true" do
+        expect(html).to include("data-github-private-repo=\"true\"")
+      end
+    end
+
+    context "when the repo has no description" do
+      let(:response) do
+        resp = onebox_response(described_class.onebox_name)
+        resp["description"] = ""
+        resp
+      end
+
+      it "includes a message about contributing to the repo" do
+        expect(html).to include(I18n.t("onebox.github.no_description", repo: "discourse/discourse"))
+      end
+    end
+  end
+
+  context "when github_onebox_access_token is configured" do
+    before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
+
+    it "sends it as part of the request" do
+      html
+      expect(WebMock).to have_requested(:get, api_uri).with(
+        headers: {
+          "Authorization" => "Bearer github_pat_1234",
+        },
+      )
+    end
+  end
+end