Some mform refactoring

Uses customdata instead of modified constructor.

diff --git a/mod/workshop/editgradingform.php b/mod/workshop/editgradingform.php
index 11fc39478f82279cb433d679a71e401b5125c323..642775eb13aca596189dd80a0b2582a4ccf646a7 100644 (file)
--- a/mod/workshop/editgradingform.php
+++ b/mod/workshop/editgradingform.php
@@ -81,13 +81,11 @@ $strategy = new $classname($workshop);
 $dimensions = $strategy->load_dimensions();
 
 // load the form to edit the grading strategy dimensions
-$mform = $strategy->get_edit_strategy_form($selfurl, true, count($dimensions));
+$mform = $strategy->get_edit_strategy_form($selfurl, count($dimensions));
 
 // initialize form data
 $formdata = new stdClass;
-$formdata->workshopid   = $workshop->id;
-$formdata->strategy     = $workshop->strategy;
-$formdata->dimensions   = $dimensions;
+$formdata->dimensions = $dimensions;
 $mform->set_data($formdata);
 
 if ($mform->is_cancelled()) {

diff --git a/mod/workshop/grading/accumulative/gradingform.php b/mod/workshop/grading/accumulative/gradingform.php
index d467730632b91c2124edb6ed949ec5ac7e2c5929..269e9dabbcaebca53464906b1b1ffa001b55fa39 100644 (file)
--- a/mod/workshop/grading/accumulative/gradingform.php
+++ b/mod/workshop/grading/accumulative/gradingform.php
@@ -68,7 +68,7 @@ class workshop_edit_accumulative_strategy_form extends workshop_edit_strategy_fo
 
         $numofdimensionstoadd   = 2;
         $numofinitialdimensions = 3;
-        $numofdisplaydimensions = max($this->numofdimensions + $numofdimensionstoadd, $numofinitialdimensions);
+        $numofdisplaydimensions = max($this->nocurrentdims + $numofdimensionstoadd, $numofinitialdimensions);
         $this->repeat_elements($repeated, $numofdisplaydimensions,  $repeatedoptions, 'numofdimensions', 'adddimensions', $numofdimensionstoadd, get_string('addmoredimensionsaccumulative', 'workshop', $numofdimensionstoadd));
     }
 

diff --git a/mod/workshop/grading/accumulative/strategy.php b/mod/workshop/grading/accumulative/strategy.php
index 0ecfd81d0d69dd5e2f1a61ad67153329e25f97e2..d08660986cfc6dc5d783398459e6628ca7048307 100644 (file)
--- a/mod/workshop/grading/accumulative/strategy.php
+++ b/mod/workshop/grading/accumulative/strategy.php
@@ -24,9 +24,7 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-if (!defined('MOODLE_INTERNAL')) {
-    die('Direct access to this script is forbidden.'); //  It must be included from a Moodle page
-}
+defined('MOODLE_INTERNAL') || die();
 
 require_once(dirname(dirname(__FILE__)) . '/strategy.php'); // parent class
 

diff --git a/mod/workshop/grading/gradingform.php b/mod/workshop/grading/gradingform.php
index 9553dafff92767d99186869a423dadeba1ced434..3e28b892f4fe0475b2ebfb2b86dee6a164f0691b 100644 (file)
--- a/mod/workshop/grading/gradingform.php
+++ b/mod/workshop/grading/gradingform.php
@@ -43,26 +43,8 @@ class workshop_edit_strategy_form extends moodleform {
     /** strategy logic instance that this class is editor of */ 
     protected $strategy;
 
-    /** number of dimensions that are populated from DB */
-    protected $numofdimensions;
-
-    /**
-     * Constructor
-     * 
-     * @param object $strategy The instance of the strategy logic
-     * @param str $actionurl URL to handle data
-     * @param bool $editable Should the form be editable?
-     * @param int $initialdimensions Number of assessment dimensions that are already filled
-     * @access public
-     * @return void
-     */
-    public function __construct(workshop_strategy $strategy, $actionurl, $editable=true, $initialdimensions=0) {
-
-        $this->strategy         = $strategy;
-        $this->numofdimensions  = $initialdimensions;
-        parent::moodleform($actionurl, null, 'post', '', array('class' => 'editstrategyform'), $editable);
-    }
-
+    /** number of current dimensions loaded from DB */
+    protected $nocurrentdims;
 
     /**
      * Add the fields that are common for all grading strategies.
@@ -79,12 +61,10 @@ class workshop_edit_strategy_form extends moodleform {
         global $CFG;
 
         $mform = $this->_form;
+        $this->strategy         = $this->_customdata['strategy'];
+        $this->nocurrentdims    = $this->_customdata['nocurrentdims']; 
 
-        $mform->addElement('hidden', 'workshopid');
-        $mform->setType('id', PARAM_INT);
-
-        $mform->addElement('hidden', 'strategy');
-        $mform->setType('id', PARAM_ALPHA);
+        $mform->addElement('hidden', 'strategyname', $this->strategy->name);
 
         $this->definition_inner($mform);
 

diff --git a/mod/workshop/grading/strategy.php b/mod/workshop/grading/strategy.php
index da42ecee66126a8bb4365e7f758589346a1ca3e0..35d45435f842838a48236698e94d20f2aef4bc08 100644 (file)
--- a/mod/workshop/grading/strategy.php
+++ b/mod/workshop/grading/strategy.php
@@ -34,15 +34,16 @@ interface workshop_strategy_interface {
     /**
      * Factory method returning an instance of an assessment form editor class
      * 
-     * The returned class will probably expand the base workshop_edit_strategy_form
+     * The returned class will probably expand the base workshop_edit_strategy_form. The number of
+     * dimensions that will be passed by set_data() must be already known here becase the
+     * definition() of the form has to know the number and it is called before set_data().
      *
      * @param string $actionurl URL of the action handler script
-     * @param boolean $edit Open the form in editing mode
-     * @param int $nodimensions Number of dimensions to be provided by set_data
+     * @param int $nocurrentdims Number of current dimensions to be set later by set_data()
      * @access public
      * @return object The instance of the assessment form editor class
      */
-    public function get_edit_strategy_form($actionurl, $edit=true, $nodimensions=0);
+    public function get_edit_strategy_form($actionurl, $nocurrentdims=0);
 
 
     /**
@@ -83,7 +84,7 @@ class workshop_strategy implements workshop_strategy_interface {
     public $name;
 
     /** the parent workshop instance */
-    protected $_workshop;
+    protected $workshop;
 
     /**
      * Constructor 
@@ -94,8 +95,8 @@ class workshop_strategy implements workshop_strategy_interface {
      */
     public function __construct($workshop) {
 
-        $this->name         = $workshop->strategy;
-        $this->_workshop    = $workshop;
+        $this->name     = $workshop->strategy;
+        $this->workshop = $workshop;
     }
 
 
@@ -105,7 +106,7 @@ class workshop_strategy implements workshop_strategy_interface {
      * By default, the class is defined in grading/{strategy}/gradingform.php and is named
      * workshop_edit_{strategy}_strategy_form
      */
-    public function get_edit_strategy_form($actionurl, $edit=true, $nodimensions=0) {
+    public function get_edit_strategy_form($actionurl, $nocurrentdims=0) {
         global $CFG;    // needed because the included files use it
     
         $strategyform = dirname(__FILE__) . '/' . $this->name . '/gradingform.php';
@@ -116,7 +117,15 @@ class workshop_strategy implements workshop_strategy_interface {
         }
         $classname = 'workshop_edit_' . $this->name . '_strategy_form';
 
-        return new $classname($this, $actionurl, $edit, $nodimensions);
+        $customdata = new stdClass;
+        $customdata = array(
+                        'strategy'      => $this,
+                        'nocurrentdims' => $nocurrentdims,
+
+                        );
+        $attributes = array('class' => 'editstrategyform');
+
+        return new $classname($actionurl, $customdata, 'post', '', $attributes);
 
     }
 
@@ -133,7 +142,7 @@ class workshop_strategy implements workshop_strategy_interface {
     public function load_dimensions() {
         global $DB;
 
-        return $DB->get_records('workshop_forms_' . $this->name, array('workshopid' => $this->_workshop->id), 'sort');
+        return $DB->get_records('workshop_forms_' . $this->name, array('workshopid' => $this->workshop->id), 'sort');
     }
 
 
@@ -153,6 +162,11 @@ class workshop_strategy implements workshop_strategy_interface {
     public function save_dimensions($data) {
         global $DB;
         
+        if (!isset($data->strategyname) || ($data->strategyname != $this->name)) {
+            // the workshop strategy has changed since the form was opened for editing
+            throw new moodle_exception('strategyhaschanged', 'workshop');
+        }
+
         $data = $this->cook_form_data($data);
 
         foreach ($data as $record) {
@@ -173,7 +187,7 @@ class workshop_strategy implements workshop_strategy_interface {
     /**
      * The default implementation transposes the returned structure
      *
-     * It automatically adds 'sort' column and 'workshopid' column into every record.
+     * It automatically adds some columns into every record.
      * The sorting is done by the order of the returned array and starts with 1.
      * 
      * @param object $raw 
@@ -185,8 +199,9 @@ class workshop_strategy implements workshop_strategy_interface {
         foreach (array_flip($this->map_dimension_fieldnames()) as $formfield => $dbfield) {
             for ($k = 0; $k < $raw->numofdimensions; $k++) {
                 $cook[$k]->{$dbfield}   = isset($raw->{$formfield}[$k]) ? $raw->{$formfield}[$k] : null;
-                $cook[$k]->sort         = $k + 1;
-                $cook[$k]->workshopid   = $raw->workshopid;
+                $cook[$k]->descriptionformat    = FORMAT_HTML;
+                $cook[$k]->sort                 = $k + 1;
+                $cook[$k]->workshopid           = $this->workshop->id;
             }
         }
         return $cook;

diff --git a/mod/workshop/simpletest/testlib.php b/mod/workshop/simpletest/testlib.php
index 93ef9890794def6cd7dee75b93337526b73b8ad1..4b8b431ae73f3c2f77f75cb762686acca3cbd1a5 100644 (file)
--- a/mod/workshop/simpletest/testlib.php
+++ b/mod/workshop/simpletest/testlib.php
@@ -24,9 +24,7 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-if (!defined('MOODLE_INTERNAL')) {
-    die('Direct access to this script is forbidden.'); //  It must be included from a Moodle page
-}
+defined('MOODLE_INTERNAL')) || die();
  
 // Make sure the code being tested is accessible.
 require_once($CFG->dirroot . '/mod/workshop/lib.php'); // Include the code to test