diff --git a/lib/onebox/engine/github_pull_request_onebox.rb b/lib/onebox/engine/github_pull_request_onebox.rb index 4588d8ea4250b..b70263b9a3677 100644 --- a/lib/onebox/engine/github_pull_request_onebox.rb +++ b/lib/onebox/engine/github_pull_request_onebox.rb @@ -33,6 +33,7 @@ def match def data result = raw(github_auth_header(match[:org])).clone result["link"] = link + result["pr_status"] = fetch_pr_status(result) created_at = Time.parse(result["created_at"]) result["created_at"] = created_at.strftime("%I:%M%p - %d %b %y %Z") @@ -111,6 +112,40 @@ def load_json(url) ::MultiJson.load( URI.parse(url).open({ read_timeout: timeout }.merge(github_auth_header(match[:org]))), ) + rescue OpenURI::HTTPError => e + Rails.logger.warn("GitHub API error: #{e.io.status[0]} fetching #{url}") + raise + end + + def fetch_pr_status(pr_data) + return unless SiteSetting.github_pr_status_enabled + + return "merged" if pr_data["merged"] + return "closed" if pr_data["state"] == "closed" + return "draft" if pr_data["draft"] + + reviews_data = load_json(url + "/reviews") + return "approved" if reviews_approved?(reviews_data) + + "open" + rescue StandardError => e + Rails.logger.warn("GitHub PR status fetch error: #{e.message}") + nil + end + + def reviews_approved?(reviews) + return false if reviews.blank? + + states = + reviews + .reject { |r| r.dig("user", "id").nil? || %w[PENDING COMMENTED].include?(r["state"]) } + .group_by { |r| r.dig("user", "id") } + .transform_values { |rs| rs.max_by { |r| r["submitted_at"] }["state"] } + .values + + return false if states.empty? + + states.all? { |s| %w[APPROVED DISMISSED].include?(s) } && states.include?("APPROVED") end end end diff --git a/lib/onebox/mixins/github_auth_header.rb b/lib/onebox/mixins/github_auth_header.rb index c96872f20e47c..5cdac6ca828ea 100644 --- a/lib/onebox/mixins/github_auth_header.rb +++ b/lib/onebox/mixins/github_auth_header.rb @@ -5,13 +5,11 @@ module Mixins module GithubAuthHeader def github_auth_header(github_org) return {} if SiteSetting.github_onebox_access_tokens.blank? - org_tokens = - SiteSetting.github_onebox_access_tokens.split("\n").map { |line| line.split("|") }.to_h - # Use the default token if no token is found for the org, - # this will be the token that used to be stored in the old - # github_onebox_access_token site setting if it was configured. + org_tokens = SiteSetting.github_onebox_access_tokens.split("\n").to_h { _1.split("|") } + token = org_tokens[github_org] || org_tokens["default"] + return {} if token.blank? { "Authorization" => "Bearer #{token}" } diff --git a/lib/onebox/templates/githubpullrequest.mustache b/lib/onebox/templates/githubpullrequest.mustache index 9bb3299bbcbd9..9383d67ed64ee 100644 --- a/lib/onebox/templates/githubpullrequest.mustache +++ b/lib/onebox/templates/githubpullrequest.mustache @@ -1,4 +1,4 @@ -
+
{{#commit}}
diff --git a/plugins/chat/app/jobs/regular/chat/process_message.rb b/plugins/chat/app/jobs/regular/chat/process_message.rb index 697c4e352d0f8..683db70535e6f 100644 --- a/plugins/chat/app/jobs/regular/chat/process_message.rb +++ b/plugins/chat/app/jobs/regular/chat/process_message.rb @@ -31,6 +31,9 @@ def execute(args = {}) # we dont process mentions when creating/updating message so we always have to do it chat_message.upsert_mentions + # extract external links for webhook-based rebaking + ::Chat::MessageLink.extract_from(chat_message) + # notifier should be idempotent and not re-notify if args[:edit_timestamp] ::Chat::Notifier.new(chat_message, args[:edit_timestamp]).notify_edit diff --git a/plugins/chat/app/models/chat/message_link.rb b/plugins/chat/app/models/chat/message_link.rb new file mode 100644 index 0000000000000..6cec45885f625 --- /dev/null +++ b/plugins/chat/app/models/chat/message_link.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module Chat + class MessageLink < ActiveRecord::Base + self.table_name = "chat_message_links" + + belongs_to :chat_message, class_name: "Chat::Message" + + validates :url, presence: true, length: { maximum: 500 } + + def self.extract_from(message) + return if message.blank? || message.deleted_at.present? + + current_urls = [] + + PrettyText + .extract_links(message.cooked) + .map { |link| UrlHelper.relaxed_parse(link.url) } + .compact + .reject { |uri| uri.scheme == "mailto" } + .uniq + .each do |parsed| + url = parsed.to_s[0...500] + next if parsed.host.blank? || parsed.host.length > 100 + + current_urls << url + upsert_link(message, url) + end + + cleanup_entries(message, current_urls) + end + + def self.upsert_link(message, url) + sql = <<~SQL + INSERT INTO chat_message_links (chat_message_id, url, created_at, updated_at) + VALUES (:chat_message_id, :url, :now, :now) + ON CONFLICT (chat_message_id, url) DO NOTHING + SQL + + DB.exec(sql, chat_message_id: message.id, url: url, now: Time.current) + end + + def self.cleanup_entries(message, current_urls) + if current_urls.present? + where( + "chat_message_id = :id AND url NOT IN (:urls)", + id: message.id, + urls: current_urls, + ).delete_all + else + where(chat_message_id: message.id).delete_all + end + end + end +end + +# == Schema Information +# +# Table name: chat_message_links +# +# id :bigint not null, primary key +# url :string(500) not null +# created_at :datetime not null +# updated_at :datetime not null +# chat_message_id :bigint not null +# +# Indexes +# +# index_chat_message_links_on_chat_message_id (chat_message_id) +# index_chat_message_links_on_url (url) +# unique_chat_message_links (chat_message_id,url) UNIQUE +# diff --git a/plugins/chat/db/migrate/20251203000001_create_chat_message_links.rb b/plugins/chat/db/migrate/20251203000001_create_chat_message_links.rb new file mode 100644 index 0000000000000..82158158100d7 --- /dev/null +++ b/plugins/chat/db/migrate/20251203000001_create_chat_message_links.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CreateChatMessageLinks < ActiveRecord::Migration[7.2] + def change + create_table :chat_message_links do |t| + t.bigint :chat_message_id, null: false + t.string :url, null: false, limit: 500 + t.timestamps + end + + add_index :chat_message_links, :chat_message_id + add_index :chat_message_links, :url + add_index :chat_message_links, + %i[chat_message_id url], + unique: true, + name: "unique_chat_message_links" + end +end diff --git a/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb b/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb index d83491e845ca6..392d2fd9a5f53 100644 --- a/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb +++ b/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb @@ -31,4 +31,12 @@ chat_message.destroy expect { described_class.new.execute(chat_message_id: chat_message.id) }.not_to raise_exception end + + it "extracts links from the message" do + described_class.new.execute(chat_message_id: chat_message.id) + + link = Chat::MessageLink.find_by(chat_message_id: chat_message.id) + expect(link).to be_present + expect(link.url).to eq("https://discourse.org/team") + end end diff --git a/plugins/chat/spec/models/chat/message_link_spec.rb b/plugins/chat/spec/models/chat/message_link_spec.rb new file mode 100644 index 0000000000000..0f6985d59f094 --- /dev/null +++ b/plugins/chat/spec/models/chat/message_link_spec.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +RSpec.describe Chat::MessageLink do + fab!(:user) + fab!(:channel, :chat_channel) + + describe ".extract_from" do + it "extracts external links from a chat message" do + message = + Fabricate( + :chat_message, + chat_channel: channel, + user: user, + message: "Check out https://github.com/discourse/discourse/pull/123", + ) + message.update!(cooked: PrettyText.cook(message.message)) + + Chat::MessageLink.extract_from(message) + + expect(Chat::MessageLink.count).to eq(1) + link = Chat::MessageLink.first + expect(link.url).to eq("https://github.com/discourse/discourse/pull/123") + expect(link.chat_message_id).to eq(message.id) + end + + it "extracts multiple links from a message" do + message = + Fabricate( + :chat_message, + chat_channel: channel, + user: user, + message: "PRs: https://github.com/org/repo/pull/1 and https://github.com/org/repo/pull/2", + ) + message.update!(cooked: PrettyText.cook(message.message)) + + Chat::MessageLink.extract_from(message) + + expect(Chat::MessageLink.count).to eq(2) + urls = Chat::MessageLink.pluck(:url) + expect(urls).to contain_exactly( + "https://github.com/org/repo/pull/1", + "https://github.com/org/repo/pull/2", + ) + end + + it "removes links that no longer exist in the message" do + message = + Fabricate( + :chat_message, + chat_channel: channel, + user: user, + message: "Check out https://github.com/org/repo/pull/1", + ) + message.update!(cooked: PrettyText.cook(message.message)) + Chat::MessageLink.extract_from(message) + + expect(Chat::MessageLink.count).to eq(1) + + message.update!( + message: "Check out https://github.com/org/repo/pull/2", + cooked: PrettyText.cook("Check out https://github.com/org/repo/pull/2"), + ) + Chat::MessageLink.extract_from(message) + + expect(Chat::MessageLink.count).to eq(1) + expect(Chat::MessageLink.first.url).to eq("https://github.com/org/repo/pull/2") + end + + it "does not extract mailto links" do + message = + Fabricate( + :chat_message, + chat_channel: channel, + user: user, + message: "Email me at mailto:test@example.com", + ) + message.update!(cooked: PrettyText.cook(message.message)) + + Chat::MessageLink.extract_from(message) + + expect(Chat::MessageLink.count).to eq(0) + end + + it "does nothing for deleted messages" do + message = + Fabricate( + :chat_message, + chat_channel: channel, + user: user, + message: "https://github.com/org/repo/pull/1", + deleted_at: Time.current, + ) + message.update!(cooked: PrettyText.cook(message.message)) + + Chat::MessageLink.extract_from(message) + + expect(Chat::MessageLink.count).to eq(0) + end + + it "is idempotent" do + message = + Fabricate( + :chat_message, + chat_channel: channel, + user: user, + message: "https://github.com/org/repo/pull/1", + ) + message.update!(cooked: PrettyText.cook(message.message)) + + 3.times { Chat::MessageLink.extract_from(message) } + + expect(Chat::MessageLink.count).to eq(1) + end + end +end diff --git a/plugins/discourse-github/app/controllers/discourse_github/webhooks_controller.rb b/plugins/discourse-github/app/controllers/discourse_github/webhooks_controller.rb new file mode 100644 index 0000000000000..e36c69f5dce82 --- /dev/null +++ b/plugins/discourse-github/app/controllers/discourse_github/webhooks_controller.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module DiscourseGithub + class WebhooksController < ::ApplicationController + requires_plugin "discourse-github" + + skip_before_action :check_xhr + skip_before_action :verify_authenticity_token + skip_before_action :redirect_to_login_if_required + + def github + return head :forbidden unless verify_signature + + event = request.headers["X-GitHub-Event"] + return head :ok if %w[pull_request pull_request_review].exclude?(event) + + pr_url = params.dig(:pull_request, :html_url) + return head :ok if pr_url.blank? + + Jobs.enqueue(:rebake_github_pr_posts, pr_url: pr_url) + + head :ok + end + + private + + def verify_signature + secret = SiteSetting.github_webhook_secret + return false if secret.blank? + + signature = request.headers["X-Hub-Signature-256"] + return false if signature.blank? + + request.body.rewind + body = request.body.read + expected = "sha256=" + OpenSSL::HMAC.hexdigest("SHA256", secret, body) + + ActiveSupport::SecurityUtils.secure_compare(expected, signature) + end + end +end diff --git a/plugins/discourse-github/app/jobs/regular/rebake_github_pr_posts.rb b/plugins/discourse-github/app/jobs/regular/rebake_github_pr_posts.rb new file mode 100644 index 0000000000000..6214f14ec43f4 --- /dev/null +++ b/plugins/discourse-github/app/jobs/regular/rebake_github_pr_posts.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Jobs + class RebakeGithubPrPosts < ::Jobs::Base + sidekiq_options queue: "low" + + def execute(args) + pr_url = args[:pr_url] + return if pr_url.blank? + + rebake_posts(pr_url) + rebake_chat_messages(pr_url) if defined?(Chat) + end + + private + + def rebake_posts(pr_url) + post_ids = + TopicLink + .where(url: pr_url) + .or(TopicLink.where("url LIKE ?", "#{pr_url}%")) + .select(:post_id) + + Post + .where(id: post_ids) + .find_each do |post| + next unless has_github_pr_onebox?(post.cooked, pr_url) + post.rebake!(invalidate_oneboxes: true, priority: :low) + end + end + + def rebake_chat_messages(pr_url) + return unless defined?(Chat::MessageLink) + + message_ids = + Chat::MessageLink + .where(url: pr_url) + .or(Chat::MessageLink.where("url LIKE ?", "#{pr_url}%")) + .select(:chat_message_id) + + Chat::Message + .where(id: message_ids) + .find_each do |message| + next unless has_github_pr_onebox?(message.cooked, pr_url) + message.rebake!(invalidate_oneboxes: true, priority: :low) + end + end + + def has_github_pr_onebox?(cooked, pr_url) + # quick & dirty check to avoid doing unnecessary rebakes + cooked.present? && cooked.include?("onebox") && cooked.include?(pr_url) + end + end +end diff --git a/plugins/discourse-github/assets/stylesheets/common/github-pr-status.scss b/plugins/discourse-github/assets/stylesheets/common/github-pr-status.scss new file mode 100644 index 0000000000000..0f1e9fc43623b --- /dev/null +++ b/plugins/discourse-github/assets/stylesheets/common/github-pr-status.scss @@ -0,0 +1,68 @@ +.onebox.githubpullrequest .github-row { + // Draft PR icon (dashed outline) + --gh-icon-draft: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16'%3E%3Cpath d='M3.25 1A2.25 2.25 0 0 1 4 5.372v5.256a2.251 2.251 0 1 1-1.5 0V5.372A2.251 2.251 0 0 1 3.25 1Zm9.5 14a2.25 2.25 0 1 1 0-4.5 2.25 2.25 0 0 1 0 4.5ZM2.5 3.25a.75.75 0 1 0 1.5 0 .75.75 0 0 0-1.5 0ZM3.25 12a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5Zm9.5 0a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5ZM14 4.25a.75.75 0 0 1-.75.75h-2a.75.75 0 0 1 0-1.5h2a.75.75 0 0 1 .75.75Zm-.75 3.75a.75.75 0 0 0 0-1.5h-2a.75.75 0 0 0 0 1.5Z'/%3E%3C/svg%3E"); + + // Open PR icon + --gh-icon-open: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16'%3E%3Cpath d='M1.5 3.25a2.25 2.25 0 1 1 3 2.122v5.256a2.251 2.251 0 1 1-1.5 0V5.372A2.25 2.25 0 0 1 1.5 3.25Zm5.677-.177L9.573.677A.25.25 0 0 1 10 .854V2.5h1A2.5 2.5 0 0 1 13.5 5v5.628a2.251 2.251 0 1 1-1.5 0V5a1 1 0 0 0-1-1h-1v1.646a.25.25 0 0 1-.427.177L7.177 3.427a.25.25 0 0 1 0-.354ZM3.75 2.5a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5Zm0 9.5a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5Zm8.25.75a.75.75 0 1 0 1.5 0 .75.75 0 0 0-1.5 0Z'/%3E%3C/svg%3E"); + + // Merged PR icon + --gh-icon-merged: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16'%3E%3Cpath d='M5.45 5.154A4.25 4.25 0 0 0 9.25 7.5h1.378a2.251 2.251 0 1 1 0 1.5H9.25A5.734 5.734 0 0 1 5 7.123v3.505a2.25 2.25 0 1 1-1.5 0V5.372a2.25 2.25 0 1 1 1.95-.218ZM4.25 13.5a.75.75 0 1 0 0-1.5.75.75 0 0 0 0 1.5Zm8.5-4.5a.75.75 0 1 0 0-1.5.75.75 0 0 0 0 1.5ZM5 3.25a.75.75 0 1 0 0 .005V3.25Z'/%3E%3C/svg%3E"); + + // Closed PR icon + --gh-icon-closed: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16'%3E%3Cpath d='M3.25 1A2.25 2.25 0 0 1 4 5.372v5.256a2.251 2.251 0 1 1-1.5 0V5.372A2.251 2.251 0 0 1 3.25 1Zm9.5 5.5a.75.75 0 0 1 .75.75v3.378a2.251 2.251 0 1 1-1.5 0V7.25a.75.75 0 0 1 .75-.75Zm-2.03-5.273a.75.75 0 0 1 1.06 0l.97.97.97-.97a.748.748 0 0 1 1.265.332.75.75 0 0 1-.205.729l-.97.97.97.97a.751.751 0 0 1-.018 1.042.751.751 0 0 1-1.042.018l-.97-.97-.97.97a.749.749 0 0 1-1.275-.326.749.749 0 0 1 .215-.734l.97-.97-.97-.97a.75.75 0 0 1 0-1.06ZM2.5 3.25a.75.75 0 1 0 1.5 0 .75.75 0 0 0-1.5 0ZM3.25 12a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5Zm9.5 0a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5Z'/%3E%3C/svg%3E"); + + // Approved check-circle icon + --gh-icon-approved: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16'%3E%3Cpath d='M8 16A8 8 0 1 1 8 0a8 8 0 0 1 0 16Zm3.78-9.72a.751.751 0 0 0-.018-1.042.751.751 0 0 0-1.042-.018L6.75 9.19 5.28 7.72a.751.751 0 0 0-1.042.018.751.751 0 0 0-.018 1.042l2 2a.75.75 0 0 0 1.06 0Z'/%3E%3C/svg%3E"); + + &.--gh-status-draft, + &.--gh-status-open, + &.--gh-status-approved, + &.--gh-status-merged, + &.--gh-status-closed { + .github-icon-container { + display: block; + grid-area: icon; + align-self: center; + justify-self: center; + + .github-icon { + display: none; + } + + &::before { + content: ""; + display: block; + width: 2.5em; + aspect-ratio: 12 / 16; + mask-size: 100% 100%; + mask-repeat: no-repeat; + mask-position: center; + } + } + } + + &.--gh-status-draft .github-icon-container::before { + mask-image: var(--gh-icon-draft); + background-color: #8b949e; + } + + &.--gh-status-open .github-icon-container::before { + mask-image: var(--gh-icon-open); + background-color: #3fb950; + } + + &.--gh-status-approved .github-icon-container::before { + mask-image: var(--gh-icon-approved); + background-color: #3fb950; + } + + &.--gh-status-merged .github-icon-container::before { + mask-image: var(--gh-icon-merged); + background-color: #a371f7; + } + + &.--gh-status-closed .github-icon-container::before { + mask-image: var(--gh-icon-closed); + background-color: #f85149; + } +} diff --git a/plugins/discourse-github/config/locales/client.en.yml b/plugins/discourse-github/config/locales/client.en.yml index c3815f45d865d..f8b732d4bc8d1 100644 --- a/plugins/discourse-github/config/locales/client.en.yml +++ b/plugins/discourse-github/config/locales/client.en.yml @@ -1,4 +1,12 @@ en: + js: + github: + pr_status: + draft: "Draft" + open: "Open" + approved: "Approved" + merged: "Merged" + closed: "Closed" admin_js: admin: site_settings: diff --git a/plugins/discourse-github/config/locales/server.en.yml b/plugins/discourse-github/config/locales/server.en.yml index cfe12d2607d20..d397d3e582fd5 100644 --- a/plugins/discourse-github/config/locales/server.en.yml +++ b/plugins/discourse-github/config/locales/server.en.yml @@ -6,6 +6,8 @@ en: github_linkback_access_token: 'A valid access token for the user who will post the linkback and for counting commits/contributions to grant badges. See here for instructions on how to get a token.' github_linkback_maximum_links: "Maximum number of linkbacks to create from one single post. When a post contains more links, none is created." github_permalinks_enabled: "Enable GitHub permalink overwrites" + github_pr_status_enabled: "Show GitHub pull request status on oneboxes (draft, open, approved, merged, closed)" + github_webhook_secret: "Secret token for GitHub webhook signature verification. Configure this value in your GitHub webhook settings." github_permalinks_exclude: "Filename or directory that should be excluded from permalink overwrites. Provide only filename or full path: user/repository/optional-directory/filename" github_badges_enabled: "Enable GitHub badges" github_badges_repos: "URLs of the GitHub repos to scan for contributions and commits" diff --git a/plugins/discourse-github/config/settings.yml b/plugins/discourse-github/config/settings.yml index d3dbee64c4011..c9d9993bf61e8 100644 --- a/plugins/discourse-github/config/settings.yml +++ b/plugins/discourse-github/config/settings.yml @@ -21,6 +21,11 @@ discourse_github: validator: "GithubBadgesRepoSettingValidator" github_permalinks_enabled: default: false + github_pr_status_enabled: + default: false + github_webhook_secret: + default: "" + secret: true github_permalinks_exclude: default: "README.md" type: list diff --git a/plugins/discourse-github/plugin.rb b/plugins/discourse-github/plugin.rb index 2b9b94d14fa30..541e499240e93 100644 --- a/plugins/discourse-github/plugin.rb +++ b/plugins/discourse-github/plugin.rb @@ -16,7 +16,16 @@ enabled_site_setting :enable_discourse_github_plugin +register_asset "stylesheets/common/github-pr-status.scss" + after_initialize do + require_relative "app/controllers/discourse_github/webhooks_controller" + require_relative "app/jobs/regular/rebake_github_pr_posts" + + Discourse::Application.routes.append do + post "/discourse-github/webhooks/github" => "discourse_github/webhooks#github" + end + %w[ ../app/models/github_commit.rb ../app/models/github_repo.rb diff --git a/plugins/discourse-github/spec/jobs/rebake_github_pr_posts_spec.rb b/plugins/discourse-github/spec/jobs/rebake_github_pr_posts_spec.rb new file mode 100644 index 0000000000000..fd0fa444f9d88 --- /dev/null +++ b/plugins/discourse-github/spec/jobs/rebake_github_pr_posts_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +RSpec.describe Jobs::RebakeGithubPrPosts do + fab!(:user) + fab!(:topic) + let(:pr_url) { "https://github.com/discourse/discourse/pull/123" } + + before { enable_current_plugin } + + describe "#execute" do + it "does nothing with blank pr_url" do + expect { described_class.new.execute(pr_url: nil) }.not_to raise_error + expect { described_class.new.execute(pr_url: "") }.not_to raise_error + end + + context "with oneboxed PR link" do + fab!(:post) { Fabricate(:post, topic: topic, user: user) } + + before do + post.update!(cooked: <<~HTML) +

Check out this PR:

+ + HTML + + TopicLink.create!( + topic_id: topic.id, + post_id: post.id, + user_id: user.id, + url: pr_url, + domain: "github.com", + ) + end + + it "rebakes posts with oneboxed PR links" do + expect_any_instance_of(Post).to receive(:rebake!).with( + invalidate_oneboxes: true, + priority: :low, + ) + described_class.new.execute(pr_url: pr_url) + end + + it "matches URLs with path suffixes" do + TopicLink.create!( + topic_id: topic.id, + post_id: post.id, + user_id: user.id, + url: "#{pr_url}/files", + domain: "github.com", + ) + + post_ids = + TopicLink + .where(url: pr_url) + .or(TopicLink.where("url LIKE ?", "#{pr_url}%")) + .pluck(:post_id) + + expect(post_ids).to include(post.id) + end + end + + context "with inline PR link (not oneboxed)" do + fab!(:post) { Fabricate(:post, topic: topic, user: user) } + + before do + post.update!(cooked: <<~HTML) +

Check out this PR for details.

+ HTML + + TopicLink.create!( + topic_id: topic.id, + post_id: post.id, + user_id: user.id, + url: pr_url, + domain: "github.com", + ) + end + + it "does not rebake posts with only inline links" do + expect_any_instance_of(Post).not_to receive(:rebake!) + described_class.new.execute(pr_url: pr_url) + end + end + + context "with no matching posts" do + it "completes without error" do + expect { described_class.new.execute(pr_url: pr_url) }.not_to raise_error + end + end + end +end diff --git a/plugins/discourse-github/spec/lib/github_pr_onebox_status_spec.rb b/plugins/discourse-github/spec/lib/github_pr_onebox_status_spec.rb new file mode 100644 index 0000000000000..e89014c9d00fa --- /dev/null +++ b/plugins/discourse-github/spec/lib/github_pr_onebox_status_spec.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +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(:reviews_api_uri) { "#{api_uri}/reviews" } + + def open_pr_response + resp = MultiJson.load(onebox_response("githubpullrequest")) + resp["state"] = "open" + resp["merged"] = false + resp["draft"] = false + resp + end + + before { enable_current_plugin } + + def onebox_html + Onebox::Engine::GithubPullRequestOnebox.new(gh_link).to_html + end + + describe "PR status" do + context "when github_pr_status_enabled is false" do + before do + SiteSetting.github_pr_status_enabled = false + stub_request(:get, api_uri).to_return(status: 200, body: MultiJson.dump(open_pr_response)) + end + + it "does not include status class" do + expect(onebox_html).not_to include("--gh-status-") + end + end + + context "when github_pr_status_enabled is true" do + before { SiteSetting.github_pr_status_enabled = true } + + context "with open PR" do + before do + stub_request(:get, api_uri).to_return(status: 200, body: MultiJson.dump(open_pr_response)) + stub_request(:get, reviews_api_uri).to_return(status: 200, body: "[]") + end + + it "includes open status" do + expect(onebox_html).to include("--gh-status-open") + end + end + + context "when PR is merged" do + before do + resp = open_pr_response + resp["merged"] = true + stub_request(:get, api_uri).to_return(status: 200, body: MultiJson.dump(resp)) + stub_request(:get, reviews_api_uri).to_return(status: 200, body: "[]") + end + + it "includes merged status" do + expect(onebox_html).to include("--gh-status-merged") + end + end + + context "when PR is closed" do + before do + resp = open_pr_response + resp["state"] = "closed" + stub_request(:get, api_uri).to_return(status: 200, body: MultiJson.dump(resp)) + stub_request(:get, reviews_api_uri).to_return(status: 200, body: "[]") + end + + it "includes closed status" do + expect(onebox_html).to include("--gh-status-closed") + end + end + + context "when PR is draft" do + before do + resp = open_pr_response + resp["draft"] = true + stub_request(:get, api_uri).to_return(status: 200, body: MultiJson.dump(resp)) + stub_request(:get, reviews_api_uri).to_return(status: 200, body: "[]") + end + + it "includes draft status" do + expect(onebox_html).to include("--gh-status-draft") + end + end + + context "when PR is approved" do + before do + stub_request(:get, api_uri).to_return(status: 200, body: MultiJson.dump(open_pr_response)) + reviews = [ + { "user" => { "id" => 1 }, "state" => "APPROVED", "submitted_at" => Time.now.iso8601 }, + ] + stub_request(:get, reviews_api_uri).to_return(status: 200, body: MultiJson.dump(reviews)) + end + + it "includes approved status" do + expect(onebox_html).to include("--gh-status-approved") + end + end + + context "when PR has changes requested" do + before do + stub_request(:get, api_uri).to_return(status: 200, body: MultiJson.dump(open_pr_response)) + reviews = [ + { + "user" => { + "id" => 1, + }, + "state" => "CHANGES_REQUESTED", + "submitted_at" => Time.now.iso8601, + }, + ] + stub_request(:get, reviews_api_uri).to_return(status: 200, body: MultiJson.dump(reviews)) + end + + it "shows as open status (not approved)" do + expect(onebox_html).to include("--gh-status-open") + expect(onebox_html).not_to include("--gh-status-approved") + end + end + + context "when reviews API fails" do + before do + stub_request(:get, api_uri).to_return(status: 200, body: MultiJson.dump(open_pr_response)) + stub_request(:get, reviews_api_uri).to_return(status: 500, body: "error") + end + + it "falls back gracefully without status" do + expect(onebox_html).not_to include("--gh-status-") + end + end + end + end +end diff --git a/plugins/discourse-github/spec/requests/webhooks_controller_spec.rb b/plugins/discourse-github/spec/requests/webhooks_controller_spec.rb new file mode 100644 index 0000000000000..f1c6434dfb784 --- /dev/null +++ b/plugins/discourse-github/spec/requests/webhooks_controller_spec.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +RSpec.describe DiscourseGithub::WebhooksController do + let(:webhook_secret) { "test_secret_123" } + let(:pr_url) { "https://github.com/discourse/discourse/pull/123" } + + let(:pull_request_payload) do + { action: "closed", pull_request: { html_url: pr_url, merged: true } } + end + + let(:pull_request_review_payload) do + { action: "submitted", review: { state: "approved" }, pull_request: { html_url: pr_url } } + end + + def sign_payload(payload, secret) + body = payload.to_json + signature = "sha256=" + OpenSSL::HMAC.hexdigest("SHA256", secret, body) + [body, signature] + end + + before do + SiteSetting.enable_discourse_github_plugin = true + SiteSetting.github_webhook_secret = webhook_secret + end + + describe "#github" do + context "when plugin is disabled" do + before { SiteSetting.enable_discourse_github_plugin = false } + + it "returns 404" do + body, signature = sign_payload(pull_request_payload, webhook_secret) + post "/discourse-github/webhooks/github", + params: body, + headers: { + "CONTENT_TYPE" => "application/json", + "X-GitHub-Event" => "pull_request", + "X-Hub-Signature-256" => signature, + } + expect(response.status).to eq(404) + end + end + + context "with invalid signature" do + it "returns 403" do + body, _signature = sign_payload(pull_request_payload, webhook_secret) + post "/discourse-github/webhooks/github", + params: body, + headers: { + "CONTENT_TYPE" => "application/json", + "X-GitHub-Event" => "pull_request", + "X-Hub-Signature-256" => "sha256=invalid", + } + expect(response.status).to eq(403) + end + end + + context "with missing signature" do + it "returns 403" do + post "/discourse-github/webhooks/github", + params: pull_request_payload.to_json, + headers: { + "CONTENT_TYPE" => "application/json", + "X-GitHub-Event" => "pull_request", + } + expect(response.status).to eq(403) + end + end + + context "with missing webhook secret setting" do + before { SiteSetting.github_webhook_secret = "" } + + it "returns 403" do + body, signature = sign_payload(pull_request_payload, webhook_secret) + post "/discourse-github/webhooks/github", + params: body, + headers: { + "CONTENT_TYPE" => "application/json", + "X-GitHub-Event" => "pull_request", + "X-Hub-Signature-256" => signature, + } + expect(response.status).to eq(403) + end + end + + context "with valid signature" do + it "enqueues rebake job for pull_request event" do + body, signature = sign_payload(pull_request_payload, webhook_secret) + + expect_enqueued_with(job: :rebake_github_pr_posts, args: { pr_url: pr_url }) do + post "/discourse-github/webhooks/github", + params: body, + headers: { + "CONTENT_TYPE" => "application/json", + "X-GitHub-Event" => "pull_request", + "X-Hub-Signature-256" => signature, + } + end + + expect(response.status).to eq(200) + end + + it "enqueues rebake job for pull_request_review event" do + body, signature = sign_payload(pull_request_review_payload, webhook_secret) + + expect_enqueued_with(job: :rebake_github_pr_posts, args: { pr_url: pr_url }) do + post "/discourse-github/webhooks/github", + params: body, + headers: { + "CONTENT_TYPE" => "application/json", + "X-GitHub-Event" => "pull_request_review", + "X-Hub-Signature-256" => signature, + } + end + + expect(response.status).to eq(200) + end + + it "ignores other event types" do + payload = { action: "created", issue: { html_url: "https://github.com/org/repo/issues/1" } } + body, signature = sign_payload(payload, webhook_secret) + + post "/discourse-github/webhooks/github", + params: body, + headers: { + "CONTENT_TYPE" => "application/json", + "X-GitHub-Event" => "issues", + "X-Hub-Signature-256" => signature, + } + + expect(response.status).to eq(200) + expect(Jobs::RebakeGithubPrPosts.jobs.size).to eq(0) + end + + it "handles missing PR URL gracefully" do + payload = { action: "closed", pull_request: {} } + body, signature = sign_payload(payload, webhook_secret) + + post "/discourse-github/webhooks/github", + params: body, + headers: { + "CONTENT_TYPE" => "application/json", + "X-GitHub-Event" => "pull_request", + "X-Hub-Signature-256" => signature, + } + + expect(response.status).to eq(200) + expect(Jobs::RebakeGithubPrPosts.jobs.size).to eq(0) + end + end + end +end