Skip to content

Add lti consumer xblock modules to LTI REST endpoints - #14349

Merged
robrap merged 1 commit into
robrap/upgrade-xblock-lti-consumerfrom
ubc-add-lti-consumer-module-to-rest-endpoints
Jan 19, 2017
Merged

Add lti consumer xblock modules to LTI REST endpoints#14349
robrap merged 1 commit into
robrap/upgrade-xblock-lti-consumerfrom
ubc-add-lti-consumer-module-to-rest-endpoints

Conversation

@robrap

@robrap robrap commented Jan 19, 2017

Copy link
Copy Markdown
Contributor

This allows LTI consumer xblock modules being hocked into get_course_lti_endpoints for
discovering LTI result/outcome endpoints.

This edx-platform change is required to complete testing of the xblock-lti-consumer upgrade being brought into the platform in this PR: https://github.com/edx/edx-platform/pull/14348

It also replaced the following PR: https://github.com/edx/edx-platform/pull/13639

NOTE: I am going to upgrade the sandbox with this branch.

Sandbox:

@robrap

robrap commented Jan 19, 2017

Copy link
Copy Markdown
Contributor Author

Reviewers: Before this change, when I went to the following url:
https://robrap.sandbox.edx.org/courses/course-v1:edX+DemoX+Demo_Course/lti_rest_endpoints/
I saw the following result:

[]

Hopefully, we will have something different after I update the sandbox. :)

@robrap
robrap requested review from douglashall and removed request for andy-armstrong January 19, 2017 15:24
@robrap

robrap commented Jan 19, 2017

Copy link
Copy Markdown
Contributor Author

@xcompass @douglashall: After the changes when I navigate to:
https://robrap.sandbox.edx.org/courses/course-v1:edX+DemoX+Demo_Course/lti_rest_endpoints/
I see:

[{
  "lti_1_1_result_service_xml_endpoint": "https://robrap.sandbox.edx.org/courses/course-v1:edX+DemoX+Demo_Course/xblock/block-v1:edX+DemoX+Demo_Course+type@lti_consumer+block@1777e9cddf5f43a98f641521ae90c574/handler_noauth/outcome_service_handler",
  "display_name": "LTI Consumer",
  "lti_2_0_result_service_json_endpoint": "https://robrap.sandbox.edx.org/courses/course-v1:edX+DemoX+Demo_Course/xblock/block-v1:edX+DemoX+Demo_Course+type@lti_consumer+block@1777e9cddf5f43a98f641521ae90c574/handler_noauth/result_service_handler/user/{anon_user_id}"
}]

I'm concerned with /user/{anon_user_id} at the end of 'lti_2_0_result_service_json_endpoint'. Is this evidence of a defect in openedx/xblock-lti-consumer#22 ?

@robrap

robrap commented Jan 19, 2017

Copy link
Copy Markdown
Contributor Author

@xcompass @douglashall: At first this looked suspicious, but now I see that /user/{anon_user_id} has been hard-coded in that result for a long time:
https://github.com/edx/edx-platform/blob/18e3e095d106d3aba3683faf53819f825c2917ca/lms/djangoapps/courseware/views/views.py#L1087

So, all looks good. Feel free to add thumbs if you wish. Looks like this will finally be merging shortly.

@robrap

robrap commented Jan 19, 2017

Copy link
Copy Markdown
Contributor Author

jenkins run bokchoy

@douglashall

Copy link
Copy Markdown
Contributor

👍

@dianakhuang dianakhuang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 happy to see that the endpoint is working now!

@robrap
robrap force-pushed the robrap/upgrade-xblock-lti-consumer branch from d436961 to 23c61ad Compare January 19, 2017 21:14
This allows LTI consumer xblock modules being hocked into get_course_lti_endpoints for
discovering LTI result/outcome endpoints.
@robrap
robrap force-pushed the ubc-add-lti-consumer-module-to-rest-endpoints branch from 171e9aa to 4f783bb Compare January 19, 2017 21:21
@robrap
robrap merged commit 5d8e8b6 into robrap/upgrade-xblock-lti-consumer Jan 19, 2017
@robrap
robrap deleted the ubc-add-lti-consumer-module-to-rest-endpoints branch January 19, 2017 22:10
@robrap
robrap restored the ubc-add-lti-consumer-module-to-rest-endpoints branch July 6, 2017 14:16
@robrap

robrap commented Jul 6, 2017

Copy link
Copy Markdown
Contributor Author

Somehow this commit got lost in the shuffle with the consumer upgrade. I am working on it in this new PR: https://github.com/edx/edx-platform/pull/15478

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants