Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-53351

lti_delete_instance() tries to get property of non-object

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      Shorter: Testing not needed, the fix is covered by a unit test.

      Longer:

      1. Add a new instance of the External tool (LTI) to a course, use default settings (in fact, only name is needed).
      2. Delete the instance and make sure no PHP notice is thrown. This means either
        1. avoid deleting via AJAX, e.g. by opening the "Delete" action link in a new tab or having javascript disabled
        2. or watching the server's error_log.
      Show
      Shorter: Testing not needed, the fix is covered by a unit test. Longer: Add a new instance of the External tool (LTI) to a course, use default settings (in fact, only name is needed). Delete the instance and make sure no PHP notice is thrown. This means either avoid deleting via AJAX, e.g. by opening the "Delete" action link in a new tab or having javascript disabled or watching the server's error_log.
    • Affected Branches:
      MOODLE_30_STABLE
    • Fixed Branches:
      MOODLE_29_STABLE, MOODLE_30_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-53351-master-lti-notice

      Description

      I found that when a mod_lti instance is being deleted, it throws a PHP notice:

      PHP Notice:  Trying to get property of non-object in mod/lti/lib.php on line 192
      

      I found that it is caused by the lti_delete_instance() code doing

          $ltitype = $DB->get_record('lti_types', array('id' => $basiclti->typeid));
          $DB->delete_records('lti_tool_settings',
              array('toolproxyid' => $ltitype->toolproxyid, 'course' => $basiclti->course, 'coursemoduleid' => $id));
      

      without checking that $ltitype actually found the record.

      Note that typeid=0 is a valid and common value representing "Automatic, based on launch URL".

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              mudrd8mz David Mudrák (@mudrd8mz)
              Reporter:
              mudrd8mz David Mudrák (@mudrd8mz)
              Peer reviewer:
              cameron1729
              Integrator:
              Andrew Nicols
              Tester:
              John Okely
              Participants:
              Component watchers:
              Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                9/Nov/15