From fa90f529867abf9036b9d9f3d5bb4e7f9e8a5d06 Mon Sep 17 00:00:00 2001 From: David Monllao Date: Thu, 25 Aug 2016 16:50:15 +0800 Subject: [PATCH] MDL-55127 mod_data: Search entries fixes - If the database contains has than 4 valid fields all fields from the 4th one to the last one will be merged into the 4th one so we index all entry contents - get_content_value now receives the whole data_content record as fields may need other values than just the content to prepare the value - phpunit database generator changes to separate checkboxes from radio buttons options and to separate multimenu options from menu options --- mod/data/classes/search/entry.php | 10 +++++++--- mod/data/field/checkbox/field.class.php | 15 +++++++++------ mod/data/field/multimenu/field.class.php | 15 +++++++++------ mod/data/field/textarea/field.class.php | 9 ++++++--- mod/data/lib.php | 9 ++++++--- mod/data/tests/generator/lib.php | 8 ++++++-- mod/data/tests/generator_test.php | 10 +++++----- mod/data/tests/search_test.php | 18 +++++++++--------- 8 files changed, 57 insertions(+), 37 deletions(-) diff --git a/mod/data/classes/search/entry.php b/mod/data/classes/search/entry.php index ee5c1ebe497..7a874a80424 100644 --- a/mod/data/classes/search/entry.php +++ b/mod/data/classes/search/entry.php @@ -97,6 +97,7 @@ class entry extends \core_search\base_mod { return false; } + // All fields should be already returned as plain text by data_field_base::get_content_value. $doc->set('title', $indexfields[0]); $doc->set('content', $indexfields[1]); @@ -263,7 +264,7 @@ class entry extends \core_search\base_mod { $indexfields = array(); $validfieldtypes = array('text', 'textarea', 'menu', 'radiobutton', 'checkbox', 'multimenu', 'url'); - $sql = "SELECT dc.id, dc.content, df.name AS fldname, + $sql = "SELECT dc.*, df.name AS fldname, df.type AS fieldtype, df.required FROM {data_content} dc, {data_fields} df WHERE dc.fieldid = df.id @@ -320,10 +321,13 @@ class entry extends \core_search\base_mod { $content = $contentqueue->extract(); $classname = $this->get_field_class_name($content->fieldtype); - - $indexfields[] = $classname::get_content_value($content->content); + $indexfields[] = $classname::get_content_value($content); } + // Limited to 4 fields as a document only has 4 content fields. + if (count($indexfields) > 4) { + $indexfields[3] = implode(' ', array_slice($indexfields, 3)); + } return $indexfields; } diff --git a/mod/data/field/checkbox/field.class.php b/mod/data/field/checkbox/field.class.php index c8675d98c35..d3726fa2bcc 100644 --- a/mod/data/field/checkbox/field.class.php +++ b/mod/data/field/checkbox/field.class.php @@ -250,17 +250,20 @@ class data_field_checkbox extends data_field_base { /** * Returns the presentable string value for a field content. - * @param string $value + * + * The returned string should be plain text. + * + * @param stdClass $content * @return string */ - public static function get_content_value($value) { - $arr = explode('##', $value); + public static function get_content_value($content) { + $arr = explode('##', $content->content); - $content = ''; + $strvalue = ''; foreach ($arr as $a) { - $content .= $a.' '; + $strvalue .= $a . ' '; } - return trim($content); + return trim($strvalue, "\r\n "); } } diff --git a/mod/data/field/multimenu/field.class.php b/mod/data/field/multimenu/field.class.php index fb7e290b6c2..a6555f64573 100644 --- a/mod/data/field/multimenu/field.class.php +++ b/mod/data/field/multimenu/field.class.php @@ -276,18 +276,21 @@ class data_field_multimenu extends data_field_base { /** * Returns the presentable string value for a field content. - * @param string $value + * + * The returned string should be plain text. + * + * @param stdClass $content * @return string */ - public static function get_content_value($value) { - $arr = explode('##', $value); + public static function get_content_value($content) { + $arr = explode('##', $content->content); - $content = ''; + $strvalue = ''; foreach ($arr as $a) { - $content .= $a.' '; + $strvalue .= $a . ' '; } - return trim($content); + return trim($strvalue, "\r\n "); } } diff --git a/mod/data/field/textarea/field.class.php b/mod/data/field/textarea/field.class.php index a6d6248c1d2..cdb46fcdfcb 100644 --- a/mod/data/field/textarea/field.class.php +++ b/mod/data/field/textarea/field.class.php @@ -284,11 +284,14 @@ class data_field_textarea extends data_field_base { /** * Returns the presentable string value for a field content. - * @param string $value + * + * The returned string should be plain text. + * + * @param stdClass $content * @return string */ - public static function get_content_value($value) { - return clean_param($value, PARAM_NOTAGS); + public static function get_content_value($content) { + return content_to_text($content->content, $content->content1); } } diff --git a/mod/data/lib.php b/mod/data/lib.php index 5accbeb46ab..01ddaa9cafb 100644 --- a/mod/data/lib.php +++ b/mod/data/lib.php @@ -551,11 +551,14 @@ class data_field_base { // Base class for Database Field Types (see field/*/ /** * Returns the presentable string value for a field content. - * @param string $value + * + * The returned string should be plain text. + * + * @param stdClass $content * @return string */ - public static function get_content_value($value) { - return trim($value); + public static function get_content_value($content) { + return trim($content->content, "\r\n "); } } diff --git a/mod/data/tests/generator/lib.php b/mod/data/tests/generator/lib.php index e511bbb8788..3fe9ef57506 100644 --- a/mod/data/tests/generator/lib.php +++ b/mod/data/tests/generator/lib.php @@ -133,10 +133,14 @@ class mod_data_generator extends testing_module_generator { } if (!isset($record['param1'])) { - if (in_array($record['type'], array('checkbox', 'radiobutton'))) { + if ($record['type'] == 'checkbox') { $record['param1'] = implode("\n", array('opt1', 'opt2', 'opt3', 'opt4')); - } else if (in_array($record['type'], array('multimenu', 'menu'))) { + } else if ($record['type'] == 'radiobutton') { + $record['param1'] = implode("\n", array('radioopt1', 'radioopt2', 'radioopt3', 'radioopt4')); + } else if ($record['type'] == 'menu') { $record['param1'] = implode("\n", array('menu1', 'menu2', 'menu3', 'menu4')); + } else if ($record['type'] == 'multimenu') { + $record['param1'] = implode("\n", array('multimenu1', 'multimenu2', 'multimenu3', 'multimenu4')); } else if (($record['type'] === 'text') || ($record['type'] === 'url')) { $record['param1'] = 1; } else { diff --git a/mod/data/tests/generator_test.php b/mod/data/tests/generator_test.php index ebcfa335dec..1f443c67eee 100644 --- a/mod/data/tests/generator_test.php +++ b/mod/data/tests/generator_test.php @@ -186,9 +186,9 @@ class mod_data_generator_testcase extends advanced_testcase { $contents[] = array('opt1', 'opt2', 'opt3', 'opt4'); $contents[] = '01-01-2037'; // It should be lower than 2038, to avoid failing on 32-bit windows. $contents[] = 'menu1'; - $contents[] = array('menu1', 'menu2', 'menu3', 'menu4'); + $contents[] = array('multimenu1', 'multimenu2', 'multimenu3', 'multimenu4'); $contents[] = '12345'; - $contents[] = 'opt1'; + $contents[] = 'radioopt1'; $contents[] = 'text for testing'; $contents[] = '

text area testing

'; $contents[] = array('example.url', 'sampleurl'); @@ -221,13 +221,13 @@ class mod_data_generator_testcase extends advanced_testcase { $this->assertEquals($contents[$contentstartid]->content, 'opt1##opt2##opt3##opt4'); $this->assertEquals($contents[++$contentstartid]->content, '2114380800'); $this->assertEquals($contents[++$contentstartid]->content, 'menu1'); - $this->assertEquals($contents[++$contentstartid]->content, 'menu1##menu2##menu3##menu4'); + $this->assertEquals($contents[++$contentstartid]->content, 'multimenu1##multimenu2##multimenu3##multimenu4'); $this->assertEquals($contents[++$contentstartid]->content, '12345'); - $this->assertEquals($contents[++$contentstartid]->content, 'opt1'); + $this->assertEquals($contents[++$contentstartid]->content, 'radioopt1'); $this->assertEquals($contents[++$contentstartid]->content, 'text for testing'); $this->assertEquals($contents[++$contentstartid]->content, '

text area testing

'); $this->assertEquals($contents[$contentstartid]->content1, '1'); $this->assertEquals($contents[++$contentstartid]->content, 'http://example.url'); $this->assertEquals($contents[$contentstartid]->content1, 'sampleurl'); } -} \ No newline at end of file +} diff --git a/mod/data/tests/search_test.php b/mod/data/tests/search_test.php index ee6b073ddae..105571f97e7 100644 --- a/mod/data/tests/search_test.php +++ b/mod/data/tests/search_test.php @@ -355,8 +355,8 @@ class mod_data_search_test extends advanced_testcase { $this->assertEquals($data1doc->get('courseid'), $course->id); $this->assertEquals($data1doc->get('title'), 'text for testing'); $this->assertEquals($data1doc->get('content'), 'menu1'); - $this->assertEquals($data1doc->get('description1'), 'opt1'); - $this->assertEquals($data1doc->get('description2'), 'opt1 opt2 opt3 opt4'); + $this->assertEquals($data1doc->get('description1'), 'radioopt1'); + $this->assertEquals($data1doc->get('description2'), 'opt1 opt2 opt3 opt4 multimenu1 multimenu2 multimenu3 multimenu4 text area testing http://example.url'); // Second Case. $data2 = $this->getDataGenerator()->create_module('data', $record); @@ -382,7 +382,7 @@ class mod_data_search_test extends advanced_testcase { $this->assertEquals($data2doc->get('title'), 'opt1 opt2 opt3 opt4'); $this->assertEquals($data2doc->get('content'), 'text for testing'); $this->assertEquals($data2doc->get('description1'), 'menu1'); - $this->assertEquals($data2doc->get('description2'), 'text area testing'); + $this->assertEquals($data2doc->get('description2'), 'text area testing http://example.url'); // Third Case. $data3 = $this->getDataGenerator()->create_module('data', $record); @@ -501,9 +501,9 @@ class mod_data_search_test extends advanced_testcase { $data9doc = $searcharea->get_document($data9entry1); $this->assertEquals($data9doc->get('courseid'), $course->id); - $this->assertEquals($data9doc->get('title'), 'opt1'); + $this->assertEquals($data9doc->get('title'), 'radioopt1'); $this->assertEquals($data9doc->get('content'), 'menu1'); - $this->assertEquals($data9doc->get('description1'), 'menu1 menu2 menu3 menu4'); + $this->assertEquals($data9doc->get('description1'), 'multimenu1 multimenu2 multimenu3 multimenu4'); // Tenth Case. $data10 = $this->getDataGenerator()->create_module('data', $record); @@ -521,7 +521,7 @@ class mod_data_search_test extends advanced_testcase { $this->assertEquals($data10doc->get('courseid'), $course->id); $this->assertEquals($data10doc->get('title'), 'opt1 opt2 opt3 opt4'); $this->assertEquals($data10doc->get('content'), 'text area testing'); - $this->assertEquals($data10doc->get('description1'), 'menu1 menu2 menu3 menu4'); + $this->assertEquals($data10doc->get('description1'), 'multimenu1 multimenu2 multimenu3 multimenu4'); } @@ -939,7 +939,7 @@ class mod_data_search_test extends advanced_testcase { break; case 'multimenu': - $fieldcontents[$fieldrecord->id] = array('menu1', 'menu2', 'menu3', 'menu4'); + $fieldcontents[$fieldrecord->id] = array('multimenu1', 'multimenu2', 'multimenu3', 'multimenu4'); break; case 'date': @@ -951,7 +951,7 @@ class mod_data_search_test extends advanced_testcase { break; case 'radiobutton': - $fieldcontents[$fieldrecord->id] = 'opt1'; + $fieldcontents[$fieldrecord->id] = 'radioopt1'; break; case 'number': @@ -996,4 +996,4 @@ class mod_data_search_test extends advanced_testcase { return $DB->get_record_sql($sql, array('drid' => $recordid)); } -} \ No newline at end of file +}