From: mjollnir_ Date: Sat, 2 May 2009 13:23:22 +0000 (+0000) Subject: MDL-18734 - documentation and coding style in portfolio/* X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=4c7a4ef9df3d426cfdc00502786428eda970c3c5;p=moodle.git MDL-18734 - documentation and coding style in portfolio/* Added much needed documentation to the portfolio "scripts" (the libs are all fine), and moved all parameter fetching from the body of the file to the top. --- diff --git a/lib/portfolio/plugin.php b/lib/portfolio/plugin.php index dd81994c99..71f59c18d6 100644 --- a/lib/portfolio/plugin.php +++ b/lib/portfolio/plugin.php @@ -766,6 +766,7 @@ abstract class portfolio_plugin_pull_base extends portfolio_plugin_base { if (!($file instanceof stored_file)) { throw new portfolio_export_exception($this->get('exporter'), 'filenotfound', 'portfolio'); } + // the last 'true' on the end of this means don't die(); afterwards, so we can clean up. send_stored_file($file, 0, 0, true, null, true); $this->get('exporter')->log_transfer(); } diff --git a/portfolio/add.php b/portfolio/add.php index 3e48979236..52b0af2501 100644 --- a/portfolio/add.php +++ b/portfolio/add.php @@ -1,27 +1,64 @@ . + * + * @package moodle + * @subpackage portfolio + * @author Penny Leach + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL + * @copyright (C) 1999 onwards Martin Dougiamas http://dougiamas.com + * + * This file is the main controller to do with the portfolio export wizard. + */ require_once(dirname(dirname(__FILE__)) . '/config.php'); if (empty($CFG->enableportfolios)) { print_error('disabled', 'portfolio'); } +// this will pull in all the other required libraries require_once($CFG->libdir . '/portfoliolib.php'); +// so plugins don't have to. require_once($CFG->libdir . '/formslib.php'); -$cancel = optional_param('cancel', 0, PARAM_RAW); +$cancel = optional_param('cancel', 0, PARAM_RAW); // user has cancelled the request +$dataid = optional_param('id', 0, PARAM_INT); // id of partially completed export (in session, everything else in portfolio_tempdata +$instanceid = optional_param('instance', 0, PARAM_INT); // instanceof of configured portfolio plugin +$courseid = optional_param('course', 0, PARAM_INT); // courseid the data being exported belongs to (caller object should provide this later) +$stage = optional_param('stage', PORTFOLIO_STAGE_CONFIG, PARAM_INT); // stage of the export we're at (stored in the exporter) +$postcontrol = optional_param('postcontrol', 0, PARAM_INT); // when returning from some bounce to an external system, this gets passed +$callbackfile = optional_param('callbackfile', null, PARAM_PATH); // callback file eg /mod/forum/lib.php - the location of the exporting content +$callbackclass = optional_param('callbackclass', null, PARAM_ALPHAEXT); // callback class eg forum_portfolio_caller - the class to handle the exporting content. -require_login(); +require_login(); // this is selectively called again with $course later when we know for sure which one we're in. $exporter = null; -$dataid = 0; -if (!$dataid = optional_param('id', '', PARAM_INT) ) { +// try and find a partial export id in the session if it's not passed explicitly +if (empty($dataid)) { if (isset($SESSION->portfolioexport)) { $dataid = $SESSION->portfolioexport; } } -if ($dataid) { +// if we have a dataid, it means we're in the middle of an export, +// so rewaken it and continue. +if (!empty($dataid)) { try { $exporter = portfolio_exporter::rewaken_object($dataid); } catch (portfolio_exception $e) { @@ -34,36 +71,44 @@ if ($dataid) { portfolio_exporter::print_expired_export(); } } + // we have to wake it up first before we can cancel it + // so temporary directories etc get cleaned up. if ($cancel) { $exporter->cancel_request(); } // verify we still belong to the correct user and session $exporter->verify_rewaken(); + // if we don't have an instanceid in the exporter + // it means we've just posted from the 'choose portfolio instance' page + // so process that and start up the portfolio plugin if (!$exporter->get('instance')) { - if ($instance = optional_param('instance', '', PARAM_INT)) { + if ($instanceid) { try { - $instance = portfolio_instance($instance); + $instance = portfolio_instance($instanceid); } catch (portfolio_exception $e) { portfolio_export_rethrow_exception($exporter, $e); } + // this technically shouldn't happen but make sure anyway if ($broken = portfolio_instance_sanity_check($instance)) { throw new portfolio_export_exception($exporter, $broken[$instance->get('id')], 'portfolio_' . $instance->get('plugin')); } + // now we're all set up, ready to go $instance->set('user', $USER); $exporter->set('instance', $instance); $exporter->save(); } } +// completely new request, look to see what information we've been passed and set up the exporter object. } else { - + // you cannot get here with no information for us, we must at least have the caller. if (empty($_GET) && empty($_POST)) { portfolio_exporter::print_expired_export(); } // we'e just posted here for the first time and have might the instance already - if ($instance = optional_param('instance', 0, PARAM_INT)) { + if ($instanceid) { // this can throw exceptions but there's no point catching and rethrowing here // as the exporter isn't created yet. - $instance = portfolio_instance($instance); + $instance = portfolio_instance($instanceid); if ($broken = portfolio_instance_sanity_check($instance)) { throw new portfolio_exception($broken[$instance->get('id')], 'portfolio_' . $instance->get('plugin')); } @@ -72,13 +117,16 @@ if ($dataid) { $instance = null; } - $callbackfile = optional_param('callbackfile', null, PARAM_PATH); - $callbackclass = optional_param('callbackclass', null, PARAM_ALPHAEXT); - + // we must be passed this from the caller, we cannot start a new export + // without knowing information about what part of moodle we come from. if (empty($callbackfile) || empty($callbackclass)) { portfolio_exporter::print_expired_export(); } + // so each place in moodle can pass callback args here + // process the entire request looking for ca_* + // be as lenient as possible while still being secure + // so only accept certain parameter types. $callbackargs = array(); foreach (array_keys(array_merge($_GET, $_POST)) as $key) { if (strpos($key, 'ca_') === 0) { @@ -87,46 +135,54 @@ if ($dataid) { $value = optional_param($key, false, PARAM_PATH); } } + // strip off ca_ for niceness $callbackargs[substr($key, 3)] = $value; } } + // righto, now we have the callback args set up + // load up the caller file and class and tell it to set up all the data + // it needs require_once($CFG->dirroot . $callbackfile); $caller = new $callbackclass($callbackargs); $caller->set('user', $USER); $caller->load_data(); + // this must check capabilities and either throw an exception or return false. if (!$caller->check_permissions()) { throw new portfolio_caller_exception('nopermissions', 'portfolio', $caller->get_return_url()); } // for build navigation if (!$course = $caller->get('course')) { - $course = optional_param('course', 0, PARAM_INT); + $course = $courseid; } - if (!empty($course) && is_numeric($course)) { - $course = $DB->get_record('course', array('id' => $course), 'id,shortname,fullname'); - } + // set up the course so that build_navigation works nice + course_setup($course); - // this is yuk but used in build_navigation - $COURSE = $course; + // and now we know the course for sure, call require_login with it + require_login($course); list($extranav, $cm) = $caller->get_navigation(); $extranav[] = array('type' => 'title', 'name' => get_string('exporting', 'portfolio')); $navigation = build_navigation($extranav, $cm); + // finally! set up the exporter object with the portfolio instance, caller information, and navigation elements $exporter = new portfolio_exporter($instance, $caller, $callbackfile, $navigation); + + // set the export-specific variables, and save. $exporter->set('user', $USER); $exporter->set('sesskey', sesskey()); $exporter->save(); + + // and finally, put it in the session for waking up again later. $SESSION->portfolioexport = $exporter->get('id'); } if (!$exporter->get('instance')) { // we've just arrived but have no instance - // so retrieve everything from the request, - // add them as hidden fields in a new form - // to select the instance and post back here again - // for the next block to catch + // in this case the exporter object and the caller object have been set up above + // so just make a little form to select the portfolio plugin instance, + // which is the last thing to do before starting the export. $mform = new portfolio_instance_select('', array('caller' => $exporter->get('caller'))); if ($mform->is_cancelled()) { $exporter->cancel_request(); @@ -144,20 +200,24 @@ if (!$exporter->get('instance')) { } } -if (!$stage = optional_param('stage', PORTFOLIO_STAGE_CONFIG)) { +// if we haven't been passed &stage= grab it from the exporter. +if (!$stage) { $stage = $exporter->get('stage'); } -$alreadystolen = false; // for places returning control to pass (rather than PORTFOLIO_STAGE_PACKAGE // which is unstable if they can't get to the constant (eg external system) -if ($postcontrol = optional_param('postcontrol', 0, PARAM_INT)) { +$alreadystolen = false; +if ($postcontrol) { // the magic request variable plugins must pass on returning here try { + // allow it to read whatever gets sent back in the request + // this is useful for plugins that redirect away and back again + // adding a token to the end of the url, for example box.net $exporter->instance()->post_control($stage, array_merge($_GET, $_POST)); } catch (portfolio_plugin_exception $e) { portfolio_export_rethrow_exception($exporter, $e); } - $alreadystolen = true; + $alreadystolen = true; // remember this so we don't get caught in a steal control loop! } // actually do the work now.. diff --git a/portfolio/already.php b/portfolio/already.php index c0ce4828dc..b4f3878c80 100644 --- a/portfolio/already.php +++ b/portfolio/already.php @@ -1,4 +1,30 @@ . + * + * @package moodle + * @subpackage portfolio + * @author Penny Leach + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL + * @copyright (C) 1999 onwards Martin Dougiamas http://dougiamas.com + * + * This file is the handler that gets invoked when there's already an export happening. + */ require_once(dirname(dirname(__FILE__)) . '/config.php'); if (empty($CFG->enableportfolios)) { @@ -10,22 +36,26 @@ require_once($CFG->libdir . '/portfoliolib.php'); require_login(); $dataid = 0; -$currentinfo = null; + +// look for the export id in the request, if it's not there, try the session if (!$dataid = optional_param('id', '', PARAM_INT) ) { if (isset($SESSION->portfolioexport)) { $dataid = $SESSION->portfolioexport; } } +// all we're going to do is print a table with some information +// about the current export, with a yes/ no option to resume or cancel. $table = new StdClass; $table->head = array( - get_string('displayarea', 'portfolio'), - get_string('destination', 'portfolio'), - get_string('displayinfo', 'portfolio'), + get_string('displayarea', 'portfolio'), // the part of moodle exporting content + get_string('destination', 'portfolio'), // the portfolio plugin instance + get_string('displayinfo', 'portfolio'), // any extra data about what it is we're exporting from the caller ); $table->data = array(); if ($dataid) { try { + // try to reawaken it and get any information about it we can $exporter = portfolio_exporter::rewaken_object($dataid); $exporter->verify_rewaken(); $table->data[] = array( @@ -33,7 +63,7 @@ if ($dataid) { ($exporter->get('instance') ? $exporter->get('instance')->get('name') : get_string('notyetselected', 'portfolio')), $exporter->get('caller')->heading_summary(), ); - } catch (portfolio_exception $e) { } + } catch (portfolio_exception $e) { } // maybe in this case we should just kill it and redirect to the new request anyway ? } $strheading = get_string('activeexport', 'portfolio'); diff --git a/portfolio/file.php b/portfolio/file.php index e56cceb564..6032e6de29 100644 --- a/portfolio/file.php +++ b/portfolio/file.php @@ -1,4 +1,32 @@ . + * + * @package moodle + * @subpackage portfolio + * @author Penny Leach + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL + * @copyright (C) 1999 onwards Martin Dougiamas http://dougiamas.com + * + * For portfolio plugins that are 'pull' - ie, send the request and then wait + * for the remote system to request the file for moodle, + * this is the script that serves up the export file to them. + */ require_once(dirname(dirname(__FILE__)) . '/config.php'); if (empty($CFG->enableportfolios)) { @@ -9,6 +37,7 @@ require_once($CFG->libdir . '/portfoliolib.php'); require_once($CFG->libdir . '/file/stored_file.php'); require_once($CFG->libdir . '/filelib.php'); +// exporter id $id = required_param('id', PARAM_INT); require_login(); @@ -16,14 +45,17 @@ require_login(); $exporter = portfolio_exporter::rewaken_object($id); $exporter->verify_rewaken(); +// push plugins don't need to access this script. if ($exporter->get('instance')->is_push()) { throw new portfolio_export_exception($exporter, 'filedenied', 'portfolio'); } +// it's up to the plugin to verify the request parameters, like a token or whatever if (!$exporter->get('instance')->verify_file_request_params(array_merge($_GET, $_POST))) { throw new portfolio_export_exception($exporter, 'filedenied', 'portfolio'); } +// ok, we're good, send the file and finish the export. $exporter->get('instance')->send_file(); $exporter->process_stage_cleanup(true); exit; diff --git a/portfolio/type/download/file.php b/portfolio/type/download/file.php index abb7f2ab30..b1c8465aee 100644 --- a/portfolio/type/download/file.php +++ b/portfolio/type/download/file.php @@ -1,5 +1,9 @@ enableportfolios)) { @@ -19,6 +23,8 @@ $exporter->print_header(get_string('downloading', 'portfolio_download'), false); $returnurl = $exporter->get('caller')->get_return_url(); notify('' . get_string('returntowhereyouwere', 'portfolio') . '
'); +// if they don't have javascript, they can submit the form here to get the file. +// if they do, it does it nicely for them. echo '