Moodle
  1. Moodle
  2. MDL-37781

Workshop scheduled phase switching requires cron.php executed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.4, 2.4.1
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Workshop
    • Labels:
    • Testing Instructions:
      Hide

      Testing difficulty: easy

      The point of the testing is to make sure that the workshop switches from the submission phase into the assessment phase even without the cron.php being executed (make sure you do not run cron.php on the test site during this test).

      As usually when dealing with the Workshop testing, you may find http://docs.moodle.org/dev/Workshop/fakesubmissions.php useful.

      1. Prepare a simple workshop in a course with at least two participants.
      2. In the workshop setting, set the Submissions deadline to a near future (like T + 15 minutes or so). Also enable Switch to the next phase after the submissions deadline.
      3. Switch the workshop manually into the submission phase and let students submit their work.
      4. Enable the scheduled allocation and make sure its state is something like 'To be executed on {the submission deadline}

        '

      5. Go to the coffee machine and have a nice coffee or tea. Wander around the office for a while and have a friendly chat with your colleagues. Come back to the workshop after the configured submissions deadline.
      6. Visit the workshop main page (view.php)
      7. TEST: Make sure the workshop is now in the assessment phase.
      8. TEST: Make sure the submissions were allocated for the review as configured.
      9. TEST: Make sure the scheduled allocator's state is something like 'Executed on {the time when you displayed the view.php}

        '

      10. Wait a minute of two (hey, what about putting the coffee cup into the dishwasher meanwhile?)
      11. Switch the workshop into the submission phase manually and reload the view.php page.
      12. TEST: Make sure the workshop stays in the submission phase.

      Thanks for testing this!

      Show
      Testing difficulty: easy The point of the testing is to make sure that the workshop switches from the submission phase into the assessment phase even without the cron.php being executed (make sure you do not run cron.php on the test site during this test). As usually when dealing with the Workshop testing, you may find http://docs.moodle.org/dev/Workshop/fakesubmissions.php useful. Prepare a simple workshop in a course with at least two participants. In the workshop setting, set the Submissions deadline to a near future (like T + 15 minutes or so). Also enable Switch to the next phase after the submissions deadline . Switch the workshop manually into the submission phase and let students submit their work. Enable the scheduled allocation and make sure its state is something like 'To be executed on {the submission deadline} ' Go to the coffee machine and have a nice coffee or tea. Wander around the office for a while and have a friendly chat with your colleagues. Come back to the workshop after the configured submissions deadline. Visit the workshop main page (view.php) TEST: Make sure the workshop is now in the assessment phase. TEST: Make sure the submissions were allocated for the review as configured. TEST: Make sure the scheduled allocator's state is something like 'Executed on {the time when you displayed the view.php} ' Wait a minute of two (hey, what about putting the coffee cup into the dishwasher meanwhile?) Switch the workshop into the submission phase manually and reload the view.php page. TEST: Make sure the workshop stays in the submission phase. Thanks for testing this!
    • Workaround:
      Hide

      Say "Bah

      Show
      Say "Bah
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      MDL-37781-workshop-schedule_24
    • Pull Master Branch:
      MDL-37781-workshop-schedule
    • Rank:
      47515

      Description

      As discussed in MDL-26099: "There is a space for improvements, for sure. For example, the Workshop could check for the need of scheduled allocation every time when some user opens the workshop main page. For example, let us say the phase switching (and thence the allocation) is scheduled to 12:40 PM and the cron runs every 30 minutes. So the cron execution at 12:30 did not trigger anything yet, the next execution at 13:00 would do it. We could improve Workshop so that when some user (teacher, student or even a guest) looks at the workshop page at 12:43 PM, it would trigger the phase switch (so the result is what the user expects). Let me know if this would be suitable for your needs and we can open a new issue for that."

        Issue Links

          Activity

          Hide
          David Mudrak added a comment -

          Petr, we were talking about this recently. If you have a minute, can you throw an eye on the patch and peer-review it? Thanks in advance.

          Show
          David Mudrak added a comment - Petr, we were talking about this recently. If you have a minute, can you throw an eye on the patch and peer-review it? Thanks in advance.
          Hide
          Petr Škoda added a comment -

          hi, I like the use of event API here - I agree it is a lot easier instead of hardcoding various expectations in workshop about subplugin types, +1

          Show
          Petr Škoda added a comment - hi, I like the use of event API here - I agree it is a lot easier instead of hardcoding various expectations in workshop about subplugin types, +1
          Hide
          David Mudrak added a comment -

          Thanks Petr for the review. I'm submitting this for the integration now.

          Show
          David Mudrak added a comment - Thanks Petr for the review. I'm submitting this for the integration now.
          Hide
          David Mudrak added a comment -

          Once this is integrated, we need to fix the docs describing the automatic phase switching feature.

          Show
          David Mudrak added a comment - Once this is integrated, we need to fix the docs describing the automatic phase switching feature.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Damyon Wiese added a comment -

          Thanks David, I like the functionality - just asking for a second opinion on the code duplication here.

          Show
          Damyon Wiese added a comment - Thanks David, I like the functionality - just asking for a second opinion on the code duplication here.
          Hide
          Damyon Wiese added a comment -

          Thanks David, this is integrated now in master, 24 and 23.

          Re: the duplication comment above - it might be nicer to also have an event handler for auto changing the stage of the workshop and then call your event handlers from both cron and view.php.

          Just a suggestion - thanks for the patch.

          Show
          Damyon Wiese added a comment - Thanks David, this is integrated now in master, 24 and 23. Re: the duplication comment above - it might be nicer to also have an event handler for auto changing the stage of the workshop and then call your event handlers from both cron and view.php. Just a suggestion - thanks for the patch.
          Hide
          David Mudrak added a comment -

          Thanks for the comments.

          We discussed this "duplication" with Petr during the peer-review. However, I believe there is no need for yet another wrapper. Looking closely at the code, you can easily spot that the real magic is in the single method call workshop::switch_phase(). All the code around is to prepare the environment for this and to respect differences in view.php (that already has the whole workshop class instance loaded) and cron (that processes all workshop candidates in a loop). In other words, I do not think it is worth of adding a shared wrapper around a single method call.

          Please note this is a subject of change in the near future anyway as it is planned to add automatic phase switching support for other phases as well.

          Show
          David Mudrak added a comment - Thanks for the comments. We discussed this "duplication" with Petr during the peer-review. However, I believe there is no need for yet another wrapper. Looking closely at the code, you can easily spot that the real magic is in the single method call workshop::switch_phase(). All the code around is to prepare the environment for this and to respect differences in view.php (that already has the whole workshop class instance loaded) and cron (that processes all workshop candidates in a loop). In other words, I do not think it is worth of adding a shared wrapper around a single method call. Please note this is a subject of change in the near future anyway as it is planned to add automatic phase switching support for other phases as well.
          Hide
          Jason Fowler added a comment -

          All works David, thanks for the patch (and the great testing instructions)

          Show
          Jason Fowler added a comment - All works David, thanks for the patch (and the great testing instructions)
          Hide
          Damyon Wiese added a comment -

          Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone!

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone! Regards, Damyon
          Hide
          Mary Cooch added a comment -

          Removing docs_required as I have added a note about this here http://docs.moodle.org/en/Workshop_settings (in 2.3 and 2.4 docs)

          Show
          Mary Cooch added a comment - Removing docs_required as I have added a note about this here http://docs.moodle.org/en/Workshop_settings (in 2.3 and 2.4 docs)

            People

            • Votes:
              7 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: