bfh / moodle-mod_verbalfeedback

The Verbal Feedback Moodle Activity
GNU General Public License v3.0
1 stars 5 forks source link

Bug: Problems in privacy provider class sql query #39

Open NJahreis opened 1 month ago

NJahreis commented 1 month ago

Hi Luca, Hi Stephan,

I just had to use the data request in moodle for the first time in quiet a while and thus found an error in the privacy provider of mod_verbalfeedback.

The following error message got displayed:

Das Plugin mod_verbalfeedback konnte die Verarbeitung der Daten nicht beenden. Folgende Informationen könnten den Entwickler/innen helfen:
Fehler beim Lesen der Datenbank

#0 /moodle/apache2/htdocs/lib/dml/moodle_read_slave_trait.php(293): moodle_database->query_end()
#1 /moodle/apache2/htdocs/lib/dml/mysqli_native_moodle_database.php(1337): mysqli_native_moodle_database->query_end()
#2 /moodle/apache2/htdocs/privacy/classes/local/request/contextlist.php(71): mysqli_native_moodle_database->get_recordset_sql()
#3 /moodle/apache2/htdocs/mod/verbalfeedback/classes/privacy/provider.php(113): core_privacy\local\request\contextlist->add_from_sql()
#4 /moodle/apache2/htdocs/lib/moodlelib.php(8296): mod_verbalfeedback\privacy\provider::get_contexts_for_userid()
#5 /moodle/apache2/htdocs/privacy/classes/manager.php(578): component_class_callback()
#6 /moodle/apache2/htdocs/privacy/classes/manager.php(611): core_privacy\manager::component_class_callback()
#7 /moodle/apache2/htdocs/privacy/classes/manager.php(238): core_privacy\manager->handled_component_class_callback()
#8 /moodle/apache2/htdocs/admin/tool/dataprivacy/classes/task/process_data_request_task.php(93): core_privacy\manager->get_contexts_for_userid()
#9 /moodle/apache2/htdocs/lib/classes/cron.php(508): tool_dataprivacy\task\process_data_request_task->execute()
#10 /moodle/apache2/htdocs/lib/classes/cron.php(348): core\cron::run_inner_adhoc_task()
#11 /moodle/apache2/htdocs/admin/cli/adhoc_task.php(148): core\cron::run_adhoc_task()
#12 {main}

I already tracked it down to a mistake (most likely due to refactoring) in the SQL request of the provider in the following function: https://github.com/bfh/moodle-mod_verbalfeedback/blob/0bf38a7b587f8e534d7b248ce97e593563c32704/classes/privacy/provider.php#L92-L116 In the SQL the field names for fromuser, touserand verbalfeedback should be changed to fromuserid,touserid and instanceid The correct SQL snippet thus should be:

$sql = "SELECT ctx.id
        FROM {context} ctx
  INNER JOIN {course_modules} cm
          ON cm.id = ctx.instanceid AND ctx.contextlevel = :contextlevel
  INNER JOIN {modules} m
          ON m.id = cm.module AND m.name = :modname
  INNER JOIN {verbalfeedback} t
          ON t.id = cm.instance
  INNER JOIN {verbalfeedback_submission} ts
          ON ts.instanceid = t.id
       WHERE ts.fromuserid = :fromuser OR ts.touserid = :touser";

While this change removed the error message for me the data export now fails quietly on my system. (But I am not sure if verbalfeedback is to blame here).

I also noticed what the other SQL statements in the class also need some work. I have started working on this already but am stuck as some SQL statements reference tables which no longer exist in the current version of the plugin. I will create a draft PR soonish.

Best Nikolai

lucaboesch commented 1 week ago

Thanks very much @NJahreis. The draft PR looks good so far.

Please do go on, if it is possible to you.

Best, Luca