search

openedx / xblock-lti-consumer

GNU Affero General Public License v3.0
28 stars 84 forks source link

Race conditions when multiple XBlocks are in a single unit #379

Open Agrendalath opened 1 year ago

Agrendalath commented 1 year ago

There is a race condition when there are multiple LTI Consumer XBlocks in a single unit. The first block has a correct LTI ID, and the second one has an incorrect one. I didn't change anything between these screenshots - I only refreshed the page.

The first refresh
The second refresh

Originally posted by @Agrendalath in https://github.com/openedx/xblock-lti-consumer/issues/378#issuecomment-1572134893

Update: we have the solution, but we still need to update the docs.

RafayGhafoor commented 1 year ago

@Agrendalath, The error is from the lti_url i.e., https://saltire.lti.app/tool and doesn't occur when the url used is from the docs i.e., https://lti.tools/saltire/tp

I have checked the network logs and the requests propagate correctly in the right manner from the launch_handler, the resource ids are sent correct, similarly the post request going to the Saltire LTI for registering the event is also correct but the responses received from them always contain the same resource id, so all the verification results are basically the same. This seems like a bug on the saltire lti app tool which is not sending the responses in correct way.

navinkarkera commented 1 year ago

@RafayGhafoor Just to be certain, the problem will be solved if we use https://lti.tools/saltire/tp as the lti_url?

RafayGhafoor commented 1 year ago

@navinkarkera yes, that's right.

Agrendalath commented 1 year ago

@RafayGhafoor, https://saltire.lti.app/ is a newer page version (with LTI 1.3 support), so we should contact them about this bug. I'll leave the details in the Jira ticket (private link).

Agrendalath commented 1 year ago

Update: Stephen from SPV Software Products resolved this by adding an option to turn off page refreshes. We can use it through the query param: https://saltire.lti.app/tool?norefresh.

The only remaining step here is updating the docs.