3321: Workbench copes when pipeline components have an odd form.
authorBrett Smith <brett@curoverse.com>
Thu, 24 Jul 2014 20:18:35 +0000 (16:18 -0400)
committerBrett Smith <brett@curoverse.com>
Thu, 24 Jul 2014 20:18:35 +0000 (16:18 -0400)
Closes #3321.

16 files changed:
apps/workbench/app/controllers/pipeline_instances_controller.rb
apps/workbench/app/controllers/pipeline_templates_controller.rb
apps/workbench/app/helpers/pipeline_components_helper.rb [new file with mode: 0644]
apps/workbench/app/helpers/pipeline_instances_helper.rb
apps/workbench/app/views/application/_pipeline_progress.html.erb
apps/workbench/app/views/application/_pipeline_status_label.html.erb
apps/workbench/app/views/pipeline_instances/_show_components.html.erb
apps/workbench/app/views/pipeline_instances/_show_components_json.html.erb [new file with mode: 0644]
apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb [new file with mode: 0644]
apps/workbench/app/views/pipeline_instances/_show_recent.html.erb
apps/workbench/app/views/pipeline_templates/_show_components.html.erb
apps/workbench/test/functional/pipeline_instances_controller_test.rb
apps/workbench/test/functional/pipeline_templates_controller_test.rb
apps/workbench/test/integration/pipeline_instances_test.rb
services/api/test/fixtures/pipeline_instances.yml
services/api/test/fixtures/pipeline_templates.yml

index e84d0e45975c7207386cb46fb2d43e71534bda5d..b5c9815ed5c51b8fce88e6f347e4d4de883bd5ff 100644 (file)
@@ -2,6 +2,7 @@ class PipelineInstancesController < ApplicationController
   skip_before_filter :find_object_by_uuid, only: :compare
   before_filter :find_objects_by_uuid, only: :compare
   include PipelineInstancesHelper
+  include PipelineComponentsHelper
 
   def copy
     @object = @object.dup
@@ -196,7 +197,7 @@ class PipelineInstancesController < ApplicationController
     if @object and @object.state.in? ['New', 'Ready']
       panes = %w(Inputs) + panes
     end
-    if not @object.components.values.collect { |x| x[:job] }.compact.any?
+    if not @object.components.values.any? { |x| x[:job] rescue false }
       panes -= ['Graph']
     end
     panes
index 2be51c6a1804968e9785dae7d5381a84b913e5fe..2b2e9a4e33925da7899007910b213ce130811303 100644 (file)
@@ -1,5 +1,6 @@
 class PipelineTemplatesController < ApplicationController
-  
+  include PipelineComponentsHelper
+
   def show
     @objects = PipelineInstance.where(pipeline_template_uuid: @object.uuid)
     super
diff --git a/apps/workbench/app/helpers/pipeline_components_helper.rb b/apps/workbench/app/helpers/pipeline_components_helper.rb
new file mode 100644 (file)
index 0000000..de951b3
--- /dev/null
@@ -0,0 +1,13 @@
+module PipelineComponentsHelper
+  def render_pipeline_components(template_suffix, fallback=nil, locals={})
+    begin
+      render(partial: "pipeline_instances/show_components_#{template_suffix}",
+             locals: locals)
+    rescue
+      case fallback
+      when :json
+        render(partial: "pipeline_instances/show_components_json")
+      end
+    end
+  end
+end
index 35d28d542b05ad8a9c71a94fc43fd1f5928d47f9..c15c94cea8677e789789195206a5a426c5d9631a 100644 (file)
@@ -52,6 +52,10 @@ module PipelineInstancesHelper
     object.components.each do |cname, c|
       i += 1
       pj = {index: i, name: cname}
+      if not c.is_a?(Hash)
+        ret << pj
+        next
+      end
       pj[:job] = c[:job].is_a?(Hash) ? c[:job] : {}
       pj[:percent_done] = 0
       pj[:percent_running] = 0
index d478f65ddc48c164acc3d96346e6e3deabfc8eaf..2ae03a00aaf46a1e6d50bd034652458c19f7320b 100644 (file)
@@ -1,7 +1,7 @@
 <% component_frac = 1.0 / p.components.length %>
 <div class="progress">
   <% p.components.each do |k,c| %>
-    <% if c[:job] %>
+    <% if c.is_a?(Hash) and c[:job] %>
       <%= render partial: "job_progress", locals: {:j => c[:job], :scaleby => component_frac } %>
     <% end %>
   <% end %>
index f68d547aa5f07d2e6e5ce688af7698c12071e266..8abf65b4537cf01f22027e8be36586e8f414bdd0 100644 (file)
@@ -5,7 +5,7 @@
 <% elsif p.state == 'RunningOnServer' || p.state == 'RunningOnClient' %>
   <span class="label label-info">running</span>
 <% else %>
-  <% if (p.components.select do |k,v| v[:job] end).length == 0 %>
+  <% if not p.components.values.any? { |c| c[:job] rescue false } %>
     <span class="label label-default">not started</span>
   <% else %>
     <span class="label label-default">not running</span>
index 385001998d4b7472e373f51be9f1bab64757122a..c55a7253b8bca440b6f58d1059995aeb24406e18 100644 (file)
@@ -6,73 +6,7 @@
     Current state: <span class="badge badge-info" data-pipeline-state="<%= @object.state %>"><%= @object.state.sub('OnServer', '') %></span>&nbsp;
   </div>
 
-  <table class="table pipeline-components-table">
-    <colgroup>
-      <col style="width: 15%" />
-      <col style="width: 25%" />
-      <col style="width: 8%" />
-      <col style="width: 13%" />
-      <col style="width: 12%" />
-      <col style="width: 14%" />
-      <col style="width: 13%" />
-    </colgroup>
-    <thead>
-      <tr>
-        <th colspan="2">
-          component
-        </th><th colspan="5">
-          job
-          <%# format:'js' here helps browsers avoid using the cached js
-          content in html context (e.g., duplicate tab -> see
-          javascript) %>
-          <%= link_to '(refresh)', {format: :js}, {class: 'refresh hide', remote: true, method: 'get'} %>
-        </th>
-      </tr>
-    </thead>
-    <tbody>
-      <% render_pipeline_jobs.each do |pj| %>
-        <% if pj[:job].andand[:uuid]
-             pipeline_job_uuids << pj[:job][:uuid]
-           end %>
-      <tr>
-        <td>
-          <%= pj[:name] %>
-        </td><td>
-          <%= pj[:script] %>
-          <br /><span class="deemphasize"><%= pj[:script_version] %></span>
-        </td><td>
-          <%= render(partial: 'job_status_label', locals: { j: pj[:job] }) %>
-        </td><td>
-          <%= pj[:progress_bar] %>
-        </td>
-        <% current_job = Job.find(pj[:job][:uuid]) rescue nil %>
-        <td>
-          <% if current_job %>
-            <%= render partial: 'show_object_button', locals: {object: current_job, size: 'xs', link_text: 'Show job details'} %>
-          <% end %>
-        </td><td>
-          <% if current_job.andand[:log] %>
-            <% fixup = /([a-f0-9]{32}\+\d+)(\+?.*)/.match(current_job[:log])%>
-            <% Collection.limit(1).where(uuid: fixup[1]).each do |c| %>
-              <% c.files.first.andand do |file| %>
-                <%= link_to url_for(controller: 'collections', action: 'show_file', uuid: current_job[:log], file: "#{file[0]}/#{file[1]}", disposition: 'inline', size: file[2]), class: 'btn btn-default btn-xs' do %>
-                  <i class="fa fa-fw fa-info"></i> Show log messages
-                <% end %>
-              <% end %>
-            <% end %>
-          <% end %>
-        </td><td>
-          <% if current_job.andand[:output] %>
-            <%= link_to_if_arvados_object current_job[:output], {thumbnail: true, link_text: raw('<i class="fa fa-fw fa-archive"></i> Show output files')}, {class: 'btn btn-default btn-xs'} %>
-          <% end %>
-        </td>
-      </tr>
-      <% end %>
-    </tbody>
-    <tfoot>
-      <tr><td colspan="7"></td></tr>
-    </tfoot>
-  </table>
+  <%= render_pipeline_components("running", :json, pipeline_job_uuids: pipeline_job_uuids) %>
 
   <% if @object.state.in? %w(RunningOnServer RunningOnClient Failed) %>
 
 
 <% else %>
   <%# state is either New or Ready %>
+  <p><i>Here are all of the pipeline's components (jobs that will need to run in order to complete the pipeline). If you know what you're doing (or you're experimenting) you can modify these parameters before starting the pipeline. Usually, you only need to edit the settings presented on the "Inputs" tab above.</i></p>
 
-  <% if @object.state.in? %w(New Ready) %>
-    <p><i>Here are all of the pipeline's components (jobs that will need to run in order to complete the pipeline). If you know what you're doing (or you're experimenting) you can modify these parameters before starting the pipeline. Usually, you only need to edit the settings presented on the "Inputs" tab above.</i></p>
-  <% end %>
-
-  <% if @object.state.in? ['New', 'Ready'] %>
-    <%= render partial: 'show_components_editable', locals: {editable: true} %>
-  <% else %>
-    <%= render partial: 'show_components_editable', locals: {editable: false} %>
-  <% end %>
+  <%= render_pipeline_components("editable", :json, editable: true) %>
 <% end %>
diff --git a/apps/workbench/app/views/pipeline_instances/_show_components_json.html.erb b/apps/workbench/app/views/pipeline_instances/_show_components_json.html.erb
new file mode 100644 (file)
index 0000000..3cdd5ae
--- /dev/null
@@ -0,0 +1,16 @@
+<p>The components of this pipeline are in a format that Workbench does not recognize.</p>
+
+    <div id="components-accordion" class="panel panel-default">
+      <div class="panel-heading">
+        <h4 class="panel-title">
+          <a data-toggle="collapse" data-parent="#components-accordion" href="#components-json">
+            Show components JSON
+          </a>
+        </h4>
+      </div>
+      <div id="components-json" class="panel-collapse collapse">
+        <div class="panel-body">
+          <pre><%= Oj.dump(@object.components, indent: 2) %></pre>
+        </div>
+      </div>
+    </div>
diff --git a/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb b/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb
new file mode 100644 (file)
index 0000000..a41fdd1
--- /dev/null
@@ -0,0 +1,67 @@
+  <table class="table pipeline-components-table">
+    <colgroup>
+      <col style="width: 15%" />
+      <col style="width: 25%" />
+      <col style="width: 8%" />
+      <col style="width: 13%" />
+      <col style="width: 12%" />
+      <col style="width: 14%" />
+      <col style="width: 13%" />
+    </colgroup>
+    <thead>
+      <tr>
+        <th colspan="2">
+          component
+        </th><th colspan="5">
+          job
+          <%# format:'js' here helps browsers avoid using the cached js
+          content in html context (e.g., duplicate tab -> see
+          javascript) %>
+          <%= link_to '(refresh)', {format: :js}, {class: 'refresh hide', remote: true, method: 'get'} %>
+        </th>
+      </tr>
+    </thead>
+    <tbody>
+      <% render_pipeline_jobs.each do |pj| %>
+        <% if pj[:job].andand[:uuid]
+             pipeline_job_uuids << pj[:job][:uuid]
+           end %>
+      <tr>
+        <td>
+          <%= pj[:name] %>
+        </td><td>
+          <%= pj[:script] %>
+          <br /><span class="deemphasize"><%= pj[:script_version] %></span>
+        </td><td>
+          <%= render(partial: 'job_status_label', locals: { j: pj[:job] }) %>
+        </td><td>
+          <%= pj[:progress_bar] %>
+        </td>
+        <% current_job = Job.find(pj[:job][:uuid]) rescue nil %>
+        <td>
+          <% if current_job %>
+            <%= render partial: 'show_object_button', locals: {object: current_job, size: 'xs', link_text: 'Show job details'} %>
+          <% end %>
+        </td><td>
+          <% if current_job.andand[:log] %>
+            <% fixup = /([a-f0-9]{32}\+\d+)(\+?.*)/.match(current_job[:log])%>
+            <% Collection.limit(1).where(uuid: fixup[1]).each do |c| %>
+              <% c.files.first.andand do |file| %>
+                <%= link_to url_for(controller: 'collections', action: 'show_file', uuid: current_job[:log], file: "#{file[0]}/#{file[1]}", disposition: 'inline', size: file[2]), class: 'btn btn-default btn-xs' do %>
+                  <i class="fa fa-fw fa-info"></i> Show log messages
+                <% end %>
+              <% end %>
+            <% end %>
+          <% end %>
+        </td><td>
+          <% if current_job.andand[:output] %>
+            <%= link_to_if_arvados_object current_job[:output], {thumbnail: true, link_text: raw('<i class="fa fa-fw fa-archive"></i> Show output files')}, {class: 'btn btn-default btn-xs'} %>
+          <% end %>
+        </td>
+      </tr>
+      <% end %>
+    </tbody>
+    <tfoot>
+      <tr><td colspan="7"></td></tr>
+    </tfoot>
+  </table>
index 0046b56f735efec16764947c5d25657015168f6b..0b78e07d65e875facfee37ee65fba34ad76847e1 100644 (file)
       </td>
       <td style="border-top: 0; opacity: 0.5;" colspan="6">
         <% ob.components.each do |cname, c| %>
-          <% if c[:job] %>
+          <% if c.is_a?(Hash) and c[:job] %>
             <%= render partial: "job_status_label", locals: {:j => c[:job], :title => cname.to_s } %>
           <% else %>
-            <span class="label label-default"><%= cname.to_s %></span>            
+            <span class="label label-default"><%= cname.to_s %></span>
           <% end %>
         <% end %>
       </td>
index bd4870000366b617726663a2291c215a89ad8a7a..1f2c1baaf4930dda3da6a179652d9fe43fdf7fe8 100644 (file)
@@ -11,8 +11,8 @@
                                                   }.to_json),
                 { class: "btn btn-primary btn-sm", remote: true, method: 'get' }
                ) do %>
-                   Run this pipeline 
+                   Run this pipeline
                  <% end %>
 <% end %>
 
-<%= render partial: 'pipeline_instances/show_components_editable', locals: {editable: false} %>
+<%= render_pipeline_components("editable", :json, editable: false) %>
index c04b5b1f90ddd96152e997b5cc0b7b973c001bc1..fbfbf34e44bc35c6641ac9b98aa2188393f031d8 100644 (file)
@@ -71,4 +71,11 @@ class PipelineInstancesControllerTest < ActionController::TestCase
       end
     end
   end
+
+  test "component rendering copes with unexpeceted components format" do
+    get(:show,
+        {id: api_fixture("pipeline_instances")["components_is_jobspec"]["uuid"]},
+        session_for(:active))
+    assert_response :success
+  end
 end
index be48e0ca939179c51d24e114269b1d0b754f3c1f..82c4faeabe37e9ea50e7d1986f254c88088a1540 100644 (file)
@@ -1,4 +1,10 @@
 require 'test_helper'
 
 class PipelineTemplatesControllerTest < ActionController::TestCase
+  test "component rendering copes with unexpeceted components format" do
+    get(:show,
+        {id: api_fixture("pipeline_templates")["components_is_jobspec"]["uuid"]},
+        session_for(:active))
+    assert_response :success
+  end
 end
index 7053d39bfda8e811793aa3b1be0e97fad293ea78..f5d6e633f340ea85cb7e438c35b208e7c3a13510 100644 (file)
@@ -103,7 +103,7 @@ class PipelineInstancesTest < ActionDispatch::IntegrationTest
     find('.btn', text: 'Run a pipeline').click
     within('.modal-dialog') do
       assert page.has_text? 'Two Part Pipeline Template'
-      find('.fa-gear').click
+      find('.selectable', text: 'Two Part Pipeline Template').click
       find('.btn', text: 'Next: choose inputs').click
     end
 
@@ -146,4 +146,14 @@ class PipelineInstancesTest < ActionDispatch::IntegrationTest
     assert page.has_text? 'script_version'
   end
 
+  test "JSON popup available for strange components" do
+    uuid = api_fixture("pipeline_instances")["components_is_jobspec"]["uuid"]
+    visit page_with_token("active", "/pipeline_instances/#{uuid}")
+    click_on "Components"
+    assert(page.has_no_text?("script_parameters"),
+           "components JSON visible without popup")
+    click_on "Show components JSON"
+    assert(page.has_text?("script_parameters"),
+           "components JSON not found")
+  end
 end
index 823116fda412b805b9bc76dbc40065b7eb99c889..f73558df401a3fd85fdced9cdb944c970dc26d08 100644 (file)
@@ -27,7 +27,6 @@ has_job:
   state: Ready
   uuid: zzzzz-d1hrv-1yfj6xkidf2muk3
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
-  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
   components:
    foo:
     script: foo
@@ -37,3 +36,24 @@ has_job:
             uuid: zzzzz-8i9sb-pshmckwoma9plh7,
             script_version: master
          }
+
+components_is_jobspec:
+  # Helps test that clients cope with funny-shaped components.
+  # For an example, see #3321.
+  uuid: zzzzz-d1hrv-jobspeccomponts
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-04-14 12:35:04 -0400
+  updated_at: 2014-04-14 12:35:04 -0400
+  modified_at: 2014-04-14 12:35:04 -0400
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  state: RunningOnServer
+  components:
+    script: foo
+    script_version: master
+    script_parameters:
+      input:
+        required: true
+        dataclass: Collection
+        title: "Foo/bar pair"
+        description: "Provide a collection containing at least two files."
index 8e3a070ee60bc47cd7bdb7d280efddf1035afe5e..012cd992306aa134705ea6b8c11b1843fa2785a8 100644 (file)
@@ -36,3 +36,24 @@ two_part:
           default: [1,1,2,3,5]
         array_with_value: # important to test repeating values in the array!
           value: [1,1,2,3,5]
+
+components_is_jobspec:
+  # Helps test that clients cope with funny-shaped components.
+  # For an example, see #3321.
+  uuid: zzzzz-p5p6p-jobspeccomponts
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-04-14 12:35:04 -0400
+  updated_at: 2014-04-14 12:35:04 -0400
+  modified_at: 2014-04-14 12:35:04 -0400
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  name: Pipeline Template with Jobspec Components
+  components:
+    script: foo
+    script_version: master
+    script_parameters:
+      input:
+        required: true
+        dataclass: Collection
+        title: "Foo/bar pair"
+        description: "Provide a collection containing at least two files."