[LTI Provider] Refactoring to remove the lti_run method - #8647
Conversation
|
Thanks for the pull request, @mcgachey! I've created OSPR-670 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams. Feel free to add as much of the following information to the ticket:
All technical communication about the code itself will still be done via the Github pull request interface. As a reminder, our process documentation is here. |
openedx-webhooks
added
open-source-contribution
|
Makes sense to me. I'm passing off to Dave to review; please make sure your branch has been rebased since this morning (some failing tests got fixed) and that tests/quality all pass. |
af1d17c to
6fd26e2
Compare
|
@sarina - it's not essential; if you're trying to reduce churn it could hold off until after the release is cut. |
|
Churn is fine :) I'm just trying to make sure we have attention on On Mon, Jun 29, 2015 at 4:20 PM, Phil McGachey notifications@github.com
|
|
👍 @mcgachey: if this is all set, I'll hit the merge button. |
openedx-webhooks
added
waiting on author
This change is a follow-up to the chages in PR 8347, which removed the edX login page from the workflow for a new user. Where previously we redirected a user to the login page, PR 8347 instead creates a new user transparently and logs them in. The initial reason for splitting the LTI view between lti_launch and lti_run was so that there was a target for the GET request that followed the login page. Since we no longer use the login page, we no longer need the second view. We also don't need to store the LTI parameters in the session any more, since they are not persisting between calls. This simplifies the view logic significantly. The other change here is to fetch the LtiConsumer object early in the view, and pass it to the SignatureValidator and scoring system. When the views were split, this required multiple DB hits for the same data; we're now only fetching it once.
6fd26e2 to
99fbf4d
Compare
|
@ormsbee: Sorry for the wait (I've been having some devstack issues). I rebased this morning and it all looks good to merge. |
[LTI Provider] Refactoring to remove the lti_run method
4 participants
This change is a follow-up to the chages in PR #8347, which removed the
edX login page from the workflow for a new user. Where previously we
redirected a user to the login page, PR #8347 instead creates a new user
transparently and logs them in.
The initial reason for splitting the LTI view between lti_launch and
lti_run was so that there was a target for the GET request that
followed the login page. Since we no longer use the login page, we
no longer need the second view. We also don't need to store the LTI
parameters in the session any more, since they are not persisting
between calls. This simplifies the view logic significantly.
The other change here is to fetch the LtiConsumer object early in
the view, and pass it to the SignatureValidator and scoring system.
When the views were split, this required multiple DB hits for the
same data; we're now only fetching it once.