From 742791485a7282978f31d38df92ccf54fcb34403 Mon Sep 17 00:00:00 2001 From: Erik Schierboom Date: Fri, 9 Jul 2021 10:25:56 +0200 Subject: [PATCH] Update org-wide-files repository dispatch --- .../dispatch_event_to_org_wide_files_repo.rb | 24 ++++++++++ ...notify_track_syncer_about_track_changes.rb | 21 -------- app/commands/webhooks/process_push_update.rb | 10 ++-- .../webhooks/push_updates_controller.rb | 2 +- ...patch_event_to_org_wide_files_repo_test.rb | 31 ++++++++++++ ...y_track_syncer_about_track_changes_test.rb | 16 ------- .../webhooks/process_push_update_test.rb | 48 +++++++------------ .../webhooks/push_updates_controller_test.rb | 10 ++-- 8 files changed, 84 insertions(+), 78 deletions(-) create mode 100644 app/commands/github/dispatch_event_to_org_wide_files_repo.rb delete mode 100644 app/commands/github/notify_track_syncer_about_track_changes.rb create mode 100644 test/commands/github/dispatch_event_to_org_wide_files_repo_test.rb delete mode 100644 test/commands/github/notify_track_syncer_about_track_changes_test.rb diff --git a/app/commands/github/dispatch_event_to_org_wide_files_repo.rb b/app/commands/github/dispatch_event_to_org_wide_files_repo.rb new file mode 100644 index 0000000000..c78eb811b1 --- /dev/null +++ b/app/commands/github/dispatch_event_to_org_wide_files_repo.rb @@ -0,0 +1,24 @@ +module Github + class DispatchEventToOrgWideFilesRepo + include Mandate + + initialize_with :event_type, :repos + + def call + raise "Unsupported event type" unless EVENT_TYPES.include?(event_type) + + Exercism.octokit_client.post("https://api.github.com/repos/exercism/org-wide-files/dispatches", body) + end + + private + def body + { + event_type: event_type.to_s, + client_payload: { repos: repos } + }.to_json + end + + EVENT_TYPES = %i[appends_update].freeze + private_constant :EVENT_TYPES + end +end diff --git a/app/commands/github/notify_track_syncer_about_track_changes.rb b/app/commands/github/notify_track_syncer_about_track_changes.rb deleted file mode 100644 index cd1d6fbd88..0000000000 --- a/app/commands/github/notify_track_syncer_about_track_changes.rb +++ /dev/null @@ -1,21 +0,0 @@ -module Github - class NotifyTrackSyncerAboutTrackChanges - include Mandate - - initialize_with :track - - def call - Exercism.octokit_client.post("https://api.github.com/repos/exercism/org-wide-files/dispatches", body) - end - - private - def body - { - event_type: 'track_changes', - client_payload: { - tracks: ["exercism/#{track.slug}"] - } - }.to_json - end - end -end diff --git a/app/commands/webhooks/process_push_update.rb b/app/commands/webhooks/process_push_update.rb index f822714f9e..5946f65052 100644 --- a/app/commands/webhooks/process_push_update.rb +++ b/app/commands/webhooks/process_push_update.rb @@ -2,7 +2,7 @@ module Webhooks class ProcessPushUpdate include Mandate - initialize_with :ref, :repo_name, :commits + initialize_with :ref, :repo_owner, :repo_name, :commits def call return unless pushed_to_main? @@ -15,7 +15,7 @@ def call else track = Track.find_by(slug: repo_name) SyncTrackJob.perform_later(track) if track - Github::NotifyTrackSyncerAboutTrackChanges.(track) if org_wide_file_changed?(track) + Github::DispatchEventToOrgWideFilesRepo.(:appends_update, [repo]) if org_wide_file_changed? end end @@ -24,9 +24,11 @@ def pushed_to_main? ref == "refs/heads/#{Git::Repository::MAIN_BRANCH_REF}" end - def org_wide_file_changed?(track) - return false unless track + def repo + "#{repo_owner}/#{repo_name}" + end + def org_wide_file_changed? commits.to_a.any? do |commit| Set.new([*commit[:added], *commit[:removed], *commit[:modified]]).any? do |file| file.starts_with?('.appends/') diff --git a/app/controllers/webhooks/push_updates_controller.rb b/app/controllers/webhooks/push_updates_controller.rb index 56d4e91293..903f076d2e 100644 --- a/app/controllers/webhooks/push_updates_controller.rb +++ b/app/controllers/webhooks/push_updates_controller.rb @@ -4,7 +4,7 @@ module Webhooks class PushUpdatesController < BaseController def create - ::Webhooks::ProcessPushUpdate.(params[:ref], params[:repository][:name], params[:commits]) + ::Webhooks::ProcessPushUpdate.(params[:ref], params[:repository][:owner][:login], params[:repository][:name], params[:commits]) head :no_content end diff --git a/test/commands/github/dispatch_event_to_org_wide_files_repo_test.rb b/test/commands/github/dispatch_event_to_org_wide_files_repo_test.rb new file mode 100644 index 0000000000..54775c08b2 --- /dev/null +++ b/test/commands/github/dispatch_event_to_org_wide_files_repo_test.rb @@ -0,0 +1,31 @@ +require "test_helper" + +class Github::DispatchEventToOrgWideFilesRepoTest < ActiveJob::TestCase + test "dispatch repository event for single repo" do + stub_request(:post, "https://api.github.com/repos/exercism/org-wide-files/dispatches"). + with(body: '{"event_type":"appends_update","client_payload":{"repos":["exercism/ruby"]}}') + + Github::DispatchEventToOrgWideFilesRepo.(:appends_update, ["exercism/ruby"]) + + assert_requested(:post, "https://api.github.com/repos/exercism/org-wide-files/dispatches", times: 1) do |req| + req.body == '{"event_type":"appends_update","client_payload":{"repos":["exercism/ruby"]}}' + end + end + + test "dispatch repository event for multiples repos" do + stub_request(:post, "https://api.github.com/repos/exercism/org-wide-files/dispatches"). + with(body: '{"event_type":"appends_update","client_payload":{"repos":["exercism/website","exercism/configlet"]}}') + + Github::DispatchEventToOrgWideFilesRepo.(:appends_update, ["exercism/website", "exercism/configlet"]) + + assert_requested(:post, "https://api.github.com/repos/exercism/org-wide-files/dispatches", times: 1) do |req| + req.body == '{"event_type":"appends_update","client_payload":{"repos":["exercism/website","exercism/configlet"]}}' + end + end + + test "raises for unknown event type" do + assert_raises do + Github::DispatchEventToOrgWideFilesRepo.(:unknown, ["exercism/website"]) + end + end +end diff --git a/test/commands/github/notify_track_syncer_about_track_changes_test.rb b/test/commands/github/notify_track_syncer_about_track_changes_test.rb deleted file mode 100644 index a9fcfd5d0e..0000000000 --- a/test/commands/github/notify_track_syncer_about_track_changes_test.rb +++ /dev/null @@ -1,16 +0,0 @@ -require "test_helper" - -class Github::NotifyTrackSyncerAboutTrackChangesTest < ActiveJob::TestCase - test "dispatch repository event for track" do - track = create :track, slug: 'ruby' - - stub_request(:post, "https://api.github.com/repos/exercism/org-wide-files/dispatches"). - with(body: '{"event_type":"track_changes","client_payload":{"tracks":["exercism/ruby"]}}') - - Github::NotifyTrackSyncerAboutTrackChanges.(track) - - assert_requested(:post, "https://api.github.com/repos/exercism/org-wide-files/dispatches", times: 1) do |req| - req.body == '{"event_type":"track_changes","client_payload":{"tracks":["exercism/ruby"]}}' - end - end -end diff --git a/test/commands/webhooks/process_push_update_test.rb b/test/commands/webhooks/process_push_update_test.rb index a6af30c094..7580cdc5ee 100644 --- a/test/commands/webhooks/process_push_update_test.rb +++ b/test/commands/webhooks/process_push_update_test.rb @@ -5,19 +5,19 @@ class Webhooks::ProcessPushUpdateTest < ActiveSupport::TestCase create :track, slug: 'ruby' assert_enqueued_jobs 1, only: SyncTrackJob do - Webhooks::ProcessPushUpdate.('refs/heads/main', 'ruby', []) + Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'ruby', []) end end test "should enqueue sync push job when pushing website-copy" do assert_enqueued_jobs 1, only: UpdateWebsiteCopyJob do - Webhooks::ProcessPushUpdate.('refs/heads/main', 'website-copy', []) + Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'website-copy', []) end end test "should enqueue sync push job when pushing docs" do assert_enqueued_jobs 1, only: SyncDocsJob do - Webhooks::ProcessPushUpdate.('refs/heads/main', 'docs', []) + Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'docs', []) end end @@ -25,7 +25,7 @@ class Webhooks::ProcessPushUpdateTest < ActiveSupport::TestCase create :track, slug: :ruby assert_enqueued_jobs 0, only: SyncTrackJob do - Webhooks::ProcessPushUpdate.('refs/heads/develop', 'ruby', []) + Webhooks::ProcessPushUpdate.('refs/heads/develop', 'exercism', 'ruby', []) end end @@ -33,53 +33,39 @@ class Webhooks::ProcessPushUpdateTest < ActiveSupport::TestCase create :track, slug: :ruby assert_enqueued_jobs 0, only: SyncTrackJob do - Webhooks::ProcessPushUpdate.('refs/heads/main', 'problem-specs', []) + Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'problem-specs', []) end end - test "should notify track syncer when commit contains added file in .appends directory" do - track = create :track, slug: 'ruby' + test "should dispatch org-wide-files event when commit contains added file in .appends directory" do + Github::DispatchEventToOrgWideFilesRepo.expects(:call).with(:appends_update, ['exercism/ruby']) - Github::NotifyTrackSyncerAboutTrackChanges.expects(:call).with(track) - - Webhooks::ProcessPushUpdate.('refs/heads/main', 'ruby', [ + Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'ruby', [ { added: ['.appends/labels.yml'], removed: [], modified: [] } ]) end - test "should notify track syncer when commit contains removed file from .appends directory" do - track = create :track, slug: 'ruby' - - Github::NotifyTrackSyncerAboutTrackChanges.expects(:call).with(track) + test "should dispatch org-wide-files event when commit contains removed file from .appends directory" do + Github::DispatchEventToOrgWideFilesRepo.expects(:call).with(:appends_update, ['exercism/website']) - Webhooks::ProcessPushUpdate.('refs/heads/main', 'ruby', [ + Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'website', [ { added: [], removed: ['.appends/issues.json'], modified: [] } ]) end - test "should notify track syncer when commit contains modified file in .appends directory" do - track = create :track, slug: 'ruby' + test "should dispatch org-wide-files event when commit contains modified file in .appends directory" do + Github::DispatchEventToOrgWideFilesRepo.expects(:call).with(:appends_update, ['exercism/configlet']) - Github::NotifyTrackSyncerAboutTrackChanges.expects(:call).with(track) - - Webhooks::ProcessPushUpdate.('refs/heads/main', 'ruby', [ + Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'configlet', [ { added: [], removed: [], modified: ['.appends/LICENSE.md'] } ]) end - test "should not notify track syncer when commit does not contain modified file in .appends directory" do - create :track, slug: :ruby - - Github::NotifyTrackSyncerAboutTrackChanges.expects(:call).never + test "should not dispatch org-wide-files event when commit does not contain modified file in .appends directory" do + Github::DispatchEventToOrgWideFilesRepo.expects(:call).never - Webhooks::ProcessPushUpdate.('refs/heads/main', 'ruby', [ + Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'ruby', [ { added: ['README.md'], removed: ['GENERATORS.md'], modified: ['CONTRIBUTING.md'] } ]) end - - test "should not notify track syncer when pushing to non-track repo" do - Github::NotifyTrackSyncerAboutTrackChanges.expects(:call).never - - Webhooks::ProcessPushUpdate.('refs/heads/main', 'problem-specs', []) - end end diff --git a/test/controllers/webhooks/push_updates_controller_test.rb b/test/controllers/webhooks/push_updates_controller_test.rb index bf290f3572..323807f410 100644 --- a/test/controllers/webhooks/push_updates_controller_test.rb +++ b/test/controllers/webhooks/push_updates_controller_test.rb @@ -4,7 +4,7 @@ class Webhooks::PushUpdatesControllerTest < Webhooks::BaseTestCase test "create should return 403 when signature is invalid" do payload = { ref: 'refs/heads/main', - repository: { name: 'csharp' }, + repository: { name: 'csharp', owner: { login: 'exercism' } }, commits: [{ added: [], removed: [], modified: ['README.md'] }] } @@ -18,7 +18,7 @@ class Webhooks::PushUpdatesControllerTest < Webhooks::BaseTestCase test "create should return 200 when signature is valid" do payload = { ref: 'refs/heads/main', - repository: { name: 'csharp' }, + repository: { name: 'csharp', owner: { login: 'exercism' } }, commits: [{ added: [], removed: [], modified: ['README.md'] }] } @@ -29,11 +29,11 @@ class Webhooks::PushUpdatesControllerTest < Webhooks::BaseTestCase test "create should process repo update when signature is valid" do payload = { ref: 'refs/heads/main', - repository: { name: 'csharp' }, + repository: { name: 'csharp', owner: { login: 'exercism' } }, commits: [{ added: [], removed: [], modified: ['README.md'] }] } Webhooks::ProcessPushUpdate.expects(:call).with( - 'refs/heads/main', 'csharp', [ + 'refs/heads/main', 'exercism', 'csharp', [ ActionController::Parameters.new(added: [], removed: [], modified: ['README.md']) ] ) @@ -44,7 +44,7 @@ class Webhooks::PushUpdatesControllerTest < Webhooks::BaseTestCase test "create should return 204 when ping event is sent" do payload = { ref: 'refs/heads/main', - repository: { name: 'csharp' }, + repository: { name: 'csharp', owner: { login: 'exercism' } }, commits: [{ added: [], removed: [], modified: ['README.md'] }] }