Skip to content
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.

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

Make default rules editable (#26) #59

Closed
wants to merge 53 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
2f56bf4
Now static filters are editable.
Canx Feb 19, 2017
20d0e52
Populate block_xp_default_filters correctly.
Canx Feb 19, 2017
a33bbcc
improving based on review
Canx Feb 25, 2017
f3b01e5
Refactoring filters, not finished
Canx Mar 7, 2017
b174f18
Refactored aiming at filter_manager::course_default_filters_to_course
Canx Mar 8, 2017
32dc4f0
Renamed classes and files to follow ruleset model
Canx Mar 9, 2017
c4cb5eb
Default filters now are stored using zero courseid in block_xp_filters
Canx Mar 9, 2017
76c8ba4
Merge branch 'issue-27-zerohack' into issue-26
Canx Mar 9, 2017
f5f0aa5
Renamed files and classes to follow ruleset pattern
Canx Mar 10, 2017
590a5a3
Fixing first failing test
Canx Mar 10, 2017
8edd6cf
Undoing bug, don't know why tests are failing...
Canx Mar 10, 2017
e6559cb
Now tests pass
Canx Mar 11, 2017
1592295
Readded non-editable static rules in rules.php, but now they are only…
Canx Mar 12, 2017
70b547c
Added header license in new files
Canx Mar 12, 2017
374a614
Wrong description
Canx Mar 12, 2017
a22c0ea
abstract functions should be protected
Canx Mar 12, 2017
9600918
Cleaned code style
Canx Mar 13, 2017
ae568c8
Added comment
Canx Mar 13, 2017
cee1259
Improving filter classes and adding tests. Still some failing old tests
Canx Mar 14, 2017
da4e569
Refactoring filter_manager and adapting old tests. manager_tests fail…
Canx Mar 16, 2017
a877742
Cleaning debug comments. Detected bug but manager_test still fails...…
Canx Mar 16, 2017
11380d8
Now course rules render ok!
Canx Mar 16, 2017
fb29ddd
Now all tests passes. I'm going to cry!
Canx Mar 16, 2017
9d7c4a9
Prepare filter_manager to manage default filters.
Canx Mar 19, 2017
a6a4a54
Added comments and renamed function
Canx Mar 20, 2017
a6b9b88
filterset lazy loading
Canx Mar 20, 2017
ce43806
Refactoring filter save function
Canx Mar 20, 2017
8c81559
Refactor filter load method
Canx Mar 20, 2017
4c2f17b
Refactor load_as_new
Canx Mar 20, 2017
d303018
Refactor load_from data and renaming
Canx Mar 20, 2017
776c042
Comment function insert_or_update
Canx Mar 20, 2017
7ae003a
Refactor load filterset method
Canx Mar 20, 2017
ae4c262
use factory from filter class
Canx Mar 20, 2017
fe57237
rules.php refactor and solved errors.
Canx Mar 21, 2017
e11d003
Readding types in params (PHP5) and refactoring filter match.
Canx Mar 21, 2017
1c1e15c
Added iterator pattern to filterset
Canx Mar 21, 2017
23555f6
Iterator->SeekableIterator interface for filterset. Removed and clean…
Canx Mar 21, 2017
547ba24
Simple factories to get more readable code.
Canx Mar 22, 2017
9e3342e
Added test to check addition of default filters to current courses.
Canx Mar 22, 2017
b584083
Renamed methods.
Canx Mar 23, 2017
149f652
properties should be protected.
Canx Mar 24, 2017
55127a9
Filtersets now have renderer.
Canx Mar 26, 2017
716ba0e
forgot to rename function.
Canx Mar 27, 2017
468f6a8
Added test to check filter_manager::save_default_filters().
Canx Mar 27, 2017
1176abd
Added admin page to modify default rules.
Canx Mar 29, 2017
a7859f5
Merged from upstream with latest version.
Canx Mar 31, 2017
f2c18bd
Added tests to check adding filters to courses and moved invalidate_f…
Canx Apr 3, 2017
2e55685
thinking of using decorator pattern to cache
Canx Apr 3, 2017
9e12b04
Only add default filters if they are not still in the course.
Canx Apr 3, 2017
08aad6d
Replaced save_default_filters by a raw DB function.
Canx Apr 9, 2017
97118ff
Added generator tool to help write raw DB commands.
Canx Apr 9, 2017
ac6f77f
append_default_rules_to_courses function implemented with low level d…
Canx Apr 9, 2017
d6d199e
Cleaned upgradelib.
Canx Apr 10, 2017
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
Prev Previous commit
Next Next commit
Refactoring filter_manager and adapting old tests. manager_tests fail…
…ing...
  • Loading branch information
Canx committed Mar 16, 2017
commit da4e56908d50fcab139b94d17552dea0415e682b
3 changes: 1 addition & 2 deletions block_xp.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ public function instance_create() {

// Copy default filters to course
// TODO: only do this the first time the block is added.
$filtermanager = new block_xp_filter_manager($manager);
$filtermanager->copy_default_filters_to_course();
$manager->get_filter_manager()->copy_default_filters();

return true;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would add the default rules again, and again, if the block was remove, then re-added. Notice in instance_delete I do not remove anything form the database, I simple update the block to be disabled. That is another improvement we could make at a later stage.

}
Expand Down
44 changes: 29 additions & 15 deletions classes/filter.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class block_xp_filter implements renderable {
*
* @var int
*/
protected $id;
public $id;

/**
* Points for this filter.
Expand Down Expand Up @@ -185,6 +185,31 @@ public static function load_from_data($record) {
return $filter;
}

// TODO: testing if we can replace static load_from_data
public function load_from_data2($object) {
//$record = (array) $record;
$object = (is_array($object)) ? (object)$object : $object;
//print_object($record);
foreach ($object as $key => $value) {
if ($key == 'rule' and !empty($value)) {
$this->set_rule($value);
continue;
}

if ($key == 'points') {
// Prevent negatives.
$value = abs(intval($value));
} else if ($key == 'sortorder') {
$value = intval($value);
}

$this->$key = $value;
}
if (is_null($this->rule) and is_null($this->ruledata)) {
throw new coding_exception("filter must have rule or ruledata property");
}
}

/**
* Load the rule from {@link self::$ruledata}.
*
Expand Down Expand Up @@ -229,20 +254,9 @@ public function save() {
$this->insert_or_update('block_xp_filters', $record);
}

/**
* Save the record as a default filter
*
* @return void
*/

public function save_default() {
$record = (object) array(
'ruledata' => $this->ruledata,
'points' => $this->points,
'sortorder' => $this->sortorder,
);

$this->insert_or_update('block_xp_default_filters', $record);
public function cmp($a, $b)
{
return strcmp($a->sortorder, $b->sortorder);
}

protected function insert_or_update($table, $record) {
Expand Down
29 changes: 22 additions & 7 deletions classes/filter_course.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,33 @@ public function __construct($courseid) {
}

/**
* Loads object properties from array, stdClass or block_xp_filter
* Loads object properties from array, stdClass or block_xp_filter.
*
*
* @param block_xp_filter|array $filter
*/
public function load($object) {
$object = (is_array($object)) ? (object)$object : $object;
//$object = (is_array($object)) ? (object)$object : $object;
$tempcourseid = $this->courseid;
$this->load_from_data2($object);
$this->courseid = $tempcourseid;
// foreach ($this as $key => $value) {
// if ($key != "courseid") {
// if (isset($object->$key)) {
// $this->$key = $object->$key;
// }
// }
// }
}

foreach ($this as $key => $value) {
if (isset($object->$key)) {
$this->$key = $object->$key;
}
}
/**
* As load but always create a new object in DB.
*
* @param block_xp_filter|array $filter
*/
public function load_as_new($object) {
$this->load($object);
$this->id = null;
}

public function save() {
Expand Down
60 changes: 31 additions & 29 deletions classes/filter_manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,17 @@
*/
class block_xp_filter_manager {

/**
* The block XP manager.
*
* @var block_xp_manager.
*/
protected $manager;
protected $courseid;

protected $filterset;
/**
* Constructor.
*
* @param block_xp_manager $manager The XP manager.
*/
public function __construct(block_xp_manager $manager) {
$this->manager = $manager;
public function __construct($courseid) {
$this->courseid = $courseid;
$this->filterset = new block_xp_filterset_course($courseid);
}

/**
Expand All @@ -58,7 +55,7 @@ public function __construct(block_xp_manager $manager) {
*/
public function get_all_filters() {
$cache = cache::make('block_xp', 'filters');
$key = 'filters_' . $this->manager->get_courseid();
$key = 'filters_' . $this->get_courseid();
if (false === ($filters = $cache->get($key))) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this method is public, I would prefer you do not remove it (also the PHP doc should have been removed). We could keep maintain the behaviour of this method as it was: "Returning all the filters to apply to the current course", and also keep get_static_filters even though the later wouldn't contain anything by default any more.

$filters = $this->get_course_filters();
$cache->set($key, $filters);
Expand All @@ -72,7 +69,7 @@ public function get_all_filters() {
* @return course id
*/
public function get_courseid() {
return $this->manager->get_courseid();
return $this->courseid;
}


Expand All @@ -97,10 +94,7 @@ public function get_points_for_event(\core\event\base $event) {
* @return array Of filter objects.
*/
public static function get_static_filters() {
$staticfilters = new block_xp_filterset_static();
$staticfilters->load();

return $staticfilters->get();
return (new block_xp_filterset_static())->get();
}

/**
Expand All @@ -109,15 +103,7 @@ public static function get_static_filters() {
* @return array of filter data from the DB, though properties is already json_decoded.
*/
public function get_course_filters() {
global $DB;
$results = $DB->get_recordset('block_xp_filters', array('courseid' => $this->get_courseid()),
'sortorder ASC, id ASC');
$filters = array();
foreach ($results as $key => $filter) {
$filters[$filter->id] = block_xp_filter::load_from_data($filter);
}
$results->close();
return $filters;
return $this->filterset->get();
}

/**
Expand All @@ -137,21 +123,33 @@ public function get_user_filters() {
public static function save_default_filters() {
$staticfilters = new block_xp_filterset_static();
$defaultfilters = new block_xp_filterset_default();

$defaultfilters->import($staticfilters);
}

/**
* Used when adding block to course
*
* @return void */
public function copy_default_filters_to_course() {
public function copy_default_filters() {
$defaultfilters = new block_xp_filterset_default();
$coursefilters = new block_xp_filterset_course($this->get_courseid());
print_object("--REGLAS DEFECTO DENTRO ANTES APPEND --");
print_object($defaultfilters->get());
$this->filterset->append($defaultfilters);
print_object("--REGLAS DEFECTO DENTRO DESPUES APPEND --");
print_object($defaultfilters->get());

if ($coursefilters->empty()) {
$coursefilters->import($defaultfilters);
}

}

public static function copy_default_filters_to_course($courseid) {
$defaultfilters = new block_xp_filterset_default();
$coursefilters = new block_xp_filterset_course($courseid);
print_object("--REGLAS DEFECTO DENTRO ANTES APPEND --");
print_object($defaultfilters->get());

$coursefilters->append($defaultfilters);
print_object("--REGLAS DEFECTO DENTRO DESPUES APPEND --");
print_object($defaultfilters->get());
}

/**
Expand All @@ -164,4 +162,8 @@ public function invalidate_filters_cache() {
$cache->delete('filters_' . $this->get_courseid());
}

public function get_filterset() {
return $this->filterset;
}

}
25 changes: 12 additions & 13 deletions classes/filterset.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,16 @@ public function save() {
}
}

public function merge(block_xp_filterset $filters) {
public function append(block_xp_filterset $filters) {
foreach ($filters->get() as $filter) {
$clonedfilter = $this->create_filter();
$clonedfilter->load($filter);
$this->add_last($clonedfilter);
$this->add_last($filter);
}

$this->reset_sortorder();
$this->save();
}

public function import(block_xp_filterset $filters) {
$this->delete_all();
$this->merge($filters);
$this->append($filters);
}

/*
Expand All @@ -96,20 +92,23 @@ public function delete_all() {
}

public function add_last(block_xp_filter $filter) {
$this->filters[] = $filter;
$clonedfilter = $this->create_filter();
$clonedfilter->load($filter);
$this->filters[] = $clonedfilter;
$this->reset_sortorder();
}

public function add_first(block_xp_filter $filter) {
$this->add($filter, 0);
$clonedfilter = $this->create_filter();
$clonedfilter->load_as_new($filter);
$this->add($clonedfilter, 0);
}

/*
* Insert filter in specific position
*/
public function add($filter, $position) {
$clonedfilter = $this->create_filter();
$clonedfilter->load_as_new($filter);
$position = min($position, $this->count()+1);
array_splice( $this->filters, $position, 0, [$filter] );
array_splice( $this->filters, $position, 0, [$clonedfilter] );
$this->reset_sortorder();
}

Expand Down
17 changes: 11 additions & 6 deletions classes/filterset_course.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,24 @@ public function __construct($id) {
parent::__construct();
}

// TODO: refactor
public function load() {
global $DB;

$records = $DB->get_recordset('block_xp_filters',
array('courseid' => $this->courseid));
$recordset = $DB->get_recordset('block_xp_filters',
array('courseid' => $this->courseid),
'sortorder ASC, id ASC');

$this->clean();

foreach ($records as $key => $filterdata) {
$filter = $this->create_filter();
$filter->load($filterdata);
$this->filters[] = $filter;
if ($recordset->valid()) {
foreach ($recordset as $key => $filterdata) {
$filter = $this->create_filter();
$filter->load($filterdata);
$this->filters[] = $filter;
}
}
$recordset->close();
}

public function create_filter() {
Expand Down
24 changes: 17 additions & 7 deletions classes/filterset_default.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,26 @@ class block_xp_filterset_default extends block_xp_filterset {
public function load() {
global $DB;

$records = $DB->get_recordset('block_xp_filters',
array('courseid' => 0));
$recordset = $DB->get_recordset('block_xp_filters',
array('courseid' => 0),
'sortorder ASC, id ASC');

unset($this->filters);
print_object("--RECORDS DEFAULT--");
print_object($recordset);

foreach ($records as $key => $filterdata) {
$filter = $this->create_filter();
$filter->load($filterdata);
$this->filters[] = $filter;

if ($recordset->valid()) {
$this->clean();
foreach ($recordset as $key => $filterdata) {
$filter = $this->create_filter();
$filter->load($filterdata);
$this->filters[] = $filter;
}
}
else {
throw new coding_exception("Default filters can't be retrieved");
}
$recordset->close();
}

public function create_filter() {
Expand Down
2 changes: 1 addition & 1 deletion classes/manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ public static function get_default_config() {
*/
public function get_filter_manager() {
if (!$this->filtermanager) {
$this->filtermanager = new block_xp_filter_manager($this);
$this->filtermanager = new block_xp_filter_manager($this->get_courseid());
}
return $this->filtermanager;
}
Expand Down
Loading