Moodle
  1. Moodle
  2. MDL-38210

Error when running participation report for Workshop activity

    Details

    • Type: Bug Bug
    • Status: Development in progress
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.4.1, 2.5.5, 2.6.2, 2.7
    • Fix Version/s: None
    • Component/s: Reports, Workshop
    • Database:
      Any
    • Testing Instructions:
      Hide

      1. As the admin, disable all the log stores. Enable the "Legacy log" store only.
      2. Check the Legacy log store settings and make sure that "Log legacy data" (logstore_legacy | loglegacy) is enabled.
      3. As the admin / teacher, set up a simple workshop and put it into the Submission phase
      4. Log in as a student and submit into the Workshop (eventually edit the submission, visit the workshop page couple of times etc).
      5. As a teacher, check the Course participation report for that Workshop
      6. TEST: Make sure no error is thrown and the number of the student's post / view actions are reported correctly.
      7. TEST: Check that the teacher's activity is reported, too.

      Show
      1. As the admin, disable all the log stores. Enable the "Legacy log" store only. 2. Check the Legacy log store settings and make sure that "Log legacy data" (logstore_legacy | loglegacy) is enabled. 3. As the admin / teacher, set up a simple workshop and put it into the Submission phase 4. Log in as a student and submit into the Workshop (eventually edit the submission, visit the workshop page couple of times etc). 5. As a teacher, check the Course participation report for that Workshop 6. TEST: Make sure no error is thrown and the number of the student's post / view actions are reported correctly. 7. TEST: Check that the teacher's activity is reported, too.
    • Workaround:
      Hide

      Use the new Standard logging

      Show
      Use the new Standard logging
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE, MOODLE_27_STABLE
    • Pull from Repository:
    • Pull 2.6 Branch:
      mdl38210-moodle26
    • Pull 2.7 Branch:
      mdl38210-moodle27
    • Pull 2.8 Branch:
      mdl38210-moodle28
    • Pull Master Branch:
      mdl38210-master

      Description

      When I try and run a participation report for a Workshop activity (which does have submissions), I receive the following error message:

      Module workshop is missing the code needed to perform this function

      Debugging reveals the following stack trace:

      Debug info:
      Error code: modulemissingcode
      Stack trace:
      line 467 of /lib/setuplib.php: moodle_exception thrown
      line 173 of /report/participation/index.php: call to print_error()

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            David Mudrak added a comment -

            Good catch! Thanks for reporting this. I can confirm that Workshop 2.x has never had required workshop_get_view_actions() and workshop_get_post_actions() callbacks. And nobody has noticed since Moodle 2.0 :-o

            Show
            David Mudrak added a comment - Good catch! Thanks for reporting this. I can confirm that Workshop 2.x has never had required workshop_get_view_actions() and workshop_get_post_actions() callbacks. And nobody has noticed since Moodle 2.0 :-o
            Hide
            Tim Lock added a comment -

            Hi David,

            Any progress on this patch?

            Show
            Tim Lock added a comment - Hi David, Any progress on this patch?
            Hide
            Tim Lock added a comment -

            Added github links.

            Show
            Tim Lock added a comment - Added github links.
            Hide
            CiBoT added a comment -

            Results for MDL-38210

            Show
            CiBoT added a comment - Results for MDL-38210 Remote repository: https://github.com/tlock/moodle.git Remote branch mdl38210-moodle25 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2681 Error: The mdl38210-moodle25 branch at https://github.com/tlock/moodle.git exceeds the maximum number of commits ( 18 > 15) Remote branch mdl38210-moodle26 to be integrated into upstream MOODLE_26_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2682 Error: The mdl38210-moodle26 branch at https://github.com/tlock/moodle.git exceeds the maximum number of commits ( 28 > 15) Remote branch mdl38210-master to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2683 Error: The mdl38210-master branch at https://github.com/tlock/moodle.git exceeds the maximum number of commits ( 90 > 15)
            Hide
            Ankit Agarwal added a comment -

            Hi,
            You might want to keep an eye on the changes in MDL-41283, you might have to update your master branch based on MDL-41283.

            Ps:- I have not reviewed the full code.

            Cheers

            Show
            Ankit Agarwal added a comment - Hi, You might want to keep an eye on the changes in MDL-41283 , you might have to update your master branch based on MDL-41283 . Ps:- I have not reviewed the full code. Cheers
            Hide
            David Mudrak added a comment -

            Good catch on MDL-41283 in progress. However, I'm not sure if rebasing against a "wip" branch is a good idea.

            Show
            David Mudrak added a comment - Good catch on MDL-41283 in progress. However, I'm not sure if rebasing against a "wip" branch is a good idea.
            Hide
            David Mudrak added a comment -

            Thanks for your work on this Tim. I have some things to discuss.

            • Firstly, I still have a feeling that the current behaviour of the Participation report is wrong. It should not die just because some activity module does not implement callbacks it expects. It should throw a DEBUG_NORMAL message and skip. But that's another issue.
            • Workshop logs more actions (like 'view example' or 'add assessment') than you included in the patch. Did you intentionally selected just some of them? If so, what was the criterion?
            • WRT the coding style, I would like to keep space after commas and some note in the phpDoc block that these two functions are used by the participation report.
            Show
            David Mudrak added a comment - Thanks for your work on this Tim. I have some things to discuss. Firstly, I still have a feeling that the current behaviour of the Participation report is wrong. It should not die just because some activity module does not implement callbacks it expects. It should throw a DEBUG_NORMAL message and skip. But that's another issue. Workshop logs more actions (like 'view example' or 'add assessment') than you included in the patch. Did you intentionally selected just some of them? If so, what was the criterion? WRT the coding style, I would like to keep space after commas and some note in the phpDoc block that these two functions are used by the participation report.
            Hide
            Ankit Agarwal added a comment -

            Yup, it is being changed a lot as we speak, would be nice to wait for a week or so for that to be integrated, before rebasing and pushing this forward.

            Show
            Ankit Agarwal added a comment - Yup, it is being changed a lot as we speak, would be nice to wait for a week or so for that to be integrated, before rebasing and pushing this forward.
            Hide
            David Mudrak added a comment -

            OK, let's wait then. I believe that the code freeze does not apply to this (as this is a bug, not a new feature) and the patch will be pretty trivial.

            Show
            David Mudrak added a comment - OK, let's wait then. I believe that the code freeze does not apply to this (as this is a bug, not a new feature) and the patch will be pretty trivial.
            Hide
            Tim Lock added a comment -

            Updated github links accordingly.

            I picked the main activities in the Moodle logs which I have included more now.

            Show
            Tim Lock added a comment - Updated github links accordingly. I picked the main activities in the Moodle logs which I have included more now.
            Hide
            CiBoT added a comment -
            Show
            CiBoT added a comment - Results for MDL-38210 Remote repository: https://github.com/tlock/moodle.git Remote branch mdl38210-moodle25 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2703 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/2703/artifact/work/smurf.html Remote branch mdl38210-moodle26 to be integrated into upstream MOODLE_26_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2704 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/2704/artifact/work/smurf.html Remote branch mdl38210-master to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2705 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/2705/artifact/work/smurf.html
            Hide
            David Mudrak added a comment -

            Thanks Tim. Looking at mod/workshop/db/log.php it seems that most of relevant actions are now returned

            $logs = array(
                // workshop instance log actions
                array('module'=>'workshop', 'action'=>'add', 'mtable'=>'workshop', 'field'=>'name'),
                array('module'=>'workshop', 'action'=>'update', 'mtable'=>'workshop', 'field'=>'name'),
                array('module'=>'workshop', 'action'=>'view', 'mtable'=>'workshop', 'field'=>'name'),
                array('module'=>'workshop', 'action'=>'view all', 'mtable'=>'workshop', 'field'=>'name'),
                // submission log actions
                array('module'=>'workshop', 'action'=>'add submission', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                array('module'=>'workshop', 'action'=>'update submission', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                array('module'=>'workshop', 'action'=>'view submission', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                // assessment log actions
                array('module'=>'workshop', 'action'=>'add assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                array('module'=>'workshop', 'action'=>'update assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                // example log actions
                array('module'=>'workshop', 'action'=>'add example', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                array('module'=>'workshop', 'action'=>'update example', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                array('module'=>'workshop', 'action'=>'view example', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                // example assessment log actions
                array('module'=>'workshop', 'action'=>'add reference assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                array('module'=>'workshop', 'action'=>'update reference assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                array('module'=>'workshop', 'action'=>'add example assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                array('module'=>'workshop', 'action'=>'update example assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'),
                // grading evaluation log actions
                array('module'=>'workshop', 'action'=>'update aggregate grades', 'mtable'=>'workshop', 'field'=>'name'),
                array('module'=>'workshop', 'action'=>'update clear aggregated grades', 'mtable'=>'workshop', 'field'=>'name'),
                array('module'=>'workshop', 'action'=>'update clear assessments', 'mtable'=>'workshop', 'field'=>'name'),
            );
            

            I think that "update assessment" should be returned too.

            But. I must admit I'm a bit confused by inconsistencies with what kind of actions are being returned here by various modules. For example, the plain "view" is considered as a "get" action by the Quiz while not by the new Assignment. Actions like "add" (when new instance is created) do not seem to be considered as relevant participation actions.

            It would be good to actually talk a bit about what the Participation report is even considered to display.

            Show
            David Mudrak added a comment - Thanks Tim. Looking at mod/workshop/db/log.php it seems that most of relevant actions are now returned $logs = array( // workshop instance log actions array('module'=>'workshop', 'action'=>'add', 'mtable'=>'workshop', 'field'=>'name'), array('module'=>'workshop', 'action'=>'update', 'mtable'=>'workshop', 'field'=>'name'), array('module'=>'workshop', 'action'=>'view', 'mtable'=>'workshop', 'field'=>'name'), array('module'=>'workshop', 'action'=>'view all', 'mtable'=>'workshop', 'field'=>'name'), // submission log actions array('module'=>'workshop', 'action'=>'add submission', 'mtable'=>'workshop_submissions', 'field'=>'title'), array('module'=>'workshop', 'action'=>'update submission', 'mtable'=>'workshop_submissions', 'field'=>'title'), array('module'=>'workshop', 'action'=>'view submission', 'mtable'=>'workshop_submissions', 'field'=>'title'), // assessment log actions array('module'=>'workshop', 'action'=>'add assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'), array('module'=>'workshop', 'action'=>'update assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'), // example log actions array('module'=>'workshop', 'action'=>'add example', 'mtable'=>'workshop_submissions', 'field'=>'title'), array('module'=>'workshop', 'action'=>'update example', 'mtable'=>'workshop_submissions', 'field'=>'title'), array('module'=>'workshop', 'action'=>'view example', 'mtable'=>'workshop_submissions', 'field'=>'title'), // example assessment log actions array('module'=>'workshop', 'action'=>'add reference assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'), array('module'=>'workshop', 'action'=>'update reference assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'), array('module'=>'workshop', 'action'=>'add example assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'), array('module'=>'workshop', 'action'=>'update example assessment', 'mtable'=>'workshop_submissions', 'field'=>'title'), // grading evaluation log actions array('module'=>'workshop', 'action'=>'update aggregate grades', 'mtable'=>'workshop', 'field'=>'name'), array('module'=>'workshop', 'action'=>'update clear aggregated grades', 'mtable'=>'workshop', 'field'=>'name'), array('module'=>'workshop', 'action'=>'update clear assessments', 'mtable'=>'workshop', 'field'=>'name'), ); I think that "update assessment" should be returned too. But. I must admit I'm a bit confused by inconsistencies with what kind of actions are being returned here by various modules. For example, the plain "view" is considered as a "get" action by the Quiz while not by the new Assignment. Actions like "add" (when new instance is created) do not seem to be considered as relevant participation actions. It would be good to actually talk a bit about what the Participation report is even considered to display.
            Hide
            Tim Lock added a comment -

            Updated github links + added 'update assessment', 'update example'

            Show
            Tim Lock added a comment - Updated github links + added 'update assessment', 'update example'
            Show
            CiBoT added a comment - Fails against automated checks. Checked MDL-38210 using repository: https://github.com/tlock/moodle.git MOODLE_26_STABLE (2 errors / 1 warnings) [branch: mdl38210-moodle26 | CI Job ] phplint (0/0) , php (0/1) , js (0/0) , css (0/0) , phpdoc (0/0) , commit (2/0) , savepoint (0/0) , thirdparty (0/0) , MOODLE_27_STABLE (2 errors / 1 warnings) [branch: mdl38210-moodle27 | CI Job ] phplint (0/0) , php (0/1) , js (0/0) , css (0/0) , phpdoc (0/0) , commit (2/0) , savepoint (0/0) , thirdparty (0/0) , MOODLE_28_STABLE (2 errors / 1 warnings) [branch: mdl38210-moodle28 | CI Job ] phplint (0/0) , php (0/1) , js (0/0) , css (0/0) , phpdoc (0/0) , commit (2/0) , savepoint (0/0) , thirdparty (0/0) , master (2 errors / 1 warnings) [branch: mdl38210-master | CI Job ] phplint (0/0) , php (0/1) , js (0/0) , css (0/0) , phpdoc (0/0) , commit (2/0) , savepoint (0/0) , thirdparty (0/0) , More information about this report
            Hide
            Tim Lock added a comment -

            Pushed revised tweaks.

            Show
            Tim Lock added a comment - Pushed revised tweaks.
            Hide
            Marina Glancy added a comment - - edited

            Thanks Tim, I added 'cime' label to the issue to trigger the automated check again.

            Please note that 2.6 is no longer supported and you don't need to provide a branch for it.

            Also in the new logging system those callbacks are ignored and instead we look at the participation level of the events. If you add callback for compatibility with legacy logging, please include the disclaimer in the phpdocs block, similar to this: https://github.com/moodle/moodle/blob/master/mod/assign/lib.php#L952..L954

            Show
            Marina Glancy added a comment - - edited Thanks Tim, I added 'cime' label to the issue to trigger the automated check again. Please note that 2.6 is no longer supported and you don't need to provide a branch for it. Also in the new logging system those callbacks are ignored and instead we look at the participation level of the events. If you add callback for compatibility with legacy logging, please include the disclaimer in the phpdocs block, similar to this: https://github.com/moodle/moodle/blob/master/mod/assign/lib.php#L952..L954
            Hide
            CiBoT added a comment -

            Code verified against automated checks.

            Checked MDL-38210 using repository: https://github.com/tlock/moodle.git

            More information about this report

            Show
            CiBoT added a comment - Code verified against automated checks. Checked MDL-38210 using repository: https://github.com/tlock/moodle.git MOODLE_26_STABLE (0 errors / 0 warnings) [branch: mdl38210-moodle26 | CI Job ] MOODLE_27_STABLE (0 errors / 0 warnings) [branch: mdl38210-moodle27 | CI Job ] MOODLE_28_STABLE (0 errors / 0 warnings) [branch: mdl38210-moodle28 | CI Job ] master (0 errors / 0 warnings) [branch: mdl38210-master | CI Job ] More information about this report
            Hide
            David Mudrak added a comment -

            Uploading a screenshot of yet another error I am getting when trying the Participation report on unpatched 2.8. Should this be reported in a separated issue?

            Notice: Undefined variable: actionsql in /home/mudrd8mz/www/mdk/m28/moodle/report/participation/locallib.php on line 136
             
            Notice: Undefined variable: actionparams in /home/mudrd8mz/www/mdk/m28/moodle/report/participation/locallib.php on line 136
             
            Warning: array_merge(): Argument #2 is not an array in /home/mudrd8mz/www/mdk/m28/moodle/report/participation/index.php on line 234
             
            ERROR: Incorrect number of query parameters. Expected 6, got 0.
             
            More information about this error
            Debug info:
            Error code: invalidqueryparam
            Stack trace:
             
                line 849 of /lib/dml/moodle_database.php: dml_exception thrown
                line 761 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->fix_sql_params()
                line 268 of /report/participation/index.php: call to pgsql_native_moodle_database->get_records_sql()
            

            Show
            David Mudrak added a comment - Uploading a screenshot of yet another error I am getting when trying the Participation report on unpatched 2.8. Should this be reported in a separated issue? Notice: Undefined variable: actionsql in /home/mudrd8mz/www/mdk/m28/moodle/report/participation/locallib.php on line 136   Notice: Undefined variable: actionparams in /home/mudrd8mz/www/mdk/m28/moodle/report/participation/locallib.php on line 136   Warning: array_merge(): Argument #2 is not an array in /home/mudrd8mz/www/mdk/m28/moodle/report/participation/index.php on line 234   ERROR: Incorrect number of query parameters. Expected 6, got 0.   More information about this error Debug info: Error code: invalidqueryparam Stack trace:   line 849 of /lib/dml/moodle_database.php: dml_exception thrown line 761 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->fix_sql_params() line 268 of /report/participation/index.php: call to pgsql_native_moodle_database->get_records_sql()
            Hide
            David Mudrak added a comment -

            Updating the testing instructions to make it clear that this needs to be tested with Legacy log store explicitly.

            Show
            David Mudrak added a comment - Updating the testing instructions to make it clear that this needs to be tested with Legacy log store explicitly.
            Hide
            David Mudrak added a comment -

            Thanks Tim for providing the patch. Looks good from my side. I agree with Marina that it will help to have the behaviour explicitly documented in the same way as mod_assign does it - can you please amend the patch yet? I'll be happy to send it for integration then.

            +1

            Show
            David Mudrak added a comment - Thanks Tim for providing the patch. Looks good from my side. I agree with Marina that it will help to have the behaviour explicitly documented in the same way as mod_assign does it - can you please amend the patch yet? I'll be happy to send it for integration then. +1

              People

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

                Dates

                • Created:
                  Updated: