diff --git a/common/lib/xmodule/xmodule/css/lti/lti.scss b/common/lib/xmodule/xmodule/css/lti/lti.scss index 97a8f62d54..a32fb06f33 100644 --- a/common/lib/xmodule/xmodule/css/lti/lti.scss +++ b/common/lib/xmodule/xmodule/css/lti/lti.scss @@ -2,29 +2,17 @@ div.lti { // align center margin: 0 auto; - h3.error_message { - display: block; - } - form.ltiLaunchForm { display: none; } + iframe.ltiLaunchFrame { width: 100%; height: 800px; - display: none; + display: block; border: 0px; overflow-x: hidden; } - &.rendered { - iframe.ltiLaunchFrame { - display: block; - } - - h3.error_message { - display: none; - } - } } diff --git a/common/lib/xmodule/xmodule/js/fixtures/lti.html b/common/lib/xmodule/xmodule/js/fixtures/lti.html deleted file mode 100644 index ae545970ce..0000000000 --- a/common/lib/xmodule/xmodule/js/fixtures/lti.html +++ /dev/null @@ -1,36 +0,0 @@ -
- -
- - - - - - - - - - - - - - - - -
- -

- Please provide launch_url. Click "Edit", and fill in the - required fields. -

- - - -
diff --git a/common/lib/xmodule/xmodule/js/fixtures/lti_iframe_url_default.html b/common/lib/xmodule/xmodule/js/fixtures/lti_iframe_url_default.html new file mode 100644 index 0000000000..1f8b8120cd --- /dev/null +++ b/common/lib/xmodule/xmodule/js/fixtures/lti_iframe_url_default.html @@ -0,0 +1,36 @@ +
+
+ +
+ + + + + + + + + + + + + + + + +
+ +

+ Please provide launch_url. Click "Edit", and fill in the + required fields. +

+ +
+
diff --git a/common/lib/xmodule/xmodule/js/fixtures/lti_iframe_url_empty.html b/common/lib/xmodule/xmodule/js/fixtures/lti_iframe_url_empty.html new file mode 100644 index 0000000000..049f87fffe --- /dev/null +++ b/common/lib/xmodule/xmodule/js/fixtures/lti_iframe_url_empty.html @@ -0,0 +1,36 @@ +
+
+ +
+ + + + + + + + + + + + + + + + +
+ +

+ Please provide launch_url. Click "Edit", and fill in the + required fields. +

+ +
+
diff --git a/common/lib/xmodule/xmodule/js/fixtures/lti_iframe_url_new.html b/common/lib/xmodule/xmodule/js/fixtures/lti_iframe_url_new.html new file mode 100644 index 0000000000..e5a909f0e1 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/fixtures/lti_iframe_url_new.html @@ -0,0 +1,37 @@ +
+
+ +
+ + + + + + + + + + + + + + + + +
+ + + +
+
diff --git a/common/lib/xmodule/xmodule/js/fixtures/lti_newpage_url_default.html b/common/lib/xmodule/xmodule/js/fixtures/lti_newpage_url_default.html new file mode 100644 index 0000000000..90cd2f7541 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/fixtures/lti_newpage_url_default.html @@ -0,0 +1,36 @@ +
+
+ +
+ + + + + + + + + + + + + + + + +
+ +

+ Please provide launch_url. Click "Edit", and fill in the + required fields. +

+ +
+
diff --git a/common/lib/xmodule/xmodule/js/fixtures/lti_newpage_url_empty.html b/common/lib/xmodule/xmodule/js/fixtures/lti_newpage_url_empty.html new file mode 100644 index 0000000000..0a4cc0088d --- /dev/null +++ b/common/lib/xmodule/xmodule/js/fixtures/lti_newpage_url_empty.html @@ -0,0 +1,36 @@ +
+
+ +
+ + + + + + + + + + + + + + + + +
+ +

+ Please provide launch_url. Click "Edit", and fill in the + required fields. +

+ +
+
diff --git a/common/lib/xmodule/xmodule/js/fixtures/lti_newpage_url_new.html b/common/lib/xmodule/xmodule/js/fixtures/lti_newpage_url_new.html new file mode 100644 index 0000000000..8fad7b05d2 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/fixtures/lti_newpage_url_new.html @@ -0,0 +1,33 @@ +
+
+ +
+ + + + + + + + + + + + + + + + +
+ + Click to view LTI in new window + +
+
diff --git a/common/lib/xmodule/xmodule/js/spec/lti/constructor.js b/common/lib/xmodule/xmodule/js/spec/lti/constructor.js index f016d42d9d..a5b2df1955 100644 --- a/common/lib/xmodule/xmodule/js/spec/lti/constructor.js +++ b/common/lib/xmodule/xmodule/js/spec/lti/constructor.js @@ -5,77 +5,126 @@ * * * The front-end part of the LTI module is really simple. If an action - * is set for the hidden LTI form, then it is submited, and the results are - * redirected to an iframe. + * is set for the hidden LTI form, then it is submitted, and the results are + * redirected to an iframe or to a new window (based on the + * "open_in_a_new_page" attribute). * - * We will test that the form is only submited when the action is set (i.e. - * not empty). + * We will test that the form is only submitted when the action is set (i.e. + * not empty, and not the default one). * * Other aspects of LTI module will be covered by Python unit tests and * acceptance tests. - * */ /* - * "Hence that general is skilful in attack whose opponent does not know what - * to defend; and he is skilful in defense whose opponent does not know what + * "Hence that general is skillful in attack whose opponent does not know what + * to defend; and he is skillful in defense whose opponent does not know what * to attack." * * ~ Sun Tzu */ (function () { + var element, form, frame, link; + + function initialize(fixture, hasLink) { + loadFixtures(fixture); + + element = $('.lti-wrapper'); + if (hasLink) { + link = element.find('a.link_lti_new_window'); + } + form = element.find('.ltiLaunchForm'); + + spyOnEvent(form, 'submit'); + + LTI(element); + } + describe('LTI', function () { - describe('constructor', function () { - describe('before settings were filled in', function () { - var element, errorMessage, frame; + describe('initialize', function () { + describe( + 'open_in_a_new_page is "true", launch URL is empty', + function () { - // This function will be executed before each of the it() specs - // in this suite. beforeEach(function () { - loadFixtures('lti.html'); - - element = $('#lti_id'); - errorMessage = element.find('.error_message'); - form = element.find('.ltiLaunchForm'); - frame = element.find('.ltiLaunchFrame'); - - spyOnEvent(form, 'submit'); - - LTI(element); + initialize('lti_newpage_url_empty.html'); }); - it( - 'when URL setting is not filled form is not submited', - function () { - + it('form is not submitted', function () { expect('submit').not.toHaveBeenTriggeredOn(form); }); }); - describe('After the settings were filled in', function () { - var element, errorMessage, frame; + describe( + 'open_in_a_new_page is "true", launch URL is default', + function () { - // This function will be executed before each of the it() specs - // in this suite. beforeEach(function () { - loadFixtures('lti.html'); - - element = $('#lti_id'); - errorMessage = element.find('.error_message'); - form = element.find('.ltiLaunchForm'); - frame = element.find('.ltiLaunchFrame'); - - spyOnEvent(form, 'submit'); - - // The user "fills in" the necessary settings, and the - // form will get an action URL. - form.attr('action', 'http://www.example.com/test_submit'); - - LTI(element); + initialize('lti_newpage_url_default.html'); }); - it('when URL setting is filled form is submited', function () { + it('form is not submitted', function () { + expect('submit').not.toHaveBeenTriggeredOn(form); + }); + }); + + describe( + 'open_in_a_new_page is "true", launch URL is not empty, and ' + + 'not default', + function () { + + beforeEach(function () { + initialize('lti_newpage_url_new.html', true); + }); + + it('form is not submitted', function () { + expect('submit').not.toHaveBeenTriggeredOn(form); + }); + + it('after link is clicked, form is submitted', function () { + link.trigger('click'); + + expect('submit').toHaveBeenTriggeredOn(form); + }); + }); + + describe( + 'open_in_a_new_page is "false", launch URL is empty', + function () { + + beforeEach(function () { + initialize('lti_iframe_url_empty.html'); + }); + + it('form is not submitted', function () { + expect('submit').not.toHaveBeenTriggeredOn(form); + }); + }); + + describe( + 'open_in_a_new_page is "false", launch URL is default', + function () { + + beforeEach(function () { + initialize('lti_iframe_url_default.html'); + }); + + it('form is not submitted', function () { + expect('submit').not.toHaveBeenTriggeredOn(form); + }); + }); + + describe( + 'open_in_a_new_page is "false", launch URL is not empty, ' + + 'and not default', + function () { + + beforeEach(function () { + initialize('lti_iframe_url_new.html'); + }); + + it('form is submitted', function () { expect('submit').toHaveBeenTriggeredOn(form); }); }); diff --git a/common/lib/xmodule/xmodule/js/src/lti/lti.js b/common/lib/xmodule/xmodule/js/src/lti/lti.js index 7d5b183f21..e001fb8d67 100644 --- a/common/lib/xmodule/xmodule/js/src/lti/lti.js +++ b/common/lib/xmodule/xmodule/js/src/lti/lti.js @@ -1,9 +1,39 @@ +/** + * File: lti.js + * + * Purpose: LTI module constructor. Given an LTI element, we process it. + * + * + * Inside the element there is a form. If that form has a valid action + * attribute, then we do one of: + * + * 1.) Submit the form. The results will be shown on the current page in an + * iframe. + * 2.) attach a handler function to a link which will submit the form. The + * results will be shown in a new window. + * + * The 'open_in_a_new_page' data attribute of the LTI element dictates which of + * the two actions will be performed. + */ + +/* + * So the thing to do when working on a motorcycle, as in any other task, is to + * cultivate the peace of mind which does not separate one's self from one's + * surroundings. When that is done successfully, then everything else follows + * naturally. Peace of mind produces right values, right values produce right + * thoughts. Right thoughts produce right actions and right actions produce + * work which will be a material reflection for others to see of the serenity + * at the center of it all. + * + * ~ Robert M. Pirsig + */ + window.LTI = (function () { // Function initialize(element) // - // Initialize the LTI iframe. + // Initialize the LTI module. function initialize(element) { - var form; + var form, open_in_a_new_page; // In cms (Studio) the element is already a jQuery object. In lms it is // a DOM object. @@ -14,11 +44,37 @@ window.LTI = (function () { form = element.find('.ltiLaunchForm'); + if ( + // Action is one of: null, undefined, 0, 000, '', false. + !Boolean(form.attr('action')) || + + // Default URL that should not cause a form submit. + form.attr('action') === 'http://www.example.com' + ) { + return; // Nothing to do - no valid action provided. + } + + // We want a Boolean 'true' or 'false'. First we will retrieve the data + // attribute, and then we will parse it via native JSON.parse(). + open_in_a_new_page = element.find('.lti').data('open_in_a_new_page'); + try { + open_in_a_new_page = JSON.parse(open_in_a_new_page); + } catch (e) { + console.log('ERROR: Parsing data attribute "open_in_a_new_page".'); + console.log('*** error = "' + e.toString() + '".'); + + open_in_a_new_page = null; + } + // If the Form's action attribute is set (i.e. we can perform a normal - // submit), then we submit the form and make the frame shown. - if (form.attr('action') && form.attr('action') !== 'http://www.example.com') { + // submit), then we submit the form immediately or when user will click + // on a link (depending on instance settings) and make the frame shown. + if (open_in_a_new_page === true) { + element.find('.link_lti_new_window').on('click', function () { + form.submit(); + }); + } else if (open_in_a_new_page === false) { form.submit(); - element.find('.lti').addClass('rendered'); } } diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index bbc39957a0..dfc24e8808 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -8,12 +8,14 @@ http://www.imsglobal.org/LTI/v1p1p1/ltiIMGv1p1p1.html import logging import oauthlib.oauth1 import urllib +import json from xmodule.editing_module import MetadataOnlyEditingDescriptor from xmodule.x_module import XModule from xmodule.course_module import CourseDescriptor from pkg_resources import resource_string from xblock.core import String, Scope, List +from xblock.fields import Boolean log = logging.getLogger(__name__) @@ -45,6 +47,7 @@ class LTIFields(object): lti_id = String(help="Id of the tool", default='', scope=Scope.settings) launch_url = String(help="URL of the tool", default='http://www.example.com', scope=Scope.settings) custom_parameters = List(help="Custom parameters (vbid, book_location, etc..)", scope=Scope.settings) + open_in_a_new_page = Boolean(help="Should LTI be opened in new page?", default=True, scope=Scope.settings) class LTIModule(LTIFields, XModule): @@ -169,14 +172,14 @@ class LTIModule(LTIFields, XModule): client_key, client_secret ) - context = { 'input_fields': input_fields, # these params do not participate in oauth signing 'launch_url': self.launch_url, 'element_id': self.location.html_id(), - 'element_class': self.location.category, + 'element_class': self.category, + 'open_in_a_new_page': self.open_in_a_new_page } return self.system.render_template('lti.html', context) diff --git a/lms/djangoapps/courseware/features/lti.feature b/lms/djangoapps/courseware/features/lti.feature index a7182a3dae..8255e515b0 100644 --- a/lms/djangoapps/courseware/features/lti.feature +++ b/lms/djangoapps/courseware/features/lti.feature @@ -2,17 +2,27 @@ Feature: LMS.LTI component As a student, I want to view LTI component in LMS. - Scenario: LTI component in LMS is not rendered + Scenario: LTI component in LMS with no launch_url is not rendered Given the course has correct LTI credentials - And the course has an LTI component with incorrect fields - Then I view the LTI and it is not rendered + And the course has an LTI component with no_launch_url fields, new_page is false + Then I view the LTI and error is shown - Scenario: LTI component in LMS is rendered + Scenario: LTI component in LMS with incorrect lti_id is rendered incorrectly Given the course has correct LTI credentials - And the course has an LTI component filled with correct fields - Then I view the LTI and it is rendered + And the course has an LTI component with incorrect_lti_id fields, new_page is false + Then I view the LTI but incorrect_signature warning is rendered Scenario: LTI component in LMS is rendered incorrectly Given the course has incorrect LTI credentials - And the course has an LTI component filled with correct fields + And the course has an LTI component with correct fields, new_page is false Then I view the LTI but incorrect_signature warning is rendered + + Scenario: LTI component in LMS is correctly rendered in new page + Given the course has correct LTI credentials + And the course has an LTI component with correct fields, new_page is true + Then I view the LTI and it is rendered in new page + + Scenario: LTI component in LMS is correctly rendered in iframe + Given the course has correct LTI credentials + And the course has an LTI component with correct fields, new_page is false + Then I view the LTI and it is rendered in iframe diff --git a/lms/djangoapps/courseware/features/lti.py b/lms/djangoapps/courseware/features/lti.py index fa76dbb4f6..2091ac74cb 100644 --- a/lms/djangoapps/courseware/features/lti.py +++ b/lms/djangoapps/courseware/features/lti.py @@ -1,5 +1,6 @@ #pylint: disable=C0111 +import os from django.contrib.auth.models import User from lettuce import world, step from lettuce.django import django_url @@ -8,33 +9,19 @@ from common import course_id from student.models import CourseEnrollment -@step('I view the LTI and it is not rendered$') +@step('I view the LTI and error is shown$') def lti_is_not_rendered(_step): - # lti div has no class rendered - assert world.is_css_not_present('div.lti.rendered') - # error is shown - assert world.css_visible('.error_message') + assert world.is_css_present('.error_message') - # iframe is not visible - assert not world.css_visible('iframe') + # iframe is not presented + assert not world.is_css_present('iframe') - location = world.scenario_dict['LTI'].location.html_id() - iframe_name = 'ltiLaunchFrame-' + location - - #inside iframe test content is not presented - with world.browser.get_iframe(iframe_name) as iframe: - # iframe does not contain functions from terrain/ui_helpers.py - world.browser.driver.implicitly_wait(1) - try: - assert iframe.is_element_not_present_by_css('.result', wait_time=1) - except: - raise - finally: - world.browser.driver.implicitly_wait(world.IMPLICIT_WAIT) + # link is not presented + assert not world.is_css_present('.link_lti_new_window') -def check_lti_ifarme_content(text): +def check_lti_iframe_content(text): #inside iframe test content is presented location = world.scenario_dict['LTI'].location.html_id() iframe_name = 'ltiLaunchFrame-' + location @@ -47,30 +34,33 @@ def check_lti_ifarme_content(text): )) -@step('I view the LTI and it is rendered$') -def lti_is_rendered(_step): - # lti div has class rendered - assert world.is_css_present('div.lti.rendered') +@step('I view the LTI and it is rendered in (.*)$') +def lti_is_rendered(_step, rendered_in): + if rendered_in.strip() == 'iframe': + assert world.is_css_present('iframe') + assert not world.is_css_present('.link_lti_new_window') + assert not world.is_css_present('.error_message') - # error is hidden - assert not world.css_visible('.error_message') + # iframe is visible + assert world.css_visible('iframe') + check_lti_iframe_content("This is LTI tool. Success.") - # iframe is visible - assert world.css_visible('iframe') - check_lti_ifarme_content("This is LTI tool. Success.") + elif rendered_in.strip() == 'new page': + assert not world.is_css_present('iframe') + assert world.is_css_present('.link_lti_new_window') + assert not world.is_css_present('.error_message') + check_lti_popup() + else: # incorrent rendered_in parametetr + assert False @step('I view the LTI but incorrect_signature warning is rendered$') def incorrect_lti_is_rendered(_step): - # lti div has class rendered - assert world.is_css_present('div.lti.rendered') - - # error is hidden - assert not world.css_visible('.error_message') - - # iframe is visible - assert world.css_visible('iframe') - check_lti_ifarme_content("Wrong LTI signature") + assert world.is_css_present('iframe') + assert not world.is_css_present('.link_lti_new_window') + assert not world.is_css_present('.error_message') + #inside iframe test content is presented + check_lti_iframe_content("Wrong LTI signature") @step('the course has correct LTI credentials$') @@ -97,44 +87,33 @@ def set_incorrect_lti_passport(_step): i_am_registered_for_the_course(coursenum, metadata) -@step('the course has an LTI component filled with correct fields$') -def add_correct_lti_to_course(_step): +@step('the course has an LTI component with (.*) fields, new_page is(.*)$') +def add_correct_lti_to_course(_step, fields, new_page): category = 'lti' - world.scenario_dict['LTI'] = world.ItemFactory.create( - # parent_location=section_location(course), - parent_location=world.scenario_dict['SEQUENTIAL'].location, - category=category, - display_name='LTI', - metadata={ - 'lti_id': 'correct_lti_id', - 'launch_url': world.lti_server.oauth_settings['lti_base'] + world.lti_server.oauth_settings['lti_endpoint'] - } - ) - course = world.scenario_dict["COURSE"] - chapter_name = world.scenario_dict['SECTION'].display_name.replace( - " ", "_") - section_name = chapter_name - path = "/courses/{org}/{num}/{name}/courseware/{chapter}/{section}".format( - org=course.org, - num=course.number, - name=course.display_name.replace(' ', '_'), - chapter=chapter_name, - section=section_name) - url = django_url(path) + lti_id = 'correct_lti_id' + launch_url = world.lti_server.oauth_settings['lti_base'] + world.lti_server.oauth_settings['lti_endpoint'] + if fields.strip() == 'incorrect_lti_id': # incorrect fields + lti_id = 'incorrect_lti_id' + elif fields.strip() == 'correct': # correct fields + pass + elif fields.strip() == 'no_launch_url': + launch_url = u'' + else: # incorrect parameter + assert False - world.browser.visit(url) + if new_page.strip().lower() == 'false': + new_page = False + else: # default is True + new_page = True - -@step('the course has an LTI component with incorrect fields$') -def add_incorrect_lti_to_course(_step): - category = 'lti' world.scenario_dict['LTI'] = world.ItemFactory.create( parent_location=world.scenario_dict['SEQUENTIAL'].location, category=category, display_name='LTI', metadata={ - 'lti_id': 'incorrect_lti_id', - 'lti_url': world.lti_server.oauth_settings['lti_base'] + world.lti_server.oauth_settings['lti_endpoint'] + 'lti_id': lti_id, + 'launch_url': launch_url, + 'open_in_a_new_page': new_page } ) course = world.scenario_dict["COURSE"] @@ -192,3 +171,28 @@ def i_am_registered_for_the_course(course, metadata): CourseEnrollment.enroll(usr, course_id(course)) world.log_in(username='robot', password='test') + + +def check_lti_popup(): + parent_window = world.browser.current_window # Save the parent window + world.css_find('.link_lti_new_window').first.click() + + assert len(world.browser.windows) != 1 + + for window in world.browser.windows: + world.browser.switch_to_window(window) # Switch to a different window (the pop-up) + # Check if this is the one we want by comparing the url + url = world.browser.url + basename = os.path.basename(url) + pathname = os.path.splitext(basename)[0] + + if pathname == u'correct_lti_endpoint': + break + + result = world.css_find('.result').first.text + assert result == u'This is LTI tool. Success.' + + world.browser.driver.close() # Close the pop-up window + world.browser.switch_to_window(parent_window) # Switch to the main window again + + diff --git a/lms/djangoapps/courseware/tests/test_lti.py b/lms/djangoapps/courseware/tests/test_lti.py index 1996f95cbf..a22db66041 100644 --- a/lms/djangoapps/courseware/tests/test_lti.py +++ b/lms/djangoapps/courseware/tests/test_lti.py @@ -75,6 +75,7 @@ class TestLTI(BaseTestXmodule): 'element_class': self.item_module.location.category, 'element_id': self.item_module.location.html_id(), 'launch_url': 'http://www.example.com', # default value + 'open_in_a_new_page': True } self.assertEqual( generated_context, diff --git a/lms/templates/lti.html b/lms/templates/lti.html index 5e1b9e5d77..aca91d1dee 100644 --- a/lms/templates/lti.html +++ b/lms/templates/lti.html @@ -1,14 +1,20 @@ -
+<%! import json %> +<%! from django.utils.translation import ugettext as _ %> - ## This form will be hidden. Once available on the client, the LTI +
+ + ## This form will be hidden. + ## If open_in_a_new_page is false then, once available on the client, the LTI ## module JavaScript will trigget a "submit" on the form, and the ## result will be rendered to the below iFrame. + ## If open_in_a_new_page is true, then link will be shown, and by clicking on it, + ## LTI will pop up in new window.
@@ -19,16 +25,22 @@
+ +% if launch_url and launch_url != 'http://www.example.com': + % if open_in_a_new_page: + ${_(u'Click to view LTI in new window')} + % else: + ## The result of the form submit will be rendered here. + + % endif +% else:

Please provide launch_url. Click "Edit", and fill in the required fields.

- - ## The result of the form submit will be rendered here. - - +%endif