Save Custom Fields

Closes PLAT-3820

Test Plan:
* Specify custom fields in the customization form
* Save the key & configuration
* create a tool from that configuration and
  verify the custom fields have been added

Change-Id: I00a89f7a5f7daceacb4260465e4c099da418bcb9
Reviewed-on: https://gerrit.instructure.com/167702
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Marc Phillips <mphillips@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>

app/controllers/lti/tool_configurations_api_controller.rb

@@ -72,13 +72,19 @@ class Lti::ToolConfigurationsApiController < ApplicationController
   #   placements should be excluded from the
   #   tool configuration.
   #
+  # @argument custom_fields [String]
+  #   A new line seperated string of key/value pairs
+  #   to be used as custom fields in the LTI launch.
+  #   Example: foo=bar\ncourse=$Canvas.course.id
+  #
   # @returns ToolConfiguration
   def create
     tool_config = Lti::ToolConfiguration.create!(
       developer_key: DeveloperKey.create!(account: account),
       settings: tool_configuration_params[:settings],
       settings_url: tool_configuration_params[:settings_url],
-      disabled_placements: tool_configuration_params[:disabled_placements]
+      disabled_placements: tool_configuration_params[:disabled_placements],
+      custom_fields: tool_configuration_params[:custom_fields]
     )
     update_developer_key!(tool_config)
     render json: tool_config
@@ -113,6 +119,10 @@ class Lti::ToolConfigurationsApiController < ApplicationController
   #   An array of strings indicating which Canvas
   #   placements should be excluded from the
   #   tool configuration.
+  # @argument custom_fields [String]
+  #   A new line seperated string of key/value pairs
+  #   to be used as custom fields in the LTI launch.
+  #   Example: foo=bar\ncourse=$Canvas.course.id
   #
   # @returns ToolConfiguration
   def update
@@ -120,7 +130,8 @@ class Lti::ToolConfigurationsApiController < ApplicationController
     tool_config.update!(
       settings: tool_configuration_params[:settings],
       settings_url: tool_configuration_params[:settings_url],
-      disabled_placements: tool_configuration_params[:disabled_placements]
+      disabled_placements: tool_configuration_params[:disabled_placements],
+      custom_fields: tool_configuration_params[:custom_fields]
     )
     update_developer_key!(tool_config)
 
@@ -169,8 +180,8 @@ class Lti::ToolConfigurationsApiController < ApplicationController
   end
 
   def tool_configuration_params
-    params.require(:tool_configuration).permit(:settings_url, disabled_placements: []).merge(
-      { settings: params.require(:tool_configuration)[:settings]&.to_unsafe_h }
+    params.require(:tool_configuration).permit(:settings_url, :custom_fields, disabled_placements: []).merge(
+      {settings: params.require(:tool_configuration)[:settings]&.to_unsafe_h}
     )
   end
 

app/jsx/developer_keys/CustomizationForm.js

@@ -51,7 +51,9 @@ export default class CustomizationForm extends React.Component {
     }
 
     // Intersection of requested scopes and valid scopes
-    return toolConfiguration.scopes.filter(scope => validScopeNames.includes(scope)).map(s => validScopes[s])
+    return toolConfiguration.scopes
+      .filter(scope => validScopeNames.includes(scope))
+      .map(s => validScopes[s])
   }
 
   get placements() {
@@ -62,9 +64,9 @@ export default class CustomizationForm extends React.Component {
     }
 
     // Get Canvas specific extensions from the tool config
-    const extension = toolConfiguration.extensions.find(ext => (
-      ext.platform === 'canvas.instructure.com'
-    ))
+    const extension = toolConfiguration.extensions.find(
+      ext => ext.platform === 'canvas.instructure.com'
+    )
 
     if (!(extension && extension.settings)) {
       return []
@@ -75,13 +77,13 @@ export default class CustomizationForm extends React.Component {
   }
 
   componentDidMount() {
-    const { dispatch, setEnabledScopes } = this.props
-    const initialScopes = this.scopes.map((s) => this.invertedScopes[s])
+    const {dispatch, setEnabledScopes} = this.props
+    const initialScopes = this.scopes.map(s => this.invertedScopes[s])
 
     dispatch(setEnabledScopes(initialScopes))
   }
 
-  handleScopeChange = (e) => {
+  handleScopeChange = e => {
     const {dispatch, setEnabledScopes} = this.props
     const value = this.invertedScopes[e.target.value]
     const newEnabledScopes = this.props.enabledScopes.slice()
@@ -89,8 +91,8 @@ export default class CustomizationForm extends React.Component {
     dispatch(setEnabledScopes(this.toggleArrayItem(newEnabledScopes, value)))
   }
 
-  handlePlacementChange = (e) => {
-    const { dispatch, setDisabledPlacements, validPlacements } = this.props
+  handlePlacementChange = e => {
+    const {dispatch, setDisabledPlacements, validPlacements} = this.props
     const value = e.target.value
     const newDisabledPlacements = this.props.disabledPlacements.slice()
 
@@ -118,7 +120,7 @@ export default class CustomizationForm extends React.Component {
         name={I18n.t('Services')}
         options={scopes}
         onOptionToggle={this.handleScopeChange}
-        selectedOptions={this.props.enabledScopes.map((s) => (this.props.validScopes[s]))}
+        selectedOptions={this.props.enabledScopes.map(s => this.props.validScopes[s])}
         type="scope"
       />
     )
@@ -147,6 +149,7 @@ export default class CustomizationForm extends React.Component {
         maxHeight="20rem"
         width="50%"
         messages={[{text: I18n.t('One per line. Format: name=value'), type: 'hint'}]}
+        name="custom_fields"
       />
     )
   }

app/jsx/developer_keys/NewKeyModal.js

@@ -59,12 +59,15 @@ export default class DeveloperKeyModal extends React.Component {
   }
 
   saveCustomizations = () => {
+    const customFields = new FormData(this.submissionForm).get('custom_fields')
     const { store, actions, createLtiKeyState, createOrEditDeveloperKeyState } = this.props
+
     store.dispatch(actions.ltiKeysUpdateCustomizations(
       createLtiKeyState.enabledScopes,
       createLtiKeyState.disabledPlacements,
       createOrEditDeveloperKeyState.developerKey.id,
-      createLtiKeyState.toolConfiguration
+      createLtiKeyState.toolConfiguration,
+      customFields
     ))
     this.closeModal()
   }

app/jsx/developer_keys/ToolConfiguration.js

@@ -64,6 +64,6 @@ ToolConfiguration.propTypes = {
     toolConfiguration: PropTypes.object.isRequired,
     toolConfigurationUrl: PropTypes.string.isRequired,
     enabledScopes: PropTypes.arrayOf(PropTypes.string).isRequired,
-    disabledPlacements: PropTypes.arrayOf(PropTypes.string).isRequired
+    disabledPlacements: PropTypes.arrayOf(PropTypes.string).isRequired,
   }).isRequired
 }

app/jsx/developer_keys/actions/__tests__/ltiKeyActions.test.js

@@ -74,13 +74,15 @@ describe('ltiKeysUpdateCustomizations', () => {
   const disabledPlacements = ['account_navigation', 'course_navigaiton']
   const developerKeyId = 123
   const toolConfiguration = {}
+  const customFields = "foo=bar\r\nkey=value"
 
   const update = (dispatch) => {
     actions.ltiKeysUpdateCustomizations(
       scopes,
       disabledPlacements,
       developerKeyId,
-      toolConfiguration
+      toolConfiguration,
+      customFields
     )(dispatch)
   }
 
@@ -95,7 +97,8 @@ describe('ltiKeysUpdateCustomizations', () => {
         },
         tool_configuration: {
           disabled_placements: disabledPlacements,
-          settings: toolConfiguration
+          settings: toolConfiguration,
+          custom_fields: customFields
         }
       }
     )

app/jsx/developer_keys/actions/ltiKeyActions.js

@@ -137,7 +137,7 @@ actions.ltiKeysUpdateCustomizationsSuccessful = payload => ({
 })
 
 actions.LTI_KEYS_UPDATE_CUSTOMIZATIONS = 'LTI_KEYS_UPDATE_CUSTOMIZATIONS'
-actions.ltiKeysUpdateCustomizations = (scopes, disabled_placements, developerKeyId, toolConfiguration) => dispatch => {
+actions.ltiKeysUpdateCustomizations = (scopes, disabled_placements, developerKeyId, toolConfiguration, customFields) => dispatch => {
   dispatch(actions.ltiKeysUpdateCustomizationsStart())
   const url = `/api/lti/developer_keys/${developerKeyId}/tool_configuration`
 
@@ -147,6 +147,7 @@ actions.ltiKeysUpdateCustomizations = (scopes, disabled_placements, developerKey
         scopes
       },
       tool_configuration: {
+        custom_fields: customFields,
         disabled_placements,
         settings: toolConfiguration
       }

app/jsx/developer_keys/reducers/createLtiKeyReducer.js

@@ -30,7 +30,7 @@ const initialState = {
   disabledPlacements: [],
   updateCustomizationsPending: false,
   updateCustomizationsSuccessful: false,
-  updateCustomizationsError: null,
+  updateCustomizationsError: null
 }
 
 const ltiKeysHandlers = {

app/models/lti/tool_configuration.rb

@@ -41,6 +41,7 @@ module Lti
         false
       )
       tool.developer_key = developer_key
+      tool.custom_fields_string = tool.custom_fields_string + "\n#{custom_fields}"
       tool
     end
 

db/migrate/20181005184917_add_custom_fields_to_lti_tool_configurations.rb

@@ -0,0 +1,24 @@
+#
+# Copyright (C) 2018 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+
+class AddCustomFieldsToLtiToolConfigurations < ActiveRecord::Migration[5.1]
+  tag :predeploy
+
+  def change
+    add_column :lti_tool_configurations, :custom_fields, :text
+  end
+end

spec/controllers/lti/tool_configurations_api_controller_spec.rb

@@ -159,7 +159,8 @@ RSpec.describe Lti::ToolConfigurationsApiController, type: :controller do
         developer_key_id: developer_key.id,
         tool_configuration: {
           settings_url: url,
-          disabled_placements: ['course_navigation', 'account_navigation']
+          disabled_placements: ['course_navigation', 'account_navigation'],
+          custom_fields: "foo=bar\r\nkey=value"
         }
       }
     end
@@ -181,6 +182,11 @@ RSpec.describe Lti::ToolConfigurationsApiController, type: :controller do
           valid_parameters.dig(:tool_configuration, :disabled_placements)
         )
       end
+
+      it 'sets the "custom_fields"' do
+        subject
+        expect(config_from_response.custom_fields).to eq valid_parameters.dig(:tool_configuration, :custom_fields)
+      end
     end
 
     context 'when the request times out' do

spec/javascripts/jsx/developer_keys/NewKeyModalSpec.js

@@ -396,6 +396,6 @@ test('saves customizations', () => {
     />
   )
   wrapper.instance().saveCustomizations()
-  ok(ltiStub.calledWith(['https://www.test.com/lineitem'], ['account_navigation'], 22, {}))
+  ok(ltiStub.calledWith(['https://www.test.com/lineitem'], ['account_navigation'], 22, {}, null))
   wrapper.unmount()
 })

spec/models/lti/tool_configuration_spec.rb

@@ -232,7 +232,10 @@ module Lti
 
       let(:extensions) { settings['extensions'].first }
 
-      before { tool_configuration.developer_key = developer_key }
+      before do
+        tool_configuration.developer_key = developer_key
+        tool_configuration.custom_fields = "key=value\nfoo=bar"
+      end
 
       shared_examples_for 'a new context external tool' do
         context 'when "disabled_placements" is set' do
@@ -273,7 +276,7 @@ module Lti
         end
 
         it 'uses the correct top-level custom params' do
-          expect(subject.custom_fields).to eq settings['custom_fields']
+          expect(subject.custom_fields).to eq({"has_expansion"=>"$Canvas.user.id", "no_expansion"=>"foo", "key"=>"value", "foo"=>"bar"})
         end
 
         it 'uses the correct icon url' do