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

Triage fixes for LTI score passback bug #1560

Conversation

clpetersonucf
Copy link
Member

The bug:

As of v10.0.1-rc.1 and a database running MySQL 8, there appears to be a bug associated with the DB session driver where values written to user sessions don't persist. Specifically, the lti-link-<play_id> values written when a user replays an instance in an LTI context goes missing. This has multiple downstream impacts, most importantly that upon submission, a replay isn't considered an LTI play and the score is not returned to the LTI provider. It's not yet clear if the underlying cause is related to:

  • Changes between MySQL 5.7 and 8 that cause session writes to sometimes go bad
  • Issues with session rotation, specifically when the Session table is under significant load
  • Issues with FuelPHP 1.9 AND MySQL 8 or some combination of the above

Until the core issue can be identified, this PR serves as a stopgap measure. Changes include:

  • Modifies scoreScreenUrl in the widget-player component to use useRef instead of state. While this didn't fix the core issue, it potentially alleviates a race condition.
  • Adds auth and environment_data values to the \Materia\Session\Play instance properties when requested via get_by_id. These are were not previously included in the DB query.
  • Adds "triage" fallback logic to the ltievents class when requesting lti-link-<play_id> from session. If the token value is not present in session, it interrogates (or creates and then interrogates) a play instance for the auth and environment_data properties. The play's environment data will include the token value that was supposed to be stored in session.

Copy link
Contributor

@cayb0rg cayb0rg left a comment

Choose a reason for hiding this comment

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

Works as expected on localhost (including when I make the token null on line 353 of ltievents.php) and QA (embedded in assignment page and in a new tab). I think this is good to go, but I'm no expert here. Also, I hadn't thought to use useRef. Nice work!

@clpetersonucf clpetersonucf merged commit dcd4ca1 into ucfopen:dev/10.1.0 Jan 25, 2024
2 checks passed
@clpetersonucf clpetersonucf deleted the issue/1558-score-screen-url-race-condition branch January 25, 2024 21:46
@clpetersonucf clpetersonucf mentioned this pull request Jan 30, 2024
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.

2 participants