diff --git a/lib/open_project/github_integration/hook_handler.rb b/lib/open_project/github_integration/hook_handler.rb index 19786b6c68..d6de2051d3 100644 --- a/lib/open_project/github_integration/hook_handler.rb +++ b/lib/open_project/github_integration/hook_handler.rb @@ -15,7 +15,7 @@ module OpenProject::GithubIntegration class HookHandler # List of the github events we can handle. - KNOWN_EVENTS = %w{ ping pull_request issue_comment } + KNOWN_EVENTS = %w[ping pull_request issue_comment].freeze # A github webhook happened. # We need to check validity of the data and send a Notification @@ -29,11 +29,12 @@ module OpenProject::GithubIntegration return 404 unless KNOWN_EVENTS.include?(event_type) && event_delivery return 403 unless user.present? - payload = Hash.new - payload.merge! params.require('webhook') - payload.merge! 'user_id' => user.id, - 'github_event' => event_type, - 'github_delivery' => event_delivery + payload = params + .permit! + .to_h + .merge('user_id' => user.id, + 'github_event' => event_type, + 'github_delivery' => event_delivery) OpenProject::Notifications.send(event_name(event_type), payload) diff --git a/spec/lib/github_integration_spec.rb b/spec/lib/github_integration_spec.rb index a449e87b85..52164a2cae 100644 --- a/spec/lib/github_integration_spec.rb +++ b/spec/lib/github_integration_spec.rb @@ -49,31 +49,29 @@ describe OpenProject::GithubIntegration do let(:wps) { [wp1, wp2, wp3, wp4] } it "should handle the pull_request creation payload" do - params = ActionController::Parameters.new({ - 'webhook' => { - 'action' => 'opened', - 'number' => '5', - 'pull_request' => { - 'title' => 'Bugfixes', - 'body' => "Fixes http://example.net/wp/#{wp1.id} and " + - "https://example.net/work_packages/#{wp2.id} and " + - "http://example.net/subdir/wp/#{wp3.id} and " + - "https://example.net/subdir/work_packages/#{wp4.id}.", - 'html_url' => 'http://pull.request', - 'base' => { - 'repo' => { - 'full_name' => 'full/name', - 'html_url' => 'http://pull.request' - } + params = ActionController::Parameters.new( + 'action' => 'opened', + 'number' => '5', + 'pull_request' => { + 'title' => 'Bugfixes', + 'body' => "Fixes http://example.net/wp/#{wp1.id} and " + + "https://example.net/work_packages/#{wp2.id} and " + + "http://example.net/subdir/wp/#{wp3.id} and " + + "https://example.net/subdir/work_packages/#{wp4.id}.", + 'html_url' => 'http://pull.request', + 'base' => { + 'repo' => { + 'full_name' => 'full/name', + 'html_url' => 'http://pull.request' } - }, - 'sender' => { - 'login' => 'github_login', - 'html_url' => 'http://user.name' - }, - 'repository' => {} - } - }) + } + }, + 'sender' => { + 'login' => 'github_login', + 'html_url' => 'http://user.name' + }, + 'repository' => {} + ) environment = { 'HTTP_X_GITHUB_EVENT' => 'pull_request', @@ -94,31 +92,29 @@ describe OpenProject::GithubIntegration do end it "should handle the pull_request close payload" do - params = ActionController::Parameters.new({ - 'webhook' => { - 'action' => 'closed', - 'number' => '5', - 'pull_request' => { - 'title' => 'Bugfixes', - 'body' => "Fixes http://example.net/wp/#{wp1.id} and " + - "https://example.net/work_packages/#{wp2.id} and " + - "http://example.net/subdir/wp/#{wp3.id} and " + - "https://example.net/subdir/work_packages/#{wp4.id}.", - 'html_url' => 'http://pull.request', - 'base' => { - 'repo' => { - 'full_name' => 'full/name', - 'html_url' => 'http://pull.request' - } + params = ActionController::Parameters.new( + 'action' => 'closed', + 'number' => '5', + 'pull_request' => { + 'title' => 'Bugfixes', + 'body' => "Fixes http://example.net/wp/#{wp1.id} and " + + "https://example.net/work_packages/#{wp2.id} and " + + "http://example.net/subdir/wp/#{wp3.id} and " + + "https://example.net/subdir/work_packages/#{wp4.id}.", + 'html_url' => 'http://pull.request', + 'base' => { + 'repo' => { + 'full_name' => 'full/name', + 'html_url' => 'http://pull.request' } - }, - 'sender' => { - 'login' => 'github_login', - 'html_url' => 'http://user.name' - }, - 'repository' => {} - } - }) + } + }, + 'sender' => { + 'login' => 'github_login', + 'html_url' => 'http://user.name' + }, + 'repository' => {} + ) environment = { 'HTTP_X_GITHUB_EVENT' => 'pull_request', @@ -139,32 +135,30 @@ describe OpenProject::GithubIntegration do end it "should handle the pull_request merged payload" do - params = ActionController::Parameters.new({ - 'webhook' => { - 'action' => 'closed', - 'number' => '5', - 'pull_request' => { - 'title' => 'Bugfixes', - 'body' => "Fixes http://example.net/wp/#{wp1.id} and " + - "https://example.net/work_packages/#{wp2.id} and " + - "http://example.net/subdir/wp/#{wp3.id} and " + - "https://example.net/subdir/work_packages/#{wp4.id}.", - 'html_url' => 'http://pull.request', - 'base' => { - 'repo' => { - 'full_name' => 'full/name', - 'html_url' => 'http://pull.request' - } - }, - 'merged' => true - }, - 'sender' => { - 'login' => 'github_login', - 'html_url' => 'http://user.name' + params = ActionController::Parameters.new( + 'action' => 'closed', + 'number' => '5', + 'pull_request' => { + 'title' => 'Bugfixes', + 'body' => "Fixes http://example.net/wp/#{wp1.id} and " + + "https://example.net/work_packages/#{wp2.id} and " + + "http://example.net/subdir/wp/#{wp3.id} and " + + "https://example.net/subdir/work_packages/#{wp4.id}.", + 'html_url' => 'http://pull.request', + 'base' => { + 'repo' => { + 'full_name' => 'full/name', + 'html_url' => 'http://pull.request' + } }, - 'repository' => {} - } - }) + 'merged' => true + }, + 'sender' => { + 'login' => 'github_login', + 'html_url' => 'http://user.name' + }, + 'repository' => {} + ) environment = { 'HTTP_X_GITHUB_EVENT' => 'pull_request', @@ -185,35 +179,33 @@ describe OpenProject::GithubIntegration do end it "should handle the pull_request comment creation payload" do - params = ActionController::Parameters.new({ - 'webhook' => { - 'action' => 'created', - 'issue' => { - 'title' => 'Bugfixes', - 'number' => '5', - 'pull_request' => { - 'html_url' => 'http://pull.request' - } - }, - 'comment' => { - 'body' => "Fixes http://example.net/wp/#{wp1.id} and " + - "https://example.net/work_packages/#{wp2.id} and " + - "http://example.net/subdir/wp/#{wp3.id} and " + - "https://example.net/subdir/work_packages/#{wp4.id}.", - 'html_url' => 'http://comment.url', - 'user' => { - 'login' => 'github_login', - 'html_url' => 'http://user.name' - } - }, - 'sender' => { - }, - 'repository' => { - 'full_name' => 'full/name', + params = ActionController::Parameters.new( + 'action' => 'created', + 'issue' => { + 'title' => 'Bugfixes', + 'number' => '5', + 'pull_request' => { 'html_url' => 'http://pull.request' } + }, + 'comment' => { + 'body' => "Fixes http://example.net/wp/#{wp1.id} and " + + "https://example.net/work_packages/#{wp2.id} and " + + "http://example.net/subdir/wp/#{wp3.id} and " + + "https://example.net/subdir/work_packages/#{wp4.id}.", + 'html_url' => 'http://comment.url', + 'user' => { + 'login' => 'github_login', + 'html_url' => 'http://user.name' + } + }, + 'sender' => { + }, + 'repository' => { + 'full_name' => 'full/name', + 'html_url' => 'http://pull.request' } - }) + ) environment = { 'HTTP_X_GITHUB_EVENT' => 'issue_comment', diff --git a/spec/lib/hook_handler_spec.rb b/spec/lib/hook_handler_spec.rb index 76b72ac4c4..77d613e940 100644 --- a/spec/lib/hook_handler_spec.rb +++ b/spec/lib/hook_handler_spec.rb @@ -32,7 +32,7 @@ describe OpenProject::GithubIntegration::HookHandler do describe '#process' do let(:handler) { OpenProject::GithubIntegration::HookHandler.new } let(:hook) { 'fake hook' } - let(:params) { ActionController::Parameters.new({ 'webhook' => {'fake' => 'value'} }) } + let(:params) { ActionController::Parameters.new({ 'fake' => 'value' }) } let(:environment) { { 'HTTP_X_GITHUB_EVENT' => 'pull_request' , 'HTTP_X_GITHUB_DELIVERY' => 'veryuniqueid' } } let(:user) do