Moodle
  1. Moodle
  2. MDL-33917

mod/resource/db/upgradelib.php typing error

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.9, 2.1.6, 2.2.3
    • Fix Version/s: 2.2.5
    • Component/s: Resource
    • Labels:
    • Testing Instructions:
      Hide
      1. Install a Moodle 1.9 site
      2. Go to Admin > Front Page > Front page settings
      3. Include a topic section
      4. On front page topic section
      5. Add a link to a file or website
      6. Upload a file
      7. Set the link to this. (click choose).
      8. Set the link explicitly so it uses the full http link (not just IMG_1.jpg explicitly to http://dan.moodle.org/moodle/file.php/1/IMG_1.jpg ) # # Save and verify that the file is linked to on front page
      9. Checkout MOODLE_22_STABLE
      10. Upgrade
      11. Check the front page resource has been upgraded correctly and is visible when you click on it
      Show
      Install a Moodle 1.9 site Go to Admin > Front Page > Front page settings Include a topic section On front page topic section Add a link to a file or website Upload a file Set the link to this. (click choose). Set the link explicitly so it uses the full http link (not just IMG_1.jpg explicitly to http://dan.moodle.org/moodle/file.php/1/IMG_1.jpg ) # # Save and verify that the file is linked to on front page Checkout MOODLE_22_STABLE Upgrade Check the front page resource has been upgraded correctly and is visible when you click on it
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Rank:
      42018

      Description

      in function resource_20_migrate(), line 79 is a typing error, i guess. (maybe nobody uses upload of resource to $siteid, so nobody recognized)

      $file_record = array('contextid'=>$context->id, 'component'=>'mod_resourse', 'filearea'=>'content', 'itemid'=>0);

      i think 'mod_resourse' should be spelled: 'mod_resource'

      Best regards
      Timo Welde

        Activity

        Hide
        Dan Poltawski added a comment -

        Wow. Thanks for the report Timo!

        Show
        Dan Poltawski added a comment - Wow. Thanks for the report Timo!
        Hide
        Dan Poltawski added a comment -

        Adding some watchers here.

        Can we fix badly previously upgraded files here? Obviously easy to spot in the database but not sure if tis possible/sensible.

        Show
        Dan Poltawski added a comment - Adding some watchers here. Can we fix badly previously upgraded files here? Obviously easy to spot in the database but not sure if tis possible/sensible.
        Hide
        Martin Dougiamas added a comment -

        Wow indeed. Agreed that we should fix the database. I don't think it's going to cause any more problems to do so but it would be good to hear from Petr about that.

        Show
        Martin Dougiamas added a comment - Wow indeed. Agreed that we should fix the database. I don't think it's going to cause any more problems to do so but it would be good to hear from Petr about that.
        Hide
        Andrew Davis added a comment -

        Does this want to go into master as well?

        Fixing the old upgrade code won't help sites that have already upgraded. Do we need additional upgrade code to essentially repeat this action and pick up anything that got missed?

        Show
        Andrew Davis added a comment - Does this want to go into master as well? Fixing the old upgrade code won't help sites that have already upgraded. Do we need additional upgrade code to essentially repeat this action and pick up anything that got missed?
        Hide
        Dan Poltawski added a comment -

        Yes, we'll need to create some upgrade code for master too to fix up too. Waiting to hear what Petr thinks.

        Show
        Dan Poltawski added a comment - Yes, we'll need to create some upgrade code for master too to fix up too. Waiting to hear what Petr thinks.
        Hide
        Petr Škoda added a comment -

        oh! sure, when fixing in database do not forget to update the pathname hash - the easiest way is probably to use recordset looping. The only problem is that the proper area may already exist when somebody fixed the missing file manually.

        Show
        Petr Škoda added a comment - oh! sure, when fixing in database do not forget to update the pathname hash - the easiest way is probably to use recordset looping. The only problem is that the proper area may already exist when somebody fixed the missing file manually.
        Hide
        Dan Poltawski added a comment -

        Yes I guess we have to look for collisions too..

        Show
        Dan Poltawski added a comment - Yes I guess we have to look for collisions too..
        Hide
        Dan Poltawski added a comment -

        Hmm, trying to find the code path which this follows. I created a moodle 1.9 site and added a front page resource but it seems to be migrated before this.

        Show
        Dan Poltawski added a comment - Hmm, trying to find the code path which this follows. I created a moodle 1.9 site and added a front page resource but it seems to be migrated before this.
        Hide
        Dan Poltawski added a comment -

        On more investigation, no bad records could've been created because the get_file_by_hash line was also completely wrong. So i'm not sure if we can fix the broken records.

        Note though that this is at least limited to resources with full urls, which isn't the default when using the resource module.

        Show
        Dan Poltawski added a comment - On more investigation, no bad records could've been created because the get_file_by_hash line was also completely wrong. So i'm not sure if we can fix the broken records. Note though that this is at least limited to resources with full urls, which isn't the default when using the resource module.
        Hide
        Dan Poltawski added a comment -

        Ok, submitting for integration for 2.2 only.

        As noted, I don't think we can do much about previously upgraded resources as they wont have been created with the incorrect details. So lets at least get it correct for new upgrades..

        Show
        Dan Poltawski added a comment - Ok, submitting for integration for 2.2 only. As noted, I don't think we can do much about previously upgraded resources as they wont have been created with the incorrect details. So lets at least get it correct for new upgrades..
        Hide
        Sam Hemelryk added a comment -

        Thanks Dan, I've integrated this now.

        Show
        Sam Hemelryk added a comment - Thanks Dan, I've integrated this now.
        Hide
        Michael de Raadt added a comment -

        Test result: Success!

        Well done all.

        Show
        Michael de Raadt added a comment - Test result: Success! Well done all.
        Hide
        Dan Poltawski added a comment -

        *Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

        Congratulations

        {tracker.user.name}

        !

        You've made into Moodle

        {tracker.fixversion-1}

        +

        I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

        cheers!

        {tracker.friendlyintegrator}
        Show
        Dan Poltawski added a comment - * Notice *: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26 Congratulations {tracker.user.name} ! You've made into Moodle {tracker.fixversion-1} + I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world. cheers! {tracker.friendlyintegrator}

          People

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

            Dates

            • Created:
              Updated:
              Resolved: