From a4b69dbc9174a71749845ffdb740827f72a6da72 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth@bethesque.com>
Date: Sun, 10 Jun 2018 08:36:49 +1000
Subject: [PATCH] fix: correctly handle template parameters in URL when
 rendering webhook resource

---
 .../api/contracts/webhook_contract.rb         |  3 ++-
 lib/pact_broker/domain/webhook_request.rb     |  2 +-
 lib/pact_broker/webhooks/render.rb            |  3 +++
 .../domain/webhook_request_spec.rb            |  8 ++++++
 spec/lib/pact_broker/webhooks/render_spec.rb  | 26 +++++++++++++++++--
 5 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/lib/pact_broker/api/contracts/webhook_contract.rb b/lib/pact_broker/api/contracts/webhook_contract.rb
index f2dadd322..463bd9b65 100644
--- a/lib/pact_broker/api/contracts/webhook_contract.rb
+++ b/lib/pact_broker/api/contracts/webhook_contract.rb
@@ -1,6 +1,7 @@
 require 'reform'
 require 'reform/form'
 require 'pact_broker/webhooks/check_host_whitelist'
+require 'pact_broker/webhooks/render'
 
 module PactBroker
   module Api
@@ -106,7 +107,7 @@ def host_whitelist
               end
 
               def parse_uri(uri_string, placeholder = 'placeholder')
-                URI(uri_string.gsub(/\$\{pactbroker\.[^\}]+\}/, placeholder))
+                URI(uri_string.gsub(PactBroker::Webhooks::Render::TEMPLATE_PARAMETER_REGEXP, placeholder))
               end
             end
 
diff --git a/lib/pact_broker/domain/webhook_request.rb b/lib/pact_broker/domain/webhook_request.rb
index a8cce4c36..698e3e7c3 100644
--- a/lib/pact_broker/domain/webhook_request.rb
+++ b/lib/pact_broker/domain/webhook_request.rb
@@ -45,7 +45,7 @@ def initialize attributes = {}
       end
 
       def description
-        "#{method.upcase} #{URI(url).host}"
+        "#{method.upcase} #{URI(url.gsub(PactBroker::Webhooks::Render::TEMPLATE_PARAMETER_REGEXP, 'placeholder')).host}"
       end
 
       def display_password
diff --git a/lib/pact_broker/webhooks/render.rb b/lib/pact_broker/webhooks/render.rb
index 52e74cf2c..3b9be428a 100644
--- a/lib/pact_broker/webhooks/render.rb
+++ b/lib/pact_broker/webhooks/render.rb
@@ -1,6 +1,9 @@
 module PactBroker
   module Webhooks
     class Render
+
+      TEMPLATE_PARAMETER_REGEXP = /\$\{pactbroker\.[^\}]+\}/
+
       def self.call(template, pact, verification = nil, &escaper)
         base_url = PactBroker.configuration.base_url
         params = {
diff --git a/spec/lib/pact_broker/domain/webhook_request_spec.rb b/spec/lib/pact_broker/domain/webhook_request_spec.rb
index cc88829fd..ab315d653 100644
--- a/spec/lib/pact_broker/domain/webhook_request_spec.rb
+++ b/spec/lib/pact_broker/domain/webhook_request_spec.rb
@@ -43,6 +43,14 @@ module Domain
         it "returns a brief description of the HTTP request" do
           expect(subject.description).to eq 'POST example.org'
         end
+
+        context "when the URL has a template parameter in it" do
+          let(:url) { "http://foo/commits/${pactbroker.consumerVersionNumber}" }
+
+          it "doesn't explode" do
+            expect(subject.description).to eq 'POST foo'
+          end
+        end
       end
 
       describe "display_password" do
diff --git a/spec/lib/pact_broker/webhooks/render_spec.rb b/spec/lib/pact_broker/webhooks/render_spec.rb
index a11d06690..cc321dcb9 100644
--- a/spec/lib/pact_broker/webhooks/render_spec.rb
+++ b/spec/lib/pact_broker/webhooks/render_spec.rb
@@ -10,7 +10,25 @@ module Webhooks
         end
 
         let(:pact) do
-          instance_double("pact", consumer_version_number: "1.2.3+foo", consumer_name: "Foo", provider_name: "Bar")
+          instance_double("pact", consumer_version_number: "1.2.3+foo", consumer_name: "Foo", provider_name: "Bar", latest_verification: nil)
+        end
+
+        let(:pact_with_no_verification) { pact }
+
+        let(:pact_with_successful_verification) do
+          instance_double("pact",
+            consumer_version_number: "1.2.3+foo",
+            consumer_name: "Foo",
+            provider_name: "Bar",
+            latest_verification: verification)
+        end
+
+        let(:pact_with_failed_verification) do
+          instance_double("pact",
+            consumer_version_number: "1.2.3+foo",
+            consumer_name: "Foo",
+            provider_name: "Bar",
+            latest_verification: failed_verification)
         end
 
         let(:verification) do
@@ -21,6 +39,7 @@ module Webhooks
           instance_double("verification", provider_version_number: "3", success: false)
         end
 
+        let(:nil_pact) { nil }
         let(:nil_verification) { nil }
 
         subject { Render.call(template, pact, verification) }
@@ -34,7 +53,10 @@ module Webhooks
           ["${pactbroker.providerName}", "Bar", :pact, :verification],
           ["${pactbroker.githubVerificationStatus}", "success", :pact, :verification],
           ["${pactbroker.githubVerificationStatus}", "failure", :pact, :failed_verification],
-          ["${pactbroker.githubVerificationStatus}", "", :pact, :nil_verification]
+          ["${pactbroker.githubVerificationStatus}", "", :nil_pact, :nil_verification],
+          ["${pactbroker.githubVerificationStatus}", "pending", :pact_with_no_verification, :nil_verification],
+          ["${pactbroker.githubVerificationStatus}", "success", :pact_with_successful_verification, :nil_verification],
+          ["${pactbroker.githubVerificationStatus}", "failure", :pact_with_failed_verification, :nil_verification]
         ]
 
         TEST_CASES.each do | (template, expected_output, pact_var_name, verification_var_name) |