Implement full PR review

README.md

@@ -1,4 +1,5 @@
 ### CI Status
+
 |develop|main|
 |:------:|:---:|
 |[![Build Status](https://renuo.semaphoreci.com/badges/gnosis/branches/develop.svg?style=shields)](https://renuo.semaphoreci.com/projects/gnosis)|[![Build Status](https://renuo.semaphoreci.com/badges/gnosis/branches/main.svg?style=shields)](https://renuo.semaphoreci.com/projects/gnosis)|
@@ -9,18 +10,21 @@ Welcome to gnosis!
 This is a Redmine Plugin with the goal of making your development process easier to keep an overview of.
 
 ## What does this plugin do?
+
 Gnosis is able to show you GitHub Pull Requests and Deployments that belong to a Redmine issue, all in its "details"
 page.  
 To make this magic work there are only a few steps you have to follow and certain conventions to keep in mind.
 
 ### GitHub Webhooks configuration
+
 To configure the GitHub Webhooks, go to github.com/your_org/your_repo/settings/hooks. There you click on
 "Add Webhook". The "Payload URL" is your Redmine URL + /github_webhook. The secret is up for you to decide. It's
 important that this string is complex and secure. Then, you get into your `config/application.yml`, where you add the
 key as your `GITHUB_WEBHOOK_SECRET`. Click "Let me select individual events." and choose "Pull Requests".  
 You should now be good to go!
 
 ### SemaphoreCI Webhooks configuration
+
 **Important:** The GitHub notifications work without the SemaphoreCI setup. If you don't use a CI or just a different
 you can skip this step.
 To set up the SemaphoreCI Webhook go to your_org.semaphoreci.com/notifications and click on "New Notification".
@@ -34,13 +38,15 @@ secret. This should match the `SEMAPHORE_WEBHOOK_SECRET` in your `config/applica
 This should now also work just fine!
 
 ### GitHub Access Token
+
 This app needs to query your repository every now and then, so its able to know which deployment belongs to which pull
 request. For that you need to set the `GITHUB_ACCESS_TOKEN` in your `config/application.yml`. To create it, go to
 github.com/settings/tokens and click on "Generate new token". There click the "(classic)" option. It only needs "repo"
 access to work.
 If you make a deploy, it should correctly show now.
 
 ### Conventions you need to keep
+
 The plugin needs information on which Pull Requests belong to which issues, which is why there are set conventions for
 branch naming. When you create a branch that is connected to a certain Issue it should be named as follows:
 something/issue_number-description. The only important part of this is the /issue_number as that's how

app/controllers/webhooks_controller.rb

@@ -2,31 +2,15 @@
 
 class WebhooksController < ApplicationController
   protect_from_forgery except: %i[github_webhook_catcher semaphore_webhook_catcher]
-
-  Octokit.configure do |config|
-    config.access_token = ENV.fetch('GITHUB_ACCESS_TOKEN', nil)
-  end
-  CLIENT = Octokit::Client.new
+  before_action :verify_signature, only: %i[github_webhook_catcher semaphore_webhook_catcher]
 
   def github_webhook_catcher
-    unless verify_signature(request.body.read, request.env['HTTP_X_HUB_SIGNATURE_256'],
-                            ENV.fetch('GITHUB_WEBHOOK_SECRET', nil))
-      return render json: {status: 403}, status: :forbidden
-    end
-
     github_webhook_handler(params)
-
     render json: {status: :ok}
   end
 
   def semaphore_webhook_catcher
-    unless verify_signature(request.body.read, "sha256=#{request.headers['X-Semaphore-Signature-256']}",
-                            ENV.fetch('SEMAPHORE_WEBHOOK_SECRET', nil))
-      return render json: {status: 403}, status: :forbidden
-    end
-
     semaphore_webhook_handler(params)
-
     render json: {status: :ok}
   end
 
@@ -50,22 +34,24 @@ def semaphore_webhook_handler(params)
     first_sha = range.split('...').first
     last_sha = range.split('...').last
 
-    sha_between = fetch_commit_history(repo, branch, first_sha, last_sha)
+    sha_between = fetch_commit_history(repo, first_sha, last_sha)
     create_deploys_for_pull_requests(sha_between, branch, passed, time)
   end
 
-  def verify_signature(payload_body, recieved_signature, secret)
-    signature = "sha256=#{OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), secret, payload_body)}"
-    Rack::Utils.secure_compare(signature, recieved_signature)
-  end
+  def verify_signature
+    recieved_signature = request.headers['HTTP_X_HUB_SIGNATURE_256'] ||
+                         request.headers['X-Semaphore-Signature-256']
+    signature = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'),
+                                        ENV.fetch('GITHUB_WEBHOOK_SECRET'),
+                                        request.body.read).to_s
+    return if Rack::Utils.secure_compare(signature, recieved_signature.gsub('sha256=', ''))
 
-  def fetch_commit_history(repo, branch, first_commit, last_commit)
-    commit_sha_list = CLIENT.commits(repo, branch).pluck(:sha)
-
-    first_commit_index = commit_sha_list.index(first_commit)
-    last_commit_index = commit_sha_list.index(last_commit)
+    render json: {status: 403}, status: :forbidden
+  end
 
-    commit_sha_list[last_commit_index..first_commit_index]
+  def fetch_commit_history(repo, first_commit, last_commit)
+    comparison = CLIENT.compare(repo, first_commit, last_commit)
+    comparison.commits.pluck(:sha)
   end
 
   def create_deploys_for_pull_requests(sha_between, branch, passed, time)

app/models/pull_request.rb

@@ -5,12 +5,18 @@ class PullRequest < GnosisApplicationRecord
   has_many :pull_request_deployments, dependent: :destroy
 
   def self.auto_create_or_update(params)
-    state = params[:pull_request][:merged] ? 'merged' : params[:pull_request][:state]
-    state = 'draft' if params[:pull_request][:draft]
-    pr = PullRequest.find_or_initialize_by(url: params[:pull_request][:html_url])
-    pr.update!(state: state, url: params[:pull_request][:html_url],
-               title: params[:pull_request][:title], source_branch: params[:pull_request][:head][:ref],
-               target_branch: params[:pull_request][:base][:ref], was_merged: params[:pull_request][:merged],
-               merge_commit_sha: params[:pull_request][:merge_commit_sha], issue_id: params[:issue_id])
+    pr = params[:pull_request]
+    pull_request = PullRequest.find_or_initialize_by(url: pr[:html_url])
+    pull_request.update!(state: state(pr), url: pr[:html_url],
+                         title: pr[:title], source_branch: pr[:head][:ref],
+                         target_branch: pr[:base][:ref], was_merged: pr[:merged],
+                         merge_commit_sha: pr[:merge_commit_sha], issue_id: params[:issue_id])
+  end
+
+  def self.state(pr_param)
+    return 'merged' if pr_param[:merged]
+    return 'draft' if pr_param[:draft]
+
+    pr_param[:state]
   end
 end

init.rb

@@ -15,6 +15,11 @@
 
 raise 'GITHUB_ACCESS_TOKEN is not set' if ENV['GITHUB_ACCESS_TOKEN'].blank? && !Rails.env.test?
 
+Octokit.configure do |config|
+  config.access_token = ENV.fetch('GITHUB_ACCESS_TOKEN')
+end
+CLIENT = Octokit::Client.new
+
 Redmine::Plugin.register :gnosis do
   name 'Gnosis plugin'
   author 'Anes Hodza'

test/functional/webhooks_controller_test.rb

@@ -6,11 +6,16 @@ class WebhookCatchControllerControllerTest < ActionController::TestCase
   def setup
     @controller = WebhooksController.new
 
-    Octokit::Client.any_instance.stubs(:commits).returns([
-                                                           { sha: 'another_hash' },
-                                                           { sha: 'in_between_hash' },
-                                                           { sha: 'one_hash' }
-                                                         ])
+    commit = Struct.new(:sha)
+    comparison_result_struct = Struct.new(:commits)
+
+    comparison_result = comparison_result_struct.new([
+                                                       commit.new('another_hash'),
+                                                       commit.new('in_between_hash'),
+                                                       commit.new('one_hash')
+                                                     ])
+
+    Octokit::Client.any_instance.stubs(:compare).returns(comparison_result)
 
     @github_webhook_hash = {
       pull_request: {