4878: Refactor Workbench "Re-run job" button for more code reuse.
authorBrett Smith <brett@curoverse.com>
Tue, 27 Jan 2015 23:09:20 +0000 (18:09 -0500)
committerBrett Smith <brett@curoverse.com>
Tue, 27 Jan 2015 23:09:20 +0000 (18:09 -0500)
* Avoid adding new routes: add the modal directly to the job show
  page, and submit the new work with the existing job creation route.
* Fix id references; e.g., for labels.

apps/workbench/app/controllers/jobs_controller.rb
apps/workbench/app/views/jobs/_rerun_job_with_options_popup.html.erb
apps/workbench/app/views/jobs/_show_job_buttons.html.erb [deleted file]
apps/workbench/app/views/jobs/rerun_job_with_options_popup.js.erb [deleted file]
apps/workbench/app/views/jobs/show.html.erb
apps/workbench/config/routes.rb
apps/workbench/test/integration/jobs_test.rb

index 685d13d1b550e628d65a1d6ef18851e288c7360f..08fb94d2f085d4d7eb777f81cea43d84f9f2dbcf 100644 (file)
@@ -82,33 +82,4 @@ class JobsController < ApplicationController
   def show_pane_list
     %w(Status Log Details Provenance Advanced)
   end
-
-  def rerun_job_with_options_popup
-    respond_to do |format|
-      format.js
-      format.html
-    end
-  end
-
-  def rerun_job_with_options
-    job_info = JSON.parse params['job_info']
-
-    @object = Job.new
-
-    @object.script = job_info['script']
-    @object.repository = job_info['repository']
-    @object.nondeterministic = job_info['nondeterministic']
-    @object.script_parameters = job_info['script_parameters']
-    @object.runtime_constraints = job_info['runtime_constraints']
-    @object.supplied_script_version = job_info['supplied_script_version']
-
-    if 'use_latest' == params['script']
-      @object.script_version = job_info['supplied_script_version']
-    else
-      @object.script_version = job_info['script_version']
-    end
-
-    @object.save!
-    show
-  end
 end
index 084965e7ed05979c3cf693e157a686e34682d68c..931b8da87af8b8a5ec8f418bcc90b1c215fd3386 100644 (file)
@@ -1,21 +1,17 @@
-<%
-  job_info = {}
-  job_info['script'] = params[:script]
-  job_info['script_version'] = params[:script_version]
-  job_info['repository'] = params[:repository]
-  job_info['supplied_script_version'] = params[:supplied_script_version]
-  job_info['nondeterministic'] = params[:nondeterministic]
-  job_info['script_parameters'] = params[:script_parameters]
-  job_info['runtime_constraints'] = params[:runtime_constraints]
-%>
-
-<div class="modal fade" role="dialog" aria-labelledby="myModalLabel" aria-hidden="true">
+<% @job = @object %>
+<div id="jobRerunModal" class="modal fade" role="dialog" aria-labelledby="jobRerunTitle" aria-hidden="true">
   <div class="modal-dialog">
     <div class="modal-content">
-      <%= form_tag rerun_job_with_options_path do |f| %>
+      <%= form_for(@job, method: :post, url: {controller: 'jobs', action: 'create'}) do |f| %>
+        <% [:script, :repository, :supplied_script_version, :nondeterministic].each do |field_sym| %>
+          <%= f.hidden_field(field_sym) %>
+        <% end %>
+        <% [:script_parameters, :runtime_constraints].each do |field_sym| %>
+          <%= f.hidden_field(field_sym, value: @job.send(field_sym).to_json) %>
+        <% end %>
         <div class="modal-header">
           <button type="button" class="close" onClick="reset_form()" data-dismiss="modal" aria-hidden="true">&times;</button>
-          <div>
+          <div id="jobRerunTitle">
             <div class="col-sm-6"> <h4 class="modal-title">Re-run job</h4> </div>
           </div>
           <br/>
           <p> The inputs and parameters will be the same as the current job.
               Thus, the new job will not reflect any changes made to the pipeline that initiated this job. </p>
           <div style="padding-left: 1em">
-            <% if !job_info['supplied_script_version'].nil? and
-                  !job_info['supplied_script_version'].empty? and
-                   job_info['supplied_script_version'] != job_info['script_version'] %>
-              <%= radio_button_tag(:script, "use_same", true) %>
-              <%= label_tag(:script_use_same, "Use same script version as this run") %>
+            <% if (@job.supplied_script_version.blank? or
+                   (@job.supplied_script_version == @job.script_version)) %>
+              <%= f.hidden_field(:script_version) %>
+            <% else %>
+              <%= f.radio_button("script_version", @job.script_version) %>
+              <%= f.label(:script_version, "Use same script version as this run", value: @job.script_version) %>
               <p style="padding-left: 1em"> Use the same script version as the current job.</p>
 
-              <%= radio_button_tag(:script, "use_latest") %>
-              <%= label_tag(:script_use_latest, "Use latest script version") %>
-              <p style="padding-left: 1em"> Use the current commit indicated by '<%=job_info['supplied_script_version']%>' in the '<%=job_info['repository']%>' repository.</p>
+              <%= f.radio_button(:script_version, @job.supplied_script_version) %>
+              <%= f.label(:script_version, "Use latest script version", value: @job.supplied_script_version) %>
+              <p style="padding-left: 1em"> Use the current commit indicated by '<%= @job.supplied_script_version %>' in the '<%= @job.repository %>' repository.</p>
             <% end %>
-
-            <input type="hidden" name="job_info" value="<%=job_info.to_json%>">
           </div>
         </div>
 
         <div class="modal-footer">
-          <button type="submit" class="btn btn-primary" name="rerun-with-options" value="RunningOnServer">Run now</button>
+          <%= f.submit(value: "Run now", class: "btn btn-primary") %>
           <button class="btn btn-default" onClick="reset_form()" data-dismiss="modal" aria-hidden="true">Cancel</button>
         </div>
       <% end %>
diff --git a/apps/workbench/app/views/jobs/_show_job_buttons.html.erb b/apps/workbench/app/views/jobs/_show_job_buttons.html.erb
deleted file mode 100644 (file)
index 8e9cff7..0000000
+++ /dev/null
@@ -1,16 +0,0 @@
-<% if @object.state != "Running" %>
-<%
-  supplied_script_version = @object[:supplied_script_version] if @object.respond_to? :supplied_script_version
-%>
-    <%= link_to rerun_job_with_options_popup_path(script: @object[:script],
-                                                  script_version: @object[:script_version],
-                                                  repository: @object[:repository],
-                                                  supplied_script_version: supplied_script_version,
-                                                  nondeterministic: @object[:nondeterministic],
-                                                  script_parameters: @object[:script_parameters],
-                                                  runtime_constraints: @object[:runtime_constraints]),
-        {class: 'btn btn-sm btn-primary', :remote => true, 'data-toggle' =>  "modal",
-          'data-target' => '#rerun-job-with-options', return_to: request.url} do %>
-        <i class="fa fa-fw fa-gear"></i> Re-run job...
-    <% end %>
-<% end %>
diff --git a/apps/workbench/app/views/jobs/rerun_job_with_options_popup.js.erb b/apps/workbench/app/views/jobs/rerun_job_with_options_popup.js.erb
deleted file mode 100644 (file)
index ff2b64d..0000000
+++ /dev/null
@@ -1,2 +0,0 @@
-$("#rerun-job-with-options-popup").html("<%= escape_javascript(render partial: 'rerun_job_with_options_popup') %>");
-$("#rerun-job-with-options-popup .modal").modal('show');
index d6e2051500313566178b714179a11e1036ef7f36..060728a41417ec98a7897844f37261c8e1ec18cf 100644 (file)
@@ -3,10 +3,14 @@
        data-pane-content-url="<%= url_for(params.merge(tab_pane: "job_buttons")) %>"
        data-object-uuid="<%= @object.uuid %>"
        style="display: inline">
-    <%= render partial: 'show_job_buttons', locals: {object: @object}%>
+    <% if @object.state != "Running" %>
+      <button type="button" class="btn btn-sm btn-primary" data-toggle="modal" data-target="#jobRerunModal">
+        <i class="fa fa-fw fa-gear"></i> Re-run job...
+      </button>
+    <% end %>
   </div>
 <% end %>
 
 <%= render partial: 'title_and_buttons' %>
 <%= render partial: 'content', layout: 'content_layout', locals: {pane_list: controller.show_pane_list }%>
-<div id="rerun-job-with-options-popup"></div>
+<%= render partial: 'rerun_job_with_options_popup' %>
index 964863aece622712ef9fd37ea37e773b5942cd8a..6b29d0553d4f0903973f40f951fde82d99ab3144 100644 (file)
@@ -12,8 +12,6 @@ ArvadosWorkbench::Application.routes.draw do
   get "users/setup" => 'users#setup', :as => :setup_user
   get "report_issue_popup" => 'actions#report_issue_popup', :as => :report_issue_popup
   post "report_issue" => 'actions#report_issue', :as => :report_issue
-  get "rerun-job-with-options-popup" => 'jobs#rerun_job_with_options_popup', :as => :rerun_job_with_options_popup
-  post "rerun-job-with-options" => 'jobs#rerun_job_with_options', :as => :rerun_job_with_options
   resources :nodes
   resources :humans
   resources :traits
index 719a8776135d519d8ae4c9edde91df07026c57a3..9ab98a051991c2c5736ce59d29e37bd5fbef5783 100644 (file)
@@ -95,14 +95,17 @@ class JobsTest < ActionDispatch::IntegrationTest
         find('a,button', text: 'Re-run job...').click
       end
       within('.modal-dialog') do
-        assert_selector 'a,button', text: 'Run now'
         assert_selector 'a,button', text: 'Cancel'
-        page.choose('script_use_latest') if use_latest
-        find('button', text: 'Run now').click
+        if use_latest
+          page.choose("job_script_version_#{job['supplied_script_version']}")
+        end
+        click_on "Run now"
       end
 
-      # Re-run job does not actually work and we see Fiddlesticks.
-      # So, let's make sure the correct script version is sought.
+      # Re-running jobs doesn't currently work because the test API
+      # server has no git repository to check against.  For now, check
+      # that the correct script version is mentioned in the
+      # Fiddlesticks error message.
       if expect_options && use_latest
         assert_text "Script version #{job['supplied_script_version']} does not resolve to a commit"
       else