add List.GetDetail and List[] and catch XML parsing errors and masque…

…rade them as API errors so they can be better dealt with

lib/campaign_monitor.rb

@@ -87,11 +87,15 @@ def initialize(api_key=CAMPAIGN_MONITOR_API_KEY)
    # returns an XmlSimple object with the response
   def request(method, params)
     request_xml=http_get(request_url(method, params))
-    response = PARSER.xml_in(request_xml, { 'keeproot' => false,
-      'forcearray' => %w[List Campaign Subscriber Client SubscriberOpen SubscriberUnsubscribe SubscriberClick SubscriberBounce],
-      'noattr' => true })
-    response.delete('d1p1:type')
-    response
+    begin
+      response = PARSER.xml_in(request_xml, { 'keeproot' => false,
+        'forcearray' => %w[List Campaign Subscriber Client SubscriberOpen SubscriberUnsubscribe SubscriberClick SubscriberBounce],
+        'noattr' => true })
+      response.delete('d1p1:type')
+      response
+    rescue XML::Parser::ParseError
+      { "Code" => 500, "Message" => request_xml.split(/\r?\n/).first, "FullError" => request_xml }
+    end
   end
 
   # Takes a CampaignMonitor API method name and set of parameters; returns the correct URL for the REST API.

lib/campaign_monitor/list.rb

@@ -53,11 +53,27 @@ def id=(v)
     def save
       id ? Update : Create
     end
+
+    # Loads a list by it's ID
+    #
+    # @list = List.GetDetail(1234)
+    def self.GetDetail(id)
+      list=self.new("ListID" => id)
+      list.GetDetail(true)
+      list.result.code == 101 ? nil : list
+    end
+
+    # loads a list by it's ID
+    #
+    # @list = List.GetDetail(1234)
+    def self.[](k)
+      GetDetail(k)
+    end
 
     def GetDetail(overwrite=false)
-      raw=cm_client.List_GetDetail("ListID" => id)
-      @attributes=raw.merge(@attributes)
-      @attributes.merge!(raw) if overwrite
+      @result=Result.new(cm_client.List_GetDetail("ListID" => id))
+      @attributes=@result.raw.merge(@attributes)
+      @attributes.merge!(@result.raw) if overwrite
     end
 
     def Update

lib/campaign_monitor/result.rb

@@ -6,16 +6,23 @@ class Result
     def initialize(response)
       @message = response["Message"]
       @code = response["Code"].to_i
+      @raw=response
     end
 
     def success?
       code == 0
     end
 
-    alias :succeeded? :success?
-
     def failed?
       !succeeded?
     end
+
+    alias :succeeded? :success?
+    alias :failure? :failed?
+
+    def raw
+      @raw
+    end
+
   end
 end

test/list_test.rb

@@ -49,13 +49,35 @@ def test_update_list
     assert_equal "Just another list", list.name
   end
 
-  def test_getinfo_list
+  def test_getdetail_for_list_instance
     list=@client.lists.first
     assert_equal "List #1", list.name
     assert_nil list["ConfirmOptIn"]
     list.GetDetail
     assert_equal "false", list["ConfirmOptIn"]
   end
+
+  def test_getdetail_to_load_list
+    list=CampaignMonitor::List.GetDetail(@list.id)
+    assert_equal "List #1", list.name
+    list=CampaignMonitor::List[@list.id]
+    assert_equal "List #1", list.name
+  end
+
+  # test that our own creative mapping of errors actually works
+  def test_save_with_missing_attribute
+    list = @client.new_list
+    list["Title"]="This is a new list"
+    assert !list.Create
+    assert list.result.failure?
+    assert_equal 500, list.result.code
+    assert_equal "System.InvalidOperationException: Missing parameter: UnsubscribePage.", list.result.message
+  end
+
+  def test_getting_a_dummy_list
+    list=CampaignMonitor::List["snickers"]
+    assert_equal nil, list
+  end
 
   def test_list_add_subscriber
     list=@client.lists.first