Skip to content
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.

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

Fix an issue that can cause LTI 1.3 grade passback to fail when called from the job queue. #2319

Conversation

drgrice1
Copy link
Member

The get_access_token method calls $c->url_for (on line 141 of WeBWorK::Authen::LTIAdvantage::SubmitGrade. This is only called if the access token is not saved in the database or if the access token that is saved is expired. When this is called from the minion job queue, $c is not a Mojolicious::Controller. Instead it is an unblessed hash reference that has the keys ce, db, and app defined. Clearly that unblessed hash reference will not have the url_for method. As such, the job will fail if a new access token is needed. So as is done on line 249 in the submit_grade method, the $c object needs to be switched to the passed in $c->{app} object, which does have the url_for method.

Note that with the current code this method will still succeed if the database has an unexpired access token stored. Also, this method will succeed if called with a Mojolicious::Controller object (for instance when called in the case that a student initially logs in via LTI 1.3 authentication or in the case that a student submits an answer to a problem or grades a test).

This could be considered for a hotfix.

@Alex-Jordan
Copy link
Contributor

It sounds like this is unlikely to explain why I have not been able to get grade passback working with D2L, because one should expect that most of the time, that access token should be present and not expired. Is that right? Or is it possible this plays into the issues I had with grade passback and LTI 1.3?

@drgrice1
Copy link
Member Author

It is probably not your problem. If you were able to log in via LTI 1.3 authentication, then it should obtain an access token at that time. If a job to pass grades back follows, then it should work.

Although, in my recent work on this, I thought of something that might be your issue, or at least something to try. I will give details next week.

@drgrice1 drgrice1 force-pushed the bugfix/lti-1.3-job-queue-submit-grade branch 4 times, most recently from 9878ad8 to 696ac55 Compare February 14, 2024 23:09
Copy link
Contributor

@Alex-Jordan Alex-Jordan left a comment

Choose a reason for hiding this comment

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

Did not test, but the explanation sounds good and code and commentary is good.

@drgrice1 drgrice1 force-pushed the bugfix/lti-1.3-job-queue-submit-grade branch 2 times, most recently from 17c97d1 to 714509f Compare February 25, 2024 12:13
@drgrice1 drgrice1 force-pushed the bugfix/lti-1.3-job-queue-submit-grade branch 3 times, most recently from 02b9c12 to 1d40ae1 Compare February 29, 2024 21:59
Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

I can't test this, but looks reasonable based on @drgrice1's explanation.

…d from the job queue.

The `get_access_token` method calls `$c->url_for` (on line 141 of
`WeBWorK::Authen::LTIAdvantage::SubmitGrade`.  This is only called if
the access token is not saved in the database or if the access token
that is saved is expired.  When this is called from the minion job
queue, `$c` is not a `Mojolicious::Controller`.  Instead it is an
unblessed hash reference that has the keys `ce`, `db`, and `app`
defined.  Clearly that unblessed hash reference will not have the
`url_for` method.  As such, the job will fail if a new access token is
needed.  So as is done on line 249 in the `submit_grade` method, the
`$c` object needs to be switched to the passed in `$c->{app}` object,
which does have the `url_for` method.

Note that with the current code this method will still succeed if the
database has an unexpired access token stored.  Also, this method will
succeed if called with a `Mojolicious::Controller` object (for instance
when called in the case that a student initially logs in via LTI 1.3
authentication or in the case that  a student submits an answer to a
problem or grades a test).

This could be considered for a hotfix.
@drgrice1 drgrice1 force-pushed the bugfix/lti-1.3-job-queue-submit-grade branch from 1d40ae1 to c10f3c7 Compare March 2, 2024 00:33
@Alex-Jordan Alex-Jordan merged commit afc36ae into openwebwork:develop Mar 5, 2024
2 checks passed
@drgrice1 drgrice1 deleted the bugfix/lti-1.3-job-queue-submit-grade branch March 5, 2024 21:13
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.

3 participants