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

q* plugin types should be updated to use cron_execute_plugin_type

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3
    • Fix Version/s: 2.2.2
    • Component/s: Questions, Quiz
    • Labels:
    • Testing Instructions:
      Hide

      1. Go to the quiz statistics report, and generate some statistics. These are cached in the quiz_statistics and related tables.

      2. Wait 5+ hours (or go into the quiz_statistics table, and hack the timestamps to be a long way in the past).

      3. Run cron.

      Verify there are no errors relating to q* plugin types.

      Verify that the old cached quiz_statistics and related data has been deleted.

      This needs to be tested on 2.2 and master, since the changes are different in each place.

      Show
      1. Go to the quiz statistics report, and generate some statistics. These are cached in the quiz_statistics and related tables. 2. Wait 5+ hours (or go into the quiz_statistics table, and hack the timestamps to be a long way in the past). 3. Run cron. Verify there are no errors relating to q* plugin types. Verify that the old cached quiz_statistics and related data has been deleted. This needs to be tested on 2.2 and master, since the changes are different in each place.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Currently

      • qtypes are fine
      • quiz reports have custom cron, and need to be converted.
      • quizaccess, qformat and qbehaviour plugins cannot have cron. They should be enabled.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            2.2 branch just adds standard cron support to quiz report plugins, so we can drop non-standard support in 2.3.

            The master branch adds support for all plugin types that did not already have it, and then drop the legacy support in quiz reports.

            Show
            timhunt Tim Hunt added a comment - 2.2 branch just adds standard cron support to quiz report plugins, so we can drop non-standard support in 2.3. The master branch adds support for all plugin types that did not already have it, and then drop the legacy support in quiz reports.
            Hide
            timhunt Tim Hunt added a comment -

            Note that this cannot be submitted for integration until after MDL-30610 has been integrated, and these branches have been rebased.

            Show
            timhunt Tim Hunt added a comment - Note that this cannot be submitted for integration until after MDL-30610 has been integrated, and these branches have been rebased.
            Hide
            skodak Petr Skoda added a comment -

            great!

            Show
            skodak Petr Skoda added a comment - great!
            Hide
            timhunt Tim Hunt added a comment -

            Finally, this can be submitted for integration.

            I have just rebased the branches.

            Show
            timhunt Tim Hunt added a comment - Finally, this can be submitted for integration. I have just rebased the branches.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Looks perfect, integrated, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Looks perfect, integrated, thanks!
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi Tim,
            As discussed, the cron is not activated atm.
            Failing this for further investigation.
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi Tim, As discussed, the cron is not activated atm. Failing this for further investigation. Thanks
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            Also, apart from the need to upgrade & change quiz's cron, I've detected that the name of the cron functions in the subplugins are incorrect. For example, the statistics one should be:

            quiz_statistics_cron()

            and currently you're using:

            quiz_report_statistics_cron()

            So it's not detected properly by get_plugin_list_with_function()

            Surely the same happens with other subplugins. (seems accessrule ones have not any cron implementation, edited)

            When I reviewed this, I forgot the annoying name that quiz reports subplugin have ('quiz'). IMO we should move to 'quizreport' or similar asap.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited Also, apart from the need to upgrade & change quiz's cron, I've detected that the name of the cron functions in the subplugins are incorrect. For example, the statistics one should be: quiz_statistics_cron() and currently you're using: quiz_report_statistics_cron() So it's not detected properly by get_plugin_list_with_function() Surely the same happens with other subplugins. (seems accessrule ones have not any cron implementation, edited) When I reviewed this, I forgot the annoying name that quiz reports subplugin have ('quiz'). IMO we should move to 'quizreport' or similar asap. Ciao
            Hide
            timhunt Tim Hunt added a comment -

            Please re-integrate

            Note that these branches include the fix for MDL-26469 for 2.2 and master, which are required for this to work. I hope that is not too confusing.

            Show
            timhunt Tim Hunt added a comment - Please re-integrate https://github.com/timhunt/moodle/compare/master...MDL-30635b https://github.com/timhunt/moodle/compare/MOODLE_22_STABLE...MDL-30635b_22 Note that these branches include the fix for MDL-26469 for 2.2 and master, which are required for this to work. I hope that is not too confusing.
            Hide
            timhunt Tim Hunt added a comment -

            ... but note that MDL-26469 also needs to be fixed in 2.1 and 2.0.

            Show
            timhunt Tim Hunt added a comment - ... but note that MDL-26469 also needs to be fixed in 2.1 and 2.0.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Re integrated, with the 2 new commits (22 and master). Now $module->cron should be considered and upgraded ok (as MDL-26469 fixed that).

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Re integrated, with the 2 new commits (22 and master). Now $module->cron should be considered and upgraded ok (as MDL-26469 fixed that).
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            This is working great now!
            Passing
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - This is working great now! Passing Thanks
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This virus has been spread upstream, everybody will be infected soon. Congrats, you did it!

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This virus has been spread upstream, everybody will be infected soon. Congrats, you did it! Closing, ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Adding the docs_required to this just in case it needs documentation in upgrade files and release notes/moodle docs.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Adding the docs_required to this just in case it needs documentation in upgrade files and release notes/moodle docs.
            Hide
            timhunt Tim Hunt added a comment -

            This patch already contained changes to upgrade.txt files to document this.

            The documentation for these plugin types already exists, and does not mention cron at all. So the fact that these plugins can now do cron in the standard way is good, and probably does not need to be documented explicitly.

            Show
            timhunt Tim Hunt added a comment - This patch already contained changes to upgrade.txt files to document this. The documentation for these plugin types already exists, and does not mention cron at all. So the fact that these plugins can now do cron in the standard way is good, and probably does not need to be documented explicitly.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            (agree, thanks!)

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - (agree, thanks!)

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Mar/12