MDL-55127 mod_data: Search entries fixes
authorDavid Monllao <davidm@moodle.com>
Thu, 25 Aug 2016 08:50:15 +0000 (16:50 +0800)
committerDavid Monllao <davidm@moodle.com>
Thu, 25 Aug 2016 08:50:15 +0000 (16:50 +0800)
- 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
mod/data/field/checkbox/field.class.php
mod/data/field/multimenu/field.class.php
mod/data/field/textarea/field.class.php
mod/data/lib.php
mod/data/tests/generator/lib.php
mod/data/tests/generator_test.php
mod/data/tests/search_test.php

index ee5c1eb..7a874a8 100644 (file)
@@ -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;
     }
 
index c8675d9..d3726fa 100644 (file)
@@ -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 ");
     }
 }
index fb7e290..a6555f6 100644 (file)
@@ -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 ");
     }
 
 }
index a6d6248..cdb46fc 100644 (file)
@@ -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);
     }
 
 }
index 5accbeb..01ddaa9 100644 (file)
@@ -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 ");
     }
 }
 
index e511bbb..3fe9ef5 100644 (file)
@@ -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 {
index ebcfa33..1f443c6 100644 (file)
@@ -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[] = '<p>text area testing<br /></p>';
         $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, '<p>text area testing<br /></p>');
         $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
+}
index ee6b073..105571f 100644 (file)
@@ -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
+}