New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BD-24] Add LTI 1.3 documentation and reorganize docs #1934

Merged

nedbat merged 7 commits into openedx:master from open-craft:giovanni/update-lti-docs

May 5, 2021

Contributor

giovannicimolin commented Apr 15, 2021 •

edited by marcotuts

Loading

BLENDED-825

This PR adds LTI 1.3 and LTI Advantage documentation while reorganizing the documentation contents.

Reviewers

Subject matter expert: @nedbat
Subject matter expert: @sarina
Doc team review (sanity check, copy edit, or dev edit?): @edx/doc
Product review: @marcotuts

Post-review

Add a comment with the description of this change or link this PR to the next release notes task.
Squash commits

openedx-webhooks added blended waiting on author labels

openedx-webhooks commented Apr 15, 2021 •

edited

Loading

Thanks for the pull request, @giovannicimolin! I've created BLENDED-825 to keep track of it in Jira. More details are on the BD-24 project page.

When this pull request is ready, tag your edX technical lead.


          docs: Update LTI component documentation

fab1a63

* Add LTI 1.3 and LTI Advantage documentation
* Reorganize docs

Signed-off-by: Giovanni Cimolin da Silva <[email protected]>

giovannicimolin force-pushed the giovanni/update-lti-docs branch from 22f2b6b to fab1a63 Compare

April 15, 2021 15:09

marcotuts reviewed

View reviewed changes

en_us/shared/exercises_tools/lti_component.rst

+              To enable the LTI tool in Studio, add the ``"lti_consumer"`` module to the
+              **Advanced Module List** on the **Advanced Settings** page. For more
+              information, see :ref:`Enable Additional Exercises and Tools`.

Contributor

marcotuts Apr 15, 2021

We really need to get rid of this silly step of enabling LTI as an advanced module. We similarly simplified the unit page experience for ORA recently. Given how close we are to done on the budget we can do this outside of BD-24 instead of adding another story here.

@sarina , @nedbat can connect on whether this is a reasonable LTI backlog item to consider or something T&L should consider as part of authoring efficiency type efforts in the future.

This comment is not blocking for the PR at all, but wanted to put this thread into the context of the educator impact today.

This comment was marked as resolved.

Sign in to view

marcotuts approved these changes

View reviewed changes


          Finish writing docs

b3d4651

giovannicimolin changed the title ~~[BD-24] Add LTI 1.3 documentation and reorganize docs WIP~~ [BD-24] Add LTI 1.3 documentation and reorganize docs

openedx-webhooks added needs triage and removed waiting on author labels

Contributor Author

giovannicimolin commented Apr 21, 2021 •

edited

Loading

@nedbat @sarina @marcotuts I've finished writing the documentation.
You can use this sandbox to test: https://lti-1p3-validation.opencraft.hosting (with the usual staff sandbox users).

sarina reviewed

View reviewed changes

en_us/shared/exercises_tools/lti_component.rst

-              version 1.1.1 specifications.
+              LTI component supports tools with the `LTI 1.1`_ and `LTI 1.3`_ specifications.
+              Additionally, the LTI 1.3 tools can use the following LTI Advantage extensions:
+              `Deep Linking`_ and `Assignments and Grades services`_.

Contributor

sarina Apr 22, 2021

The image below (L48) is broken

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

Contributor

nedbat Apr 23, 2021

You can't trust GitHub rendering for links like this. They depend on .rst features GitHub doesn't support, mostly multi-file features. If you checkout this PR and run ./run_tests.sh locally, all the links resolve with no warnings.

sarina reviewed

View reviewed changes

Contributor

sarina left a comment

I did a pass at formatting and wording. Overall it is clear. I think I need an LTI component to practice the setup instructions?

en_us/shared/exercises_tools/lti_component.rst

-              version 1.1.1 specifications.
+              LTI component supports tools with the `LTI 1.1`_ and `LTI 1.3`_ specifications.
+              Additionally, the LTI 1.3 tools can use the following LTI Advantage extensions:
+              `Deep Linking`_ and `Assignments and Grades services`_.

This comment was marked as resolved.

Sign in to view

en_us/shared/exercises_tools/lti_component.rst

-              version 1.1.1 specifications.
+              LTI component supports tools with the `LTI 1.1`_ and `LTI 1.3`_ specifications.
+              Additionally, the LTI 1.3 tools can use the following LTI Advantage extensions:
+              `Deep Linking`_ and `Assignments and Grades services`_.

This comment was marked as resolved.

Sign in to view

en_us/shared/exercises_tools/lti_component.rst

-              version 1.1.1 specifications.
+              LTI component supports tools with the `LTI 1.1`_ and `LTI 1.3`_ specifications.
+              Additionally, the LTI 1.3 tools can use the following LTI Advantage extensions:
+              `Deep Linking`_ and `Assignments and Grades services`_.

This comment was marked as resolved.

Sign in to view

en_us/shared/exercises_tools/lti_component.rst

+              **Advanced Module List** on the **Advanced Settings** page. For more
+              information, see :ref:`Enable Additional Exercises and Tools`.
+              .. note::

This comment was marked as resolved.

Sign in to view

en_us/shared/exercises_tools/lti_component.rst

+              To enable the LTI tool in Studio, add the ``"lti_consumer"`` module to the
+              **Advanced Module List** on the **Advanced Settings** page. For more
+              information, see :ref:`Enable Additional Exercises and Tools`.

This comment was marked as resolved.

Sign in to view

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst


		#. The content you selected in the tool will be presented to your students in the LMS.

		.. note:: LTI 1.3 launches only work with published blocks, make sure the block is

This comment was marked as resolved.

Sign in to view

nedbat suggested changes

View reviewed changes

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved


          docs: Address review comments

e7c7288

Contributor Author

giovannicimolin commented Apr 26, 2021

@sarina @nedbat I addressed the review comments. This is ready for another review pass.


          fix: Fix quality issue

8a7de48

nedbat suggested changes

View reviewed changes

Contributor

nedbat left a comment

Ugh, I caught one more typo, then good to merge!

en_us/shared/exercises_tools/lti_component.rst Outdated Show resolved Hide resolved


          Update en_us/shared/exercises_tools/lti_component.rst

a5d4d86

Co-authored-by: Ned Batchelder <[email protected]>

sarina approved these changes

View reviewed changes

viadanna approved these changes

View reviewed changes

viadanna left a comment

Good to go, just a comment on a link getting Access Denied.

I read through this.
I tested the links.

en_us/links/links.rst Outdated

@@ @@ -373,10 +373,17 @@ @@
               .. _python-social-auth backend documentation: http://python-social-auth.readthedocs.io/en/latest/backends/index.html#social-backends
-              .. _IMS LTI 1.1 Consumer Launch: http://www.imsglobal.org/developers/LTI/test/v1p1/lms.php
+              .. _IMS LTI 1.1 Consumer Launch: https://www.imsglobal.org/developers/LTI/test/v1p1/lms.php

viadanna Apr 28, 2021

Got an Access Denied here :(

giovannicimolin added 2 commits

April 28, 2021 16:51


          fix: Revert change in LTI Provider URL

a3e113c


          Merge branch 'giovanni/update-lti-docs' of github.com:open-craft/edx-…

0f73ab0

…documentation into giovanni/update-lti-docs

Contributor Author

giovannicimolin commented Apr 28, 2021 •

edited

Loading

@viadanna @nedbat

The broken link above is only used for the LTI Provider setup (on https://github.com/edx/edx-documentation/blob/9468a2681170a2b5cdbe46e4193939f94a4833df/en_us/install_operations/source/configuration/lti/tpa_lti.rst#L87-L88).
~~It's probably returning a Access Denied here because it's expecting a LTI 1.1 authentication message. I've reverted my changes to this line.~~

My mistake here, the tool referenced there was a LTI consumer to test the provider of Open edX. This is not in the scope of the LTI 1.3 documentation, but I've found a similar tool that allows testing (http://www.ltiapps.net/test/tc.php) but the instructions for using it differ from the docs.

@nedbat Should I keep the only link and leave the provider docs as is, or point to the new test tool (even without matching instructions)?

Related issue: #1938

nedbat approved these changes

View reviewed changes

nedbat merged commit ed79509 into openedx:master

openedx-webhooks added merged and removed needs triage labels

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels