Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update org-wide-files repository dispatch #1231

Merged
merged 3 commits into from
Jul 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions app/commands/github/dispatch_event_to_org_wide_files_repo.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
module Github
class DispatchEventToOrgWideFilesRepo
include Mandate

initialize_with :event_type, :repos, :pusher_github_username

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,
pusher: pusher_github_username
}
}.to_json
end

EVENT_TYPES = %i[appends_update].freeze
private_constant :EVENT_TYPES
end
end
21 changes: 0 additions & 21 deletions app/commands/github/notify_track_syncer_about_track_changes.rb

This file was deleted.

10 changes: 6 additions & 4 deletions app/commands/webhooks/process_push_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module Webhooks
class ProcessPushUpdate
include Mandate

initialize_with :ref, :repo_name, :commits
initialize_with :ref, :repo_owner, :repo_name, :pusher_username, :commits

def call
return unless pushed_to_main?
Expand All @@ -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], pusher_username) if appends_file_changed?
end
end

Expand All @@ -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 appends_file_changed?
commits.to_a.any? do |commit|
Set.new([*commit[:added], *commit[:removed], *commit[:modified]]).any? do |file|
file.starts_with?('.appends/')
Expand Down
8 changes: 7 additions & 1 deletion app/controllers/webhooks/push_updates_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@
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[:pusher][:name],
params[:commits]
)

head :no_content
end
Expand Down
Original file line number Diff line number Diff line change
@@ -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"],"pusher":"user17"}}')

Github::DispatchEventToOrgWideFilesRepo.(:appends_update, ["exercism/ruby"], 'user17')

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"],"pusher":"user17"}}'
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"],"pusher":"user17"}}') # rubocop:disable Layout/LineLength

Github::DispatchEventToOrgWideFilesRepo.(:appends_update, ["exercism/website", "exercism/configlet"], 'user17')

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"],"pusher":"user17"}}' # rubocop:disable Layout/LineLength
end
end

test "raises for unknown event type" do
assert_raises do
Github::DispatchEventToOrgWideFilesRepo.(:unknown, ["exercism/website"], 'user17')
end
end
end

This file was deleted.

48 changes: 17 additions & 31 deletions test/commands/webhooks/process_push_update_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,81 +5,67 @@ 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', 'user17', [])
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', 'user17', [])
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', 'user17', [])
end
end

test "should not enqueue sync push job when pushing to non-main branch" do
create :track, slug: :ruby

assert_enqueued_jobs 0, only: SyncTrackJob do
Webhooks::ProcessPushUpdate.('refs/heads/develop', 'ruby', [])
Webhooks::ProcessPushUpdate.('refs/heads/develop', 'exercism', 'ruby', 'user17', [])
end
end

test "should not enqueue sync push job when pushing to non-track repo" do
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', 'user17', [])
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'], 'user17')

Github::NotifyTrackSyncerAboutTrackChanges.expects(:call).with(track)

Webhooks::ProcessPushUpdate.('refs/heads/main', 'ruby', [
Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'ruby', 'user17', [
{ 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'], 'user17')

Webhooks::ProcessPushUpdate.('refs/heads/main', 'ruby', [
Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'website', 'user17', [
{ 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'], 'user17')

Github::NotifyTrackSyncerAboutTrackChanges.expects(:call).with(track)

Webhooks::ProcessPushUpdate.('refs/heads/main', 'ruby', [
Webhooks::ProcessPushUpdate.('refs/heads/main', 'exercism', 'configlet', 'user17', [
{ 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', 'user17', [
{ 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
14 changes: 9 additions & 5 deletions test/controllers/webhooks/push_updates_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ 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' } },
pusher: { name: 'user17' },
commits: [{ added: [], removed: [], modified: ['README.md'] }]
}

Expand All @@ -18,7 +19,8 @@ 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' } },
pusher: { name: 'user17' },
commits: [{ added: [], removed: [], modified: ['README.md'] }]
}

Expand All @@ -29,11 +31,12 @@ 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' } },
pusher: { name: 'user17' },
commits: [{ added: [], removed: [], modified: ['README.md'] }]
}
Webhooks::ProcessPushUpdate.expects(:call).with(
'refs/heads/main', 'csharp', [
'refs/heads/main', 'exercism', 'csharp', 'user17', [
ActionController::Parameters.new(added: [], removed: [], modified: ['README.md'])
]
)
Expand All @@ -44,7 +47,8 @@ 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' } },
pusher: { name: 'user17' },
commits: [{ added: [], removed: [], modified: ['README.md'] }]
}

Expand Down