Skip to content

Commit

Permalink
MDL-59458 mod_data: Fix sorting data query
Browse files Browse the repository at this point in the history
* When activating sorting on a different field than the original field
the list of records is empty
  • Loading branch information
Laurent David authored and laurentdavid committed Aug 15, 2022
1 parent 4ed613f commit 739b551
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 4 deletions.
3 changes: 2 additions & 1 deletion mod/data/lib.php
Expand Up @@ -4179,7 +4179,8 @@ function data_get_advanced_search_sql($sort, $data, $recordids, $selectdata, $so

// Find the field we are sorting on
if ($sort > 0 or data_get_field_from_id($sort, $data)) {
$selectdata .= ' AND c.fieldid = :sort';
$selectdata .= ' AND c.fieldid = :sort AND s.recordid = r.id';
$nestselectsql .= ',{data_content} s ';
}

// If there are no record IDs then return an sql statment that will return no rows.
Expand Down
7 changes: 4 additions & 3 deletions mod/data/locallib.php
Expand Up @@ -1116,7 +1116,7 @@ function data_search_entries($data, $cm, $context, $mode, $currentgroup, $search
$namefields = $userfieldsapi->get_sql('u', false, '', '', false)->selects;

// Find the field we are sorting on.
if ($sort <= 0 or !$sortfield = data_get_field_from_id($sort, $data)) {
if ($sort <= 0 || !($sortfield = data_get_field_from_id($sort, $data))) {

switch ($sort) {
case DATA_LASTNAME:
Expand Down Expand Up @@ -1168,7 +1168,7 @@ function data_search_entries($data, $cm, $context, $mode, $currentgroup, $search

} else {

$sortcontent = $DB->sql_compare_text('c.' . $sortfield->get_sort_field());
$sortcontent = $DB->sql_compare_text('s.' . $sortfield->get_sort_field());
$sortcontentfull = $sortfield->get_sort_sql($sortcontent);

$what = ' DISTINCT r.id, r.approved, r.timecreated, r.timemodified, r.userid, r.groupid, r.dataid, ' . $namefields . ',
Expand All @@ -1179,7 +1179,8 @@ function data_search_entries($data, $cm, $context, $mode, $currentgroup, $search
AND r.dataid = :dataid
AND r.userid = u.id ';
if (!$advanced) {
$where .= 'AND c.fieldid = :sort';
$where .= 'AND s.fieldid = :sort AND s.recordid = r.id';
$tables .= ',{data_content} s ';
}
$params['dataid'] = $data->id;
$params['sort'] = $sort;
Expand Down
118 changes: 118 additions & 0 deletions mod/data/tests/locallib_test.php
@@ -0,0 +1,118 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace mod_data;

use mod_data\external\record_exporter;

defined('MOODLE_INTERNAL') || die();

global $CFG;
require_once($CFG->dirroot . '/mod/data/locallib.php');

/**
* Unit tests for locallib.php
*
* @package mod_data
* @copyright 2022 Laurent David <laurent.david@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class locallib_test extends \advanced_testcase {

/**
* Confirms that search is working
* @covers ::data_search_entries
*/
public function test_data_search_entries() {
$this->resetAfterTest();
$this->setAdminUser();
$course = $this->getDataGenerator()->create_course();
$record = new \stdClass();
$record->course = $course->id;
$record->name = "Mod data delete test";
$record->intro = "Some intro of some sort";

$module = $this->getDataGenerator()->create_module('data', $record);
$titlefield = $this->getDataGenerator()->get_plugin_generator('mod_data')->create_field(
(object) [
'name' => 'title',
'type' => 'text',
'required' => 1
],
$module);
$captionfield = $this->getDataGenerator()->get_plugin_generator('mod_data')->create_field(
(object) [
'name' => 'caption',
'type' => 'text',
'required' => 1
],
$module);
$this->getDataGenerator()->get_plugin_generator('mod_data')->create_entry($module, [
$titlefield->field->id => 'Entry 1',
$captionfield->field->id => 'caption'
]);
$this->getDataGenerator()->get_plugin_generator('mod_data')->create_entry($module, [
$titlefield->field->id => 'Entry 2',
$captionfield->field->id => ''
]);
$cm = get_coursemodule_from_id('data', $module->cmid);
// Search for entries without any search query set, we should return them all.
list($records, $maxcount, $totalcount, $page, $nowperpage, $sort, $mode) =
data_search_entries($module, $cm, \context_course::instance($course->id), 'list', 0);
$this->assertCount(2, $records);
// Search for entries for "caption" we should return only one of them.
list($records, $maxcount, $totalcount, $page, $nowperpage, $sort, $mode) =
data_search_entries($module, $cm, \context_course::instance($course->id), 'list', 0, 'caption');
$this->assertCount(1, $records);
// Same search but we order by title.
list($records, $maxcount, $totalcount, $page, $nowperpage, $sort, $mode) =
data_search_entries($module, $cm, \context_course::instance($course->id), 'list', 0, 'caption',
$titlefield->field->id, 'ASC');
$this->assertCount(1, $records);
$this->assert_record_entries_contains($records, $captionfield->field->id, 'caption');

// Now with advanced search.
$defaults = [];
$fn = $ln = ''; // Defaults for first and last name.
// Force value for advanced search.
$_GET['f_' . $captionfield->field->id] = 'caption';
list($searcharray, $searchtext) = data_build_search_array($module, false, [], $defaults, $fn, $ln);
list($records, $maxcount, $totalcount, $page, $nowperpage, $sort, $mode) =
data_search_entries($module, $cm, \context_course::instance($course->id), 'list', 0, $searchtext,
$titlefield->field->id, 'ASC', 0, 0, true, $searcharray);
$this->assertCount(1, $records);
$this->assert_record_entries_contains($records, $captionfield->field->id, 'caption');
}

/**
* Assert that all records contains a value for the matching field id.
*
* @param array $records
* @param int $fieldid
* @param string $content
* @return void
*/
private function assert_record_entries_contains($records, $fieldid, $content) {
global $DB;
foreach ($records as $record) {
$fieldscontent = $DB->get_records('data_content', ['recordid' => $record->id]);
foreach ($fieldscontent as $fieldcontent) {
if ($fieldcontent->id == $fieldid) {
$this->assertStringContainsString($fieldcontent->content, $content);
}
}
}
}
}

0 comments on commit 739b551

Please sign in to comment.