basic lti custom fields

added UI for setting custom fields on tool creation

fixes #5813

Change-Id: Ia3b46c040fb951c7c9c78692c627383a2a437be2
Reviewed-on: https://gerrit.instructure.com/5606
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>

Author: whitmer

app/models/context_external_tool.rb

@@ -2,7 +2,7 @@ class ContextExternalTool < ActiveRecord::Base
   include Workflow
   has_many :content_tags, :as => :content
   belongs_to :context, :polymorphic => true
-  attr_accessible :privacy_level, :domain, :url, :shared_secret, :consumer_key, :name, :description, :custom_fields
+  attr_accessible :privacy_level, :domain, :url, :shared_secret, :consumer_key, :name, :description, :custom_fields, :custom_fields_string
   validates_presence_of :name
   validates_presence_of :consumer_key
   validates_presence_of :shared_secret
@@ -36,11 +36,23 @@ def privacy_level=(val)
     end
   end
 
-  def custom_fields=(hash)
-    settings[:custom_fields] ||= {}
-    hash.each do |key, val|
-      settings[:custom_fields][key] = val.to_s
+  def custom_fields_string
+    (settings[:custom_fields] || {}).map{|key, val|
+      "#{key}=#{val}"
+    }.join("\n")
+  end
+  
+  def custom_fields_string=(str)
+    hash = {}
+    str.split(/\n/).each do |line|
+      key, val = line.split(/=/)
+      hash[key] = val if key.present? && val.present?
     end
+    settings[:custom_fields] = hash
+  end
+  
+  def custom_fields=(hash)
+    settings[:custom_fields] = hash if hash.is_a?(Hash)
   end
 
   def shared_secret=(val)
@@ -169,6 +181,8 @@ def self.find_external_tool(url, context)
 
   def self.serialization_excludes; [:shared_secret,:settings]; end
 
+  def self.serialization_methods; [:custom_fields_string]; end
+  
   def self.process_migration(data, migration)
     tools = data['external_tools'] ? data['external_tools']: []
     to_import = migration.to_import 'external_tools'

app/views/external_tools/_external_tool.html.erb

@@ -7,6 +7,7 @@
       <a href="#" class="delete_tool_link" title="<%= t :delete_tool, "Delete Tool" %>"><%= image_tag "delete.png" %></a>
     </div>
     <div class="clear"></div>
+    <div class="custom_fields_string" style="display: none;"><%= tool.try(:custom_fields_string) %></div>
   </div>
   <div class="content">
     <table class="formtable">

app/views/external_tools/_external_tools.html.erb

@@ -38,10 +38,17 @@
       </tr><tr>
         <td><%= f.blabel :privacy_level, :en => "Privacy" %></td>
         <td><%= f.select :privacy_level, [[t(:anonymous, "Anonymous"),'anonymous'],[t(:name_only, "Name Only"),'name_only'],[t(:public, "Public"),'public']] %></td>
+      </tr><tr>
+        <td colspan="2">
+          <%= f.blabel :custom_fields_string, :en => "Custom Fields" %>
+          <span style="font-size: 0.8em; color: #888;"><%= t('custom_fields_explanation', '(one per line, format: name=value)') %></span>
+          <br/>
+          <%= f.text_area :custom_fields_string, :style => "width: 550px; height: 30px;" %>
+        </td>
       </tr><tr>
         <td colspan="2">
           <%= f.blabel :description, :en => "Description" %><br/>
-          <%= f.text_area :description, :style => "width: 550px; height: 100px;" %>
+          <%= f.text_area :description, :style => "width: 550px; height: 75px;" %>
         </td>
       </tr><tr>
         <td colspan="2">

public/javascripts/external_tools.js

@@ -8,6 +8,7 @@ $(document).ready(function() {
       url: "",
       description: "",
       name: "",
+      custom_fields_string: "",
       privacy: "anonymous",
       consumer_key: "",
       shared_secret: ""
@@ -16,7 +17,7 @@ $(document).ready(function() {
       autoOpen: false,
       title: I18n.t('titles.edit_external_tool', "Edit External Tool"),
       width: 600,
-      height: 400
+      height: 420
     }).dialog('open');
     $dialog.find(".shared_secret_note").hide();
     $dialog.find("form")
@@ -57,7 +58,7 @@ $(document).ready(function() {
   $("#external_tools").delegate('.edit_tool_link', 'click', function(event) {
     event.preventDefault();
     var $tool = $(this).parents(".external_tool");
-    var data = $tool.getTemplateData({textValues: ['name', 'description', 'domain', 'url', 'consumer_key'], dataValues: ['id', 'workflow_state']});
+    var data = $tool.getTemplateData({textValues: ['name', 'description', 'domain', 'url', 'consumer_key', 'custom_fields_string'], dataValues: ['id', 'workflow_state']});
 
     data.privacy_level = data.workflow_state;
     $("#external_tool_match_by").val(data.url ? 'url' : 'domain').change();
@@ -70,7 +71,7 @@ $(document).ready(function() {
       autoOpen: false,
       title: I18n.t('titles.edit_external_tool', "Edit External Tool"),
       width: 600,
-      height: 400
+      height: 420
     }).dialog('open');
   }).delegate('.delete_tool_link', 'click', function(event) {
     event.preventDefault();

spec/models/context_external_tool_spec.rb

@@ -108,6 +108,25 @@
     end
   end
 
+  describe "custom fields" do
+    it "should parse custom_fields_string from a text field" do
+      tool = @course.context_external_tools.create!(:name => "a", :url => "http://www.google.com", :consumer_key => '12345', :shared_secret => 'secret')
+      tool.custom_fields_string=("a=1\nbT^@!#n_40=123\n\nc=")
+      tool.settings[:custom_fields].should_not be_nil
+      tool.settings[:custom_fields].keys.length.should == 2
+      tool.settings[:custom_fields]['a'].should == '1'
+      tool.settings[:custom_fields]['bT^@!#n_40'].should == '123'
+      tool.settings[:custom_fields]['c'].should == nil
+    end
+    
+    it "should return custom_fields_string as a text-formatted field" do
+      tool = @course.context_external_tools.create!(:name => "a", :url => "http://www.google.com", :consumer_key => '12345', :shared_secret => 'secret', :custom_fields => {'a' => '123', 'b' => '456'})
+      tool.custom_fields_string.should == "a=123\nb=456"
+    end
+    
+    
+  end
+  
   describe "all_tools_for" do
     it "should retrieve all tools in alphabetical order" do
       @tools = []

spec/selenium/external_tools_sel.rb

@@ -0,0 +1,69 @@
+require File.expand_path(File.dirname(__FILE__) + '/common')
+
+describe "editing external tools" do
+  it_should_behave_like "in-process server selenium tests"
+  
+  it "should allow creating a new course external tool with custom fields" do
+    course_with_teacher_logged_in
+    get "/courses/#{@course.id}/settings"
+    
+    keep_trying_until { driver.find_element(:css, "#tab-tools-link").displayed? }
+    driver.find_element(:css, "#tab-tools-link").click
+    driver.find_element(:css, ".add_tool_link").click
+    driver.find_element(:css, "#external_tools_dialog").should be_displayed
+    driver.find_element(:css, "#external_tool_name").send_keys "Tool"
+    driver.find_element(:css, "#external_tool_consumer_key").send_keys "Key"
+    driver.find_element(:css, "#external_tool_shared_secret").send_keys "Secret"
+    driver.find_element(:css, "#external_tool_domain").send_keys "example.com"
+    driver.find_element(:css, "#external_tool_custom_fields_string").send_keys "a=1\nb=123"
+    driver.find_element(:css, "#external_tools_dialog .save_button").click
+    
+    keep_trying_until { !driver.find_element(:css, "#external_tools_dialog").displayed? }
+    
+    tool = ContextExternalTool.last
+    driver.find_element(:css, "#external_tool_#{tool.id}").should be_displayed
+    tool.should_not be_nil
+    tool.name.should == "Tool"
+    tool.consumer_key.should == "Key"
+    tool.shared_secret.should == "Secret"
+    tool.domain.should == "example.com"
+    tool.settings[:custom_fields].should == {'a' => '1', 'b' => '123'}
+  end
+  
+  it "should allow editing an existing external tool with custom fields" do
+    course_with_teacher_logged_in
+    tool = @course.context_external_tools.create!(:name => "new tool", :consumer_key => "key", :shared_secret => "secret", :domain => 'example.com', :custom_fields => {'a' => '1', 'b' => '2'})
+    get "/courses/#{@course.id}/settings"
+    
+    keep_trying_until { driver.find_element(:css, "#tab-tools-link").displayed? }
+    driver.find_element(:css, "#tab-tools-link").click
+    tool_elem = driver.find_element(:css, "#external_tool_#{tool.id}")
+    tool_elem.find_element(:css, ".edit_tool_link").click
+
+    driver.find_element(:css, "#external_tools_dialog").should be_displayed
+
+    driver.find_element(:css, "#external_tool_name").clear
+    driver.find_element(:css, "#external_tool_consumer_key").clear
+    driver.find_element(:css, "#external_tool_shared_secret").clear
+    driver.find_element(:css, "#external_tool_domain").clear
+    driver.find_element(:css, "#external_tool_custom_fields_string").clear
+
+    driver.find_element(:css, "#external_tool_name").send_keys "new tool (updated)"
+    driver.find_element(:css, "#external_tool_consumer_key").send_keys "key (updated)"
+    driver.find_element(:css, "#external_tool_shared_secret").send_keys "secret (updated)"
+    driver.find_element(:css, "#external_tool_domain").send_keys "example2.com"
+    driver.find_element(:css, "#external_tool_custom_fields_string").send_keys "a=9\nb=8"
+    driver.find_element(:css, "#external_tools_dialog .save_button").click
+    
+    keep_trying_until { !driver.find_element(:css, "#external_tools_dialog").displayed? }
+    
+    tool_elem = driver.find_elements(:css, "#external_tools .external_tool").detect(&:displayed?)
+    tool_elem.should_not be_nil
+    tool.reload
+    tool.name.should == "new tool (updated)"
+    tool.consumer_key.should == "key (updated)"
+    tool.shared_secret.should == "secret (updated)"
+    tool.domain.should == "example2.com"
+    tool.settings[:custom_fields].should == {'a' => '9', 'b' => '8'}
+  end
+end