Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-32301

LTI Tools don't allow instructor custom parameters

          Details

          • Testing Instructions:
            Hide
            1. Create an External Tool item (You can use a launch URL from https://lti-examples.heroku.com/index.html)
            2. Make sure Advanced options are shown, then enter something into the Custom Parameters field (like test=testing)
            3. Save the External Tool item, and open your favorite web inspector (Firebug, Chrome inspector, etc.)
            4. Perform a launch of the External Tool, and check the post data. You should see a custom_test=testing parameter passed in the launch (Look for launch.php under the network tab)
            Show
            Create an External Tool item (You can use a launch URL from https://lti-examples.heroku.com/index.html ) Make sure Advanced options are shown, then enter something into the Custom Parameters field (like test=testing) Save the External Tool item, and open your favorite web inspector (Firebug, Chrome inspector, etc.) Perform a launch of the External Tool, and check the post data. You should see a custom_test=testing parameter passed in the launch (Look for launch.php under the network tab)
          • Affected Branches:
            MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
          • Fixed Branches:
            MOODLE_23_STABLE, MOODLE_24_STABLE
          • Pull from Repository:
          • Pull Master Branch:
            MDL-32301_lti_instructor_params
          • Pull Master Diff URL:

            Description

            In the launch code, if allowinstructorcustom is blank or set to "never", the instructor is not allowed to supply their own custom parameters. When tool configurations are written, the allowinstructorcustom setting is left blank.

            We should change the behavior to allow custom parameters if allowinstructorcustom is not set, as there is not really a good reason to restrict people from editing them.

            Note that some tool providers require information in the custom parameters to indicate which resource should be launched and will not work until this is resolved. This will be a major issue for some tool providers.

              Gliffy Diagrams

                Attachments

                  Activity

                  Ascending order - Click to sort in descending order
                  Hide
                  Permalink
                  bushido Mark Nielsen added a comment -

                  Attaching a patch that changes the default behavior to allowing custom instructor params in the LTI launch.

                  Show
                  bushido Mark Nielsen added a comment - Attaching a patch that changes the default behavior to allowing custom instructor params in the LTI launch.
                  Hide
                  Permalink
                  salvetore Michael de Raadt added a comment -

                  Thanks for providing a fix for that.

                  I'll see if we can get this peer reviewed soon.

                  Show
                  salvetore Michael de Raadt added a comment - Thanks for providing a fix for that. I'll see if we can get this peer reviewed soon.
                  Hide
                  Permalink
                  poltawski Dan Poltawski added a comment -

                  Stealing this off Eloy for peer review

                  Show
                  poltawski Dan Poltawski added a comment - Stealing this off Eloy for peer review
                  Hide
                  Permalink
                  poltawski Dan Poltawski added a comment -

                  Hi Guys,

                  Makes sense to me. Are you able to supply testing instructions and a git branch so we can properly credit you?

                  thanks,
                  Dan

                  Show
                  poltawski Dan Poltawski added a comment - Hi Guys, Makes sense to me. Are you able to supply testing instructions and a git branch so we can properly credit you? thanks, Dan
                  Hide
                  Permalink
                  scriby Chris Scribner added a comment -

                  Testing instructions:

                  1. Create an External Tool item (You can use a launch URL from https://lti-examples.heroku.com/index.html)
                  2. Make sure Advanced options are shown, then enter something into the Custom Parameters field (like test=testing)
                  3. Save the External Tool item, and open your favorite web inspector (Firebug, Chrome inspector, etc.)
                  4. Perform a launch of the External Tool, and check the post data. You should see a custom_test=testing parameter passed in the launch

                  Show
                  scriby Chris Scribner added a comment - Testing instructions: 1. Create an External Tool item (You can use a launch URL from https://lti-examples.heroku.com/index.html ) 2. Make sure Advanced options are shown, then enter something into the Custom Parameters field (like test=testing) 3. Save the External Tool item, and open your favorite web inspector (Firebug, Chrome inspector, etc.) 4. Perform a launch of the External Tool, and check the post data. You should see a custom_test=testing parameter passed in the launch
                  Hide
                  Permalink
                  scriby Chris Scribner added a comment -

                  We don't really care about receiving credit, but if it makes it easier for you to get this included let me know and I'll set up a branch.

                  Show
                  scriby Chris Scribner added a comment - We don't really care about receiving credit, but if it makes it easier for you to get this included let me know and I'll set up a branch.
                  Hide
                  Permalink
                  bushido Mark Nielsen added a comment -

                  The fix can be cherry picked to stable branches.

                  Show
                  bushido Mark Nielsen added a comment - The fix can be cherry picked to stable branches.
                  Hide
                  Permalink
                  poltawski Dan Poltawski added a comment -

                  Sent to integration, sorry I dunno how I keep missing these issues. If you could rebase it would be great, otherwise I think the integrator should be able to cherr-pick.

                  Show
                  poltawski Dan Poltawski added a comment - Sent to integration, sorry I dunno how I keep missing these issues. If you could rebase it would be great, otherwise I think the integrator should be able to cherr-pick.
                  Hide
                  Permalink
                  bushido Mark Nielsen added a comment -

                  Rebased onto master, it's ready to go!

                  Show
                  bushido Mark Nielsen added a comment - Rebased onto master, it's ready to go!
                  Hide
                  Permalink
                  stronk7 Eloy Lafuente (stronk7) added a comment -

                  Integrated (23, 24 & master), thanks!

                  Show
                  stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
                  Hide
                  Permalink
                  andyjdavis Andrew Davis added a comment -

                  Seems to be working as described. Passing.

                  Show
                  andyjdavis Andrew Davis added a comment - Seems to be working as described. Passing.
                  Hide
                  Permalink
                  stronk7 Eloy Lafuente (stronk7) added a comment -

                  Did you think this day was not going to arrive ever?

                  Your patience has been rewarded, yay, sent upstream, thanks!

                  Closing...ciao

                  Show
                  stronk7 Eloy Lafuente (stronk7) added a comment - Did you think this day was not going to arrive ever? Your patience has been rewarded, yay, sent upstream, thanks! Closing...ciao

                    People

                    • Votes:
                      2 Vote for this issue
                      Watchers:
                      5 Start watching this issue

                      Dates

                      • Created:
                        Updated:
                        Resolved:
                        Fix Release Date:
                        13/May/13