Bug: Problems in privacy provider class sql query

[NJahreis]

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:

moodle-mod_verbalfeedback/classes/privacy/provider.php

Lines 92 to 116 in 0bf38a7

	public static function get_contexts_for_userid(int $userid) : contextlist {
	// Fetch all verbalfeedback activity contexts where the user is participating.
	$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.verbalfeedback = t.id
	WHERE ts.fromuser = :fromuser OR ts.touser = :touser";
	$params = [
	'modname' => 'verbalfeedback',
	'contextlevel' => CONTEXT_MODULE,
	'fromuser' => $userid,
	'touser' => $userid,
	];
	$contextlist = new contextlist();
	$contextlist->add_from_sql($sql, $params);
	return $contextlist;
	}

In the SQL the field names for fromuser, touser and 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]

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

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

Best,
Luca