Merge pull request #46 from imtayadeway/fix-alert-defs

Preserve contract for expressions of alert definitions

app/controllers/api/alert_definitions_controller.rb

@@ -2,13 +2,15 @@ module Api
   class AlertDefinitionsController < BaseController
     REQUIRED_FIELDS = %w(description db expression options).freeze
 
+    before_action :set_additional_attributes
+
     def create_resource(type, id, data = {})
       assert_id_not_specified(data, type)
       assert_all_required_fields_exists(data, type, REQUIRED_FIELDS)
       begin
         data["expression"] = MiqExpression.new(data["expression"])
         data["enabled"] = true if data["enabled"].nil?
-        super(type, id, data)
+        super(type, id, data).serializable_hash.merge("expression" => data["expression"])
       rescue => err
         raise BadRequestError, "Failed to create a new alert definition - #{err}"
       end
@@ -23,5 +25,11 @@ def edit_resource(type, id = nil, data = {})
         raise BadRequestError, "Failed to update alert definition - #{err}"
       end
     end
+
+    private
+
+    def set_additional_attributes
+      @additional_attributes = %w(expression notify_email)
+    end
   end
 end

spec/requests/alert_definitions_spec.rb

@@ -42,14 +42,18 @@
 
   it "reads an alert as a resource" do
     api_basic_authorize action_identifier(:alert_definitions, :read, :resource_actions, :get)
-    alert_definition = FactoryGirl.create(:miq_alert)
+    alert_definition = FactoryGirl.create(
+      :miq_alert,
+      :miq_expression => MiqExpression.new("=" => {"field" => "Vm-name", "value" => "foo"})
+    )
     run_get(alert_definitions_url(alert_definition.id))
     expect(response).to have_http_status(:ok)
     expect(response.parsed_body).to include(
       "href"        => a_string_matching(alert_definitions_url(alert_definition.compressed_id)),
       "id"          => alert_definition.compressed_id,
       "description" => alert_definition.description,
-      "guid"        => alert_definition.guid
+      "guid"        => alert_definition.guid,
+      "expression"  => {"exp" => {"=" => {"field" => "Vm-name", "value" => "foo"}}, "context_type" => nil}
     )
   end
 
@@ -115,19 +119,34 @@
   end
 
   it "edits an alert definition" do
-    sample_alert_definition = {
-      :description => "Test Alert Definition",
-      :db          => "ContainerNode",
-      :expression  => { :eval_method => "mw_heap_used", :mode => "internal", :options => {} },
-      :options     => { :notifications => {:delay_next_evaluation => 0, :evm_event => {} } },
-      :enabled     => true
+    api_basic_authorize(action_identifier(:alert_definitions, :edit, :resource_actions, :post))
+    alert_definition = FactoryGirl.create(
+      :miq_alert,
+      :expression => { :eval_method => "mw_heap_used", :mode => "internal", :options => {} },
+      :options    => { :notifications => {:delay_next_evaluation => 0, :evm_event => {} } }
+    )
+
+    run_post(
+      alert_definitions_url(alert_definition.id),
+      :action  => "edit",
+      :options => { :notifications => {:delay_next_evaluation => 60, :evm_event => {} } }
+    )
+
+    expected = {
+      "expression" => {
+        "eval_method" => "mw_heap_used",
+        "mode"        => "internal",
+        "options"     => {}
+      },
+      "options"    => {
+        "notifications" => {
+          "delay_next_evaluation" => 60,
+          "evm_event"             => {}
+        }
+      }
     }
-    updated_options = { :notifications => {:delay_next_evaluation => 60, :evm_event => {} } }
-    api_basic_authorize action_identifier(:alert_definitions, :edit, :resource_actions, :post)
-    alert_definition = FactoryGirl.create(:miq_alert, sample_alert_definition)
-    run_post(alert_definitions_url(alert_definition.id), gen_request(:edit, :options => updated_options))
     expect(response).to have_http_status(:ok)
-    expect(response.parsed_body["options"]).to eq(updated_options.deep_stringify_keys)
+    expect(response.parsed_body).to include(expected)
   end
 
   it "edits alert definitions" do