Merge branch 'accept-2xx-status-codes-for-webhooks' into 'master'

Accept 2xx status codes for successful Web hook triggers

Closes https://github.com/gitlabhq/gitlabhq/issues/9956

See merge request !2332

3 files changed, 14 insertions, 1 deletions

diff --git a/CHANGELOG b/CHANGELOG
index d1500ab7d3a..b3b8b1c5ba4 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.4.0 (unreleased)
+  - Accept 2xx status codes for successful Web hook triggers (Stan Hu)
   - Fix missing date of month in network graph when commits span a month (Stan Hu)
   - Expire view caches when application settings change (e.g. Gravatar disabled) (Stan Hu)
   - Don't notify users twice if they are both project watchers and subscribers (Stan Hu)
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 7164c0e1e90..d0aadfc330a 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -61,7 +61,7 @@ class WebHook < ActiveRecord::Base
                               basic_auth: auth)
     end
 
-    [response.code == 200, ActionView::Base.full_sanitizer.sanitize(response.to_s)]
+    [(response.code >= 200 && response.code < 300), ActionView::Base.full_sanitizer.sanitize(response.to_s)]
   rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e
     logger.error("WebHook Error => #{e}")
     [false, e.to_s]
diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb
index 2d90b0793cc..7070aa4ac62 100644
--- a/spec/models/hooks/web_hook_spec.rb
+++ b/spec/models/hooks/web_hook_spec.rb
@@ -77,5 +77,17 @@ describe ProjectHook, models: true do
 
       expect(@project_hook.execute(@data, 'push_hooks')).to eq([false, 'SSL error'])
     end
+
+    it "handles 200 status code" do
+      WebMock.stub_request(:post, @project_hook.url).to_return(status: 200, body: "Success")
+
+      expect(@project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success'])
+    end
+
+    it "handles 2xx status codes" do
+      WebMock.stub_request(:post, @project_hook.url).to_return(status: 201, body: "Success")
+
+      expect(@project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success'])
+    end
   end
 end