search

packbackbooks / lti-1-3-php-library

A library used for building IMS-certified LTI 1.3 tool providers in PHP.
Apache License 2.0
39 stars 25 forks source link

Missing properties and mismatches in Deep Link Resources #92

Closed alessandrodolci closed 1 year ago

alessandrodolci commented 1 year ago

Hello!

First of all, a big thank you for your effort with this library, we just replaced the abandoned IMSGlobal version with yours, and it saved us a lot of pain with the development of our Tool service.

During the implementation of the Deep Linking flow, we noticed that the class that maps ltiResourceLink resources (\Packback\Lti1p3\LtiDeepLinkResource) does not fully comply with the protocol specification, missing some optional properties and including some "deprecated" ones.

In particular, with reference to what is listed here, the class should include fields for the available and submission properties. At the same time, the document target is currently specified via the presentation.documentTarget property, set to either iframe or window, as per older protocol versions or drafts; while this is still supported by some platforms (or at least is by Moodle, which is what we're testing on), it would be better to migrate to the official iframe and window resource properties.

Is there a reason for this class to maintain the current structure, or is it just something you still haven't had the chance to work on? If this is the case, I'm willing to prepare a pull request with the required modifications, just let me know if you're ok with this.

Have a nice day, Alessandro

dbhynds commented 1 year ago

Alessandro,

Thanks for reaching out. Of the new features implemented in LTI Advantage, deep linking is the only one we don't actively use. As such, this area the code probably has a handful of problems with it. I suspect the issues you cited and your proposed changes are accurate. If you have a PR, we'd absolutely welcome that. If you have any questions or run into any issues as you get into it, please reach out and let me know.

alessandrodolci commented 1 year ago

Hi @dbhynds, thank you for your feedback! The modifications shouldn't require too much effort, I'll try and submit a PR in the very next days.

Have a nice weekend, Alessandro