From 7edbf92f017f05ed0867d005ab2279f3b0cb1e71 Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Thu, 18 Sep 2014 12:25:53 +0900
Subject: [PATCH 1/8] Give user an option to drop pending events when enabling
 an agent.

This is shown in a confirmation dialog introduced for enable/disable
actions.
---
 app/controllers/agents_controller.rb       |  8 ++-
 app/views/agents/_action_menu.html.erb     | 58 +++++++++++++++++++++-
 spec/controllers/agents_controller_spec.rb | 12 +++++
 3 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/app/controllers/agents_controller.rb b/app/controllers/agents_controller.rb
index a459e8b5..d53cea95 100644
--- a/app/controllers/agents_controller.rb
+++ b/app/controllers/agents_controller.rb
@@ -122,7 +122,13 @@ class AgentsController < ApplicationController
     @agent = current_user.agents.find(params[:id])
 
     respond_to do |format|
-      if @agent.update_attributes(params[:agent])
+      if @agent.with_transaction_returning_status {
+          @agent.attributes = params[:agent]
+          if params[:drop_pending_events] && @agent.can_receive_events?
+            @agent.set_last_checked_event_id
+          end
+          @agent.save
+        }
         format.html { redirect_back "'#{@agent.name}' was successfully updated." }
         format.json { render json: @agent, status: :ok, location: agent_path(@agent) }
       else
diff --git a/app/views/agents/_action_menu.html.erb b/app/views/agents/_action_menu.html.erb
index 4a202f3d..4b3ed2bc 100644
--- a/app/views/agents/_action_menu.html.erb
+++ b/app/views/agents/_action_menu.html.erb
@@ -21,9 +21,13 @@
 
   <li>
     <% if agent.disabled? %>
-      <%= link_to '<i class="glyphicon glyphicon-play"></i> Enable agent'.html_safe, agent_path(agent, :agent => { :disabled => false }, :return => returnTo), :method => :put %>
+      <%= link_to '#', 'data-toggle' => 'modal', 'data-target' => "#confirm-enable-agent#{agent.id}" do %>
+        <i class="glyphicon glyphicon-play"></i> Enable agent
+      <% end %>
     <% else %>
-      <%= link_to '<i class="glyphicon glyphicon-pause"></i> Disable agent'.html_safe, agent_path(agent, :agent => { :disabled => true }, :return => returnTo), :method => :put %>
+      <%= link_to '#', 'data-toggle' => 'modal', 'data-target' => "#confirm-disable-agent#{agent.id}" do %>
+        <i class="glyphicon glyphicon-pause"></i> Disable agent
+      <% end %>
     <% end %>
   </li>
 
@@ -49,3 +53,53 @@
     <%= link_to '<span class="color-danger glyphicon glyphicon-remove"></span> Delete agent'.html_safe, agent_path(agent, :return => returnTo), method: :delete, data: { confirm: 'Are you sure that you want to permanently delete this Agent?' }, :tabindex => "-1" %>
   </li>
 </ul>
+
+<% if agent.disabled? %>
+<div id="confirm-enable-agent<%= agent.id %>" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="confirmEnableAgentLabel" aria-hidden="true">
+  <div class="modal-dialog modal-sm">
+    <div class="modal-content">
+      <div class="modal-header">
+        <button type="button" class="close" data-dismiss="modal"><span aria-hidden="true">&times;</span><span class="sr-only">Close</span></button>
+        <h4 class="modal-title">Confirm</h4>
+      </div>
+      <div class="modal-body">
+        <p>Enable &quot;<%= agent.name %>&quot;?</p>
+      </div>
+      <div class="modal-footer">
+        <%= form_for(agent, as: :agent, url: agent_path(agent, return: returnTo), method: 'PUT') do |f| %>
+          <% if agent.can_receive_events? || true %>
+            <div class="form-group">
+              <%= check_box_tag check_box_id = "agent#{agent.id}_drop_pending_events", 'true', false, name: :drop_pending_events %>
+              <%= label_tag check_box_id, 'Drop pending events' %>
+            </div>
+          <% end %>
+          <%= f.hidden_field :disabled, value: 'false' %>
+          <%= f.button 'No', class: 'btn btn-default', 'data-dismiss' => 'modal' %>
+          <%= f.submit 'Yes', class: 'btn btn-primary' %>
+        <% end %>
+      </div>
+    </div>
+  </div>
+</div>
+<% else %>
+<div id="confirm-disable-agent<%= agent.id %>" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="confirmDisableAgentLabel" aria-hidden="true">
+  <div class="modal-dialog modal-sm">
+    <div class="modal-content">
+      <div class="modal-header">
+        <button type="button" class="close" data-dismiss="modal"><span aria-hidden="true">&times;</span><span class="sr-only">Close</span></button>
+        <h4 class="modal-title">Confirm</h4>
+      </div>
+      <div class="modal-body">
+        <p>Disable &quot;<%= agent.name %>&quot;?</p>
+      </div>
+      <div class="modal-footer">
+        <%= form_for(agent, as: :agent, url: agent_path(agent, return: returnTo), method: 'PUT') do |f| %>
+          <%= f.hidden_field :disabled, value: 'true' %>
+          <%= f.button 'No', class: 'btn btn-default', 'data-dismiss' => 'modal' %>
+          <%= f.submit 'Yes', class: 'btn btn-primary' %>
+        <% end %>
+      </div>
+    </div>
+  </div>
+</div>
+<% end %>
diff --git a/spec/controllers/agents_controller_spec.rb b/spec/controllers/agents_controller_spec.rb
index bb77629f..06235b89 100644
--- a/spec/controllers/agents_controller_spec.rb
+++ b/spec/controllers/agents_controller_spec.rb
@@ -251,6 +251,18 @@ describe AgentsController do
         response.should redirect_to(agents_path)
       end
     end
+
+    it "updates last_checked_event_id when drop_pending_events is given" do
+      sign_in users(:bob)
+      agent = agents(:bob_website_agent)
+      agent.disabled = true
+      agent.last_checked_event_id = nil
+      agent.save!
+      post :update, id: agents(:bob_website_agent).to_param, agent: { disabled: 'false' }, drop_pending_events: true
+      agent.reload
+      agent.disabled.should == false
+      agent.last_checked_event_id.should == Event.maximum(:id)
+    end
   end
 
   describe "PUT leave_scenario" do

From c138a49824fe71dcda40436d8cd55be03a284352 Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Thu, 18 Sep 2014 14:11:19 +0900
Subject: [PATCH 2/8] Fix debug code.

---
 app/views/agents/_action_menu.html.erb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/agents/_action_menu.html.erb b/app/views/agents/_action_menu.html.erb
index 4b3ed2bc..cfe1ef9f 100644
--- a/app/views/agents/_action_menu.html.erb
+++ b/app/views/agents/_action_menu.html.erb
@@ -67,7 +67,7 @@
       </div>
       <div class="modal-footer">
         <%= form_for(agent, as: :agent, url: agent_path(agent, return: returnTo), method: 'PUT') do |f| %>
-          <% if agent.can_receive_events? || true %>
+          <% if agent.can_receive_events? %>
             <div class="form-group">
               <%= check_box_tag check_box_id = "agent#{agent.id}_drop_pending_events", 'true', false, name: :drop_pending_events %>
               <%= label_tag check_box_id, 'Drop pending events' %>

From edb87b5d388626085730397f35642a44b397bdb8 Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Thu, 18 Sep 2014 14:41:24 +0900
Subject: [PATCH 3/8] Reduce duplicates.

---
 app/views/agents/_action_menu.html.erb | 40 ++++++--------------------
 1 file changed, 8 insertions(+), 32 deletions(-)

diff --git a/app/views/agents/_action_menu.html.erb b/app/views/agents/_action_menu.html.erb
index cfe1ef9f..eb3d9802 100644
--- a/app/views/agents/_action_menu.html.erb
+++ b/app/views/agents/_action_menu.html.erb
@@ -20,12 +20,10 @@
   </li>
 
   <li>
-    <% if agent.disabled? %>
-      <%= link_to '#', 'data-toggle' => 'modal', 'data-target' => "#confirm-enable-agent#{agent.id}" do %>
+    <%= link_to '#', 'data-toggle' => 'modal', 'data-target' => "#confirm-agent#{agent.id}" do %>
+      <% if agent.disabled? %>
         <i class="glyphicon glyphicon-play"></i> Enable agent
-      <% end %>
-    <% else %>
-      <%= link_to '#', 'data-toggle' => 'modal', 'data-target' => "#confirm-disable-agent#{agent.id}" do %>
+      <% else %>
         <i class="glyphicon glyphicon-pause"></i> Disable agent
       <% end %>
     <% end %>
@@ -54,8 +52,7 @@
   </li>
 </ul>
 
-<% if agent.disabled? %>
-<div id="confirm-enable-agent<%= agent.id %>" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="confirmEnableAgentLabel" aria-hidden="true">
+<div id="confirm-agent<%= agent.id %>" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="confirmAgentLabel" aria-hidden="true">
   <div class="modal-dialog modal-sm">
     <div class="modal-content">
       <div class="modal-header">
@@ -63,17 +60,17 @@
         <h4 class="modal-title">Confirm</h4>
       </div>
       <div class="modal-body">
-        <p>Enable &quot;<%= agent.name %>&quot;?</p>
+        <p><% if agent.disabled? %>Enable<% else %>Disable<% end %> &quot;<%= agent.name %>&quot;?</p>
       </div>
       <div class="modal-footer">
         <%= form_for(agent, as: :agent, url: agent_path(agent, return: returnTo), method: 'PUT') do |f| %>
-          <% if agent.can_receive_events? %>
+          <% if agent.disabled && agent.can_receive_events? %>
             <div class="form-group">
               <%= check_box_tag check_box_id = "agent#{agent.id}_drop_pending_events", 'true', false, name: :drop_pending_events %>
               <%= label_tag check_box_id, 'Drop pending events' %>
             </div>
           <% end %>
-          <%= f.hidden_field :disabled, value: 'false' %>
+          <%= f.hidden_field :disabled, value: (!agent.disabled).to_s %>
           <%= f.button 'No', class: 'btn btn-default', 'data-dismiss' => 'modal' %>
           <%= f.submit 'Yes', class: 'btn btn-primary' %>
         <% end %>
@@ -81,25 +78,4 @@
     </div>
   </div>
 </div>
-<% else %>
-<div id="confirm-disable-agent<%= agent.id %>" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="confirmDisableAgentLabel" aria-hidden="true">
-  <div class="modal-dialog modal-sm">
-    <div class="modal-content">
-      <div class="modal-header">
-        <button type="button" class="close" data-dismiss="modal"><span aria-hidden="true">&times;</span><span class="sr-only">Close</span></button>
-        <h4 class="modal-title">Confirm</h4>
-      </div>
-      <div class="modal-body">
-        <p>Disable &quot;<%= agent.name %>&quot;?</p>
-      </div>
-      <div class="modal-footer">
-        <%= form_for(agent, as: :agent, url: agent_path(agent, return: returnTo), method: 'PUT') do |f| %>
-          <%= f.hidden_field :disabled, value: 'true' %>
-          <%= f.button 'No', class: 'btn btn-default', 'data-dismiss' => 'modal' %>
-          <%= f.submit 'Yes', class: 'btn btn-primary' %>
-        <% end %>
-      </div>
-    </div>
-  </div>
-</div>
-<% end %>
+

From b933a886eab6a037cc01ac5f61e01a3dfcdd0477 Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Thu, 18 Sep 2014 14:42:15 +0900
Subject: [PATCH 4/8] Add a tooltip for the "Drop pending events" option.

---
 app/assets/stylesheets/application.css.scss.erb | 4 ++++
 app/views/agents/_action_menu.html.erb          | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/assets/stylesheets/application.css.scss.erb b/app/assets/stylesheets/application.css.scss.erb
index fcff6585..b4b35df1 100644
--- a/app/assets/stylesheets/application.css.scss.erb
+++ b/app/assets/stylesheets/application.css.scss.erb
@@ -228,3 +228,7 @@ h2 .scenario, a span.label.scenario {
     display: none;
   }
 }
+
+.confirm-agent .popover {
+  width: 200px;
+}
diff --git a/app/views/agents/_action_menu.html.erb b/app/views/agents/_action_menu.html.erb
index eb3d9802..97ade3a6 100644
--- a/app/views/agents/_action_menu.html.erb
+++ b/app/views/agents/_action_menu.html.erb
@@ -52,7 +52,7 @@
   </li>
 </ul>
 
-<div id="confirm-agent<%= agent.id %>" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="confirmAgentLabel" aria-hidden="true">
+<div id="confirm-agent<%= agent.id %>" class="confirm-agent modal fade" tabindex="-1" role="dialog" aria-labelledby="confirmAgentLabel" aria-hidden="true">
   <div class="modal-dialog modal-sm">
     <div class="modal-content">
       <div class="modal-header">
@@ -68,6 +68,7 @@
             <div class="form-group">
               <%= check_box_tag check_box_id = "agent#{agent.id}_drop_pending_events", 'true', false, name: :drop_pending_events %>
               <%= label_tag check_box_id, 'Drop pending events' %>
+              <span class="glyphicon glyphicon-question-sign hover-help" data-content="As soon as you enable this agent, it starts to receive pending events that have not been processed while it was disabled.  To prevent that from happening, you can check this option."></span>
             </div>
           <% end %>
           <%= f.hidden_field :disabled, value: (!agent.disabled).to_s %>

From ea0fb2cf909e61ccdc3840c4b250ae472665b404 Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Thu, 18 Sep 2014 14:53:56 +0900
Subject: [PATCH 5/8] Make drop_pending_events a virtual attribute of Agent.

---
 app/controllers/agents_controller.rb       |  8 +-------
 app/models/agent.rb                        | 10 +++++++++-
 app/views/agents/_action_menu.html.erb     |  4 ++--
 spec/controllers/agents_controller_spec.rb |  2 +-
 spec/models/agent_spec.rb                  | 14 ++++++++++++++
 5 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/app/controllers/agents_controller.rb b/app/controllers/agents_controller.rb
index d53cea95..a459e8b5 100644
--- a/app/controllers/agents_controller.rb
+++ b/app/controllers/agents_controller.rb
@@ -122,13 +122,7 @@ class AgentsController < ApplicationController
     @agent = current_user.agents.find(params[:id])
 
     respond_to do |format|
-      if @agent.with_transaction_returning_status {
-          @agent.attributes = params[:agent]
-          if params[:drop_pending_events] && @agent.can_receive_events?
-            @agent.set_last_checked_event_id
-          end
-          @agent.save
-        }
+      if @agent.update_attributes(params[:agent])
         format.html { redirect_back "'#{@agent.name}' was successfully updated." }
         format.json { render json: @agent, status: :ok, location: agent_path(@agent) }
       else
diff --git a/app/models/agent.rb b/app/models/agent.rb
index 2a1d0f68..bb803d07 100644
--- a/app/models/agent.rb
+++ b/app/models/agent.rb
@@ -22,7 +22,7 @@ class Agent < ActiveRecord::Base
 
   EVENT_RETENTION_SCHEDULES = [["Forever", 0], ["1 day", 1], *([2, 3, 4, 5, 7, 14, 21, 30, 45, 90, 180, 365].map {|n| ["#{n} days", n] })]
 
-  attr_accessible :options, :memory, :name, :type, :schedule, :controller_ids, :control_target_ids, :disabled, :source_ids, :scenario_ids, :keep_events_for, :propagate_immediately
+  attr_accessible :options, :memory, :name, :type, :schedule, :controller_ids, :control_target_ids, :disabled, :source_ids, :scenario_ids, :keep_events_for, :propagate_immediately, :drop_pending_events
 
   json_serialize :options, :memory
 
@@ -196,6 +196,14 @@ class Agent < ActiveRecord::Base
     update_column :last_error_log_at, nil
   end
 
+  def drop_pending_events
+    false
+  end
+
+  def drop_pending_events=(bool)
+    set_last_checked_event_id if bool
+  end
+
   # Callbacks
 
   def set_default_schedule
diff --git a/app/views/agents/_action_menu.html.erb b/app/views/agents/_action_menu.html.erb
index 97ade3a6..bc415859 100644
--- a/app/views/agents/_action_menu.html.erb
+++ b/app/views/agents/_action_menu.html.erb
@@ -66,8 +66,8 @@
         <%= form_for(agent, as: :agent, url: agent_path(agent, return: returnTo), method: 'PUT') do |f| %>
           <% if agent.disabled && agent.can_receive_events? %>
             <div class="form-group">
-              <%= check_box_tag check_box_id = "agent#{agent.id}_drop_pending_events", 'true', false, name: :drop_pending_events %>
-              <%= label_tag check_box_id, 'Drop pending events' %>
+              <%= f.check_box :drop_pending_events %>
+              <%= f.label :drop_pending_events, 'Drop pending events' %>
               <span class="glyphicon glyphicon-question-sign hover-help" data-content="As soon as you enable this agent, it starts to receive pending events that have not been processed while it was disabled.  To prevent that from happening, you can check this option."></span>
             </div>
           <% end %>
diff --git a/spec/controllers/agents_controller_spec.rb b/spec/controllers/agents_controller_spec.rb
index 06235b89..e7ffb7ce 100644
--- a/spec/controllers/agents_controller_spec.rb
+++ b/spec/controllers/agents_controller_spec.rb
@@ -258,7 +258,7 @@ describe AgentsController do
       agent.disabled = true
       agent.last_checked_event_id = nil
       agent.save!
-      post :update, id: agents(:bob_website_agent).to_param, agent: { disabled: 'false' }, drop_pending_events: true
+      post :update, id: agents(:bob_website_agent).to_param, agent: { disabled: 'false', drop_pending_events: 'true' }
       agent.reload
       agent.disabled.should == false
       agent.last_checked_event_id.should == Event.maximum(:id)
diff --git a/spec/models/agent_spec.rb b/spec/models/agent_spec.rb
index 05379512..a1a10301 100644
--- a/spec/models/agent_spec.rb
+++ b/spec/models/agent_spec.rb
@@ -852,4 +852,18 @@ describe AgentDrop do
     interpolate(t, @wsa2).should eq('1: Formatter')
     interpolate(t, @efa).should eq('0: ')
   end
+
+  describe 'last_checked_event_id' do
+    it "should be updated by setting drop_pending_events to true" do
+      @wsa1.last_checked_event_id = nil
+      @wsa1.save!
+      @wsa1.update!(drop_pending_events: true)
+      @wsa1.reload.last_checked_event_id.should == Event.maximum(:id)
+    end
+
+    it "should not affect a virtual attribute drop_pending_events" do
+      @wsa1.update!(drop_pending_events: true)
+      @wsa1.reload.drop_pending_events.should == false
+    end
+  end
 end

From a7f5ee3051904dcd6aacdc596cf9ebdeaefe00c5 Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Thu, 18 Sep 2014 14:54:18 +0900
Subject: [PATCH 6/8] Do not update last_checked_event_id if an agent cannot
 receive events.

Use `maximum` while at it.
---
 app/models/agent.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/agent.rb b/app/models/agent.rb
index bb803d07..0f24e8fb 100644
--- a/app/models/agent.rb
+++ b/app/models/agent.rb
@@ -215,7 +215,7 @@ class Agent < ActiveRecord::Base
   end
 
   def set_last_checked_event_id
-    if newest_event_id = Event.order("id desc").limit(1).pluck(:id).first
+    if can_receive_events? && newest_event_id = Event.maximum(:id)
       self.last_checked_event_id = newest_event_id
     end
   end

From 4a06afafde5e47c3c4871936312dde73a03ae53b Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Thu, 18 Sep 2014 16:47:04 +0900
Subject: [PATCH 7/8] Move these specs to the right splace.

---
 spec/models/agent_spec.rb | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/spec/models/agent_spec.rb b/spec/models/agent_spec.rb
index a1a10301..8224727d 100644
--- a/spec/models/agent_spec.rb
+++ b/spec/models/agent_spec.rb
@@ -757,6 +757,22 @@ describe Agent do
       end
     end
   end
+
+  describe '.last_checked_event_id' do
+    it "should be updated by setting drop_pending_events to true" do
+      agent = agents(:bob_rain_notifier_agent)
+      agent.last_checked_event_id = nil
+      agent.save!
+      agent.update!(drop_pending_events: true)
+      agent.reload.last_checked_event_id.should == Event.maximum(:id)
+    end
+
+    it "should not affect a virtual attribute drop_pending_events" do
+      agent = agents(:bob_rain_notifier_agent)
+      agent.update!(drop_pending_events: true)
+      agent.reload.drop_pending_events.should == false
+    end
+  end
 end
 
 describe AgentDrop do
@@ -852,18 +868,4 @@ describe AgentDrop do
     interpolate(t, @wsa2).should eq('1: Formatter')
     interpolate(t, @efa).should eq('0: ')
   end
-
-  describe 'last_checked_event_id' do
-    it "should be updated by setting drop_pending_events to true" do
-      @wsa1.last_checked_event_id = nil
-      @wsa1.save!
-      @wsa1.update!(drop_pending_events: true)
-      @wsa1.reload.last_checked_event_id.should == Event.maximum(:id)
-    end
-
-    it "should not affect a virtual attribute drop_pending_events" do
-      @wsa1.update!(drop_pending_events: true)
-      @wsa1.reload.drop_pending_events.should == false
-    end
-  end
 end

From faed2845c687fd863ff90c545611a6f21d9edd91 Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Thu, 18 Sep 2014 17:08:02 +0900
Subject: [PATCH 8/8] Add a spec for how drop_pending_events= should work.

---
 spec/models/agent_spec.rb | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/spec/models/agent_spec.rb b/spec/models/agent_spec.rb
index 8224727d..af56e917 100644
--- a/spec/models/agent_spec.rb
+++ b/spec/models/agent_spec.rb
@@ -773,6 +773,43 @@ describe Agent do
       agent.reload.drop_pending_events.should == false
     end
   end
+
+  describe ".drop_pending_events" do
+    before do
+      stub_request(:any, /wunderground/).to_return(body: File.read(Rails.root.join("spec/data_fixtures/weather.json")), status: 200)
+      stub.any_instance_of(Agents::WeatherAgent).is_tomorrow?(anything) { true }
+    end
+
+    it "should drop pending events while the agent was disabled when set to true" do
+      agent1 = agents(:bob_weather_agent)
+      agent2 = agents(:bob_rain_notifier_agent)
+
+      -> {
+        -> {
+          Agent.async_check(agent1.id)
+          Agent.receive!
+        }.should change { agent1.events.count }.by(1)
+      }.should change { agent2.events.count }.by(1)
+
+      agent2.disabled = true
+      agent2.save!
+
+      -> {
+        -> {
+          Agent.async_check(agent1.id)
+          Agent.receive!
+        }.should change { agent1.events.count }.by(1)
+      }.should_not change { agent2.events.count }
+
+      agent2.disabled = false
+      agent2.drop_pending_events = true
+      agent2.save!
+
+      -> {
+        Agent.receive!
+      }.should_not change { agent2.events.count }
+    end
+  end
 end
 
 describe AgentDrop do