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

Fix fetching of line items and eliminate a PHP warning for missing key #35

Closed mattwright closed 3 years ago

mattwright commented 3 years ago

Summary of Changes

I couldn't get getLineItems() to return any data, and findOrCreateLineItem() was creating new line items constantly. It looks like the line items are actually returned into $response['body'] and not $response['body']['lineitems'], and this is backed up by the original PHP library and the Python library in these files:

https://github.com/IMSGlobal/lti-1-3-php-library/blob/master/src/lti/LTI_Assignments_Grades_Service.php https://github.com/dmitry-viskov/pylti1.3/blob/master/pylti1p3/assignments_grades.py

Additionally, the 'tag' key was not being isset() tested like the other fields in findOrCreateLineitem() and so was producing a warning in PHP logs if the tag was not set in a returned line item.

Testing

Manual testing that shows the line items are now being returned from Canvas LMS.

I am wondering how this regression has occurred from the library it was forked from, and whether this code is being used in any production Canvas LTI tool integrations? I really appreciate all the work on it, just wondering how production-ready this code is. Does this return behavior differ by LMS systems and some do return it in the 'lineitems' key of the response? Thanks!

dbhynds commented 3 years ago

Tagging @EricTendian since I'm OOO

mattwright commented 3 years ago

OK, I think this is ready to merge, but there is no rush.