Commit b6ea8510 authored by Robert Lyon's avatar Robert Lyon Committed by Cecilia Vela Gurovic

Bug 1786427: Move page sign-off to its own blocktype

It shall be possible to sign off on portfolio pages without using the
peer assessment block.

behatnotneeded

Change-Id: I2abb903cafd91766e2d72cef4031af31958c8ded
Signed-off-by: Robert Lyon's avatarRobert Lyon <robertl@catalyst.net.nz>
parent 367a6c44
......@@ -21,5 +21,3 @@ $string['draft'] = 'Save draft';
$string['publish'] = 'Publish';
$string['savepublishhelp'] = '<p><strong>Save draft:</strong> Only you can view it. While your assessment is in draft status, you can make changes.</p>
<p><strong>Publish:</strong> The person for whom you are giving the peer assessment can see your assessment. Everybody else who has access to the portfolio can view it as well, unless the portfolio also contains the signoff block and is not signed off by the portfolio owner. You cannot revert a published assessment to draft status.</p>';
$string['signoff'] = 'Signed off';
$string['verify'] = 'Verified';
......@@ -142,26 +142,6 @@ class PluginBlocktypePeerassessment extends MaharaCoreBlocktype {
);
}
public static function get_instance_toolbars(BlockInstance $bi) {
global $USER;
$view = $bi->get_view();
safe_require('artefact', 'peerassessment');
$smarty = smarty_core();
$smarty->assign('WWWROOT', get_config('wwwroot'));
$smarty->assign('view', $view->get('id'));
$smarty->assign('verifiable', ArtefactTypePeerassessment::is_verifiable($view));
$smarty->assign('signable', ArtefactTypePeerassessment::is_signable($view));
$smarty->assign('verified', ArtefactTypePeerassessment::is_verified($view));
$smarty->assign('signoff', ArtefactTypePeerassessment::is_signed_off($view));
return array(
array(
'toolbarhtml' => $smarty->fetch('blocktype:peerassessment:verifyform.tpl')
)
);
}
public static function delete_instance(BlockInstance $instance) {
$id = $instance->get('id');
require_once('embeddedimage.php');
......
<?php
/**
*
* @package mahara
* @subpackage blocktype-signoff
* @author Catalyst IT Ltd
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL version 3 or later
* @copyright For copyright information on Mahara, please see the README file distributed with this software.
*
*/
defined('INTERNAL') || die();
$string['title'] = 'Sign-off';
$string['description'] = 'A block to display page sign-off and verification options';
$string['placeholder'] = 'This block\'s content is displayed below the page heading rather than in a block itself on the page.';
$string['signoff'] = 'Sign-off';
$string['signoffdesc'] = 'The portfolio owner can sign off a page when all requirements have been met to indicate that it is ready for assessment.';
$string['verify'] = 'Verify';
$string['verifydesc'] = 'Decide whether a moderator needs to verify this page as part of the portfolio assessment process.';
$string['signedoff'] = 'Signed off';
$string['verified'] = 'Verified';
$string['signoffpagetitle'] = 'Sign-off page';
$string['signoffpagedesc'] = 'Select "Yes" to sign off this page and indicate that you have met all requirements. Select "No" to abort.';
$string['signoffpageundodesc'] = 'If you select "Yes", you will remove the signed off status. That will also remove the verification if that had been part of the assessment work flow. Select "No" to abort.';
$string['signoffpageconfirm'] = 'Confirm this action?';
$string['verifypagetitle'] = 'Verify page';
$string['verifypagedesc'] = 'Select "Yes" to verify that the portfolio owner has met all requirements for this page. Select "No" to return to the page without verifying it.';
$string['updatesignoff'] = 'Update page sign-off';
$string['updateverify'] = 'Update page verification';
$string['removedverifynotificationsubject'] = 'Verification for %s removed';
$string['removedverifynotification'] = 'The owner of the page %s has removed their sign-off. Therefore, your verification has also been removed. Please go to the page to see if it is ready to be marked as verified again.';
$string['signoffviewupdated'] = 'Sign-off status updated';
$string['verifyviewupdated'] = 'Verification status updated';
$string['wrongsignoffviewrequest'] = 'You do not have permission to perform the requested action';
<?php
/**
*
* @package mahara
* @subpackage blocktype-peerassessment
* @author Catalyst IT Ltd
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL version 3 or later
* @copyright For copyright information on Mahara, please see the README file distributed with this software.
*
*/
defined ('INTERNAL') || die();
class PluginBlocktypeSignoff extends MaharaCoreBlocktype {
public static function should_ajaxify() {
// TinyMCE doesn't play well with loading by ajax
return false;
}
public static function postinst($oldversion) {
set_config_plugin('blocktype', 'signoff', 'notretractable', true);
}
public static function single_only() {
return true;
}
public static function get_title() {
return get_string('title', 'blocktype.peerassessment/signoff');
}
public static function hide_title_on_empty_content() {
return true;
}
public static function get_description() {
return get_string('description', 'blocktype.peerassessment/signoff');
}
public static function get_categories() {
return array("general" => 14650);
}
public static function get_viewtypes() {
return array('portfolio');
}
public static function display_for_roles($roles) {
return true;
}
public static function render_instance(BlockInstance $instance, $editing=false) {
global $USER;
$configdata = $instance->get('configdata');
$smarty = smarty_core();
if ($editing) {
$smarty->assign('editing', $editing);
$smarty->assign('placeholder', get_string('placeholder', 'blocktype.peerassessment/signoff'));
$html = $smarty->fetch('blocktype:signoff:signoff.tpl');
}
else {
$html = '';
}
return $html;
}
public static function has_instance_config() {
return true;
}
public static function instance_config_form(BlockInstance $instance) {
$configdata = $instance->get('configdata');
$elements = array (
'signoff' => array (
'type' => 'switchbox',
'title' => get_string('signoff', 'blocktype.peerassessment/signoff'),
'description' => get_string('signoffdesc', 'blocktype.peerassessment/signoff'),
'value' => true,
'disabled' => true,
),
'verify' => array (
'type' => 'switchbox',
'title' => get_string('verify', 'blocktype.peerassessment/signoff'),
'description' => get_string('verifydesc', 'blocktype.peerassessment/signoff'),
'defaultvalue' => !empty($configdata['verify']) ? 1 : 0,
),
);
return $elements;
}
public static function instance_config_save($values, $instance) {
$viewid = $instance->get_view()->get('id');
ensure_record_exists('view_signoff_verify', (object) array('view' => $viewid), (object) array('view' => $viewid), 'id', true);
return $values;
}
public static function get_artefacts(BlockInstance $instance) {
return array();
}
public static function get_instance_toolbars(BlockInstance $bi) {
global $USER;
$configdata = $bi->get('configdata');
$view = $bi->get_view();
safe_require('artefact', 'peerassessment');
$smarty = smarty_core();
$smarty->assign('WWWROOT', get_config('wwwroot'));
$smarty->assign('view', $view->get('id'));
// Verify option
$smarty->assign('showverify', !empty($configdata['verify']));
$smarty->assign('verifiable', ArtefactTypePeerassessment::is_verifiable($view, false));
$smarty->assign('verified', ArtefactTypePeerassessment::is_verified($view, false));
// Signoff option
$smarty->assign('showsignoff', !empty($configdata['signoff']));
$smarty->assign('signable', ArtefactTypePeerassessment::is_signable($view, false));
$smarty->assign('signoff', ArtefactTypePeerassessment::is_signed_off($view, false));
return array(
array(
'toolbarhtml' => $smarty->fetch('blocktype:signoff:verifyform.tpl')
)
);
}
public static function delete_instance(BlockInstance $instance) {
$viewid = $instance->get_view()->get('id');
execute_sql("DELETE FROM {view_signoff_verify} WHERE view = ?", array($viewid));
}
/**
* We will use rewrite_blockinstance_extra_config to save the new view_signoff_verify row
*/
public static function rewrite_blockinstance_extra_config(View $view, BlockInstance $block, $configdata, $artefactcopies) {
$viewid = $view->get('id');
ensure_record_exists('view_signoff_verify', (object) array('view' => $viewid), (object) array('view' => $viewid), 'id', true);
return $configdata;
}
}
<?php
/**
*
* @package mahara
* @subpackage artefact-comment
* @author Catalyst IT Ltd
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL version 3 or later
* @copyright For copyright information on Mahara, please see the README file distributed with this software.
*
*/
defined('INTERNAL') || die();
$config = new StdClass;
$config->version = 2018073000;
$config->release = '1.0.0';
......@@ -15,7 +15,7 @@ define('JSON', 1);
require(dirname(dirname(dirname(__FILE__))) . '/init.php');
require_once(get_config('libroot') . 'view.php');
safe_require('artefact', 'peerassessment');
safe_require('blocktype', 'peerassessment');
safe_require('blocktype', 'signoff');
$blockid = param_integer('block', null);
$viewid = param_integer('view', null);
......@@ -24,42 +24,81 @@ $verify = param_integer('verify', null);
if (empty($viewid) && !empty($blockid)) {
// try to find view from blockid
$bi = new ArtefactTypePeerassessment($blockid);
$bi = new BlockInstance($blockid);
$viewid = $bi->get_view()->get('id');
}
// Does the view have a peerassessment block
if (!$blocks = get_records_sql_array("SELECT id FROM {block_instance} WHERE view = ? AND blocktype = ?", array($viewid, 'peerassessment'))) {
// Does the view have a signoff block
if (!$block = get_field('block_instance', 'id', 'view', $viewid, 'blocktype', 'signoff')) {
json_reply('local', get_string('wrongblocktype', 'view'));
}
else {
$bi = new BlockInstance($block);
}
$view = new View($viewid);
$configdata = $bi->get('configdata');
$showsignoff = !empty($configdata['signoff']) ? true : false;
$showverify = !empty($configdata['verify']) ? true : false;
$signable = ArtefactTypePeerassessment::is_signable($view);
$verifiable = ArtefactTypePeerassessment::is_verifiable($view);
$data = new stdClass();
if ($signable && $signoff !== null) {
if ($showsignoff && $signable && $signoff !== null) {
$currentstate = ArtefactTypePeerassessment::is_signed_off($view);
$newstate = 1 - (int)$currentstate;
execute_sql("UPDATE {view_signoff_verify} SET signoff = ? WHERE view = ?", array($newstate, $viewid));
execute_sql("UPDATE {view_signoff_verify} SET signoff = ?, signofftime = ? WHERE view = ?", array($newstate, db_format_timestamp(time()), $viewid));
if ($currentstate == 1) {
// we are removing sign-off so we should also remove verify as well
execute_sql("UPDATE {view_signoff_verify} SET verified = ? WHERE view = ?", array(0, $viewid));
// We are removing sign-off so we should also remove verify as well
// We send a notification to the manager that did the verification to alert them it has been removed
$sendto = get_field('view_signoff_verify', 'verifier', 'view', $viewid);
$url = $view->get_url(false);
if (!empty($sendto)) {
// Notify author
$title = $view->get('title');
$title = hsc($title);
$data = (object) array(
'subject' => false,
'message' => false,
'strings' => (object) array(
'subject' => (object) array(
'key' => 'removedverifynotificationsubject',
'section' => 'blocktype.peerassessment/signoff',
'args' => array($title),
),
'message' => (object) array(
'key' => 'removedverifynotification',
'section' => 'blocktype.peerassessment/signoff',
'args' => array($title),
),
'urltext' => (object) array(
'key' => 'view',
),
),
'users' => array($sendto),
'url' => $url,
);
activity_occurred('maharamessage', $data);
}
execute_sql("UPDATE {view_signoff_verify} SET verified = ?, verifier = NULL, verifiedtime = NULL WHERE view = ?", array(0, $viewid));
$data->verify_change = 1;
}
$data->signoff_newstate = $newstate;
}
else if ($verifiable && !empty($verify)) {
else if ($showverify && $verifiable && !empty($verify)) {
$currentstate = ArtefactTypePeerassessment::is_verified($view);
$newstate = 1;
if ((int)$currentstate != $newstate) {
execute_sql("UPDATE {view_signoff_verify} SET verified = ? WHERE view = ?", array($newstate, $viewid));
execute_sql("UPDATE {view_signoff_verify} SET verified = ?, verifier = ?, verifiedtime = ? WHERE view = ?", array($newstate, $USER->get('id'), db_format_timestamp(time()), $viewid));
}
$data->verify_newstate = $newstate;
}
else {
// No rights to do what is requested
json_reply('local', get_string('wrongassessmentviewrequest', 'artefact.peerassessment'));
json_reply('local', get_string('wrongsignoffviewrequest', 'blocktype.peerassessment/signoff'));
}
json_reply(false, array('message' => get_string('assessmentviewupdated', 'artefact.peerassessment'),
$message = $verify ? get_string('verifyviewupdated', 'blocktype.peerassessment/signoff') : get_string('signoffviewupdated', 'blocktype.peerassessment/signoff');
json_reply(false, array('message' => $message,
'data' => $data));
......@@ -24,8 +24,6 @@ $string['assessmentsubmitteddraft'] = 'Peer assessment saved as draft';
$string['reallydeletethisassessment'] = 'Delete this peer assessment';
$string['thisassessmentisprivate'] = 'Saved as draft';
$string['assessmentremoved'] = 'Peer assessment deleted';
$string['assessmentviewupdated'] = 'Peer assessment status updated';
$string['wrongassessmentviewrequest'] = 'You do not have permission to perform the requested action';
// peer assessment notifications
$string['deletednotificationsubject'] = 'Peer assessment on page "%s" deleted';
......
......@@ -238,7 +238,6 @@ class ArtefactTypePeerassessment extends ArtefactType {
if ($new) {
insert_record('artefact_peer_assessment', $data);
ensure_record_exists('view_signoff_verify', (object) array('view' => $this->get('view')), (object) array('view' => $this->get('view')), 'id', true);
}
else {
update_record('artefact_peer_assessment', $data, 'assessment');
......@@ -363,23 +362,35 @@ class ArtefactTypePeerassessment extends ArtefactType {
$where = 'pa.view = ? ';
// If the view is signed off, select public assessments
// or if viewing as page owner, select public assessments
// or if viewing as assessment author, select assessments owned by author
$where.= 'AND ((vsv.signoff = 1 AND pa.private = 0) ';
$where.= ' OR (a.author = ?)';
$where.= ' OR (pa.private = 0 AND a.owner = ?))';
// If the signoff block is also present on the page then we restrict things differently
$withsignoff = record_exists('block_instance', 'view', $viewid, 'blocktype', 'signoff');
if ($withsignoff) {
// If the view is signed off, select public assessments
// or if viewing as page owner, select public assessments
// or if viewing as assessment author, select assessments owned by author
$where.= 'AND ((vsv.signoff = 1 AND pa.private = 0) ';
$where.= ' OR (a.author = ?)';
$where.= ' OR (pa.private = 0 AND a.owner = ?))';
$values = array($viewid, $userid, $userid, $block);
$values = array($viewid, $userid, $userid, $block);
$joinsignoff = ' JOIN {view_signoff_verify} vsv ON vsv.view = pa.view ';
}
else {
// select assessments that are published
// or select assessments where the user is the author, published or not
$where.= 'AND ( (pa.private = 0) ';
$where.= ' OR (a.author = ?))';
$values = array($viewid, $userid, $block);
$joinsignoff = '';
}
$result->count = count_records_sql('
SELECT COUNT(*)
FROM
{artefact} a
JOIN {artefact_peer_assessment} pa
ON a.id = pa.assessment
JOIN {view_signoff_verify} vsv
ON vsv.view = pa.view
ON a.id = pa.assessment' . $joinsignoff . '
LEFT JOIN {artefact} p
ON a.parent = p.id
WHERE ' . $where . '
......@@ -404,8 +415,8 @@ class ArtefactTypePeerassessment extends ArtefactType {
$ids = get_column_sql('
SELECT a.id
FROM {artefact} a JOIN {artefact_peer_assessment} pa ON a.id = pa.assessment
JOIN {view_signoff_verify} vsv ON vsv.view = pa.view
LEFT JOIN {artefact} p ON a.parent = p.id
' . $joinsignoff . '
LEFT JOIN {artefact} p ON a.parent = p.id
WHERE ' . $where . '
AND pa.block = ?
ORDER BY ' . $orderby,
......@@ -435,7 +446,7 @@ class ArtefactTypePeerassessment extends ArtefactType {
u.deleted, u.profileicon, u.urlid, p.id AS parent, p.author AS parentauthor
FROM {artefact} a
INNER JOIN {artefact_peer_assessment} pa ON a.id = pa.assessment
JOIN {view_signoff_verify} vsv ON vsv.view = pa.view
' . $joinsignoff . '
LEFT JOIN {artefact} p
ON a.parent = p.id
LEFT JOIN {usr} u ON a.author = u.id
......@@ -521,8 +532,12 @@ class ArtefactTypePeerassessment extends ArtefactType {
$authors = array();
$lastcomment = self::last_public_assessment($data->view);
$editableafter = time() - 60 * get_config_plugin('artefact', 'comment', 'commenteditabletime');
$signedoff = null;
foreach ($data->data as &$item) {
if ($signedoff === null) {
$view = new View($item->view);
$signedoff = self::is_signed_off($view);
}
$item->ts = strtotime($item->ctime);
$timelapse = format_timelapse($item->ts);
$item->date = ($timelapse) ? $timelapse : format_date($item->ts, 'strftimedatetime');
......@@ -554,7 +569,7 @@ class ArtefactTypePeerassessment extends ArtefactType {
}
$submittedcheck = get_record_sql('SELECT v.* FROM {view} v WHERE v.id = ?', array($data->view), ERROR_MULTIPLE);
if (($candelete || ($item->isauthor && !$is_export_preview)) && $submittedcheck->submittedstatus == View::UNSUBMITTED) {
if (($candelete || ($item->isauthor && !$signedoff && !$is_export_preview)) && $submittedcheck->submittedstatus == View::UNSUBMITTED) {
$item->deleteform = pieform(self::delete_assessment_form($item->id, $item->view, $item->block));
}
if ($item->canedit && $submittedcheck->submittedstatus == View::UNSUBMITTED) {
......
......@@ -1229,6 +1229,8 @@ class BlockInstance {
return $renderedform;
}
$notretractable = get_config_plugin('blocktype', $this->get('blocktype'), 'notretractable');
safe_require('blocktype', $this->get('blocktype'));
$blocktypeclass = generate_class_name('blocktype', $this->get('blocktype'));
$elements = call_static_method($blocktypeclass, 'instance_config_form', $this, $this->get_view()->get('template'));
......@@ -1278,22 +1280,28 @@ class BlockInstance {
'value' => $new,
),
),
$elements,
array (
'retractable' => array(
'type' => 'select',
'title' => get_string('retractable', 'view'),
'description' => get_string('retractabledescription', 'view'),
'options' => array(
BlockInstance::RETRACTABLE_NO => get_string('no'),
BlockInstance::RETRACTABLE_YES => get_string('yes'),
BlockInstance::RETRACTABLE_RETRACTED => get_string('retractedonload', 'view')
),
'defaultvalue' => $retractable + $retractedonload,
),
)
$elements
);
if (!$notretractable) {
$elements = array_merge(
$elements,
array (
'retractable' => array(
'type' => 'select',
'title' => get_string('retractable', 'view'),
'description' => get_string('retractabledescription', 'view'),
'options' => array(
BlockInstance::RETRACTABLE_NO => get_string('no'),
BlockInstance::RETRACTABLE_YES => get_string('yes'),
BlockInstance::RETRACTABLE_RETRACTED => get_string('retractedonload', 'view')
),
'defaultvalue' => $retractable + $retractedonload,
),
)
);
}
if ($new) {
$cancel = get_string('remove');
$elements['removeoncancel'] = array('type' => 'hidden', 'value' => 1);
......
......@@ -729,7 +729,7 @@ EOD;
/**
* A fixture to set up page & collection permissions. Currently it only supports setting a blanket permission of
* "public", "loggedin", "friends", or "private", and allowcomments & approvecomments
* "public", "loggedin", "friends", "private", "user + role", and allowcomments & approvecomments
*
* Example:
* Given the following "permissions" exist:
......@@ -768,6 +768,7 @@ EOD;
$accesslist = array();
}
else {
$role = null;
switch ($record['accesstype']) {
case 'user':
$ids = get_records_sql_array('SELECT id FROM {usr} WHERE LOWER(TRIM(username)) = ?', array(strtolower(trim($record['accessname']))));
......@@ -776,6 +777,9 @@ EOD;
}
$id = $ids[0]->id;
$type = 'user';
if (!empty($record['role']) && $userrole = get_field('usr_roles', 'role', 'role', $record['role'])) {
$role = $userrole;
}
break;
case 'public':
case 'friends':
......@@ -789,10 +793,18 @@ EOD;
'startdate' => null,
'stopdate' => null,
'type' => $type,
'role' => $role,
'id' => $id,
)
);
}
if (!empty($record['multiplepermissions'])) {
require_once('view.php');
$firstview = new View($viewids[0]);
$currentaccess = $firstview->get_access();
$accesslist = array_merge($currentaccess, $accesslist);
}
$viewconfig = array(
'startdate' => null,
'stopdate' => null,
......
......@@ -137,6 +137,8 @@ class BehatDataGenerators extends BehatBase {
'accessname' => 'text',
'allowcomments' => 'bool',
'approvecomments' => 'bool',
'role' => 'text',
'multiplepermissions' => 'bool', // Set to true if wanting to add multiple access rules to a view
),
'required' => array('title', 'accesstype'),
),
......
......@@ -62,13 +62,15 @@ define ("LOCATOR_CONSTANTS", json_encode(array(
'Matrix table' => array("#tablematrix", "css_element"),
'Toolbar buttons' => array("#toolbar-buttons", "css_element"),
'User menu' => array(".icon-chevron-down.collapsed", "css_element"),
'Settings sub-menu' => array("//span[@innertext='Settings']", "xpath_element"),
'Settings' => array("//ul[#'userchildmenu-8']/?/?/a[@innertext='Settings']", "xpath_element"),
'Legal' => array("//ul[#'userchildmenu-8']/?/?/a[@innertext='Legal']", "xpath_element"),
'Signoff page' => array("#signoff-confirm-form", "css_element"),
'Verify page' => array("#verify-confirm-form", "css_element"),
#xpath_elements
'Secret urls - table row 1' => array("//table/tbody/tr[1]/td[4]/a", "xpath_element"),
'File Size' => array("//table[@id='files_filebrowser_filelist']/tbody/tr[1]/td[4]", "xpath_element"),
'Multirecipientnotification' => array("//li[@id='module.multirecipientnotification']", "xpath_element"),
'Settings sub-menu' => array("//span[@innertext='Settings']", "xpath_element"),
'Settings' => array("//ul[#'userchildmenu-8']/?/?/a[@innertext='Settings']", "xpath_element"),
'Legal' => array("//ul[#'userchildmenu-8']/?/?/a[@innertext='Legal']", "xpath_element"),
// xpath related to participation report
'Group views report tr1 tc1' => array("//*[@id='groupviewsreport']/tbody/tr[1]/td[1]", "xpath_element"),
'Group views report tr1 tc2' => array("//*[@id='groupviewsreport']/tbody/tr[1]/td[2]", "xpath_element"),
......
{if $placeholder && $editing}
<p class="editor-description">{$placeholder}</p>
{else}
<!-- block has toolbarhtml option -->
{/if}
<div id="verifyform" class="toolbarhtml">
{if $showsignoff}
<div>
{str tag=signoff section=blocktype.peerassessment/peerassessment}
{str tag=signedoff section=blocktype.peerassessment/signoff}
{if $signable}
<a href="#" id="signoff">
<span class="icon {if $signoff}icon-check-circle completed {else}icon-circle incomplete{/if} icon-lg"></span>
<span class="sr-only">{str tag=updatesignoff section=blocktype.peerassessment/signoff}</span>
</a>
{elseif $signoff}
<span class="icon icon-check-circle completed icon-lg"></span>
......@@ -11,11 +13,14 @@
<span class="icon icon-circle dot disabled icon-lg"></span>
{/if}
</div>
{/if}
{if $showverify}
<div>
{str tag=verify section=blocktype.peerassessment/peerassessment}
{str tag=verified section=blocktype.peerassessment/signoff}
{if $verifiable && $signoff}
<a href="#" id="verify">