the import process now allows you to merge your agents with the incoming ones; next step is better UI

Andrew Cantino 10 years ago
parent
commit
827b62356a

+ 7 - 4
app/assets/javascripts/application.js.coffee.erb

@@ -9,14 +9,17 @@
9 9
 #= require ./worker-checker
10 10
 #= require_self
11 11
 
12
-window.setupJsonEditor = ($editor = $(".live-json-editor")) ->
12
+window.setupJsonEditor = ($editors = $(".live-json-editor")) ->
13 13
   JSONEditor.prototype.ADD_IMG = '<%= image_path 'json-editor/add.png' %>'
14 14
   JSONEditor.prototype.DELETE_IMG = '<%= image_path 'json-editor/delete.png' %>'
15
-  if $editor.length
15
+  editors = []
16
+  $editors.each ->
17
+    $editor = $(this)
16 18
     jsonEditor = new JSONEditor($editor, $editor.data('width') || 400, $editor.data('height') || 500)
17 19
     jsonEditor.doTruncation true
18 20
     jsonEditor.showFunctionButtons()
19
-    return jsonEditor
21
+    editors.push jsonEditor
22
+  return editors
20 23
 
21 24
 hideSchedule = ->
22 25
   $(".schedule-region select").hide()
@@ -55,7 +58,7 @@ showEventDescriptions = ->
55 58
 
56 59
 $(document).ready ->
57 60
   # JSON Editor
58
-  window.jsonEditor = setupJsonEditor()
61
+  window.jsonEditor = setupJsonEditor()[0]
59 62
 
60 63
   # Flash
61 64
   if $(".flash").length

+ 5 - 1
app/assets/stylesheets/application.css.scss.erb

@@ -140,7 +140,11 @@ span.not-applicable:after {
140 140
   opacity: 0.5;
141 141
 }
142 142
 
143
-// Fix JSON Editor
143
+// JSON Editor
144
+
145
+.live-json-editor {
146
+  font-family: "Courier New", Courier, monospace;
147
+}
144 148
 
145 149
 .json-editor blockquote {
146 150
   font-size: 14px;

+ 0 - 7
app/assets/stylesheets/scenarios.css.scss

@@ -1,9 +1,2 @@
1 1
 .scenario-import {
2
-  .danger {
3
-    color: red;
4
-    font-weight: strong;
5
-    border: 1px solid red;
6
-    padding: 10px;
7
-    margin: 10px 0;
8
-  }
9 2
 }

+ 2 - 7
app/controllers/scenario_imports_controller.rb

@@ -11,13 +11,8 @@ class ScenarioImportsController < ApplicationController
11 11
       render :text => 'Sorry, you cannot import a Scenario by URL from your own Huginn server.' and return
12 12
     end
13 13
 
14
-    if @scenario_import.valid?
15
-      if @scenario_import.do_import?
16
-        @scenario_import.import!
17
-        redirect_to @scenario_import.scenario, notice: "Import successful!"
18
-      else
19
-        render action: "new"
20
-      end
14
+    if @scenario_import.valid? && @scenario_import.should_import? && @scenario_import.import
15
+      redirect_to @scenario_import.scenario, notice: "Import successful!"
21 16
     else
22 17
       render action: "new"
23 18
     end

+ 138 - 19
app/models/scenario_import.rb

@@ -1,4 +1,7 @@
1
-# This is a helper class for managing Scenario imports.
1
+require 'ostruct'
2
+
3
+# This is a helper class for managing Scenario imports, used by the ScenarioImportsController.  This class behaves much
4
+# like a normal ActiveRecord object, with validations and callbacks.  However, it is never persisted to the database.
2 5
 class ScenarioImport
3 6
   include ActiveModel::Model
4 7
   include ActiveModel::Callbacks
@@ -7,7 +10,7 @@ class ScenarioImport
7 10
   DANGEROUS_AGENT_TYPES = %w[Agents::ShellCommandAgent]
8 11
   URL_REGEX = /\Ahttps?:\/\//i
9 12
 
10
-  attr_accessor :file, :url, :data, :do_import
13
+  attr_accessor :file, :url, :data, :do_import, :merges
11 14
 
12 15
   attr_reader :user
13 16
 
@@ -17,13 +20,14 @@ class ScenarioImport
17 20
   validate :validate_presence_of_file_url_or_data
18 21
   validates_format_of :url, :with => URL_REGEX, :allow_nil => true, :allow_blank => true, :message => "appears to be invalid"
19 22
   validate :validate_data
23
+  validate :generate_diff
20 24
 
21 25
   def step_one?
22 26
     data.blank?
23 27
   end
24 28
 
25 29
   def step_two?
26
-    valid?
30
+    data.present?
27 31
   end
28 32
 
29 33
   def set_user(user)
@@ -31,7 +35,7 @@ class ScenarioImport
31 35
   end
32 36
 
33 37
   def existing_scenario
34
-    @existing_scenario ||= user.scenarios.find_by_guid(parsed_data["guid"])
38
+    @existing_scenario ||= user.scenarios.find_by(:guid => parsed_data["guid"])
35 39
   end
36 40
 
37 41
   def dangerous?
@@ -39,18 +43,22 @@ class ScenarioImport
39 43
   end
40 44
 
41 45
   def parsed_data
42
-    @parsed_data ||= data && JSON.parse(data) rescue {}
46
+    @parsed_data ||= (data && JSON.parse(data) rescue {}) || {}
47
+  end
48
+
49
+  def agent_diffs
50
+    @agent_diffs || generate_diff
43 51
   end
44 52
 
45
-  def do_import?
53
+  def should_import?
46 54
     do_import == "1"
47 55
   end
48 56
 
49
-  def import!(options = {})
57
+  def import(options = {})
58
+    success = true
50 59
     guid = parsed_data['guid']
51 60
     description = parsed_data['description']
52 61
     name = parsed_data['name']
53
-    agents = parsed_data['agents']
54 62
     links = parsed_data['links']
55 63
     source_url = parsed_data['source_url'].presence || nil
56 64
     @scenario = user.scenarios.where(:guid => guid).first_or_initialize
@@ -58,17 +66,20 @@ class ScenarioImport
58 66
                                  :source_url => source_url, :public => false)
59 67
 
60 68
     unless options[:skip_agents]
61
-      created_agents = agents.map do |agent_data|
62
-        agent = @scenario.agents.find_by(:guid => agent_data['guid']) || Agent.build_for_type(agent_data['type'], user)
63
-        agent.guid = agent_data['guid']
64
-        agent.attributes = { :name => agent_data['name'],
65
-                             :schedule => agent_data['schedule'],
66
-                             :keep_events_for => agent_data['keep_events_for'],
67
-                             :propagate_immediately => agent_data['propagate_immediately'],
68
-                             :disabled => agent_data['disabled'],
69
-                             :options => agent_data['options'],
69
+      created_agents = agent_diffs.map do |agent_diff|
70
+        agent = agent_diff.agent || Agent.build_for_type("Agents::" + agent_diff.type.incoming, user)
71
+        agent.guid = agent_diff.guid.incoming
72
+        agent.attributes = { :name => agent_diff.name.updated,
73
+                             :disabled => agent_diff.disabled.updated, # == "true"
74
+                             :options => agent_diff.options.updated,
70 75
                              :scenario_ids => [@scenario.id] }
71
-        agent.save!
76
+        agent.schedule = agent_diff.schedule.updated if agent_diff.schedule.present?
77
+        agent.keep_events_for = agent_diff.keep_events_for.updated if agent_diff.keep_events_for.present?
78
+        agent.propagate_immediately = agent_diff.propagate_immediately.updated if agent_diff.propagate_immediately.present? # == "true"
79
+        unless agent.save
80
+          success = false
81
+          errors.add(:base, "Errors when saving '#{agent_diff.name.incoming}': #{agent.errors.full_messages.to_sentence}")
82
+        end
72 83
         agent
73 84
       end
74 85
 
@@ -78,6 +89,8 @@ class ScenarioImport
78 89
         receiver.sources << source unless receiver.sources.include?(source)
79 90
       end
80 91
     end
92
+
93
+    success
81 94
   end
82 95
 
83 96
   def scenario
@@ -119,4 +132,110 @@ class ScenarioImport
119 132
       errors.add(:base, "Please provide either a Scenario JSON File or a Public Scenario URL.")
120 133
     end
121 134
   end
122
-end
135
+
136
+  def generate_diff
137
+    @agent_diffs = (parsed_data['agents'] || []).map.with_index do |agent_data, index|
138
+      # AgentDiff is defined at the end of this file.
139
+      agent_diff = AgentDiff.new(agent_data)
140
+      if existing_scenario
141
+        # If this Agent exists already, update the AgentDiff with the local version's information.
142
+        agent_diff.diff_with! existing_scenario.agents.find_by(:guid => agent_data['guid'])
143
+
144
+        begin
145
+          # Update the AgentDiff with any hand-merged changes coming from the UI.  This only happens when this
146
+          # Agent already exists locally and has conflicting changes.
147
+          agent_diff.update_from! merges[index.to_s] if merges
148
+        rescue JSON::ParserError
149
+          errors.add(:base, "Your updated options for '#{agent_data['name']}' were unparsable.")
150
+        end
151
+      end
152
+      agent_diff
153
+    end
154
+  end
155
+
156
+  # AgentDiff is a helper object that encapsulates an incoming Agent.  All fields will be returned as an array
157
+  # of either one or two values.  The first value is the incoming value, the second is the existing value, if
158
+  # it differs from the incoming value.
159
+  class AgentDiff < OpenStruct
160
+    class FieldDiff
161
+      attr_accessor :incoming, :current, :updated
162
+
163
+      def initialize(incoming)
164
+        @incoming = incoming
165
+        @updated = incoming
166
+      end
167
+
168
+      def set_current(current)
169
+        @current = current
170
+        @requires_merge = (incoming != current)
171
+      end
172
+
173
+      def requires_merge?
174
+        @requires_merge
175
+      end
176
+    end
177
+
178
+    def initialize(agent_data)
179
+      super()
180
+      @requires_merge = false
181
+      self.agent = nil
182
+      store! agent_data
183
+    end
184
+
185
+    BASE_FIELDS = %w[name schedule keep_events_for propagate_immediately disabled guid]
186
+
187
+    def agent_exists?
188
+      !!agent
189
+    end
190
+
191
+    def requires_merge?
192
+      @requires_merge
193
+    end
194
+
195
+    def store!(agent_data)
196
+      self.type = FieldDiff.new(agent_data["type"].split("::").pop)
197
+      self.options = FieldDiff.new(agent_data['options'] || {})
198
+      BASE_FIELDS.each do |option|
199
+        self[option] = FieldDiff.new(agent_data[option]) if agent_data.has_key?(option)
200
+      end
201
+    end
202
+
203
+    def diff_with!(agent)
204
+      return unless agent.present?
205
+
206
+      self.agent = agent
207
+
208
+      type.set_current(agent.short_type)
209
+      options.set_current(agent.options || {})
210
+
211
+      @requires_merge ||= type.requires_merge?
212
+      @requires_merge ||= options.requires_merge?
213
+
214
+      BASE_FIELDS.each do |field|
215
+        next unless self[field].present?
216
+        self[field].set_current(agent.send(field))
217
+
218
+        @requires_merge ||= self[field].requires_merge?
219
+      end
220
+    end
221
+
222
+    def update_from!(merges)
223
+      each_field do |field, value, selection_options|
224
+        value.updated = merges[field]
225
+      end
226
+
227
+      if options.requires_merge?
228
+        options.updated = JSON.parse(merges['options'])
229
+      end
230
+    end
231
+
232
+    def each_field
233
+      boolean = [["True", "true"], ["False", "false"]]
234
+      yield 'name', name if name.requires_merge?
235
+      yield 'schedule', schedule, Agent::SCHEDULES.map {|s| [s.humanize.titleize, s] } if self['schedule'].present? && schedule.requires_merge?
236
+      yield 'keep_events_for', keep_events_for, Agent::EVENT_RETENTION_SCHEDULES if self['keep_events_for'].present? && keep_events_for.requires_merge?
237
+      yield 'propagate_immediately', propagate_immediately, boolean if self['propagate_immediately'].present? && propagate_immediately.requires_merge?
238
+      yield 'disabled', disabled, boolean if disabled.requires_merge?
239
+    end
240
+  end
241
+end

+ 1 - 1
app/views/agents/agent_views/manual_event_agent/_show.html.erb

@@ -14,7 +14,7 @@
14 14
 
15 15
 <script>
16 16
   $(function () {
17
-    var payloadJsonEditor = window.setupJsonEditor($(".payload-editor"));
17
+    var payloadJsonEditor = window.setupJsonEditor($(".payload-editor"))[0];
18 18
     $("#create-event-form").submit(function (e) {
19 19
       e.preventDefault();
20 20
       var $form = $("#create-event-form");

+ 26 - 17
app/views/scenario_imports/_step_one.html.erb

@@ -1,23 +1,32 @@
1
-<div class="page-header">
2
-  <h2>
3
-    Import a Public Scenario
4
-  </h2>
1
+<div class="row">
2
+  <div class="page-header">
3
+    <h2>
4
+      Import a Public Scenario
5
+    </h2>
6
+  </div>
5 7
 </div>
6 8
 
7
-<blockquote>You can import Scenarios, either from a <code>.json</code> file, or via a public Scenario URL.  When you import a Scenario, Huginn will keep track of where it came from and later let you update it.</blockquote>
9
+<div class='row'>
10
+  <blockquote>
11
+    You can import Scenarios, either from a <code>.json</code> file, or via a public Scenario URL. When you
12
+    import a Scenario, Huginn will keep track of where it came from and later let you update it.
13
+  </blockquote>
14
+</div>
8 15
 
9
-<div class="col-md-4">
10
-  <div class="form-group">
11
-    <%= f.label :url, 'Option 1: Provide a Public Scenario URL' %>
12
-    <%= f.text_field :url, :class => 'form-control', :placeholder => "Public Scenario URL" %>
13
-  </div>
16
+<div class='row'>
17
+  <div class="col-md-4">
18
+    <div class="form-group">
19
+      <%= f.label :url, 'Option 1: Provide a Public Scenario URL' %>
20
+      <%= f.text_field :url, :class => 'form-control', :placeholder => "Public Scenario URL" %>
21
+    </div>
14 22
 
15
-  <div class="form-group">
16
-    <%= f.label :file, 'Option 2: Upload a Scenario JSON File' %>
17
-    <%= f.file_field :file, :class => 'form-control' %>
18
-  </div>
23
+    <div class="form-group">
24
+      <%= f.label :file, 'Option 2: Upload a Scenario JSON File' %>
25
+      <%= f.file_field :file, :class => 'form-control' %>
26
+    </div>
19 27
 
20
-  <div class='form-actions'>
21
-    <%= f.submit "Start Import", :class => "btn btn-primary" %>
28
+    <div class='form-actions'>
29
+      <%= f.submit "Start Import", :class => "btn btn-primary" %>
30
+    </div>
22 31
   </div>
23
-</div>
32
+</div>

+ 142 - 50
app/views/scenario_imports/_step_two.html.erb

@@ -1,60 +1,152 @@
1
-<div class="col-md-12">
2
-  <div class="page-header">
3
-    <h2><%= @scenario_import.parsed_data["name"] %> (exported <%= time_ago_in_words Time.parse(@scenario_import.parsed_data["exported_at"]) %> ago)</h2>
4
-  </div>
5
-
6
-  <% if @scenario_import.parsed_data["description"].present? %>
7
-    <blockquote><%= @scenario_import.parsed_data["description"] %></blockquote>
8
-  <% end %>
1
+<div class="row">
2
+  <div class="col-md-12">
3
+    <% if @scenario_import.dangerous? %>
4
+      <div class="alert alert-danger">
5
+        <span class='glyphicon glyphicon-warning-sign'></span>
6
+        This Scenario contains one or more potentially dangerous Agents.
7
+        These may be able to run local commands or execute code.
8
+        Please be sure that you understand the Agent configurations before importing!
9
+      </div>
10
+    <% end %>
9 11
 
10
-  <p>
11
-    This import contains <%= pluralize @scenario_import.parsed_data["agents"].length, "Agent" %>:
12
-  </p>
13
-
14
-  <ul class='agent-import-list'>
15
-    <% @scenario_import.parsed_data["agents"].each do |agent_data| %>
16
-      <li>
17
-        <%= link_to agent_data['name'], '#', :class => 'options-toggle' %>
18
-        <span class='text-muted'>
19
-          (<%= agent_data["type"].split("::").pop.titleize %>)
20
-        </span>
21
-        <pre class='options' style='display: none;'><%= Utils.pretty_jsonify agent_data["options"] || {} %></pre>
22
-      </li>
12
+    <% if @scenario_import.existing_scenario.present? %>
13
+      <div class="alert alert-warning">
14
+        <span class='glyphicon glyphicon-warning-sign'></span>
15
+        This Scenario already exists in your system. The import will update your existing
16
+        <span class='label label-info scenario'><%= @scenario_import.existing_scenario.name %></span> Scenario's title
17
+        and
18
+        description. Below you can customize how the individual agents get updated.
19
+      </div>
23 20
     <% end %>
24
-  </ul>
25
-
26
-  <script>
27
-    $(function() {
28
-      $('.agent-import-list .options-toggle').on('click', function(e) {
29
-        e.preventDefault();
30
-        $(this).siblings('.options').fadeToggle();
31
-      });
32
-    });
33
-  </script>
34
-
35
-  <% if @scenario_import.dangerous? %>
36
-    <div class="danger">
37
-      This Scenario contains one or more potentially dangerous Agents.
38
-      These may be able to run local commands or execute code.
39
-      Please be sure that you understand the above Agent configurations before importing!
40
-    </div>
41
-  <% end %>
42 21
 
43
-  <% if @scenario_import.existing_scenario.present? %>
44
-    <div class="danger">
45
-      This Scenario already exists in your system.
46
-      If you continue, the import will overwrite your existing
47
-      <span class='label label-info scenario'><%= @scenario_import.existing_scenario.name %></span> Scenario and the Agents in it.
22
+    <div class="page-header">
23
+      <h2>
24
+        <%= @scenario_import.parsed_data["name"] %>
25
+        <span class='text-muted'>(<%= pluralize @scenario_import.parsed_data["agents"].length, "Agent" %>
26
+          ; exported <%= time_ago_in_words Time.parse(@scenario_import.parsed_data["exported_at"]) %> ago)</span>
27
+      </h2>
48 28
     </div>
49
-  <% end %>
50 29
 
51
-  <div class="checkbox">
52
-    <%= f.label :do_import do %>
53
-      <%= f.check_box :do_import %> I confirm that I want to import these Agents.
30
+    <% if @scenario_import.parsed_data["description"].present? %>
31
+      <blockquote><%= @scenario_import.parsed_data["description"] %></blockquote>
54 32
     <% end %>
33
+
55 34
   </div>
35
+</div>
36
+
37
+<div class='agent-import-list'>
38
+  <% @scenario_import.agent_diffs.each.with_index do |agent_diff, index| %>
39
+    <div class='agent-import' data-index='<%= index %>'>
40
+
41
+      <div class='row'>
42
+        <div class='col-md-12'>
43
+          <h3>
44
+            <a href='#' data-toggle="modal" data-target="#agent_options_<%= index %>"><%= agent_diff.name.incoming %></a>
45
+            <span class='text-muted'>
46
+              (<%= agent_diff.type.incoming %><% " -- WARNING: this Agent's type has been changed.  This import will likely fail!" if agent_diff.type.requires_merge? %>)
47
+            </span>
48
+          </h3>
49
+
50
+          <% if agent_diff.agent_exists? %>
51
+            <div>
52
+              This Agent exists in your Huginn system.
53
+
54
+              <% if agent_diff.requires_merge? %>
55
+                Here are the differences between the incoming version and the one you have now. For each field, please
56
+                select which value you'd like to keep.
57
+              <% else %>
58
+                It's already up-to-date.
59
+              <% end %>
60
+            </div>
61
+          <% end %>
62
+        </div>
63
+      </div>
56 64
 
57
-  <div class='form-actions'>
58
-    <%= f.submit "Finish Import", :class => "btn btn-primary" %>
65
+      <div class="modal fade" id="agent_options_<%= index %>" tabindex="-1" role="dialog" aria-labelledby="modalLabel<%= index %>" aria-hidden="true">
66
+        <div class="modal-dialog modal-lg">
67
+          <div class="modal-content">
68
+            <div class="modal-header">
69
+              <button type="button" class="close" data-dismiss="modal" aria-hidden="true">&times;</button>
70
+              <h4 class="modal-title" id="modalLabel<%= index %>">Options for '<%= agent_diff.name.updated %>'</h4>
71
+            </div>
72
+            <div class="modal-body">
73
+              <pre class='options'><%= Utils.pretty_jsonify agent_diff.options.incoming %></pre>
74
+            </div>
75
+          </div>
76
+        </div>
77
+      </div>
78
+
79
+      <% agent_diff.each_field do |field, value, selection_options| %>
80
+        <div class='row'>
81
+          <div class='col-md-4'>
82
+            <div class="form-group">
83
+              <%= label_tag "scenario_import[merges][#{index}][#{field}]", field.titleize %>
84
+              <% if selection_options.present? %>
85
+                <div>
86
+                  Your current Agent's value is:
87
+                  <span class='current'><%= selection_options.find { |s| s.last.to_s == value.current.to_s }.first %></span>
88
+                </div>
89
+                <%= select_tag "scenario_import[merges][#{index}][#{field}]", options_for_select(selection_options, value.updated), :class => 'form-control' %>
90
+              <% else %>
91
+                <div>
92
+                  Your current Agent's value is: <span class='current'><%= value.current.to_s %></span>
93
+                </div>
94
+                <%= text_field_tag "scenario_import[merges][#{index}][#{field}]", value.updated, :class => 'form-control' %>
95
+              <% end %>
96
+            </div>
97
+          </div>
98
+        </div>
99
+      <% end %>
100
+
101
+      <div class='row'>
102
+        <% if agent_diff.options.requires_merge? %>
103
+          <div class='col-md-12'>
104
+            <label>Options</label>
105
+          </div>
106
+
107
+          <div class='col-md-6'>
108
+            <textarea name="scenario_import[merges][<%= index %>][options]" rows='10' class="form-control live-json-editor">
109
+              <%= Utils.pretty_jsonify(agent_diff.options.updated) %>
110
+            </textarea>
111
+          </div>
112
+
113
+          <div class='col-md-6'>
114
+            <div>
115
+              Your current options:
116
+            </div>
117
+            <pre class='options'><%= Utils.pretty_jsonify agent_diff.options.current %></pre>
118
+          </div>
119
+        <% end %>
120
+      </div>
121
+    </div>
122
+  <% end %>
123
+</div>
124
+
125
+<div class='row'>
126
+  <div class='col-md-12'>
127
+    <div class="checkbox">
128
+      <%= f.label :do_import do %>
129
+        <%= f.check_box :do_import %> I confirm that I want to import these Agents.
130
+      <% end %>
131
+    </div>
132
+
133
+    <div class='form-actions'>
134
+      <%= f.submit "Finish Import", :class => "btn btn-primary" %>
135
+    </div>
59 136
   </div>
60 137
 </div>
138
+
139
+
140
+<script>
141
+//  $(function() {
142
+//    $('.agent-import-list .options-toggle').on('click', function (e) {
143
+//      e.preventDefault();
144
+//      $(this).siblings('.options').slideToggle()
145
+//      if ($(this).text() == "Show Options") {
146
+//        $(this).text("Hide Options");
147
+//      } else {
148
+//        $(this).text("Show Options");
149
+//      }
150
+//    });
151
+//  });
152
+</script>

+ 16 - 18
app/views/scenario_imports/new.html.erb

@@ -1,6 +1,6 @@
1 1
 <div class='container scenario-import'>
2
-  <div class='row'>
3
-    <div class='col-md-12'>
2
+  <div class="row">
3
+    <div class="col-md-12">
4 4
       <% if @scenario_import.errors.any? %>
5 5
         <div class="row well">
6 6
           <h2><%= pluralize(@scenario_import.errors.count, "error") %> prohibited this Scenario from being imported:</h2>
@@ -9,26 +9,24 @@
9 9
           <% end %>
10 10
         </div>
11 11
       <% end %>
12
+    </div>
13
+  </div>
12 14
 
13
-      <%= form_for @scenario_import, :multipart => true do |f| %>
14
-        <%= f.hidden_field :data %>
15
+  <%= form_for @scenario_import, :multipart => true do |f| %>
16
+    <%= f.hidden_field :data %>
15 17
 
16
-        <div class="row">
17
-          <% if @scenario_import.step_one? %>
18
-            <%= render 'step_one', :f => f %>
19
-          <% elsif @scenario_import.step_two? %>
20
-            <%= render 'step_two', :f => f %>
21
-          <% end %>
22
-        </div>
23
-      <% end %>
18
+    <% if @scenario_import.step_one? %>
19
+      <%= render 'step_one', :f => f %>
20
+    <% elsif @scenario_import.step_two? %>
21
+      <%= render 'step_two', :f => f %>
22
+    <% end %>
23
+  <% end %>
24 24
 
25
-      <hr />
25
+  <hr />
26 26
 
27
-      <div class="row">
28
-        <div class="col-md-12">
29
-          <%= link_to '<span class="glyphicon glyphicon-chevron-left"></span> Back'.html_safe, scenarios_path, class: "btn btn-default" %>
30
-        </div>
31
-      </div>
27
+  <div class="row">
28
+    <div class="col-md-12">
29
+      <%= link_to '<span class="glyphicon glyphicon-chevron-left"></span> Back'.html_safe, scenarios_path, class: "btn btn-default" %>
32 30
     </div>
33 31
   </div>
34 32
 </div>

+ 5 - 4
lib/agents_exporter.rb

@@ -42,12 +42,13 @@ class AgentsExporter
42 42
     {
43 43
       :type => agent.type,
44 44
       :name => agent.name,
45
-      :schedule => agent.schedule,
46
-      :keep_events_for => agent.keep_events_for,
47
-      :propagate_immediately => agent.propagate_immediately,
48 45
       :disabled => agent.disabled,
49 46
       :guid => agent.guid,
50 47
       :options => agent.options
51
-    }
48
+    }.tap do |options|
49
+      options[:schedule] = agent.schedule if agent.can_be_scheduled?
50
+      options[:keep_events_for] = agent.keep_events_for if agent.can_create_events?
51
+      options[:propagate_immediately] = agent.propagate_immediately if agent.can_receive_events?
52
+    end
52 53
   end
53 54
 end

+ 1 - 4
spec/controllers/scenario_imports_controller_spec.rb

@@ -1,10 +1,6 @@
1 1
 require 'spec_helper'
2 2
 
3 3
 describe ScenarioImportsController do
4
-  def valid_attributes(options = {})
5
-    { :name => "some_name" }.merge(options)
6
-  end
7
-
8 4
   before do
9 5
     sign_in users(:bob)
10 6
   end
@@ -22,6 +18,7 @@ describe ScenarioImportsController do
22 18
       post :create, :scenario_import => { :url => "bad url" }
23 19
       assigns(:scenario_import).user.should == users(:bob)
24 20
       assigns(:scenario_import).url.should == "bad url"
21
+      assigns(:scenario_import).should_not be_valid
25 22
       response.should render_template(:new)
26 23
     end
27 24
   end

+ 4 - 1
spec/lib/agents_exporter_spec.rb

@@ -21,9 +21,12 @@ describe AgentsExporter do
21 21
       data[:links].should == [{ :source => 0, :receiver => 1 }]
22 22
       data[:agents].should == agent_list.map { |agent| exporter.agent_as_json(agent) }
23 23
       data[:agents].all? { |agent_json| agent_json[:guid].present? && agent_json[:type].present? && agent_json[:name].present? }.should be_true
24
+
25
+      data[:agents][0].should_not have_key(:propagate_immediately) # can't receive events
26
+      data[:agents][1].should_not have_key(:schedule) # can't be scheduled
24 27
     end
25 28
 
26
-    it "does not output links to other agents" do
29
+    it "does not output links to other agents outside of the incoming set" do
27 30
       Link.create!(:source_id => agents(:jane_weather_agent).id, :receiver_id => agents(:jane_website_agent).id)
28 31
       Link.create!(:source_id => agents(:jane_website_agent).id, :receiver_id => agents(:jane_rain_notifier_agent).id)
29 32
 

+ 263 - 112
spec/models/scenario_import_spec.rb

@@ -1,6 +1,7 @@
1 1
 require 'spec_helper'
2 2
 
3 3
 describe ScenarioImport do
4
+  let(:user) { users(:bob) }
4 5
   let(:guid) { "somescenarioguid" }
5 6
   let(:description) { "This is a cool Huginn Scenario that does something useful!" }
6 7
   let(:name) { "A useful Scenario" }
@@ -22,6 +23,28 @@ describe ScenarioImport do
22 23
       'message' => "Looks like rain!"
23 24
     }
24 25
   }
26
+  let(:valid_parsed_weather_agent_data) do
27
+    {
28
+      :type => "Agents::WeatherAgent",
29
+      :name => "a weather agent",
30
+      :schedule => "5pm",
31
+      :keep_events_for => 14,
32
+      :disabled => true,
33
+      :guid => "a-weather-agent",
34
+      :options => weather_agent_options
35
+    }
36
+  end
37
+  let(:valid_parsed_trigger_agent_data) do
38
+    {
39
+      :type => "Agents::TriggerAgent",
40
+      :name => "listen for weather",
41
+      :keep_events_for => 0,
42
+      :propagate_immediately => true,
43
+      :disabled => false,
44
+      :guid => "a-trigger-agent",
45
+      :options => trigger_agent_options
46
+    }
47
+  end
25 48
   let(:valid_parsed_data) do
26 49
     { 
27 50
       :name => name,
@@ -30,26 +53,8 @@ describe ScenarioImport do
30 53
       :source_url => source_url,
31 54
       :exported_at => 2.days.ago.utc.iso8601,
32 55
       :agents => [
33
-        {
34
-          :type => "Agents::WeatherAgent",
35
-          :name => "a weather agent",
36
-          :schedule => "5pm",
37
-          :keep_events_for => 14,
38
-          :propagate_immediately => false,
39
-          :disabled => false,
40
-          :guid => "a-weather-agent",
41
-          :options => weather_agent_options
42
-        },
43
-        {
44
-          :type => "Agents::TriggerAgent",
45
-          :name => "listen for weather",
46
-          :schedule => nil,
47
-          :keep_events_for => 0,
48
-          :propagate_immediately => true,
49
-          :disabled => true,
50
-          :guid => "a-trigger-agent",
51
-          :options => trigger_agent_options
52
-        }
56
+        valid_parsed_weather_agent_data,
57
+        valid_parsed_trigger_agent_data
53 58
       ],
54 59
       :links => [
55 60
         { :source => 0, :receiver => 1 }
@@ -66,7 +71,11 @@ describe ScenarioImport do
66 71
   end
67 72
 
68 73
   describe "validations" do
69
-    subject { ScenarioImport.new }
74
+    subject do
75
+      _import = ScenarioImport.new
76
+      _import.set_user(user)
77
+      _import
78
+    end
70 79
 
71 80
     it "is not valid when none of file, url, or data are present" do
72 81
       subject.should_not be_valid
@@ -145,7 +154,7 @@ describe ScenarioImport do
145 154
     end
146 155
   end
147 156
 
148
-  describe "#import!" do
157
+  describe "#import and #generate_diff" do
149 158
     let(:scenario_import) do
150 159
       _import = ScenarioImport.new(:data => valid_data)
151 160
       _import.set_user users(:bob)
@@ -153,107 +162,249 @@ describe ScenarioImport do
153 162
     end
154 163
 
155 164
     context "when this scenario has never been seen before" do
156
-      it "makes a new scenario" do
157
-        lambda {
158
-          scenario_import.import!(:skip_agents => true)
159
-        }.should change { users(:bob).scenarios.count }.by(1)
160
-
161
-        scenario_import.scenario.name.should == name
162
-        scenario_import.scenario.description.should == description
163
-        scenario_import.scenario.guid.should == guid
164
-        scenario_import.scenario.source_url.should == source_url
165
-        scenario_import.scenario.public.should be_false
166
-      end
167
-
168
-      it "creates the Agents" do
169
-        lambda {
170
-          scenario_import.import!
171
-        }.should change { users(:bob).agents.count }.by(2)
172
-
173
-        weather_agent = scenario_import.scenario.agents.find_by(:guid => "a-weather-agent")
174
-        trigger_agent = scenario_import.scenario.agents.find_by(:guid => "a-trigger-agent")
175
-
176
-        weather_agent.name.should == "a weather agent"
177
-        weather_agent.schedule.should == "5pm"
178
-        weather_agent.keep_events_for.should == 14
179
-        weather_agent.propagate_immediately.should be_false
180
-        weather_agent.should_not be_disabled
181
-        weather_agent.memory.should be_empty
182
-        weather_agent.options.should == weather_agent_options
183
-
184
-        trigger_agent.name.should == "listen for weather"
185
-        trigger_agent.sources.should == [weather_agent]
186
-        trigger_agent.schedule.should be_nil
187
-        trigger_agent.keep_events_for.should == 0
188
-        trigger_agent.propagate_immediately.should be_true
189
-        trigger_agent.should be_disabled
190
-        trigger_agent.memory.should be_empty
191
-        trigger_agent.options.should == trigger_agent_options
165
+      describe "#import" do
166
+        it "makes a new scenario" do
167
+          lambda {
168
+            scenario_import.import(:skip_agents => true)
169
+          }.should change { users(:bob).scenarios.count }.by(1)
170
+
171
+          scenario_import.scenario.name.should == name
172
+          scenario_import.scenario.description.should == description
173
+          scenario_import.scenario.guid.should == guid
174
+          scenario_import.scenario.source_url.should == source_url
175
+          scenario_import.scenario.public.should be_false
176
+        end
177
+
178
+        it "creates the Agents" do
179
+          lambda {
180
+            scenario_import.import
181
+          }.should change { users(:bob).agents.count }.by(2)
182
+
183
+          weather_agent = scenario_import.scenario.agents.find_by(:guid => "a-weather-agent")
184
+          trigger_agent = scenario_import.scenario.agents.find_by(:guid => "a-trigger-agent")
185
+
186
+          weather_agent.name.should == "a weather agent"
187
+          weather_agent.schedule.should == "5pm"
188
+          weather_agent.keep_events_for.should == 14
189
+          weather_agent.propagate_immediately.should be_false
190
+          weather_agent.should be_disabled
191
+          weather_agent.memory.should be_empty
192
+          weather_agent.options.should == weather_agent_options
193
+
194
+          trigger_agent.name.should == "listen for weather"
195
+          trigger_agent.sources.should == [weather_agent]
196
+          trigger_agent.schedule.should be_nil
197
+          trigger_agent.keep_events_for.should == 0
198
+          trigger_agent.propagate_immediately.should be_true
199
+          trigger_agent.should_not be_disabled
200
+          trigger_agent.memory.should be_empty
201
+          trigger_agent.options.should == trigger_agent_options
202
+        end
203
+
204
+        it "creates new Agents, even if one already exists with the given guid (so that we don't overwrite a user's work outside of the scenario)" do
205
+          agents(:bob_weather_agent).update_attribute :guid, "a-weather-agent"
206
+
207
+          lambda {
208
+            scenario_import.import
209
+          }.should change { users(:bob).agents.count }.by(2)
210
+        end
192 211
       end
193 212
 
194
-      it "creates new Agents, even if one already exists with the given guid (so that we don't overwrite a user's work outside of the scenario)" do
195
-        agents(:bob_weather_agent).update_attribute :guid, "a-weather-agent"
196
-
197
-        lambda {
198
-          scenario_import.import!
199
-        }.should change { users(:bob).agents.count }.by(2)
213
+      describe "#generate_diff" do
214
+        it "returns AgentDiff objects for the incoming Agents" do
215
+          scenario_import.should be_valid
216
+
217
+          agent_diffs = scenario_import.agent_diffs
218
+
219
+          weather_agent_diff = agent_diffs[0]
220
+          trigger_agent_diff = agent_diffs[1]
221
+
222
+          valid_parsed_weather_agent_data.each do |key, value|
223
+            if key == :type
224
+              value = value.split("::").last
225
+            end
226
+            weather_agent_diff.should respond_to(key)
227
+            field = weather_agent_diff.send(key)
228
+            field.should be_a(ScenarioImport::AgentDiff::FieldDiff)
229
+            field.incoming.should == value
230
+            field.updated.should == value
231
+            field.current.should be_nil
232
+          end
233
+          weather_agent_diff.should_not respond_to(:propagate_immediately)
234
+
235
+          valid_parsed_trigger_agent_data.each do |key, value|
236
+            if key == :type
237
+              value = value.split("::").last
238
+            end
239
+            trigger_agent_diff.should respond_to(key)
240
+            field = trigger_agent_diff.send(key)
241
+            field.should be_a(ScenarioImport::AgentDiff::FieldDiff)
242
+            field.incoming.should == value
243
+            field.updated.should == value
244
+            field.current.should be_nil
245
+          end
246
+          trigger_agent_diff.should_not respond_to(:schedule)
247
+        end
200 248
       end
201 249
     end
202 250
 
203 251
     context "when an a scenario already exists with the given guid" do
204
-      let!(:existing_scenario) {
205
-        _existing_scenerio = users(:bob).scenarios.build(:name => "an existing scenario")
252
+      let!(:existing_scenario) do
253
+        _existing_scenerio = users(:bob).scenarios.build(:name => "an existing scenario", :description => "something")
206 254
         _existing_scenerio.guid = guid
207 255
         _existing_scenerio.save!
256
+
257
+        agents(:bob_weather_agent).update_attribute :guid, "a-weather-agent"
258
+        agents(:bob_weather_agent).scenarios << _existing_scenerio
259
+
208 260
         _existing_scenerio
209
-      }
210
-
211
-      it "uses the existing scenario, updating it's data" do
212
-        lambda {
213
-          scenario_import.import!(:skip_agents => true)
214
-          scenario_import.scenario.should == existing_scenario
215
-        }.should_not change { users(:bob).scenarios.count }
216
-
217
-        existing_scenario.reload
218
-        existing_scenario.guid.should == guid
219
-        existing_scenario.description.should == description
220
-        existing_scenario.name.should == name
221
-        existing_scenario.source_url.should == source_url
222
-        existing_scenario.public.should be_false
223 261
       end
224 262
 
225
-      it "updates any existing agents in the scenario, and makes new ones as needed" do
226
-        agents(:bob_weather_agent).update_attribute :guid, "a-weather-agent"
227
-        agents(:bob_weather_agent).scenarios << existing_scenario
228
-
229
-        lambda {
230
-          # Shouldn't matter how many times we do it!
231
-          scenario_import.import!
232
-          scenario_import.import!
233
-          scenario_import.import!
234
-        }.should change { users(:bob).agents.count }.by(1)
235
-
236
-        weather_agent = existing_scenario.agents.find_by(:guid => "a-weather-agent")
237
-        trigger_agent = existing_scenario.agents.find_by(:guid => "a-trigger-agent")
238
-
239
-        weather_agent.should == agents(:bob_weather_agent)
240
-
241
-        weather_agent.name.should == "a weather agent"
242
-        weather_agent.schedule.should == "5pm"
243
-        weather_agent.keep_events_for.should == 14
244
-        weather_agent.propagate_immediately.should be_false
245
-        weather_agent.should_not be_disabled
246
-        weather_agent.memory.should be_empty
247
-        weather_agent.options.should == weather_agent_options
248
-
249
-        trigger_agent.name.should == "listen for weather"
250
-        trigger_agent.sources.should == [weather_agent]
251
-        trigger_agent.schedule.should be_nil
252
-        trigger_agent.keep_events_for.should == 0
253
-        trigger_agent.propagate_immediately.should be_true
254
-        trigger_agent.should be_disabled
255
-        trigger_agent.memory.should be_empty
256
-        trigger_agent.options.should == trigger_agent_options
263
+      describe "#import" do
264
+        it "uses the existing scenario, updating its data" do
265
+          lambda {
266
+            scenario_import.import(:skip_agents => true)
267
+            scenario_import.scenario.should == existing_scenario
268
+          }.should_not change { users(:bob).scenarios.count }
269
+
270
+          existing_scenario.reload
271
+          existing_scenario.guid.should == guid
272
+          existing_scenario.description.should == description
273
+          existing_scenario.name.should == name
274
+          existing_scenario.source_url.should == source_url
275
+          existing_scenario.public.should be_false
276
+        end
277
+
278
+        it "updates any existing agents in the scenario, and makes new ones as needed" do
279
+          scenario_import.should be_valid
280
+
281
+          lambda {
282
+            scenario_import.import
283
+          }.should change { users(:bob).agents.count }.by(1) # One, because the weather agent already existed.
284
+
285
+          weather_agent = existing_scenario.agents.find_by(:guid => "a-weather-agent")
286
+          trigger_agent = existing_scenario.agents.find_by(:guid => "a-trigger-agent")
287
+
288
+          weather_agent.should == agents(:bob_weather_agent)
289
+
290
+          weather_agent.name.should == "a weather agent"
291
+          weather_agent.schedule.should == "5pm"
292
+          weather_agent.keep_events_for.should == 14
293
+          weather_agent.propagate_immediately.should be_false
294
+          weather_agent.should be_disabled
295
+          weather_agent.memory.should be_empty
296
+          weather_agent.options.should == weather_agent_options
297
+
298
+          trigger_agent.name.should == "listen for weather"
299
+          trigger_agent.sources.should == [weather_agent]
300
+          trigger_agent.schedule.should be_nil
301
+          trigger_agent.keep_events_for.should == 0
302
+          trigger_agent.propagate_immediately.should be_true
303
+          trigger_agent.should_not be_disabled
304
+          trigger_agent.memory.should be_empty
305
+          trigger_agent.options.should == trigger_agent_options
306
+        end
307
+
308
+        it "honors updates coming from the UI" do
309
+          scenario_import.merges = {
310
+            "0" => {
311
+              "name" => "updated name",
312
+              "schedule" => "6pm",
313
+              "keep_events_for" => "2",
314
+              "disabled" => "false",
315
+              "options" => weather_agent_options.merge("api_key" => "foo").to_json
316
+            }
317
+          }
318
+
319
+          scenario_import.should be_valid
320
+
321
+          scenario_import.import.should be_true
322
+
323
+          weather_agent = existing_scenario.agents.find_by(:guid => "a-weather-agent")
324
+          weather_agent.name.should == "updated name"
325
+          weather_agent.schedule.should == "6pm"
326
+          weather_agent.keep_events_for.should == 2
327
+          weather_agent.should_not be_disabled
328
+          weather_agent.options.should == weather_agent_options.merge("api_key" => "foo")
329
+        end
330
+
331
+        it "adds errors when updated agents are invalid" do
332
+          scenario_import.merges = {
333
+            "0" => {
334
+              "name" => "",
335
+              "schedule" => "foo",
336
+              "keep_events_for" => "2",
337
+              "options" => weather_agent_options.merge("api_key" => "").to_json
338
+            }
339
+          }
340
+
341
+          scenario_import.import.should be_false
342
+
343
+          errors = scenario_import.errors.full_messages.to_sentence
344
+          errors.should =~ /Name can't be blank/
345
+          errors.should =~ /api_key is required/
346
+          errors.should =~ /Schedule is not a valid schedule/
347
+        end
348
+      end
349
+
350
+      describe "#generate_diff" do
351
+        it "returns AgentDiff objects that include 'current' values from any agents that already exist" do
352
+          agent_diffs = scenario_import.agent_diffs
353
+          weather_agent_diff = agent_diffs[0]
354
+          trigger_agent_diff = agent_diffs[1]
355
+
356
+          # Already exists
357
+          weather_agent_diff.agent.should == agents(:bob_weather_agent)
358
+          valid_parsed_weather_agent_data.each do |key, value|
359
+            next if key == :type
360
+            weather_agent_diff.send(key).current.should == agents(:bob_weather_agent).send(key)
361
+          end
362
+
363
+          # Doesn't exist yet
364
+          valid_parsed_trigger_agent_data.each do |key, value|
365
+            trigger_agent_diff.send(key).current.should be_nil
366
+          end
367
+        end
368
+
369
+        it "sets the 'updated' FieldDiff values based on any feedback from the user" do
370
+          scenario_import.merges = {
371
+            "0" => {
372
+              "name" => "a new name",
373
+              "schedule" => "6pm",
374
+              "keep_events_for" => "2",
375
+              "disabled" => "true",
376
+              "options" => weather_agent_options.merge("api_key" => "foo").to_json
377
+            },
378
+            "1" => {
379
+              "name" => "another new name"
380
+            }
381
+          }
382
+
383
+          scenario_import.should be_valid
384
+
385
+          agent_diffs = scenario_import.agent_diffs
386
+          weather_agent_diff = agent_diffs[0]
387
+          trigger_agent_diff = agent_diffs[1]
388
+
389
+          weather_agent_diff.name.current.should == agents(:bob_weather_agent).name
390
+          weather_agent_diff.name.incoming.should == valid_parsed_weather_agent_data[:name]
391
+          weather_agent_diff.name.updated.should == "a new name"
392
+
393
+          weather_agent_diff.schedule.updated.should == "6pm"
394
+          weather_agent_diff.keep_events_for.updated.should == "2"
395
+          weather_agent_diff.disabled.updated.should == "true"
396
+          weather_agent_diff.options.updated.should == weather_agent_options.merge("api_key" => "foo")
397
+        end
398
+
399
+        it "adds errors on validation when updated options are unparsable" do
400
+          scenario_import.merges = {
401
+            "0" => {
402
+              "options" => '{'
403
+            }
404
+          }
405
+          scenario_import.should_not be_valid
406
+          scenario_import.should have(1).error_on(:base)
407
+        end
257 408
       end
258 409
     end
259 410
   end