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

When preconfigured LTI tools are added to a course, the typeid is not set correctly.

    Details

    • Testing Instructions:
      Hide

      1. Configure an LTI tool as admin. It doesn't have to be real, just a name and url.
      2. In a course, add an External Tool activity, using the url from step 1. Make sure to use the auto matched type from the dropdown.
      3. SELECT typeid FROM mdl_lti should be the same as SELECT id from mdl_lti_types.

      Show
      1. Configure an LTI tool as admin. It doesn't have to be real, just a name and url. 2. In a course, add an External Tool activity, using the url from step 1. Make sure to use the auto matched type from the dropdown. 3. SELECT typeid FROM mdl_lti should be the same as SELECT id from mdl_lti_types.
    • Affected Branches:
      MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-44170-master

      Description

      The typeid column of the mdl_lti table doesn't get set correctly when you add an lti tool to a course. It defaults to 0 every time. This is incorrect behaviour; the typeid should be the same as the id column of mdl_lti_types for the tool in question.

      Steps to reproduce:

      1. Configure an LTI tool as admin. It doesn't have to be real, just a name and url.
      2. In a course, add an External Tool activity, using the url from step 1. Make sure to use the auto matched type from the dropdown.
      3. SELECT * FROM mdl_lti and you'll see the typeid is 0. SELECT * from mdl_lti_types and you'll see the id column of the tool you added in step 1. The typeid of mdl_lti should be the same, but it isn't.

      This causes problems when you need to lookup the permissions of the lti type; it forces you to match on tool url rather than just use the typeid.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            cibot CiBoT added a comment -
            Show
            cibot CiBoT added a comment - Results for MDL-44170 Remote repository: https://github.com/adrianfish/moodle Remote branch MDL-44170 -master to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1389 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1389/artifact/work/smurf.html
            Hide
            poltawski Dan Poltawski added a comment -

            Sending all 'waiting for peer review' issues for integration review. The integration team are doing this to ensure any 'integratable issues' will not got missed for freeze.

            Note: We will prioritise peer reviewed issues and may not spend as much time examining non-integratable, non peer-reviewed issues.

            This is a present from the iTeam - it means that peer review is not working well enough! We really do not want to do this again! Lets improve our peer review process!

            Show
            poltawski Dan Poltawski added a comment - Sending all 'waiting for peer review' issues for integration review. The integration team are doing this to ensure any 'integratable issues' will not got missed for freeze. Note: We will prioritise peer reviewed issues and may not spend as much time examining non-integratable, non peer-reviewed issues. This is a present from the iTeam - it means that peer review is not working well enough! We really do not want to do this again! Lets improve our peer review process!
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Adrian,

            I've integrated this now for 25, 26 and master (bug fixes are backported).

            Note: - I noticed too late, but the format of the commit message is slightly wrong (please remove the [] from around the issue number in future).

            Show
            damyon Damyon Wiese added a comment - Thanks Adrian, I've integrated this now for 25, 26 and master (bug fixes are backported). Note: - I noticed too late, but the format of the commit message is slightly wrong (please remove the [] from around the issue number in future).
            Hide
            damyon Damyon Wiese added a comment -

            Passing this test - tested on 25, 26 and master and it works as described.

            Show
            damyon Damyon Wiese added a comment - Passing this test - tested on 25, 26 and master and it works as described.
            Hide
            damyon Damyon Wiese added a comment -

            Grr - failed. This is failing unit tests.

            Show
            damyon Damyon Wiese added a comment - Grr - failed. This is failing unit tests.
            Hide
            damyon Damyon Wiese added a comment -

            Unit tests are passing again. Passing this issue again.

            Show
            damyon Damyon Wiese added a comment - Unit tests are passing again. Passing this issue again.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks for your efforts, this change is now part of Moodle!

            Show
            poltawski Dan Poltawski added a comment - Thanks for your efforts, this change is now part of Moodle!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/May/14