Skip to content

Optimizamos código, además de hacerlo más mantenible a futuro #166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tmas0 wants to merge 2 commits into
base: main_old
Choose a base branch
from
Open

Optimizamos código, además de hacerlo más mantenible a futuro #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36e68ee
Optimizamos código, además de hacerlo más mantenible a futuro
tmas0 Apr 14, 2025
c0adec0
Generación libería para los tests.
tmas0 Apr 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 35 additions & 30 deletions classes/local/entities/rule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ protected function get_all_columns(): array {
* @return array
*/
protected function get_all_filters(): array {
global $USER, $DB, $PAGE;
global $USER, $DB, $PAGE, $COURSE;
$narralias = $this->get_table_alias('notificationsagent_report');
$narrualias = $this->get_table_alias('notificationsagent_rule');
$coursealias = $this->get_table_alias('course');
Expand All @@ -202,8 +202,8 @@ protected function get_all_filters(): array {
)
) {
$query = 'SELECT id
FROM {notificationsagent_rule}
WHERE template = 1';
FROM {notificationsagent_rule}
WHERE template = 1';
$rulenames = $DB->get_fieldset_sql($query);
// User can see rules of current course.
} else if (
Expand All @@ -214,8 +214,8 @@ protected function get_all_filters(): array {
)
) {
$query = 'SELECT DISTINCT {notificationsagent_report}.ruleid
FROM {notificationsagent_report}
WHERE {notificationsagent_report}.courseid = :courseid';
FROM {notificationsagent_report}
WHERE {notificationsagent_report}.courseid = :courseid';
$params = ['courseid' => $coursecontext->instanceid];
$rulenames = $DB->get_fieldset_sql($query, $params);
// User can see rules of its own.
Expand All @@ -229,11 +229,11 @@ protected function get_all_filters(): array {
$key = array_keys(enrol_get_my_courses(['id', 'cacherev']));
[$insql, $inparams] = $DB->get_in_or_equal($key, SQL_PARAMS_NAMED, 'courseid');
$query = 'SELECT {notificationsagent_rule}.id
FROM {notificationsagent_rule}
JOIN {notificationsagent_report}
ON {notificationsagent_report}.ruleid = {notificationsagent_rule}.id
WHERE {notificationsagent_rule}.createdby = :userid
AND {notificationsagent_report}.courseid ' . $insql;
FROM {notificationsagent_rule}
JOIN {notificationsagent_report}
ON {notificationsagent_report}.ruleid = {notificationsagent_rule}.id
WHERE {notificationsagent_rule}.createdby = :userid
AND {notificationsagent_report}.courseid ' . $insql;
$params = ['userid' => $userid, ...$inparams];
$rulenames = $DB->get_fieldset_sql($query, $params);
}
Expand Down Expand Up @@ -280,7 +280,7 @@ protected function get_all_filters(): array {
"{$coursealias}.id"
))
->add_joins($this->get_joins())
->set_options_callback(static function () use ($DB, $PAGE): array {
->set_options_callback(static function () use ($DB, $PAGE, $COURSE): array {
$coursecontext = $PAGE->context;
$options = [];
if (
Expand All @@ -302,12 +302,8 @@ protected function get_all_filters(): array {
$coursecontext
)
) {
$query = "SELECT id FROM {course} WHERE id = :courseid";
$params = ['courseid' => $coursecontext->instanceid];
$courses = $DB->get_fieldset_sql($query, $params);
foreach ($courses as $course) {
$options[$course] = get_course($course)->fullname;
}
// Only filter the current course.
$options[$COURSE->id] = $COURSE->fullname;
}

return $options;
Expand Down Expand Up @@ -341,12 +337,16 @@ protected function get_all_filters(): array {
$coursecontext
)
) {
$query
= "SELECT DISTINCT
{user}.id, {user}.firstname, {user}.lastname, {user}.firstnamephonetic,
{user}.lastnamephonetic, {user}.middlename, {user}.alternatename
FROM {notificationsagent_report}
JOIN {user} ON {user}.id={notificationsagent_report}.userid";
$query = "SELECT DISTINCT
{user}.id
, {user}.firstname
, {user}.lastname
, {user}.firstnamephonetic
, {user}.lastnamephonetic
, {user}.middlename
, {user}.alternatename
FROM {notificationsagent_report}
JOIN {user} ON {user}.id = {notificationsagent_report}.userid";
$users = $DB->get_recordset_sql($query);

foreach ($users as $user) {
Expand All @@ -359,14 +359,19 @@ protected function get_all_filters(): array {
$coursecontext
)
) {
$key = array_keys(enrol_get_my_courses(['id', 'cacherev']));
$key = array_keys(enrol_get_my_courses());
[$insql, $params] = $DB->get_in_or_equal($key, SQL_PARAMS_NAMED, 'courseid');
$query = 'SELECT DISTINCT
{user}.id, {user}.firstname, {user}.lastname, {user}.firstnamephonetic,
{user}.lastnamephonetic, {user}.middlename, {user}.alternatename
FROM {notificationsagent_report}
JOIN {user} ON {user}.id = {notificationsagent_report}.userid
WHERE {notificationsagent_report}.courseid ' . $insql;
{user}.id
, {user}.firstname
, {user}.lastname
, {user}.firstnamephonetic
, {user}.lastnamephonetic
, {user}.middlename
, {user}.alternatename
FROM {notificationsagent_report}
JOIN {user} ON {user}.id = {notificationsagent_report}.userid
WHERE {notificationsagent_report}.courseid ' . $insql;

$users = $DB->get_recordset_sql($query, $params);

Expand All @@ -380,7 +385,7 @@ protected function get_all_filters(): array {
$coursecontext
)
) {
$options[$USER->id] = $USER->firstname . ' ' . $USER->lastname;
$options[$USER->id] = fullname($USER);
}

return $options;
Expand Down
6 changes: 5 additions & 1 deletion classes/rule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1595,7 +1595,11 @@ public function create($data) {
$record = $this->to_record();
$record->name = $data->title;
$record->createdat = time();
$record->createdby = $USER->id;

if (!isset($data->createdby)) {
$record->createdby = $USER->id;
}

$record->template = $data->type;

if (isset($data->timesfired) && !empty($data->timesfired)) {
Expand Down
114 changes: 114 additions & 0 deletions tests/generator/lib.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
<?php
// This file is part of Moodle - https://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 <https://www.gnu.org/licenses/>.

// Project implemented by the "Recovery, Transformation and Resilience Plan.
// Funded by the European Union - Next GenerationEU".
//
// Produced by the UNIMOODLE University Group: Universities of
// Valladolid, Complutense de Madrid, UPV/EHU, León, Salamanca,
// Illes Balears, Valencia, Rey Juan Carlos, La Laguna, Zaragoza, Málaga,
// Córdoba, Extremadura, Vigo, Las Palmas de Gran Canaria y Burgos.

/**
* Notificationsagent test data generator class.
*
* @package local_notificationsagent
* @copyright 2025 Proyecto UNIMOODLE
* @author UNIMOODLE Group (Coordinator) <direccion.area.estrategia.digital@uva.es>
* @author PLANIFICACIÓN DE ENTORNOS TECNOLÓGICOS, S.L. <admon@pentec.es>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class local_notificationsagent_generator extends testing_data_generator {
/** @var int Number of created rules */
protected $numofrules = 0;


/**
* Create rule.
*
* @param stdClass $rule Rule object to will be created.
* @return int The new rule id.
*/
public function create_rule(stdClass $rule): int {
$newrule = new local_notificationsagent\rule();

if (!isset($rule->title)) {
$rule->title = "Rule $this->numofrules";
}

if (!isset($rule->type)) {
$rule->type = local_notificationsagent\rule::RULE_TYPE;
}

if (!isset($rule->courseid)) {
// Create new course.
$course = $this->create_course(['startdate' => time() - WEEKSECS, 'enddate' => time() + YEARSECS]);
$rule->courseid = $course->id;
}

if (!isset($rule->timesfired)) {
$rule->timesfired = 1;
}

if (!isset($rule->runtime_group)) {
$rule->runtime_group = ['runtime_days' => 5, 'runtime_hours' => 0, 'runtime_minutes' => 0];
}

if (!isset($rules->createdby)) {
// Create new user.
$user = $this->create_user();
$rule->createdby = $user->id;
}

return $newrule->create($rule);
}

/**
* Create condition.
*
* @param stdClass $condition The condition that will be created.
* @return int The new condition id.
*/
public function create_condition(stdClass $condition): int {
global $DB;

$newcondition = new stdClass();

if (!isset($condition->ruleid)) {
$newrule = new stdClass();
$newrule->timesfired = 2;
$condition->ruleid = $this->create_rule($newrule);
}

if (!isset($condition->type)) {
$condition->type = local_notificationsagent\notificationplugin::TYPE_CONDITION;
}

if (!isset($condition->complementary)) {
$condition->complementary = local_notificationsagent\notificationplugin::COMPLEMENTARY_CONDITION;
}

if (!isset($condition->parameters)) {
$condition->parameters = '{"time": 300}';
}

if (!isset($condition->pluginname)) {
$condition->pluginname = 'coursestart';
}

return $DB->insert_record('notificationsagent_condition', (array)$condition);
}
}
41 changes: 41 additions & 0 deletions tests/notificationsagent_rule_test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
namespace local_notificationsagent;

use local_notificationsagent\helper\helper;
use stdClass;

/**
* Testing rule class
Expand Down Expand Up @@ -110,6 +111,46 @@ final public function setUp(): void {
]);
}

/**
* Multiple evaluations.
*/
public function test_evaluate_performance_v1() {
$generator = $this->getDataGenerator()->get_plugin_generator('local_notificationsagent');
$futurerule = new stdClass();
$futurerule->courseid = self::$course->id;
$futurerule->timesfired = 2;

$ruleid = $generator->create_rule($futurerule);

// Conditions.
$newcondition = new stdClass();
$newcondition->ruleid = $ruleid;
$condition1 = $generator->create_condition($newcondition);

// Context.
$context = new evaluationcontext();
$context->set_userid(self::$user->id);
$context->set_courseid(self::$course->id);
$context->set_timeaccess(time() - YEARSECS);
$context->set_triggercondition($condition1);

$rule = new rule($ruleid);
$result = $rule->evaluate($context);
$this->assertTrue($result);

$conditions = $rule->get_conditions_to_evaluate();
$this->assertCount(1, $conditions);

foreach ($conditions as $condition) {
$context->set_params($condition->get_parameters());
$context->set_complementary(false);
$result = $condition->evaluate($context);
$this->assertTrue($result);
}

print_object($conditions);
}

/**
* Testing evaluate function
*
Expand Down