DE-1771: Domain Webhooks - Refactor and deprecate existing methods

lib/mailgun/webhooks/webhooks.rb

@@ -4,6 +4,8 @@ module Mailgun
   # A Mailgun::Webhooks object is a simple CRUD interface to Mailgun Webhooks.
   # Uses Mailgun
   class Webhooks
+    include ApiVersionChecker
+
     ACTIONS = %w[accepted clicked complained delivered opened permanent_fail temporary_fail unsubscribed].freeze
 
     # Public creates a new Mailgun::Webhooks instance.
@@ -22,7 +24,14 @@ def list(domain, options = {})
       res = @client.get("domains/#{domain}/webhooks", options)
       res.to_h['webhooks']
     end
-    alias get_webhooks list
+
+    # :nocov:
+
+    def get_webhooks(domain, _options = {})
+      warn('`get_webhooks` method will be deprecated in future versions of Mailgun. Please use `list` instead.')
+      list(domain, {})
+    end
+    # :nocov:
 
     # Public: Get webook information for a specific action
     #
@@ -31,13 +40,19 @@ def list(domain, options = {})
     #
     # Returns a String of the url for the identified webhook or an
     #   empty String if one is not set
-    def info(domain, action)
+    def get(domain, action)
       res = @client.get("domains/#{domain}/webhooks/#{action}")
       res.to_h['webhook']['urls'] || ''
-    rescue NoMethodError
-      ''
     end
-    alias get_webhook_url info
+
+    # :nocov:
+    %i[info get_webhook_url].each do |method|
+      define_method(method) do |domain, action|
+        warn("`#{method}` method will be deprecated in future versions of Mailgun. Please use `get` instead.")
+        get(domain, action)
+      end
+    end
+    # :nocov:
 
     # Public: Add webhook
     #
@@ -50,24 +65,34 @@ def create(domain, action, url = '')
       res = @client.post("domains/#{domain}/webhooks", id: action, url: url)
       res.to_h['webhook']['urls'].include?(url) && res.to_h['message'] == 'Webhook has been created'
     end
-    alias add create
-    alias add_webhook create
+
+    # :nocov:
+    %i[add add_webhook].each do |method|
+      define_method(method) do |domain, action, url|
+        url ||= ''
+        warn("`#{method}` method will be deprecated in future versions of Mailgun. Please use `create` instead.")
+        create(domain, action, url)
+      end
+    end
+    # :nocov:
 
     # Public: Sets all webhooks to the same URL
     #
     # domain - A String of the domain name
     # url    - A String of the url to set all webhooks to
     #
     # Returns true or false
-    def create_all(domain, url = '')
-      ACTIONS.each do |action|
-        add_webhook domain, action, url
+    # :nocov:
+    %i[create_all add_all_webhooks].each do |method|
+      define_method(method) do |domain|
+        warn("`#{method}` method will be deprecated in future versions of Mailgun. Please use `create` instead.")
+
+        ACTIONS.each do |action|
+          create domain, action, url
+        end
       end
-      true
-    rescue StandardError
-      false
     end
-    alias add_all_webhooks create_all
+    # :nocov:
 
     # Public: Update webhook
     #
@@ -81,9 +106,15 @@ def update(domain, action, url = '')
       raise Mailgun::ParameterError('Action not provided to identify webhook to update') unless action
 
       res = @client.put("domains/#{domain}/webhooks/#{action}", id: action, url: url)
-      res.to_h['webhook']['urls'] == url && res.to_h['message'] == 'Webhook has been updated'
+      res.to_h['message'] == 'Webhook has been updated'
     end
-    alias update_webhook update
+
+    # :nocov:
+    def update_webhook(domain, action, url = '')
+      warn('`update_webhook` method will be deprecated in future versions of Mailgun. Please use `update` instead.')
+      update(domain, action, url)
+    end
+    # :nocov:
 
     # Public: Delete a specific webhook
     #
@@ -96,25 +127,36 @@ def remove(domain, action)
       raise Mailgun::ParameterError('Action not provided to identify webhook to remove') unless action
 
       @client.delete("domains/#{domain}/webhooks/#{action}").to_h['message'] == 'Webhook has been deleted'
-    rescue Mailgun::CommunicationError
-      false
     end
-    alias delete remove
-    alias delete_webhook remove
+
+    # :nocov:
+    %i[delete delete_webhook].each do |method|
+      define_method(method) do |domain, action|
+        warn("`#{method}` method will be deprecated in future versions of Mailgun. Please use `remove` instead.")
+        remove(domain, action)
+      end
+    end
+    # :nocov:
 
     # Public: Delete all webhooks for a domain
     #
     # domain - A required String of the domain to remove all webhooks for
     #
     # Returns a Boolean on the success
-    def remove_all(domain)
-      raise Mailgun::ParameterError('Domain not provided to remove webhooks from') unless domain
+    # :nocov:
+    %i[remove_all delete_all delete_all_webooks].each do |method|
+      define_method(method) do |domain|
+        warn("`#{method}` method will be deprecated in future versions of Mailgun. Please use `remove` instead.")
 
-      ACTIONS.each do |action|
-        delete_webhook domain, action
+        raise Mailgun::ParameterError('Domain not provided to remove webhooks from') unless domain
+
+        ACTIONS.each do |action|
+          remove domain, action
+        end
       end
     end
-    alias delete_all remove_all
-    alias delete_all_webooks remove_all
+    # :nocov:
+
+    enforces_api_version 'v3', :list, :get, :create, :update, :remove
   end
 end

spec/integration/webhook_spec.rb

@@ -6,55 +6,40 @@
 vcr_opts = { cassette_name: 'webhooks' }
 
 describe 'For the webhooks endpoint', order: :defined, vcr: vcr_opts do
-  before(:all) do
-    @mg_obj = Mailgun::Client.new(APIKEY, APIHOST, APIVERSION, SSL)
-    @domain = 'DOMAIN.TEST'
-    @testhook = 'accepted'
-    @testhookup = 'accepted'
-  end
+  let(:api_version) { 'v3' }
+  let(:mg_client) { Mailgun::Client.new(APIKEY, APIHOST, api_version, SSL) }
+  let(:mg_obj) { Mailgun::Webhooks.new(mg_client) }
+  let(:domain) { 'DOMAIN.TEST' }
+  let(:testhook) { 'accepted' }
+  let(:testhookup) { 'accepted' }
 
   it 'creates a webhook' do
-    result = @mg_obj.post("domains/#{@domain}/webhooks", { id: @testhook,
-                                                           url: "http://example.com/mailgun/events/#{@testhook}" })
+    result = mg_obj.create(domain, testhook, "http://example.com/mailgun/events/#{testhook}")
 
-    result.to_h!
-    expect(result.body['message']).to eq('Webhook has been created')
-    expect(result.body['webhook']['urls']).to include("http://example.com/mailgun/events/#{@testhook}")
+    expect(result).to be_truthy
   end
 
-  it 'gets a webhook.' do
-    result = @mg_obj.get("domains/#{@domain}/webhooks/#{@testhook}")
+  it 'gets a webhook' do
+    result = mg_obj.get(domain, testhook)
 
-    result.to_h!
-    expect(result.body['webhook']['urls']).to include("http://example.com/mailgun/events/#{@testhook}")
+    expect(result).to include("http://example.com/mailgun/events/#{testhook}")
   end
 
-  it 'gets a list of all webhooks.' do
-    result = @mg_obj.get("domains/#{@domain}/webhooks")
+  it 'gets a list of all webhooks' do
+    result = mg_obj.list(domain)
 
-    result.to_h!
-    expect(result.body['webhooks']['accepted']['urls']).to include("http://example.com/mailgun/events/#{@testhook}")
+    expect(result['accepted']['urls'][0]).to include("http://example.com/mailgun/events/#{testhook}")
   end
 
-  it 'updates a webhook.' do
-    result = @mg_obj.put(
-      "domains/#{@domain}/webhooks/#{@testhook}",
-      {
-        id: @testhook,
-        url: "http://example.com/mailgun/events/#{@testhookup}"
-      }
-    )
-
-    result.to_h!
-    expect(result.body['message']).to eq('Webhook has been updated')
-    expect(result.body['webhook']['urls']).to include("http://example.com/mailgun/events/#{@testhookup}")
+  it 'updates a webhook' do
+    result = mg_obj.update(domain, testhook, "http://example.com/mailgun/events/#{testhookup}")
+
+    expect(result).to be_truthy
   end
 
   it 'removes a webhook' do
-    result = @mg_obj.delete("domains/#{@domain}/webhooks/#{@testhook}")
+    result = mg_obj.remove(domain, testhook)
 
-    result.to_h!
-    expect(result.body['message']).to eq('Webhook has been deleted')
-    expect(result.body['webhook']['urls']).to include("http://example.com/mailgun/events/#{@testhookup}")
+    expect(result).to be_truthy
   end
 end