Skip to content

Commit

Permalink
MDL-83859 core_question: Update filter condition API
Browse files Browse the repository at this point in the history
This corrects some definitions of the methods in the base condition
class to make things more obvious to developers implementing new
filters.

Previously if your filter wanted to use the default
`core/datafilter/filtertype` class, you still had to implement
`get_filter_class` to return `null`, since it was declared as abstract.
This change defines it as returning `null` by default, so this is no
longer necessary.

Also, this removes the default definitions for `get_condition_key` and
`build_query_from_functions`, and declares them abstract. Currently it
is necessary to override these to implement a functional filter so it
doesn't make sense to have a useless default definition.

This will not cause any breakages with existing filters. All filters
must already be defining the methods that are now abstract, otherwise
they will not function. Any filter that is now overriding
`get_filter_class` to return `null` will continue to work as before,
even though this is no longer necessary.
  • Loading branch information
marxjohnson committed Dec 5, 2024
1 parent a798b1d commit c65d069
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 11 deletions.
20 changes: 20 additions & 0 deletions .upgradenotes/MDL-83859-2024120511024767.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
issueNumber: MDL-83859
notes:
core_question:
- message: >
The definition of the abstract `core_question\local\bank\condition`
class has changed to make it clearer which methods are required
in child classes.
The `get_filter_class` method is no longer declared as abstract, and
will return `null` by default to use the base
`core/datafilter/filtertype` class. If you have defined this method
to return `null` in your own class, it will continue to work, but
it is no longer necessary.
`build_query_from_filter` and `get_condition_key` are now declared as
abstract, since all filter condition classes must define these
(as well as existing abstract methods) to function. Again, exsiting
child classes will continue to work if they did before, as they
already needed these methods.
type: changed
25 changes: 14 additions & 11 deletions question/classes/local/bank/condition.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
abstract class condition {

/** @var int The default filter type */
const JOINTYPE_DEFAULT = datafilter::JOINTYPE_ANY;

Expand All @@ -48,11 +47,16 @@ abstract class condition {
abstract public function get_title();

/**
* Return filter class associated with this condition
* Return the Javascript filter class to provide the UI for this condition.
*
* If left as null, this will use the default core/datafilter/filtertype class. Otherwise, override it to return
* the full path to the Javascript module path for the class.
*
* @return string filter class
*/
abstract public function get_filter_class();
public function get_filter_class() {
return null;
}

/**
* Extract the required filter from the provided question bank view.
Expand Down Expand Up @@ -140,9 +144,7 @@ public function get_condition_class() {
*
* @return string
*/
public static function get_condition_key() {
return '';
}
abstract public static function get_condition_key();

/**
* Return an SQL fragment to be ANDed into the WHERE clause to filter which questions are shown.
Expand Down Expand Up @@ -211,12 +213,13 @@ public static function get_filter_from_list(array $filters): ?array {
}

/**
* Build query from filter value
* Return the SQL WHERE condition and parameters to be ANDed with other filter conditions.
*
* The $filter parameter recieves an array with a 'values' key, containing an array of the filter values selected,
* and a 'jointype' key containing the selected join type.
*
* @param array $filter filter properties
* @return array where sql and params
* @return array ['SQL where condition', ['param1' => 'value1', 'param2' => 'value2', ...]]
*/
public static function build_query_from_filter(array $filter): array {
return ['', []];
}
abstract public static function build_query_from_filter(array $filter);
}

0 comments on commit c65d069

Please sign in to comment.