From 30310f37a9139fc5f015feadbeb6ed1945859dbf Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Mon, 19 Aug 2024 16:34:05 +1000 Subject: [PATCH 01/12] Issue #66: Add search API --- classes/search/cms.php | 188 +++++++++++++++++++++++++++++++++++++++++ lang/en/cms.php | 6 ++ settings.php | 18 ++++ 3 files changed, 212 insertions(+) create mode 100644 classes/search/cms.php diff --git a/classes/search/cms.php b/classes/search/cms.php new file mode 100644 index 0000000..8b8bcc5 --- /dev/null +++ b/classes/search/cms.php @@ -0,0 +1,188 @@ +. + +/** + * Define search area. + * + * @package mod_cms + * @author Tomo Tsuyuki + * @copyright 2024 Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace mod_cms\search; + +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->dirroot . '/mod/cms/lib.php'); + +/** + * Define search area. + * + * @package mod_cms + * @author Tomo Tsuyuki + * @copyright 2024 Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class cms extends \core_search\base_mod { + + /** + * @var array Internal quick static cache. + */ + protected $cmsdata = []; + + /** + * Returns recordset containing required data for indexing cms data records. + * + * @param int $modifiedfrom timestamp + * @param \context|null $context Optional context to restrict scope of returned results + * @return moodle_recordset|null Recordset (or null if no results) + */ + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { + global $DB; + + list ($contextjoin, $contextparams) = $this->get_context_restriction_sql( + $context, 'cms', 'mc'); + if ($contextjoin === null) { + return null; + } + + $searcharea = explode(',', get_config('mod_cms', 'search_area')); + if (empty($searcharea)) { + return null; + } + list($insql, $inparams) = $DB->get_in_or_equal($searcharea); + + $sql = "SELECT mcd.*, mc.id AS cmsid, mc.course AS courseid + FROM {customfield_data} mcd + JOIN {cms} mc ON mc.id = mcd.instanceid + $contextjoin + WHERE mcd.timemodified >= ? AND mcd.fieldid " . $insql . " ORDER BY mcd.timemodified ASC"; + return $DB->get_recordset_sql($sql, array_merge($contextparams, [$modifiedfrom], $inparams)); + } + + /** + * Returns the document associated with this data id. + * + * @param stdClass $record + * @param array $options + * @return \core_search\document + */ + public function get_document($record, $options = []) { + try { + $cm = $this->get_cm('cms', $record->cmsid, $record->courseid); + $context = \context_module::instance($cm->id); + } catch (\dml_missing_record_exception $ex) { + // Notify it as we run here as admin, we should see everything. + debugging('Error retrieving ' . $this->areaid . ' ' . $record->id . ' document, not all required data is available: ' . + $ex->getMessage(), DEBUG_DEVELOPER); + return false; + } catch (\dml_exception $ex) { + // Notify it as we run here as admin, we should see everything. + debugging('Error retrieving ' . $this->areaid . ' ' . $record->id . ' document: ' . $ex->getMessage(), DEBUG_DEVELOPER); + return false; + } + + // Prepare associative array with data from DB. + $doc = \core_search\document_factory::instance($record->id, $this->componentname, $this->areaname); + $doc->set('title', content_to_text($record->value, false)); + $doc->set('content', content_to_text($record->value, $record->valueformat)); + $doc->set('contextid', $context->id); + $doc->set('courseid', $record->courseid); + $doc->set('owneruserid', \core_search\manager::NO_OWNER_ID); + $doc->set('modified', $record->timemodified); + + // Check if this document should be considered new. + if (isset($options['lastindexedtime']) && ($options['lastindexedtime'] < $record->timecreated)) { + // If the document was created after the last index time, it must be new. + $doc->set_is_new(true); + } + + return $doc; + } + + /** + * Whether the user can access the document or not. + * + * @param int $id data id + * @return bool + */ + public function check_access($id) { + try { + $data = $this->get_data($id); + $cminfo = $this->get_cm('cms', $data->instanceid, $data->courseid); + } catch (\dml_missing_record_exception $ex) { + return \core_search\manager::ACCESS_DELETED; + } catch (\dml_exception $ex) { + return \core_search\manager::ACCESS_DENIED; + } + + // Recheck uservisible although it should have already been checked in core_search. + if ($cminfo->uservisible === false) { + return \core_search\manager::ACCESS_DENIED; + } + + $context = \context_module::instance($cminfo->id); + + if (!has_capability('mod/cms:view', $context)) { + return \core_search\manager::ACCESS_DENIED; + } + + return \core_search\manager::ACCESS_GRANTED; + } + + /** + * Link to the cms. + * + * @param \core_search\document $doc + * @return \moodle_url + */ + public function get_doc_url(\core_search\document $doc) { + return $this->get_context_url($doc); + } + + /** + * Link to the cms. + * + * @param \core_search\document $doc + * @return \moodle_url + */ + public function get_context_url(\core_search\document $doc) { + $contextmodule = \context::instance_by_id($doc->get('contextid')); + return new \moodle_url('/mod/cms/view.php', ['id' => $contextmodule->instanceid]); + } + + /** + * Returns the specified data from its internal cache. + * + * @throws \dml_missing_record_exception + * @param int $id + * @return stdClass + */ + protected function get_data($id) { + global $DB; + if (empty($this->cmsdata[$id])) { + $sql = "SELECT mcd.*, mc.id AS cmsid, mc.course AS courseid + FROM {customfield_data} mcd + JOIN {cms} mc ON mc.id = mcd.instanceid + WHERE mcd.id = :id"; + $this->cmsdata[$id] = $DB->get_record_sql($sql, ['id' => $id]); + if (!$this->cmsdata[$id]) { + throw new \dml_missing_record_exception('cms_data'); + } + } + return $this->cmsdata[$id]; + } +} diff --git a/lang/en/cms.php b/lang/en/cms.php index f69fea6..16268f1 100644 --- a/lang/en/cms.php +++ b/lang/en/cms.php @@ -103,6 +103,12 @@ $string['error:no_instance_hash'] = 'Module {$a} has no instance hash.'; $string['error:no_config_hash'] = 'Module {$a} has no config hash.'; +// Search strings. +$string['search:cms'] = 'CMS'; +$string['search:settings'] = 'Search settings'; +$string['search:settings:area'] = 'Search area'; +$string['search:settings:area_desc'] = 'Search area for global search'; + // Site datasource strings. $string['site:displayname'] = 'Site Info'; diff --git a/settings.php b/settings.php index fd299ef..54c8556 100644 --- a/settings.php +++ b/settings.php @@ -56,5 +56,23 @@ $ADMIN->add('modcmsfolder', $settings); +$settings = new admin_settingpage('mod_cms_search_settings', new lang_string('search:settings', 'mod_cms')); +if ($ADMIN->fulltree) { + $sql = "SELECT mcf.id, mcf.name + FROM {customfield_category} mcc + JOIN {customfield_field} mcf ON mcf.categoryid = mcc.id + WHERE mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' AND mcf.type IN ('text', 'textarea')"; + $areas = $DB->get_records_sql_menu($sql); + + $settings->add(new admin_setting_configmulticheckbox( + 'mod_cms/search_area', + get_string('search:settings:area', 'mod_cms'), + get_string('search:settings:area_desc', 'mod_cms'), + null, + $areas, + )); +} +$ADMIN->add('modcmsfolder', $settings); + // Tell core we already added the settings structure. $settings = null; From 03aed7e2723f9c9a398a6217d81c9e7194c94dfe Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Tue, 20 Aug 2024 14:04:26 +1000 Subject: [PATCH 02/12] Issue #66: Rename search class and debug for php7.2 --- classes/search/{cms.php => cmsfield.php} | 4 ++-- lang/en/cms.php | 2 +- settings.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename classes/search/{cms.php => cmsfield.php} (99%) diff --git a/classes/search/cms.php b/classes/search/cmsfield.php similarity index 99% rename from classes/search/cms.php rename to classes/search/cmsfield.php index 8b8bcc5..ab70b2c 100644 --- a/classes/search/cms.php +++ b/classes/search/cmsfield.php @@ -36,7 +36,7 @@ * @copyright 2024 Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class cms extends \core_search\base_mod { +class cmsfield extends \core_search\base_mod { /** * @var array Internal quick static cache. @@ -44,7 +44,7 @@ class cms extends \core_search\base_mod { protected $cmsdata = []; /** - * Returns recordset containing required data for indexing cms data records. + * Returns recordset containing required data for indexing cmsfield records. * * @param int $modifiedfrom timestamp * @param \context|null $context Optional context to restrict scope of returned results diff --git a/lang/en/cms.php b/lang/en/cms.php index 16268f1..c0f33d4 100644 --- a/lang/en/cms.php +++ b/lang/en/cms.php @@ -104,7 +104,7 @@ $string['error:no_config_hash'] = 'Module {$a} has no config hash.'; // Search strings. -$string['search:cms'] = 'CMS'; +$string['search:cmsfield'] = 'CMS'; $string['search:settings'] = 'Search settings'; $string['search:settings:area'] = 'Search area'; $string['search:settings:area_desc'] = 'Search area for global search'; diff --git a/settings.php b/settings.php index 54c8556..8567265 100644 --- a/settings.php +++ b/settings.php @@ -69,7 +69,7 @@ get_string('search:settings:area', 'mod_cms'), get_string('search:settings:area_desc', 'mod_cms'), null, - $areas, + $areas )); } $ADMIN->add('modcmsfolder', $settings); From c3eef515cad19caac1dd6cc7ba676b732b2aeae2 Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Tue, 20 Aug 2024 16:42:38 +1000 Subject: [PATCH 03/12] Issue #66: Remove setting and add phpunit test --- classes/search/cmsfield.php | 21 ++-- settings.php | 18 ---- tests/generator/lib.php | 20 ++++ tests/search/search_test.php | 185 +++++++++++++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 29 deletions(-) create mode 100644 tests/search/search_test.php diff --git a/classes/search/cmsfield.php b/classes/search/cmsfield.php index ab70b2c..835cee1 100644 --- a/classes/search/cmsfield.php +++ b/classes/search/cmsfield.php @@ -59,18 +59,15 @@ public function get_document_recordset($modifiedfrom = 0, \context $context = nu return null; } - $searcharea = explode(',', get_config('mod_cms', 'search_area')); - if (empty($searcharea)) { - return null; - } - list($insql, $inparams) = $DB->get_in_or_equal($searcharea); - - $sql = "SELECT mcd.*, mc.id AS cmsid, mc.course AS courseid + $sql = "SELECT mcd.*, mc.id AS cmsid, mc.course AS courseid, mcf.name AS fieldname FROM {customfield_data} mcd JOIN {cms} mc ON mc.id = mcd.instanceid + JOIN {customfield_field} mcf ON mcf.id = mcd.fieldid + JOIN {customfield_category} mcc ON mcf.categoryid = mcc.id $contextjoin - WHERE mcd.timemodified >= ? AND mcd.fieldid " . $insql . " ORDER BY mcd.timemodified ASC"; - return $DB->get_recordset_sql($sql, array_merge($contextparams, [$modifiedfrom], $inparams)); + WHERE mcd.timemodified >= ? AND mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' + ORDER BY mcd.timemodified ASC"; + return $DB->get_recordset_sql($sql, array_merge($contextparams, [$modifiedfrom])); } /** @@ -97,7 +94,7 @@ public function get_document($record, $options = []) { // Prepare associative array with data from DB. $doc = \core_search\document_factory::instance($record->id, $this->componentname, $this->areaname); - $doc->set('title', content_to_text($record->value, false)); + $doc->set('title', content_to_text($record->fieldname, false)); $doc->set('content', content_to_text($record->value, $record->valueformat)); $doc->set('contextid', $context->id); $doc->set('courseid', $record->courseid); @@ -150,7 +147,9 @@ public function check_access($id) { * @return \moodle_url */ public function get_doc_url(\core_search\document $doc) { - return $this->get_context_url($doc); + $contextmodule = \context::instance_by_id($doc->get('contextid')); + $cm = get_coursemodule_from_id('cms', $contextmodule->instanceid, $doc->get('courseid'), true); + return new \moodle_url('/course/view.php', ['id' => $doc->get('courseid'), 'section' => $cm->sectionnum]); } /** diff --git a/settings.php b/settings.php index 8567265..fd299ef 100644 --- a/settings.php +++ b/settings.php @@ -56,23 +56,5 @@ $ADMIN->add('modcmsfolder', $settings); -$settings = new admin_settingpage('mod_cms_search_settings', new lang_string('search:settings', 'mod_cms')); -if ($ADMIN->fulltree) { - $sql = "SELECT mcf.id, mcf.name - FROM {customfield_category} mcc - JOIN {customfield_field} mcf ON mcf.categoryid = mcc.id - WHERE mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' AND mcf.type IN ('text', 'textarea')"; - $areas = $DB->get_records_sql_menu($sql); - - $settings->add(new admin_setting_configmulticheckbox( - 'mod_cms/search_area', - get_string('search:settings:area', 'mod_cms'), - get_string('search:settings:area_desc', 'mod_cms'), - null, - $areas - )); -} -$ADMIN->add('modcmsfolder', $settings); - // Tell core we already added the settings structure. $settings = null; diff --git a/tests/generator/lib.php b/tests/generator/lib.php index 58e9bfc..d3d4601 100644 --- a/tests/generator/lib.php +++ b/tests/generator/lib.php @@ -15,6 +15,7 @@ // along with Moodle. If not, see . use mod_cms\local\model\cms_types; +use mod_cms\customfield\cmsfield_handler; use core_customfield\category_controller; use core_customfield\field_controller; @@ -47,6 +48,25 @@ public function create_instance($record = null, array $options = null) { return parent::create_instance($record, (array) $options); } + /** + * Create new cms module instance with custom data + * + * @param array|stdClass $record + * @param null|array $options + * @return stdClass + */ + public function create_instance_with_data($record = null, ?array $options = null): object { + $record = (object)$record; + $cms = parent::create_instance($record, $options); + + // Save customfield. + $handler = cmsfield_handler::create($cms->typeid); + $record->id = $cms->id; + $handler->instance_form_save($record); + + return $cms; + } + /** * Get generator for custom fields. * @return core_customfield_generator diff --git a/tests/search/search_test.php b/tests/search/search_test.php new file mode 100644 index 0000000..bc7af22 --- /dev/null +++ b/tests/search/search_test.php @@ -0,0 +1,185 @@ +. + +/** + * CMS search unit tests. + * + * @package mod_cms + * @category test + * @author Tomo Tsuyuki + * @copyright 2024 Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \mod_cms\search\cms + */ + +namespace mod_cms\search; + +use mod_cms\local\model\cms_types; +use mod_cms\customfield\cmsfield_handler; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/search/tests/fixtures/testable_core_search.php'); + +class search_test extends \advanced_testcase { + + /** + * @var string Area id + */ + protected $cmsareaid = null; + + /** + * Set up. + */ + public function setUp(): void { + $this->resetAfterTest(); + set_config('enableglobalsearch', true); + + $this->cmsareaid = \core_search\manager::generate_areaid('mod_cms', 'cmsfield'); + + // Set \core_search::instance to the mock_search_engine as we don't require the search engine to be working to test this. + $search = \testable_core_search::instance(); + } + + /** + * Test search enabled. + * + * @return void + */ + public function test_search_enabled() { + + $searcharea = \core_search\manager::get_search_area($this->cmsareaid); + list($componentname, $varname) = $searcharea->get_config_var_name(); + + // Enabled by default once global search is enabled. + $this->assertTrue($searcharea->is_enabled()); + + set_config($varname . '_enabled', 0, $componentname); + $this->assertFalse($searcharea->is_enabled()); + + set_config($varname . '_enabled', 1, $componentname); + $this->assertTrue($searcharea->is_enabled()); + } + + /** + * Indexing mod cms contents. + * + * @return void + */ + public function test_get_document_recordset() { + global $DB; + + // Returns the instance as long as the area is supported. + $searcharea = \core_search\manager::get_search_area($this->cmsareaid); + $this->assertInstanceOf('\mod_cms\search\cmsfield', $searcharea); + + $course = self::getDataGenerator()->create_course(); + + $cmstype = new cms_types(); + $cmstype->set('name', 'Overview') + ->set('idnumber', 'overview') + ->set('title_mustache', 'Overview'); + $cmstype->save(); + + $fieldcategory = self::getDataGenerator()->create_custom_field_category([ + 'name' => 'Other fields', + 'component' => 'mod_cms', + 'area' => 'cmsfield', + 'itemid' => $cmstype->get('id'), + ]); + $field = self::getDataGenerator()->create_custom_field([ + 'name' => 'Overview', + 'shortname' => 'overview', + 'type' => 'text', + 'categoryid' => $fieldcategory->get('id'), + ]); + + // Name for cms is coming from "title_mustache" in cms_type. + $generator = self::getDataGenerator()->get_plugin_generator('mod_cms'); + $record = new \stdClass(); + $record->course = $course->id; + $record->customfield_overview = 'Test overview text 1'; + $record->typeid = $cmstype->get('id'); + $generator->create_instance_with_data($record); + + $record = new \stdClass(); + $record->course = $course->id; + $record->customfield_overview = 'Test overview text 2'; + $record->typeid = $cmstype->get('id'); + $generator->create_instance_with_data($record); + + // All records. + $recordset = $searcharea->get_document_recordset(); + $this->assertTrue($recordset->valid()); + foreach ($recordset as $record) { + $this->assertInstanceOf('stdClass', $record); + $data = $DB->get_record('customfield_data', ['id' => $record->id]); + $doc = $searcharea->get_document($record); + $this->assertInstanceOf('\core_search\document', $doc); + $this->assertEquals('mod_cms-cmsfield-' . $data->id, $doc->get('id')); + $this->assertEquals($data->id, $doc->get('itemid')); + $this->assertEquals($course->id, $doc->get('courseid')); + $this->assertEquals($data->contextid, $doc->get('contextid')); + $this->assertEquals($field->get('name'), $doc->get('title')); + $this->assertEquals($data->value, $doc->get('content')); + + // Static caches are working. + $dbreads = $DB->perf_get_reads(); + $doc = $searcharea->get_document($record); + $this->assertEquals($dbreads, $DB->perf_get_reads()); + $this->assertInstanceOf('\core_search\document', $doc); + } + $recordset->close(); + + // The +2 is to prevent race conditions. + $recordset = $searcharea->get_document_recordset(time() + 2); + + // No new records. + $this->assertFalse($recordset->valid()); + $recordset->close(); + + // Wait 1 sec to have new search string. + sleep(1); + $record = new \stdClass(); + $record->course = $course->id; + $record->customfield_overview = 'Test overview text 3'; + $record->typeid = $cmstype->get('id'); + $generator->create_instance_with_data($record); + + // Return only new search. + $recordset = $searcharea->get_document_recordset(time()); + foreach ($recordset as $record) { + $this->assertInstanceOf('stdClass', $record); + $data = $DB->get_record('customfield_data', ['id' => $record->id]); + $doc = $searcharea->get_document($record); + $this->assertInstanceOf('\core_search\document', $doc); + $this->assertEquals('mod_cms-cmsfield-' . $data->id, $doc->get('id')); + $this->assertEquals($data->id, $doc->get('itemid')); + $this->assertEquals($course->id, $doc->get('courseid')); + $this->assertEquals($data->contextid, $doc->get('contextid')); + $this->assertEquals($field->get('name'), $doc->get('title')); + $this->assertEquals($data->value, $doc->get('content')); + + // Static caches are working. + $dbreads = $DB->perf_get_reads(); + $doc = $searcharea->get_document($record); + $this->assertEquals($dbreads, $DB->perf_get_reads()); + $this->assertInstanceOf('\core_search\document', $doc); + } + $recordset->close(); + } +} From 1649afbec7d460f22323a3a15d78252a902e6786 Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Thu, 22 Aug 2024 13:55:54 +1000 Subject: [PATCH 04/12] Issue #66: Add phpunit test for check_access --- tests/search/search_test.php | 141 +++++++++++++++++++++++------------ 1 file changed, 95 insertions(+), 46 deletions(-) diff --git a/tests/search/search_test.php b/tests/search/search_test.php index bc7af22..8425d53 100644 --- a/tests/search/search_test.php +++ b/tests/search/search_test.php @@ -22,19 +22,27 @@ * @author Tomo Tsuyuki * @copyright 2024 Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - * @covers \mod_cms\search\cms */ namespace mod_cms\search; use mod_cms\local\model\cms_types; -use mod_cms\customfield\cmsfield_handler; defined('MOODLE_INTERNAL') || die(); global $CFG; require_once($CFG->dirroot . '/search/tests/fixtures/testable_core_search.php'); +/** + * Test class for cmsfield search. + * + * @package mod_cms + * @category test + * @author Tomo Tsuyuki + * @copyright 2024 Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @coversDefaultClass \mod_cms\search\cmsfield + */ class search_test extends \advanced_testcase { /** @@ -42,6 +50,21 @@ class search_test extends \advanced_testcase { */ protected $cmsareaid = null; + /** + * @var cms_types CMS type object + */ + protected $cmstype = null; + + /** + * @var \core_customfield\category_controller Custom field category object + */ + protected $fieldcategory = null; + + /** + * @var \core_customfield\field_controller Custom field object + */ + protected $field = null; + /** * Set up. */ @@ -53,6 +76,27 @@ public function setUp(): void { // Set \core_search::instance to the mock_search_engine as we don't require the search engine to be working to test this. $search = \testable_core_search::instance(); + + $cmstype = new cms_types(); + $cmstype->set('name', 'Overview') + ->set('idnumber', 'overview') + ->set('title_mustache', 'Overview'); + $cmstype->save(); + $fieldcategory = self::getDataGenerator()->create_custom_field_category([ + 'name' => 'Other fields', + 'component' => 'mod_cms', + 'area' => 'cmsfield', + 'itemid' => $cmstype->get('id'), + ]); + $field = self::getDataGenerator()->create_custom_field([ + 'name' => 'Overview', + 'shortname' => 'overview', + 'type' => 'text', + 'categoryid' => $fieldcategory->get('id'), + ]); + $this->cmstype = $cmstype; + $this->fieldcategory = $fieldcategory; + $this->field = $field; } /** @@ -60,8 +104,7 @@ public function setUp(): void { * * @return void */ - public function test_search_enabled() { - + public function test_search_enabled(): void { $searcharea = \core_search\manager::get_search_area($this->cmsareaid); list($componentname, $varname) = $searcharea->get_config_var_name(); @@ -80,7 +123,7 @@ public function test_search_enabled() { * * @return void */ - public function test_get_document_recordset() { + public function test_get_document_recordset(): void { global $DB; // Returns the instance as long as the area is supported. @@ -89,60 +132,24 @@ public function test_get_document_recordset() { $course = self::getDataGenerator()->create_course(); - $cmstype = new cms_types(); - $cmstype->set('name', 'Overview') - ->set('idnumber', 'overview') - ->set('title_mustache', 'Overview'); - $cmstype->save(); - - $fieldcategory = self::getDataGenerator()->create_custom_field_category([ - 'name' => 'Other fields', - 'component' => 'mod_cms', - 'area' => 'cmsfield', - 'itemid' => $cmstype->get('id'), - ]); - $field = self::getDataGenerator()->create_custom_field([ - 'name' => 'Overview', - 'shortname' => 'overview', - 'type' => 'text', - 'categoryid' => $fieldcategory->get('id'), - ]); - // Name for cms is coming from "title_mustache" in cms_type. $generator = self::getDataGenerator()->get_plugin_generator('mod_cms'); $record = new \stdClass(); $record->course = $course->id; $record->customfield_overview = 'Test overview text 1'; - $record->typeid = $cmstype->get('id'); + $record->typeid = $this->cmstype->get('id'); $generator->create_instance_with_data($record); $record = new \stdClass(); $record->course = $course->id; $record->customfield_overview = 'Test overview text 2'; - $record->typeid = $cmstype->get('id'); + $record->typeid = $this->cmstype->get('id'); $generator->create_instance_with_data($record); // All records. $recordset = $searcharea->get_document_recordset(); $this->assertTrue($recordset->valid()); - foreach ($recordset as $record) { - $this->assertInstanceOf('stdClass', $record); - $data = $DB->get_record('customfield_data', ['id' => $record->id]); - $doc = $searcharea->get_document($record); - $this->assertInstanceOf('\core_search\document', $doc); - $this->assertEquals('mod_cms-cmsfield-' . $data->id, $doc->get('id')); - $this->assertEquals($data->id, $doc->get('itemid')); - $this->assertEquals($course->id, $doc->get('courseid')); - $this->assertEquals($data->contextid, $doc->get('contextid')); - $this->assertEquals($field->get('name'), $doc->get('title')); - $this->assertEquals($data->value, $doc->get('content')); - - // Static caches are working. - $dbreads = $DB->perf_get_reads(); - $doc = $searcharea->get_document($record); - $this->assertEquals($dbreads, $DB->perf_get_reads()); - $this->assertInstanceOf('\core_search\document', $doc); - } + $this->assertEquals(2, iterator_count($recordset)); $recordset->close(); // The +2 is to prevent race conditions. @@ -157,11 +164,12 @@ public function test_get_document_recordset() { $record = new \stdClass(); $record->course = $course->id; $record->customfield_overview = 'Test overview text 3'; - $record->typeid = $cmstype->get('id'); + $record->typeid = $this->cmstype->get('id'); $generator->create_instance_with_data($record); // Return only new search. $recordset = $searcharea->get_document_recordset(time()); + $count = 0; foreach ($recordset as $record) { $this->assertInstanceOf('stdClass', $record); $data = $DB->get_record('customfield_data', ['id' => $record->id]); @@ -171,7 +179,7 @@ public function test_get_document_recordset() { $this->assertEquals($data->id, $doc->get('itemid')); $this->assertEquals($course->id, $doc->get('courseid')); $this->assertEquals($data->contextid, $doc->get('contextid')); - $this->assertEquals($field->get('name'), $doc->get('title')); + $this->assertEquals($this->field->get('name'), $doc->get('title')); $this->assertEquals($data->value, $doc->get('content')); // Static caches are working. @@ -179,7 +187,48 @@ public function test_get_document_recordset() { $doc = $searcharea->get_document($record); $this->assertEquals($dbreads, $DB->perf_get_reads()); $this->assertInstanceOf('\core_search\document', $doc); + $count++; } + $this->assertEquals(1, $count); $recordset->close(); } + + /** + * Document contents. + * + * @return void + */ + public function test_check_access(): void { + global $DB; + + // Returns the instance as long as the area is supported. + $searcharea = \core_search\manager::get_search_area($this->cmsareaid); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $course = self::getDataGenerator()->create_course(); + + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + + // Name for cms is coming from "title_mustache" in cms_type. + $generator = self::getDataGenerator()->get_plugin_generator('mod_cms'); + $record = new \stdClass(); + $record->course = $course->id; + $record->customfield_overview = 'Test overview text 1'; + $record->typeid = $this->cmstype->get('id'); + $generator->create_instance_with_data($record); + + $records = $DB->get_records('customfield_data', ['fieldid' => $this->field->get('id')]); + $this->assertCount(1, $records); + $data = current($records); + + $this->setAdminUser(); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($data->id)); + + $this->setUser($user1); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($data->id)); + + $this->setUser($user2); + $this->assertEquals(\core_search\manager::ACCESS_DENIED, $searcharea->check_access($data->id)); + } } From 7ab1b5dc67a12f12c41fc7c079c458962e7bf2f1 Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Tue, 27 Aug 2024 10:35:05 +1000 Subject: [PATCH 05/12] Issue #66: Version bump --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index 17c6f1b..a378958 100644 --- a/version.php +++ b/version.php @@ -25,7 +25,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2023120100; +$plugin->version = 2024082700; $plugin->requires = 2020061500; // Moodle 3.9.0 and above. $plugin->supported = [39, 401]; // Moodle 3.9 to 4.1 inclusive. $plugin->component = 'mod_cms'; From f40d50ce88b281dc4e69265c3d7f479cc58a450e Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Thu, 29 Aug 2024 11:32:24 +1000 Subject: [PATCH 06/12] Issue #66: Add default value and update unit test --- classes/search/cmsfield.php | 77 +++++++++++++++++++++-------- lang/en/cms.php | 3 -- tests/search/search_test.php | 94 ++++++++++++++++++++++++++++++++---- 3 files changed, 143 insertions(+), 31 deletions(-) diff --git a/classes/search/cmsfield.php b/classes/search/cmsfield.php index 835cee1..ed38ec6 100644 --- a/classes/search/cmsfield.php +++ b/classes/search/cmsfield.php @@ -14,14 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Define search area. - * - * @package mod_cms - * @author Tomo Tsuyuki - * @copyright 2024 Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ namespace mod_cms\search; defined('MOODLE_INTERNAL') || die(); @@ -43,12 +35,17 @@ class cmsfield extends \core_search\base_mod { */ protected $cmsdata = []; + /** + * @var array Internal quick static cache. + */ + protected $defaultvalues = null; + /** * Returns recordset containing required data for indexing cmsfield records. * * @param int $modifiedfrom timestamp * @param \context|null $context Optional context to restrict scope of returned results - * @return moodle_recordset|null Recordset (or null if no results) + * @return \moodle_recordset|null Recordset (or null if no results) */ public function get_document_recordset($modifiedfrom = 0, \context $context = null) { global $DB; @@ -59,15 +56,25 @@ public function get_document_recordset($modifiedfrom = 0, \context $context = nu return null; } - $sql = "SELECT mcd.*, mc.id AS cmsid, mc.course AS courseid, mcf.name AS fieldname - FROM {customfield_data} mcd - JOIN {cms} mc ON mc.id = mcd.instanceid + $sql = "SELECT mc.id, mc.course AS courseid, mc.typeid, mcf.name AS fieldname, mcd.id dataid, + mcd.value AS value, mcd.valueformat AS valueformat, + mcd.timecreated AS timecreated, mcd.timemodified AS timemodified + FROM {cms} mc + JOIN {customfield_data} mcd ON mc.id = mcd.instanceid JOIN {customfield_field} mcf ON mcf.id = mcd.fieldid JOIN {customfield_category} mcc ON mcf.categoryid = mcc.id $contextjoin WHERE mcd.timemodified >= ? AND mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' - ORDER BY mcd.timemodified ASC"; - return $DB->get_recordset_sql($sql, array_merge($contextparams, [$modifiedfrom])); + AND mcf.type IN ('textarea', 'text') + UNION + SELECT mc.id, mc.course AS courseid, mc.typeid, null AS fieldname, null dataid, + null AS value, null AS valueformat, + mc.timecreated timecreated, mc.timemodified timemodified + FROM {cms} mc + LEFT JOIN {customfield_data} mcd ON mc.id = mcd.instanceid + WHERE mcd.id IS NULL AND mc.timecreated >= ? + ORDER BY timemodified ASC"; + return $DB->get_recordset_sql($sql, array_merge($contextparams, [$modifiedfrom, $modifiedfrom])); } /** @@ -78,8 +85,9 @@ public function get_document_recordset($modifiedfrom = 0, \context $context = nu * @return \core_search\document */ public function get_document($record, $options = []) { + global $DB; try { - $cm = $this->get_cm('cms', $record->cmsid, $record->courseid); + $cm = $this->get_cm('cms', $record->id, $record->courseid); $context = \context_module::instance($cm->id); } catch (\dml_missing_record_exception $ex) { // Notify it as we run here as admin, we should see everything. @@ -92,10 +100,42 @@ public function get_document($record, $options = []) { return false; } + if (is_null($this->defaultvalues)) { + $defaultvalues = []; + // Get default value for cms custom field. + $sql = "SELECT mct.id typeid, mcf.configdata, mcf.name fieldname + FROM {cms_types} mct + JOIN {customfield_category} mcc ON mcc.itemid = mct.id + JOIN {customfield_field} mcf ON mcf.categoryid = mcc.id + WHERE mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' AND mcf.type IN ('textarea', 'text')"; + $cmstypes = $DB->get_records_sql($sql); + $defaultvalues = []; + foreach ($cmstypes as $cmstype) { + $data = new \stdClass(); + $configdata = json_decode($cmstype->configdata); + $data->value = $configdata->defaultvalue ?? 'Default value'; + $data->valueformat = $configdata->defaultvalueformat ?? 0; + $data->fieldname = $cmstype->fieldname; + $defaultvalues[$cmstype->typeid] = $data; + } + $this->defaultvalues = $defaultvalues; + } + + // Check if it's default value or not. + if (empty($record->dataid)) { + $title = $this->defaultvalues[$record->typeid]->fieldname ?? 'Default title'; + $value = $this->defaultvalues[$record->typeid]->value ?? 'Default value'; + $valueformat = $this->defaultvalues[$record->typeid]->valueformat ?? 'Default valueformat'; + } else { + $title = $record->fieldname; + $value = $record->value; + $valueformat = $record->valueformat; + } + // Prepare associative array with data from DB. $doc = \core_search\document_factory::instance($record->id, $this->componentname, $this->areaname); - $doc->set('title', content_to_text($record->fieldname, false)); - $doc->set('content', content_to_text($record->value, $record->valueformat)); + $doc->set('title', content_to_text($title, false)); + $doc->set('content', content_to_text($value, $valueformat)); $doc->set('contextid', $context->id); $doc->set('courseid', $record->courseid); $doc->set('owneruserid', \core_search\manager::NO_OWNER_ID); @@ -120,6 +160,7 @@ public function check_access($id) { try { $data = $this->get_data($id); $cminfo = $this->get_cm('cms', $data->instanceid, $data->courseid); + $context = \context_module::instance($cminfo->id); } catch (\dml_missing_record_exception $ex) { return \core_search\manager::ACCESS_DELETED; } catch (\dml_exception $ex) { @@ -131,8 +172,6 @@ public function check_access($id) { return \core_search\manager::ACCESS_DENIED; } - $context = \context_module::instance($cminfo->id); - if (!has_capability('mod/cms:view', $context)) { return \core_search\manager::ACCESS_DENIED; } diff --git a/lang/en/cms.php b/lang/en/cms.php index c0f33d4..9e88fda 100644 --- a/lang/en/cms.php +++ b/lang/en/cms.php @@ -105,9 +105,6 @@ // Search strings. $string['search:cmsfield'] = 'CMS'; -$string['search:settings'] = 'Search settings'; -$string['search:settings:area'] = 'Search area'; -$string['search:settings:area_desc'] = 'Search area for global search'; // Site datasource strings. $string['site:displayname'] = 'Site Info'; diff --git a/tests/search/search_test.php b/tests/search/search_test.php index 8425d53..9e522bc 100644 --- a/tests/search/search_test.php +++ b/tests/search/search_test.php @@ -26,6 +26,7 @@ namespace mod_cms\search; +use mod_cms\customfield\cmsfield_handler; use mod_cms\local\model\cms_types; defined('MOODLE_INTERNAL') || die(); @@ -93,6 +94,7 @@ public function setUp(): void { 'shortname' => 'overview', 'type' => 'text', 'categoryid' => $fieldcategory->get('id'), + 'configdata' => json_encode(['defaultvalue' => 'Default Text Overview']), ]); $this->cmstype = $cmstype; $this->fieldcategory = $fieldcategory; @@ -138,13 +140,13 @@ public function test_get_document_recordset(): void { $record->course = $course->id; $record->customfield_overview = 'Test overview text 1'; $record->typeid = $this->cmstype->get('id'); - $generator->create_instance_with_data($record); + $cms1 = $generator->create_instance_with_data($record); $record = new \stdClass(); $record->course = $course->id; $record->customfield_overview = 'Test overview text 2'; $record->typeid = $this->cmstype->get('id'); - $generator->create_instance_with_data($record); + $cms2 = $generator->create_instance_with_data($record); // All records. $recordset = $searcharea->get_document_recordset(); @@ -152,15 +154,15 @@ public function test_get_document_recordset(): void { $this->assertEquals(2, iterator_count($recordset)); $recordset->close(); - // The +2 is to prevent race conditions. + // Search again by current time + 2 sec. $recordset = $searcharea->get_document_recordset(time() + 2); - // No new records. $this->assertFalse($recordset->valid()); $recordset->close(); // Wait 1 sec to have new search string. sleep(1); + $time = time(); $record = new \stdClass(); $record->course = $course->id; $record->customfield_overview = 'Test overview text 3'; @@ -168,15 +170,15 @@ public function test_get_document_recordset(): void { $generator->create_instance_with_data($record); // Return only new search. - $recordset = $searcharea->get_document_recordset(time()); + $recordset = $searcharea->get_document_recordset($time); $count = 0; foreach ($recordset as $record) { $this->assertInstanceOf('stdClass', $record); - $data = $DB->get_record('customfield_data', ['id' => $record->id]); + $data = $DB->get_record('customfield_data', ['id' => $record->dataid]); $doc = $searcharea->get_document($record); $this->assertInstanceOf('\core_search\document', $doc); - $this->assertEquals('mod_cms-cmsfield-' . $data->id, $doc->get('id')); - $this->assertEquals($data->id, $doc->get('itemid')); + $this->assertEquals('mod_cms-cmsfield-' . $record->id, $doc->get('id')); + $this->assertEquals($record->id, $doc->get('itemid')); $this->assertEquals($course->id, $doc->get('courseid')); $this->assertEquals($data->contextid, $doc->get('contextid')); $this->assertEquals($this->field->get('name'), $doc->get('title')); @@ -191,10 +193,84 @@ public function test_get_document_recordset(): void { } $this->assertEquals(1, $count); $recordset->close(); + + // Update existing data. + $cms1->customfield_overview = 'Update test 1'; + $handler = cmsfield_handler::create($cms1->typeid); + $handler->instance_form_save($cms1); + // Return 2 records. + $recordset = $searcharea->get_document_recordset($time); + $this->assertTrue($recordset->valid()); + $this->assertEquals(2, iterator_count($recordset)); + $recordset->close(); + } + + /** + * Test default value from cms content type + * + * @return void + */ + public function test_default_content(): void { + global $DB; + + // Returns the instance as long as the area is supported. + $searcharea = \core_search\manager::get_search_area($this->cmsareaid); + $this->assertInstanceOf('\mod_cms\search\cmsfield', $searcharea); + + $course = self::getDataGenerator()->create_course(); + + // Name for cms is coming from "title_mustache" in cms_type. + $generator = self::getDataGenerator()->get_plugin_generator('mod_cms'); + $record = new \stdClass(); + $record->course = $course->id; + $record->typeid = $this->cmstype->get('id'); + $cms1 = $generator->create_instance_with_data($record); + $context = \context_module::instance($cms1->cmid); + + $recordset = $searcharea->get_document_recordset(); + $count = 0; + foreach ($recordset as $record) { + $this->assertInstanceOf('stdClass', $record); + $this->assertEmpty($record->dataid); + $doc = $searcharea->get_document($record); + $this->assertInstanceOf('\core_search\document', $doc); + $this->assertEquals('mod_cms-cmsfield-' . $record->id, $doc->get('id')); + $this->assertEquals($record->id, $doc->get('itemid')); + $this->assertEquals($course->id, $doc->get('courseid')); + $this->assertEquals($context->id, $doc->get('contextid')); + $this->assertEquals($this->field->get('name'), $doc->get('title')); + $this->assertEquals('Default Text Overview', $doc->get('content')); + $count++; + } + $this->assertEquals(1, $count); + $recordset->close(); + + // Add data for the cms activity. + $cms1->customfield_overview = 'Update test 1'; + $handler = cmsfield_handler::create($cms1->typeid); + $handler->instance_form_save($cms1); + $recordset = $searcharea->get_document_recordset(); + $count = 0; + foreach ($recordset as $record) { + $this->assertInstanceOf('stdClass', $record); + $data = $DB->get_record('customfield_data', ['id' => $record->dataid]); + $doc = $searcharea->get_document($record); + $this->assertInstanceOf('\core_search\document', $doc); + $this->assertEquals('mod_cms-cmsfield-' . $record->id, $doc->get('id')); + $this->assertEquals($record->id, $doc->get('itemid')); + $this->assertEquals($course->id, $doc->get('courseid')); + $this->assertEquals($data->contextid, $doc->get('contextid')); + $this->assertEquals($this->field->get('name'), $doc->get('title')); + $this->assertEquals($data->value, $doc->get('content')); + $this->assertEquals('Update test 1', $doc->get('content')); + $count++; + } + $this->assertEquals(1, $count); + $recordset->close(); } /** - * Document contents. + * Test check_access. * * @return void */ From 5c4b30951b0a3b09cc0a646d931d90fe87b124be Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Thu, 29 Aug 2024 12:31:29 +1000 Subject: [PATCH 07/12] Issue #66: Add comments and version bump --- classes/search/cmsfield.php | 27 ++++++++++++++++----------- tests/search/search_test.php | 4 ++-- version.php | 2 +- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/classes/search/cmsfield.php b/classes/search/cmsfield.php index ed38ec6..f94d8b3 100644 --- a/classes/search/cmsfield.php +++ b/classes/search/cmsfield.php @@ -56,8 +56,9 @@ public function get_document_recordset($modifiedfrom = 0, \context $context = nu return null; } - $sql = "SELECT mc.id, mc.course AS courseid, mc.typeid, mcf.name AS fieldname, mcd.id dataid, - mcd.value AS value, mcd.valueformat AS valueformat, + // Search area is from customfield_data, but if the record is missing from activity, use default value. + $sql = "SELECT mc.id, mc.course AS courseid, mc.typeid, mcf.name AS fieldname, mcf.type, + mcd.id dataid, mcd.value AS value, mcd.valueformat AS valueformat, mcd.timecreated AS timecreated, mcd.timemodified AS timemodified FROM {cms} mc JOIN {customfield_data} mcd ON mc.id = mcd.instanceid @@ -100,16 +101,15 @@ public function get_document($record, $options = []) { return false; } + // Get default value for cms custom field. if (is_null($this->defaultvalues)) { $defaultvalues = []; - // Get default value for cms custom field. $sql = "SELECT mct.id typeid, mcf.configdata, mcf.name fieldname FROM {cms_types} mct JOIN {customfield_category} mcc ON mcc.itemid = mct.id JOIN {customfield_field} mcf ON mcf.categoryid = mcc.id WHERE mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' AND mcf.type IN ('textarea', 'text')"; $cmstypes = $DB->get_records_sql($sql); - $defaultvalues = []; foreach ($cmstypes as $cmstype) { $data = new \stdClass(); $configdata = json_decode($cmstype->configdata); @@ -123,9 +123,17 @@ public function get_document($record, $options = []) { // Check if it's default value or not. if (empty($record->dataid)) { - $title = $this->defaultvalues[$record->typeid]->fieldname ?? 'Default title'; - $value = $this->defaultvalues[$record->typeid]->value ?? 'Default value'; - $valueformat = $this->defaultvalues[$record->typeid]->valueformat ?? 'Default valueformat'; + $title = $this->defaultvalues[$record->typeid]->fieldname ?? ''; + $value = $this->defaultvalues[$record->typeid]->value ?? ''; + if (isset($this->defaultvalues[$record->typeid]->valueformat)) { + $valueformat = $this->defaultvalues[$record->typeid]->valueformat; + } else { + if ($record->type == 'textarea') { + $valueformat = FORMAT_HTML; + } else { + $valueformat = FORMAT_PLAIN; + } + } } else { $title = $record->fieldname; $value = $record->value; @@ -216,10 +224,7 @@ protected function get_data($id) { FROM {customfield_data} mcd JOIN {cms} mc ON mc.id = mcd.instanceid WHERE mcd.id = :id"; - $this->cmsdata[$id] = $DB->get_record_sql($sql, ['id' => $id]); - if (!$this->cmsdata[$id]) { - throw new \dml_missing_record_exception('cms_data'); - } + $this->cmsdata[$id] = $DB->get_record_sql($sql, ['id' => $id], MUST_EXIST); } return $this->cmsdata[$id]; } diff --git a/tests/search/search_test.php b/tests/search/search_test.php index 9e522bc..6c73cdb 100644 --- a/tests/search/search_test.php +++ b/tests/search/search_test.php @@ -78,6 +78,7 @@ public function setUp(): void { // Set \core_search::instance to the mock_search_engine as we don't require the search engine to be working to test this. $search = \testable_core_search::instance(); + // Name for cms activity is using from "title_mustache". $cmstype = new cms_types(); $cmstype->set('name', 'Overview') ->set('idnumber', 'overview') @@ -134,7 +135,7 @@ public function test_get_document_recordset(): void { $course = self::getDataGenerator()->create_course(); - // Name for cms is coming from "title_mustache" in cms_type. + // The name of cms activity is from cms_type, so we do not set when creating the activity. $generator = self::getDataGenerator()->get_plugin_generator('mod_cms'); $record = new \stdClass(); $record->course = $course->id; @@ -219,7 +220,6 @@ public function test_default_content(): void { $course = self::getDataGenerator()->create_course(); - // Name for cms is coming from "title_mustache" in cms_type. $generator = self::getDataGenerator()->get_plugin_generator('mod_cms'); $record = new \stdClass(); $record->course = $course->id; diff --git a/version.php b/version.php index a378958..a028e97 100644 --- a/version.php +++ b/version.php @@ -25,7 +25,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2024082700; +$plugin->version = 2024082900; $plugin->requires = 2020061500; // Moodle 3.9.0 and above. $plugin->supported = [39, 401]; // Moodle 3.9 to 4.1 inclusive. $plugin->component = 'mod_cms'; From ccadfac2bbab15a6ad9ab5e60bffb39d64bb0a06 Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Fri, 30 Aug 2024 11:23:14 +1000 Subject: [PATCH 08/12] Issue #66: Fix CI issues --- classes/search/cmsfield.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/classes/search/cmsfield.php b/classes/search/cmsfield.php index f94d8b3..600297f 100644 --- a/classes/search/cmsfield.php +++ b/classes/search/cmsfield.php @@ -68,8 +68,8 @@ public function get_document_recordset($modifiedfrom = 0, \context $context = nu WHERE mcd.timemodified >= ? AND mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' AND mcf.type IN ('textarea', 'text') UNION - SELECT mc.id, mc.course AS courseid, mc.typeid, null AS fieldname, null dataid, - null AS value, null AS valueformat, + SELECT mc.id, mc.course AS courseid, mc.typeid, null AS fieldname, null AS type, + null AS dataid, null AS value, null AS valueformat, mc.timecreated timecreated, mc.timemodified timemodified FROM {cms} mc LEFT JOIN {customfield_data} mcd ON mc.id = mcd.instanceid @@ -123,7 +123,7 @@ public function get_document($record, $options = []) { // Check if it's default value or not. if (empty($record->dataid)) { - $title = $this->defaultvalues[$record->typeid]->fieldname ?? ''; + $title = $this->defaultvalues[$record->typeid]->fieldname ?? ''; $value = $this->defaultvalues[$record->typeid]->value ?? ''; if (isset($this->defaultvalues[$record->typeid]->valueformat)) { $valueformat = $this->defaultvalues[$record->typeid]->valueformat; From 67fc491a1f5fbc89b55e18509119d557daf2e9ef Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Mon, 2 Sep 2024 14:07:42 +1000 Subject: [PATCH 09/12] Issue #66: Debug search result --- classes/search/cmsfield.php | 9 ++++----- tests/search/search_test.php | 16 ++++++++++------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/classes/search/cmsfield.php b/classes/search/cmsfield.php index 600297f..11644b5 100644 --- a/classes/search/cmsfield.php +++ b/classes/search/cmsfield.php @@ -167,7 +167,7 @@ public function get_document($record, $options = []) { public function check_access($id) { try { $data = $this->get_data($id); - $cminfo = $this->get_cm('cms', $data->instanceid, $data->courseid); + $cminfo = $this->get_cm('cms', $data->id, $data->courseid); $context = \context_module::instance($cminfo->id); } catch (\dml_missing_record_exception $ex) { return \core_search\manager::ACCESS_DELETED; @@ -220,10 +220,9 @@ public function get_context_url(\core_search\document $doc) { protected function get_data($id) { global $DB; if (empty($this->cmsdata[$id])) { - $sql = "SELECT mcd.*, mc.id AS cmsid, mc.course AS courseid - FROM {customfield_data} mcd - JOIN {cms} mc ON mc.id = mcd.instanceid - WHERE mcd.id = :id"; + $sql = "SELECT mc.id, mc.course AS courseid + FROM {cms} mc + WHERE mc.id = :id"; $this->cmsdata[$id] = $DB->get_record_sql($sql, ['id' => $id], MUST_EXIST); } return $this->cmsdata[$id]; diff --git a/tests/search/search_test.php b/tests/search/search_test.php index 6c73cdb..8cbc24c 100644 --- a/tests/search/search_test.php +++ b/tests/search/search_test.php @@ -106,6 +106,7 @@ public function setUp(): void { * Test search enabled. * * @return void + * @covers \core_search\manager::get_search_area */ public function test_search_enabled(): void { $searcharea = \core_search\manager::get_search_area($this->cmsareaid); @@ -125,6 +126,8 @@ public function test_search_enabled(): void { * Indexing mod cms contents. * * @return void + * @covers ::get_document_recordset + * @covers ::get_document */ public function test_get_document_recordset(): void { global $DB; @@ -210,6 +213,8 @@ public function test_get_document_recordset(): void { * Test default value from cms content type * * @return void + * @covers ::get_document_recordset + * @covers ::get_document */ public function test_default_content(): void { global $DB; @@ -273,6 +278,7 @@ public function test_default_content(): void { * Test check_access. * * @return void + * @covers ::check_access */ public function test_check_access(): void { global $DB; @@ -286,25 +292,23 @@ public function test_check_access(): void { $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); - // Name for cms is coming from "title_mustache" in cms_type. $generator = self::getDataGenerator()->get_plugin_generator('mod_cms'); $record = new \stdClass(); $record->course = $course->id; $record->customfield_overview = 'Test overview text 1'; $record->typeid = $this->cmstype->get('id'); - $generator->create_instance_with_data($record); + $cms = $generator->create_instance_with_data($record); $records = $DB->get_records('customfield_data', ['fieldid' => $this->field->get('id')]); $this->assertCount(1, $records); - $data = current($records); $this->setAdminUser(); - $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($data->id)); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($cms->id)); $this->setUser($user1); - $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($data->id)); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($cms->id)); $this->setUser($user2); - $this->assertEquals(\core_search\manager::ACCESS_DENIED, $searcharea->check_access($data->id)); + $this->assertEquals(\core_search\manager::ACCESS_DENIED, $searcharea->check_access($cms->id)); } } From 028fa0f6db6d4b16b6a51c4ff2dc3678db07b826 Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Tue, 3 Sep 2024 12:44:13 +1000 Subject: [PATCH 10/12] Issue #66: Concatenate texts in same activity --- classes/search/cmsfield.php | 92 +++++++++++++++++++++++------------- tests/search/search_test.php | 86 +++++++++++++++++++++++++-------- 2 files changed, 126 insertions(+), 52 deletions(-) diff --git a/classes/search/cmsfield.php b/classes/search/cmsfield.php index 11644b5..e7e571d 100644 --- a/classes/search/cmsfield.php +++ b/classes/search/cmsfield.php @@ -57,16 +57,25 @@ public function get_document_recordset($modifiedfrom = 0, \context $context = nu } // Search area is from customfield_data, but if the record is missing from activity, use default value. - $sql = "SELECT mc.id, mc.course AS courseid, mc.typeid, mcf.name AS fieldname, mcf.type, - mcd.id dataid, mcd.value AS value, mcd.valueformat AS valueformat, - mcd.timecreated AS timecreated, mcd.timemodified AS timemodified - FROM {cms} mc - JOIN {customfield_data} mcd ON mc.id = mcd.instanceid - JOIN {customfield_field} mcf ON mcf.id = mcd.fieldid - JOIN {customfield_category} mcc ON mcf.categoryid = mcc.id - $contextjoin - WHERE mcd.timemodified >= ? AND mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' - AND mcf.type IN ('textarea', 'text') + $sqlgroupconcat = $DB->sql_group_concat("mcd.value", ', ', 'mcf.sortorder'); + $sql = "SELECT ccms.id, ccms.course AS courseid, ccms.typeid, cmcf.name AS fieldname, cmcf.type, + cdata.dataid dataid, cdata.value AS value, cdata.valueformat AS valueformat, + cdata.timecreated AS timecreated, cdata.timemodified AS timemodified + FROM {cms} ccms + JOIN ( + SELECT mc.id, MAX(mcf.id) AS fieldid, + MAX(mcd.id) dataid, {$sqlgroupconcat} AS value, MAX(mcd.valueformat) AS valueformat, + MAX(mcd.timecreated) AS timecreated, MAX(mcd.timemodified) AS timemodified + FROM {cms} mc + JOIN {customfield_data} mcd ON mc.id = mcd.instanceid + JOIN {customfield_field} mcf ON mcf.id = mcd.fieldid + JOIN {customfield_category} mcc ON mcf.categoryid = mcc.id + $contextjoin + WHERE mcd.timemodified >= ? AND mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' + AND mcf.type IN ('textarea', 'text') + GROUP BY mc.id + ) cdata ON ccms.id = cdata.id + JOIN {customfield_field} cmcf ON cmcf.id = cdata.fieldid UNION SELECT mc.id, mc.course AS courseid, mc.typeid, null AS fieldname, null AS type, null AS dataid, null AS value, null AS valueformat, @@ -101,32 +110,14 @@ public function get_document($record, $options = []) { return false; } - // Get default value for cms custom field. - if (is_null($this->defaultvalues)) { - $defaultvalues = []; - $sql = "SELECT mct.id typeid, mcf.configdata, mcf.name fieldname - FROM {cms_types} mct - JOIN {customfield_category} mcc ON mcc.itemid = mct.id - JOIN {customfield_field} mcf ON mcf.categoryid = mcc.id - WHERE mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' AND mcf.type IN ('textarea', 'text')"; - $cmstypes = $DB->get_records_sql($sql); - foreach ($cmstypes as $cmstype) { - $data = new \stdClass(); - $configdata = json_decode($cmstype->configdata); - $data->value = $configdata->defaultvalue ?? 'Default value'; - $data->valueformat = $configdata->defaultvalueformat ?? 0; - $data->fieldname = $cmstype->fieldname; - $defaultvalues[$cmstype->typeid] = $data; - } - $this->defaultvalues = $defaultvalues; - } + $defaultvalues = $this->get_default_values(); // Check if it's default value or not. if (empty($record->dataid)) { - $title = $this->defaultvalues[$record->typeid]->fieldname ?? ''; - $value = $this->defaultvalues[$record->typeid]->value ?? ''; - if (isset($this->defaultvalues[$record->typeid]->valueformat)) { - $valueformat = $this->defaultvalues[$record->typeid]->valueformat; + $title = $defaultvalues[$record->typeid]->fieldname ?? ''; + $value = $defaultvalues[$record->typeid]->value ?? ''; + if (isset($defaultvalues[$record->typeid]->valueformat)) { + $valueformat = $defaultvalues[$record->typeid]->valueformat; } else { if ($record->type == 'textarea') { $valueformat = FORMAT_HTML; @@ -158,6 +149,41 @@ public function get_document($record, $options = []) { return $doc; } + /** + * Get default value for cms custom field. + * + * @return array + */ + protected function get_default_values() { + global $DB; + if (is_null($this->defaultvalues)) { + $defaultvalues = []; + $sql = "SELECT mcf.id fieldid, mct.id typeid, mcf.configdata, mcf.name fieldname + FROM {cms_types} mct + JOIN {customfield_category} mcc ON mcc.itemid = mct.id + JOIN {customfield_field} mcf ON mcf.categoryid = mcc.id + WHERE mcc.component = 'mod_cms' AND mcc.area = 'cmsfield' AND mcf.type IN ('textarea', 'text') + ORDER BY mct.id, mcf.sortorder"; + $cmstypes = $DB->get_records_sql($sql); + foreach ($cmstypes as $cmstype) { + if (empty($defaultvalues[$cmstype->typeid])) { + $data = new \stdClass(); + $configdata = json_decode($cmstype->configdata); + $data->value = $configdata->defaultvalue ?? 'Default value'; + $data->valueformat = $configdata->defaultvalueformat ?? 0; + $data->fieldname = $cmstype->fieldname; + } else { + $data = $defaultvalues[$cmstype->typeid]; + $configdata = json_decode($cmstype->configdata); + $data->value .= ', ' . $configdata->defaultvalue; + } + $defaultvalues[$cmstype->typeid] = $data; + } + $this->defaultvalues = $defaultvalues; + } + return $this->defaultvalues; + } + /** * Whether the user can access the document or not. * diff --git a/tests/search/search_test.php b/tests/search/search_test.php index 8cbc24c..c628481 100644 --- a/tests/search/search_test.php +++ b/tests/search/search_test.php @@ -171,7 +171,8 @@ public function test_get_document_recordset(): void { $record->course = $course->id; $record->customfield_overview = 'Test overview text 3'; $record->typeid = $this->cmstype->get('id'); - $generator->create_instance_with_data($record); + $cms3 = $generator->create_instance_with_data($record); + $context = \context_module::instance($cms3->cmid); // Return only new search. $recordset = $searcharea->get_document_recordset($time); @@ -184,7 +185,7 @@ public function test_get_document_recordset(): void { $this->assertEquals('mod_cms-cmsfield-' . $record->id, $doc->get('id')); $this->assertEquals($record->id, $doc->get('itemid')); $this->assertEquals($course->id, $doc->get('courseid')); - $this->assertEquals($data->contextid, $doc->get('contextid')); + $this->assertEquals($context->id, $doc->get('contextid')); $this->assertEquals($this->field->get('name'), $doc->get('title')); $this->assertEquals($data->value, $doc->get('content')); @@ -217,6 +218,56 @@ public function test_get_document_recordset(): void { * @covers ::get_document */ public function test_default_content(): void { + $searcharea = \core_search\manager::get_search_area($this->cmsareaid); + $this->assertInstanceOf('\mod_cms\search\cmsfield', $searcharea); + + $course = self::getDataGenerator()->create_course(); + + // Create cms activity without customfield. + $generator = self::getDataGenerator()->get_plugin_generator('mod_cms'); + $record = new \stdClass(); + $record->course = $course->id; + $record->typeid = $this->cmstype->get('id'); + $cms1 = $generator->create_instance_with_data($record); + + $recordset = $searcharea->get_document_recordset(); + $count = 0; + foreach ($recordset as $record) { + $this->assertInstanceOf('stdClass', $record); + $this->assertEmpty($record->dataid); + $doc = $searcharea->get_document($record); + $this->assertInstanceOf('\core_search\document', $doc); + // Confirm the content is from defaultvalue from cms fieldtype. + $this->assertEquals('Default Text Overview', $doc->get('content')); + $count++; + } + $this->assertEquals(1, $count); + $recordset->close(); + + // Add custom data for the cms activity. + $cms1->customfield_overview = 'Update test 1'; + $handler = cmsfield_handler::create($cms1->typeid); + $handler->instance_form_save($cms1); + $recordset = $searcharea->get_document_recordset(); + $count = 0; + foreach ($recordset as $record) { + $this->assertInstanceOf('stdClass', $record); + $doc = $searcharea->get_document($record); + $this->assertEquals('Update test 1', $doc->get('content')); + $count++; + } + $this->assertEquals(1, $count); + $recordset->close(); + } + + /** + * Test multiple contents in one cms activity + * + * @return void + * @covers ::get_document_recordset + * @covers ::get_document + */ + public function test_multiple_contents(): void { global $DB; // Returns the instance as long as the area is supported. @@ -224,13 +275,19 @@ public function test_default_content(): void { $this->assertInstanceOf('\mod_cms\search\cmsfield', $searcharea); $course = self::getDataGenerator()->create_course(); + $field = self::getDataGenerator()->create_custom_field([ + 'name' => 'Details', + 'shortname' => 'details', + 'type' => 'text', + 'categoryid' => $this->fieldcategory->get('id'), + 'configdata' => json_encode(['defaultvalue' => 'Default Text Details']), + ]); $generator = self::getDataGenerator()->get_plugin_generator('mod_cms'); $record = new \stdClass(); $record->course = $course->id; $record->typeid = $this->cmstype->get('id'); $cms1 = $generator->create_instance_with_data($record); - $context = \context_module::instance($cms1->cmid); $recordset = $searcharea->get_document_recordset(); $count = 0; @@ -239,35 +296,26 @@ public function test_default_content(): void { $this->assertEmpty($record->dataid); $doc = $searcharea->get_document($record); $this->assertInstanceOf('\core_search\document', $doc); - $this->assertEquals('mod_cms-cmsfield-' . $record->id, $doc->get('id')); - $this->assertEquals($record->id, $doc->get('itemid')); - $this->assertEquals($course->id, $doc->get('courseid')); - $this->assertEquals($context->id, $doc->get('contextid')); - $this->assertEquals($this->field->get('name'), $doc->get('title')); - $this->assertEquals('Default Text Overview', $doc->get('content')); + $this->assertStringContainsString('Default Text Overview', $doc->get('content')); + $this->assertStringContainsString('Default Text Details', $doc->get('content')); $count++; } $this->assertEquals(1, $count); $recordset->close(); // Add data for the cms activity. - $cms1->customfield_overview = 'Update test 1'; + $cms1->customfield_overview = 'Overview test 1'; + $cms1->customfield_details = 'Details test 1'; $handler = cmsfield_handler::create($cms1->typeid); $handler->instance_form_save($cms1); $recordset = $searcharea->get_document_recordset(); $count = 0; foreach ($recordset as $record) { $this->assertInstanceOf('stdClass', $record); - $data = $DB->get_record('customfield_data', ['id' => $record->dataid]); $doc = $searcharea->get_document($record); - $this->assertInstanceOf('\core_search\document', $doc); - $this->assertEquals('mod_cms-cmsfield-' . $record->id, $doc->get('id')); - $this->assertEquals($record->id, $doc->get('itemid')); - $this->assertEquals($course->id, $doc->get('courseid')); - $this->assertEquals($data->contextid, $doc->get('contextid')); - $this->assertEquals($this->field->get('name'), $doc->get('title')); - $this->assertEquals($data->value, $doc->get('content')); - $this->assertEquals('Update test 1', $doc->get('content')); + // Both strings are contained in the cms activity. + $this->assertStringContainsString('Overview test 1', $doc->get('content')); + $this->assertStringContainsString('Details test 1', $doc->get('content')); $count++; } $this->assertEquals(1, $count); From 0cff7583bb03bd7c9df4ef53ff054f2717668cf6 Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Tue, 3 Sep 2024 14:54:31 +1000 Subject: [PATCH 11/12] Issue #66: Set support version to 4.1 --- version.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/version.php b/version.php index a028e97..a79caa6 100644 --- a/version.php +++ b/version.php @@ -25,9 +25,9 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2024082900; -$plugin->requires = 2020061500; // Moodle 3.9.0 and above. -$plugin->supported = [39, 401]; // Moodle 3.9 to 4.1 inclusive. +$plugin->version = 2024090300; +$plugin->requires = 2022112800; // Moodle 4.1 and above. +$plugin->supported = [401, 401]; // Moodle 4.1. $plugin->component = 'mod_cms'; $plugin->maturity = MATURITY_STABLE; $plugin->release = 2023051800; From 6eace550fd96276cd622489fc56d0517e98165f7 Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Tue, 3 Sep 2024 15:11:41 +1000 Subject: [PATCH 12/12] Issue #66: Update README.md --- README.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index f69a702..8a8555b 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,14 @@ An activity module for managing custom defined content types which are 'first class' concepts in Moodle. This is to enable course author to think in proper concepts that matter to them and not worry about the rendering of each content type which will be defined centrally. +## Branches ## +The following maps the plugin version to use depending on your Moodle version. + +| Moodle version | Branch | +|-------------------| ------------------| +| Moodle 3.9 to 4.0 | MOODLE_39_STABLE | +| Moodle 4.1 | MOODLE_401_STABLE | + ## Installation Step 1: Install the activity module @@ -13,7 +21,7 @@ Step 1: Install the activity module Using git submodule: ``` -git submodule add git@github.com:catalyst/moodle-mod_cms.git mod/cms +git submodule add -b MOODLE_401_STABLE git@github.com:catalyst/moodle-mod_cms.git mod/cms ``` Or you can download as a zip from github