From ca2cfc4d3f470ae6a85f258e9409cc70a48f1983 Mon Sep 17 00:00:00 2001 From: loiswells97 Date: Thu, 16 Feb 2023 12:52:36 +0000 Subject: [PATCH 1/7] Create draft PR for #129 From c043c688fc37b3ab55e12558b7aecfaeaf4f6860 Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Thu, 16 Feb 2023 14:47:21 +0000 Subject: [PATCH 2/7] Checking which files were modifed --- app/controllers/github_webhooks_controller.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/app/controllers/github_webhooks_controller.rb b/app/controllers/github_webhooks_controller.rb index 0a71e0b7d..6ffb121b1 100644 --- a/app/controllers/github_webhooks_controller.rb +++ b/app/controllers/github_webhooks_controller.rb @@ -4,7 +4,7 @@ class GithubWebhooksController < ActionController::API include GithubWebhook::Processor def github_push(payload) - UploadJob.perform_later if payload['ref'] == ENV.fetch('GITHUB_WEBHOOK_REF') + UploadJob.perform_later if payload[:ref] == ENV.fetch('GITHUB_WEBHOOK_REF') && edited_code?(payload) head :ok end @@ -13,4 +13,16 @@ def github_push(payload) def webhook_secret(_payload) ENV.fetch('GITHUB_WEBHOOK_SECRET') end + + def edited_code?(payload) + commits = payload[:commits] + modified_paths = commits.map { |commit| commit[:added] | commit[:modified] | commit[:removed] }.flatten + + modified_code_paths = modified_paths.filter do |path| + path_components = path.split('/') + path_components[0] == 'en' && path_components[1] == 'code' + end + + modified_code_paths.length.positive? + end end From 68ad022f8eec85a366660676e2cf2560bd00b73c Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Thu, 16 Feb 2023 15:37:56 +0000 Subject: [PATCH 3/7] WIP: trying to test github webhooks controller --- .../github_webhooks/github_push_spec.rb | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 spec/request/github_webhooks/github_push_spec.rb diff --git a/spec/request/github_webhooks/github_push_spec.rb b/spec/request/github_webhooks/github_push_spec.rb new file mode 100644 index 000000000..733b61701 --- /dev/null +++ b/spec/request/github_webhooks/github_push_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe GithubWebhooksController, type: :request do + # let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } + # let(:project) { create(:project, user_id:) } + + describe 'github_push' do + # context 'when auth is correct' do + # before do + # mock_oauth_user(user_id) + + # response = OperationResponse.new + # response[:project] = project + # allow(Project::Create).to receive(:call).and_return(response) + # end + params = { + ref: '/branches/main', + commits: [] + } + + it 'returns success' do + post '/github_webhooks', params: params, headers: {'x-hub-signature-256': ENV.fetch('GITHUB_WEBHOOK_SECRET')} + expect(response).to have_http_status(:ok) + end + # end + + end +end From c148319374e87a051ef914bb1c151bb975a1677f Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Thu, 16 Feb 2023 16:38:27 +0000 Subject: [PATCH 4/7] fixing test for webhook controller --- Gemfile | 1 + Gemfile.lock | 2 ++ app/controllers/github_webhooks_controller.rb | 9 +---- .../github_webhooks/github_push_spec.rb | 36 +++++++++++-------- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/Gemfile b/Gemfile index adb5f433d..24e93f47c 100644 --- a/Gemfile +++ b/Gemfile @@ -34,6 +34,7 @@ group :development, :test do end group :test do + gem 'climate_control' gem 'shoulda-matchers', '~> 5.0' gem 'webmock' end diff --git a/Gemfile.lock b/Gemfile.lock index e755a3307..c5cca8ce3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -90,6 +90,7 @@ GEM builder (3.2.4) byebug (11.1.3) cancancan (3.4.0) + climate_control (1.2.0) coderay (1.1.3) concurrent-ruby (1.2.0) crack (0.4.5) @@ -307,6 +308,7 @@ DEPENDENCIES aws-sdk-s3 bootsnap cancancan (~> 3.3) + climate_control dotenv-rails factory_bot_rails faker diff --git a/app/controllers/github_webhooks_controller.rb b/app/controllers/github_webhooks_controller.rb index 6ffb121b1..164078a71 100644 --- a/app/controllers/github_webhooks_controller.rb +++ b/app/controllers/github_webhooks_controller.rb @@ -5,7 +5,6 @@ class GithubWebhooksController < ActionController::API def github_push(payload) UploadJob.perform_later if payload[:ref] == ENV.fetch('GITHUB_WEBHOOK_REF') && edited_code?(payload) - head :ok end private @@ -17,12 +16,6 @@ def webhook_secret(_payload) def edited_code?(payload) commits = payload[:commits] modified_paths = commits.map { |commit| commit[:added] | commit[:modified] | commit[:removed] }.flatten - - modified_code_paths = modified_paths.filter do |path| - path_components = path.split('/') - path_components[0] == 'en' && path_components[1] == 'code' - end - - modified_code_paths.length.positive? + modified_paths.count { |path| path.start_with?('en/code') }.positive? end end diff --git a/spec/request/github_webhooks/github_push_spec.rb b/spec/request/github_webhooks/github_push_spec.rb index 733b61701..1fbd6b15a 100644 --- a/spec/request/github_webhooks/github_push_spec.rb +++ b/spec/request/github_webhooks/github_push_spec.rb @@ -6,25 +6,31 @@ # let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } # let(:project) { create(:project, user_id:) } + around do |example| + ClimateControl.modify GITHUB_WEBHOOK_SECRET: 'secret', GITHUB_WEBHOOK_REF: 'branches/whatever' do + example.run + end + end + describe 'github_push' do - # context 'when auth is correct' do - # before do - # mock_oauth_user(user_id) + let(:params) {{ + ref: '/branches/main', + commits: [] + }} - # response = OperationResponse.new - # response[:project] = project - # allow(Project::Create).to receive(:call).and_return(response) - # end - params = { - ref: '/branches/main', - commits: [] + let(:headers) { + { + 'X-Hub-Signature-256': "sha256=#{OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), ENV.fetch('GITHUB_WEBHOOK_SECRET'), params.to_json)}", + 'X-GitHub-Event': 'push' } + } - it 'returns success' do - post '/github_webhooks', params: params, headers: {'x-hub-signature-256': ENV.fetch('GITHUB_WEBHOOK_SECRET')} - expect(response).to have_http_status(:ok) - end - # end + before do + post '/github_webhooks', params: params.to_json, headers: headers + end + it 'returns success' do + expect(response).to have_http_status(:ok) end + end end From 6de2597abdc1ed04ab309ccbf081bb015b8425e7 Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Thu, 16 Feb 2023 18:34:04 +0000 Subject: [PATCH 5/7] getting webhook job scheduling tests working --- .../github_webhooks/github_push_spec.rb | 36 --------- .../github_webhooks/github_push_spec.rb | 77 +++++++++++++++++++ 2 files changed, 77 insertions(+), 36 deletions(-) delete mode 100644 spec/request/github_webhooks/github_push_spec.rb create mode 100644 spec/requests/github_webhooks/github_push_spec.rb diff --git a/spec/request/github_webhooks/github_push_spec.rb b/spec/request/github_webhooks/github_push_spec.rb deleted file mode 100644 index 1fbd6b15a..000000000 --- a/spec/request/github_webhooks/github_push_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe GithubWebhooksController, type: :request do - # let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } - # let(:project) { create(:project, user_id:) } - - around do |example| - ClimateControl.modify GITHUB_WEBHOOK_SECRET: 'secret', GITHUB_WEBHOOK_REF: 'branches/whatever' do - example.run - end - end - - describe 'github_push' do - let(:params) {{ - ref: '/branches/main', - commits: [] - }} - - let(:headers) { - { - 'X-Hub-Signature-256': "sha256=#{OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), ENV.fetch('GITHUB_WEBHOOK_SECRET'), params.to_json)}", - 'X-GitHub-Event': 'push' - } - } - - before do - post '/github_webhooks', params: params.to_json, headers: headers - end - - it 'returns success' do - expect(response).to have_http_status(:ok) - end - end -end diff --git a/spec/requests/github_webhooks/github_push_spec.rb b/spec/requests/github_webhooks/github_push_spec.rb new file mode 100644 index 000000000..72c57071b --- /dev/null +++ b/spec/requests/github_webhooks/github_push_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe GithubWebhooksController do + + around do |example| + ClimateControl.modify GITHUB_WEBHOOK_SECRET: 'secret', GITHUB_WEBHOOK_REF: 'branches/whatever' do + example.run + end + end + + describe 'github_push' do + let(:params) {{ + ref: ref, + commits: commits + }} + + let(:headers) { + { + 'X-Hub-Signature-256': "sha256=#{OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), ENV.fetch('GITHUB_WEBHOOK_SECRET'), params.to_json)}", + 'X-GitHub-Event': 'push', + 'Content-Type': 'application/json' + } + } + + before(:example) do + allow(UploadJob).to receive(:perform_later) + post '/github_webhooks', env: { 'RAW_POST_DATA': params.to_json }, headers: headers + end + + context 'when webhook ref matches branch of interest' do + let(:ref) { 'branches/whatever' } + + context 'when code has been added' do + let(:commits) { [ { added: [], modified: [], removed: ['en/code/project1/main.py'] } ] } + + it 'schedules the upload job' do + expect(UploadJob).to have_received(:perform_later) + end + end + + context 'when code has been modified' do + let(:commits) { [ { added: [], modified: [ 'en/code/project1/main.py' ], removed: [] } ] } + + it 'schedules the upload job' do + expect(UploadJob).to have_received(:perform_later) + end + end + + context 'when code has been removed' do + let(:commits) { [ { added: [], modified: [], removed: ['en/code/project1/main.py'] } ] } + + it 'schedules the upload job' do + expect(UploadJob).to have_received(:perform_later) + end + end + + context 'when code has not been changed' do + let(:commits) { [ { added: ['en/step2.md'], modified: [ 'en/step1.md' ], removed: ['en/step0.md'] } ] } + + it 'does not schedule the upload job' do + expect(UploadJob).not_to have_received(:perform_later) + end + end + end + + context 'when webhook ref does not match branch of interest' do + let(:ref) { 'branches/master' } + let(:commits) { [ { added: [], modified: [ 'en/code/project1/main.py' ], removed: [] } ] } + + it 'does not schedule the upload job' do + expect(UploadJob).not_to have_received(:perform_later) + end + end + end +end From e15106c7d6bb46ac928261399c8fff9b8c9f5920 Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Fri, 17 Feb 2023 09:29:55 +0000 Subject: [PATCH 6/7] rubocop fixes --- ...> github_webhooks_controller_push_spec.rb} | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) rename spec/requests/github_webhooks/{github_push_spec.rb => github_webhooks_controller_push_spec.rb} (69%) diff --git a/spec/requests/github_webhooks/github_push_spec.rb b/spec/requests/github_webhooks/github_webhooks_controller_push_spec.rb similarity index 69% rename from spec/requests/github_webhooks/github_push_spec.rb rename to spec/requests/github_webhooks/github_webhooks_controller_push_spec.rb index 72c57071b..84d67bc6b 100644 --- a/spec/requests/github_webhooks/github_push_spec.rb +++ b/spec/requests/github_webhooks/github_webhooks_controller_push_spec.rb @@ -3,7 +3,6 @@ require 'rails_helper' RSpec.describe GithubWebhooksController do - around do |example| ClimateControl.modify GITHUB_WEBHOOK_SECRET: 'secret', GITHUB_WEBHOOK_REF: 'branches/whatever' do example.run @@ -11,29 +10,31 @@ end describe 'github_push' do - let(:params) {{ - ref: ref, - commits: commits - }} + let(:params) do + { + ref:, + commits: + } + end - let(:headers) { + let(:headers) do { 'X-Hub-Signature-256': "sha256=#{OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), ENV.fetch('GITHUB_WEBHOOK_SECRET'), params.to_json)}", 'X-GitHub-Event': 'push', 'Content-Type': 'application/json' } - } + end - before(:example) do + before do allow(UploadJob).to receive(:perform_later) - post '/github_webhooks', env: { 'RAW_POST_DATA': params.to_json }, headers: headers + post '/github_webhooks', env: { RAW_POST_DATA: params.to_json }, headers: end context 'when webhook ref matches branch of interest' do let(:ref) { 'branches/whatever' } context 'when code has been added' do - let(:commits) { [ { added: [], modified: [], removed: ['en/code/project1/main.py'] } ] } + let(:commits) { [{ added: ['en/code/project1/main.py'], modified: [], removed: [] }] } it 'schedules the upload job' do expect(UploadJob).to have_received(:perform_later) @@ -41,7 +42,7 @@ end context 'when code has been modified' do - let(:commits) { [ { added: [], modified: [ 'en/code/project1/main.py' ], removed: [] } ] } + let(:commits) { [{ added: [], modified: ['en/code/project1/main.py'], removed: [] }] } it 'schedules the upload job' do expect(UploadJob).to have_received(:perform_later) @@ -49,7 +50,7 @@ end context 'when code has been removed' do - let(:commits) { [ { added: [], modified: [], removed: ['en/code/project1/main.py'] } ] } + let(:commits) { [{ added: [], modified: [], removed: ['en/code/project1/main.py'] }] } it 'schedules the upload job' do expect(UploadJob).to have_received(:perform_later) @@ -57,7 +58,7 @@ end context 'when code has not been changed' do - let(:commits) { [ { added: ['en/step2.md'], modified: [ 'en/step1.md' ], removed: ['en/step0.md'] } ] } + let(:commits) { [{ added: ['en/step2.md'], modified: ['en/step1.md'], removed: ['en/step0.md'] }] } it 'does not schedule the upload job' do expect(UploadJob).not_to have_received(:perform_later) @@ -67,7 +68,7 @@ context 'when webhook ref does not match branch of interest' do let(:ref) { 'branches/master' } - let(:commits) { [ { added: [], modified: [ 'en/code/project1/main.py' ], removed: [] } ] } + let(:commits) { [{ added: [], modified: ['en/code/project1/main.py'], removed: [] }] } it 'does not schedule the upload job' do expect(UploadJob).not_to have_received(:perform_later) From e71358c04d1ecbb428795bd2eec7de7d5c02dffd Mon Sep 17 00:00:00 2001 From: Izzy Smillie Date: Fri, 17 Feb 2023 10:27:30 +0000 Subject: [PATCH 7/7] Refactor webhook tests --- .../github_webhooks_controller_push_spec.rb | 94 +++++++++---------- 1 file changed, 46 insertions(+), 48 deletions(-) diff --git a/spec/requests/github_webhooks/github_webhooks_controller_push_spec.rb b/spec/requests/github_webhooks/github_webhooks_controller_push_spec.rb index 84d67bc6b..3f48f496b 100644 --- a/spec/requests/github_webhooks/github_webhooks_controller_push_spec.rb +++ b/spec/requests/github_webhooks/github_webhooks_controller_push_spec.rb @@ -9,70 +9,68 @@ end end - describe 'github_push' do - let(:params) do - { - ref:, - commits: - } - end - - let(:headers) do - { - 'X-Hub-Signature-256': "sha256=#{OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), ENV.fetch('GITHUB_WEBHOOK_SECRET'), params.to_json)}", - 'X-GitHub-Event': 'push', - 'Content-Type': 'application/json' - } - end + let(:params) do + { + ref:, + commits: + } + end - before do - allow(UploadJob).to receive(:perform_later) - post '/github_webhooks', env: { RAW_POST_DATA: params.to_json }, headers: - end + let(:headers) do + { + 'X-Hub-Signature-256': "sha256=#{OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), ENV.fetch('GITHUB_WEBHOOK_SECRET'), params.to_json)}", + 'X-GitHub-Event': 'push', + 'Content-Type': 'application/json' + } + end - context 'when webhook ref matches branch of interest' do - let(:ref) { 'branches/whatever' } + before do + allow(UploadJob).to receive(:perform_later) + post '/github_webhooks', env: { RAW_POST_DATA: params.to_json }, headers: + end - context 'when code has been added' do - let(:commits) { [{ added: ['en/code/project1/main.py'], modified: [], removed: [] }] } + shared_examples 'upload job' do + it 'schedules the job' do + expect(UploadJob).to have_received(:perform_later) + end + end - it 'schedules the upload job' do - expect(UploadJob).to have_received(:perform_later) - end - end + describe 'when webhook ref matches branch of interest on github push' do + let(:ref) { 'branches/whatever' } - context 'when code has been modified' do - let(:commits) { [{ added: [], modified: ['en/code/project1/main.py'], removed: [] }] } + context 'when code has been added' do + let(:commits) { [{ added: ['en/code/project1/main.py'], modified: [], removed: [] }] } - it 'schedules the upload job' do - expect(UploadJob).to have_received(:perform_later) - end - end + it_behaves_like 'upload job' + end - context 'when code has been removed' do - let(:commits) { [{ added: [], modified: [], removed: ['en/code/project1/main.py'] }] } + context 'when code has been modified' do + let(:commits) { [{ added: [], modified: ['en/code/project1/main.py'], removed: [] }] } - it 'schedules the upload job' do - expect(UploadJob).to have_received(:perform_later) - end - end + it_behaves_like 'upload job' + end - context 'when code has not been changed' do - let(:commits) { [{ added: ['en/step2.md'], modified: ['en/step1.md'], removed: ['en/step0.md'] }] } + context 'when code has been removed' do + let(:commits) { [{ added: [], modified: [], removed: ['en/code/project1/main.py'] }] } - it 'does not schedule the upload job' do - expect(UploadJob).not_to have_received(:perform_later) - end - end + it_behaves_like 'upload job' end - context 'when webhook ref does not match branch of interest' do - let(:ref) { 'branches/master' } - let(:commits) { [{ added: [], modified: ['en/code/project1/main.py'], removed: [] }] } + context 'when code has not been changed' do + let(:commits) { [{ added: ['en/step2.md'], modified: ['en/step1.md'], removed: ['en/step0.md'] }] } it 'does not schedule the upload job' do expect(UploadJob).not_to have_received(:perform_later) end end end + + describe 'when webhook ref does not match branch of interest on github push' do + let(:ref) { 'branches/master' } + let(:commits) { [{ added: [], modified: ['en/code/project1/main.py'], removed: [] }] } + + it 'does not schedule the upload job' do + expect(UploadJob).not_to have_received(:perform_later) + end + end end