Moodle
  1. Moodle
  2. MDL-32630

workshop incorrectly expects that admin is logged in during upgrade

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1.5, 2.2, 2.3
    • Fix Version/s: 2.1.6, 2.2.3
    • Component/s: Workshop
    • Labels:
    • Testing Instructions:
      Hide

      Testing difficulty: HARD (requires control over installed Moodle code version)

      1. Install a fresh new Moodle using the code that does not contain MDL-25660 fix yet. Hint: you can use 2.2.0 or 2.1.0 as the fix was added later on those branches
      2. Create a new course and a new workshop in it. Specify some deadlines in the "Access control" section.
      3. TEST: Make sure that no calendar events were created in the calendar (because MDL-25660 is not fixed yet in your install)
      4. Log out (this is important, we need to run the upgrade as anonymous user, not as admins)
      5. Change the code of the Moodle to the version from integration.git for the given branch (so, if your test install was based on 2.2.0, update the code to integration/MOODLE_22_STABLE etc)
      6. Visit the /admin page to execute to upgrade
      7. TEST: Make sure that the upgrade executes without the reported error
      8. TEST: Visit the course and make sure that events were created in the calendar

      (I already tested this at PostgreSQL 9.0 on the 2.2.0 -> 2.2.2+ upgrade path)

      Show
      Testing difficulty: HARD (requires control over installed Moodle code version) 1. Install a fresh new Moodle using the code that does not contain MDL-25660 fix yet. Hint: you can use 2.2.0 or 2.1.0 as the fix was added later on those branches 2. Create a new course and a new workshop in it. Specify some deadlines in the "Access control" section. 3. TEST: Make sure that no calendar events were created in the calendar (because MDL-25660 is not fixed yet in your install) 4. Log out (this is important, we need to run the upgrade as anonymous user, not as admins) 5. Change the code of the Moodle to the version from integration.git for the given branch (so, if your test install was based on 2.2.0, update the code to integration/MOODLE_22_STABLE etc) 6. Visit the /admin page to execute to upgrade 7. TEST: Make sure that the upgrade executes without the reported error 8. TEST: Visit the course and make sure that events were created in the calendar (I already tested this at PostgreSQL 9.0 on the 2.2.0 -> 2.2.2+ upgrade path)
    • Workaround:
      • use CLI upgrade script
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-32630-workshop-calendar
    • Rank:
      39559

      Description

      see http://moodle.org/mod/forum/discuss.php?d=201260

      Sorry, but you do not currently have permissions to update calendar event
      More information about this error
      Stack trace:

      • line 435 of /lib/setuplib.php: moodle_exception thrown
      • line 1912 of /calendar/lib.php: call to print_error()
      • line 2306 of /calendar/lib.php: call to calendar_event->update()
      • line 1445 of /mod/workshop/lib.php: call to calendar_event::create()
      • line 356 of /mod/workshop/db/upgrade.php: call to workshop_calendar_update()
      • line 540 of /lib/upgradelib.php: call to xmldb_workshop_upgrade()
      • line 271 of /lib/upgradelib.php: call to upgrade_plugins_modules()
      • line 1437 of /lib/upgradelib.php: call to upgrade_plugins()
      • line 269 of /admin/index.php: call to upgrade_noncore()

      Solution:
      1/ modify events api to include $checkcapability=true parameter the same way as update() method
      2/ use this new parameter in workshop_calendar_update()

        Issue Links

          Activity

          Hide
          David Mudrak added a comment -

          This was discussed in the HQ chat (see http://moodle.org/local/chatlogs/index.php?conversationid=10062#c348049 and following). The real problem is that calendar_event::update() does check permissions by default while it should not - it's obviously the caller's responsibility to do so.

          So the alternative approach is to modify calendar API so that it does not check for permissions at all (note it does not check for them in case of delete() method, for example). Then, when this is done, the workshop upgrade path would be modified so that:

          • workshop calendar events are only deleted at stable branches without an attempt to recreate them
          • workshop calendar events are recreted during the 2.2 -> 2.3 upgrade step using the new calendar_event API that does not check for permissions.

          Petr Skoda suggested to use the same version numbers in workshop upgrade code so that those who successfully upgraded (because they were logged in as admins) would have the calendar events correctly recreated even at stable branches.

          Show
          David Mudrak added a comment - This was discussed in the HQ chat (see http://moodle.org/local/chatlogs/index.php?conversationid=10062#c348049 and following). The real problem is that calendar_event::update() does check permissions by default while it should not - it's obviously the caller's responsibility to do so. So the alternative approach is to modify calendar API so that it does not check for permissions at all (note it does not check for them in case of delete() method, for example). Then, when this is done, the workshop upgrade path would be modified so that: workshop calendar events are only deleted at stable branches without an attempt to recreate them workshop calendar events are recreted during the 2.2 -> 2.3 upgrade step using the new calendar_event API that does not check for permissions. Petr Skoda suggested to use the same version numbers in workshop upgrade code so that those who successfully upgraded (because they were logged in as admins) would have the calendar events correctly recreated even at stable branches.
          Hide
          David Mudrak added a comment -

          Finally, I decided to go with the most simplest way, given the severity of this blocker and the time press we are in before the 2.3 release. The patch just makes sure that the workshop explicitly asks calendar API to not check permissions. See the commit message why I think it is safe enough.

          Submitting for integration.

          Show
          David Mudrak added a comment - Finally, I decided to go with the most simplest way, given the severity of this blocker and the time press we are in before the 2.3 release. The patch just makes sure that the workshop explicitly asks calendar API to not check permissions. See the commit message why I think it is safe enough. Submitting for integration.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (21, 22 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (21, 22 & master), thanks!
          Hide
          Rajesh Taneja added a comment -

          Works Great
          Workshop events were created, without error.
          Thanks for fixing this David.

          Show
          Rajesh Taneja added a comment - Works Great Workshop events were created, without error. Thanks for fixing this David.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This has been near becoming rejected, because it's not the best code you are able to produce.

          But, luckily, at the end, it has landed and has been spread to all repos out there.

          Many thanks and, don't forget it, keep improving your skills, you can!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - This has been near becoming rejected, because it's not the best code you are able to produce. But, luckily, at the end, it has landed and has been spread to all repos out there. Many thanks and, don't forget it, keep improving your skills, you can! Closing, ciao
          Hide
          Kimball Johnson added a comment -

          I am still getting this error upgrading mod_hotpot, so I don't think it is fixed yet.

          Stack trace:

          line 435 of /lib/setuplib.php: moodle_exception thrown
          line 2009 of /calendar/lib.php: call to print_error()
          line 1440 of /mod/hotpot/lib.php: call to calendar_event->update()
          line 1305 of /mod/hotpot/lib.php: call to hotpot_update_events()
          line 656 of /mod/hotpot/db/upgrade.php: call to hotpot_refresh_events()
          line 540 of /lib/upgradelib.php: call to xmldb_hotpot_upgrade()
          line 271 of /lib/upgradelib.php: call to upgrade_plugins_modules()
          line 1437 of /lib/upgradelib.php: call to upgrade_plugins()
          line 269 of /admin/index.php: call to upgrade_noncore()

          Thanks,

          Kimball

          Show
          Kimball Johnson added a comment - I am still getting this error upgrading mod_hotpot, so I don't think it is fixed yet. Stack trace: line 435 of /lib/setuplib.php: moodle_exception thrown line 2009 of /calendar/lib.php: call to print_error() line 1440 of /mod/hotpot/lib.php: call to calendar_event->update() line 1305 of /mod/hotpot/lib.php: call to hotpot_update_events() line 656 of /mod/hotpot/db/upgrade.php: call to hotpot_refresh_events() line 540 of /lib/upgradelib.php: call to xmldb_hotpot_upgrade() line 271 of /lib/upgradelib.php: call to upgrade_plugins_modules() line 1437 of /lib/upgradelib.php: call to upgrade_plugins() line 269 of /admin/index.php: call to upgrade_noncore() Thanks, Kimball
          Hide
          Rajesh Taneja added a comment -

          Hello Kimball,

          This fix was for workshop and error seems to be coming from hotpot. Not sure if this is related issue.

          Show
          Rajesh Taneja added a comment - Hello Kimball, This fix was for workshop and error seems to be coming from hotpot. Not sure if this is related issue.
          Hide
          Kimball Johnson added a comment -

          Andrew has informed my comment was missplaced, I will file a new bug agaisnt hotpot and reference MDL-32631 Sorry.

          Show
          Kimball Johnson added a comment - Andrew has informed my comment was missplaced, I will file a new bug agaisnt hotpot and reference MDL-32631 Sorry.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: