Commit 6d19103a authored by Kevin Dibble's avatar Kevin Dibble Committed by Robert Lyon
Browse files

Bug 1803101: peer assessment artefact import

Need to define owner and block when creating a
peer assessment artefact while importing

Create a temp block to assign on artefact creation
that later needs to be replaced by the actual block
when it's available

Draft assessments should not be exported or imported.
For exported portfolios created on older version of
Mahara, there is no way of knowing if they are drafts
at the moment of importing because this information
was not included in the leap2A export xml file.
Not importing draft peer assessments should work
from this patch.

It also adds a fix to support
Postgres and MySQL when a table
column name is a reserved word.

Removing not null restriction for "usr" field in "artefact_peer_assessment"
table:
- The author in peer assessment table should be working as comments table.
- For each comment, there is an entry in 'artefact' table that contains
author and authorname, and another entry in 'artefact_comment_comment'
that doesn't have an author field
because we already have one in 'artefact' table.
- We should have something similar on 'artefact_peer_assessment'.
There is already an author field in 'artefact' table,
then the 'usr' field in 'artefact_peer_assessment' should not to have a
'not null' restriction. If the peer assessment artefact has been imported
and the author is not present in this site, then the 'usr'
should be allowed to be null.

behatnotneeded

Change-Id: I182520159d1fcbc072081c2d976cdc68cde5e19d
parent ad6652d2
......@@ -38,9 +38,8 @@ class HtmlExportFile extends HtmlExportArtefactPlugin {
}
}
// include the comments profile icons
if ($artefact->get('artefacttype') == 'comment') {
$author = $artefact->get('author');
if ($author && $profileicon = get_field('usr', 'profileicon', 'id', $author)) {
if ($artefact->get('artefacttype') == 'comment' && $author = $artefact->get('author')) {
if ($profileicon = get_field('usr', 'profileicon', 'id', $author)) {
$profileiconartefact = new ArtefactTypeProfileIcon($profileicon);
$this->artefactdata[$profileicon] = $profileiconartefact;
}
......
......@@ -88,6 +88,15 @@ abstract class PluginArtefact extends Plugin implements IPluginArtefact {
return array();
}
/**
* Returns any artefacts that should not be included in an export
* @param array $userid
* @return array of artefact ids
*/
public static function exclude_artefacts_in_export($userid) {
return array();
}
/**
* When filtering searches, some artefact types are classified the same way
* even when they come from different artefact plugins. This function allows
......
......@@ -75,7 +75,7 @@ class PluginBlocktypePeerassessment extends MaharaCoreBlocktype {
$options->showcomment = $showcomment;
$options->view = $instance->get_view();
$options->block = $instance->get('id');
$feedback = ArtefactTypePeerassessment::get_assessments($options, $versioning);
$feedback = ArtefactTypePeerassessment::get_assessments($options, $versioning, $exporter);
$feedbackform = ArtefactTypePeerassessment::add_assessment_form(true, $instance->get('id'), 0);
$feedbackform = pieform($feedbackform);
$smarty = smarty_core();
......
......@@ -8,7 +8,7 @@
<FIELDS>
<FIELD NAME="assessment" TYPE="int" LENGTH="10" NOTNULL="true"/>
<FIELD NAME="block" TYPE="int" LENGTH="10" NOTNULL="true"/>
<FIELD NAME="usr" TYPE="int" LENGTH="10" NOTNULL="true"/>
<FIELD NAME="usr" TYPE="int" LENGTH="10" NOTNULL="false"/>
<FIELD NAME="view" TYPE="int" LENGTH="10" NOTNULL="true"/>
<FIELD NAME="private" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" UNSIGNED="true"/>
</FIELDS>
......
......@@ -18,9 +18,10 @@ class LeapExportElementPeerassessment extends LeapExportElement {
$artefactlist = join(',', array_map('intval', $artefactids));
// Get the peer assessments that are on these views.
// and are not in draft status
$records = get_records_select_array(
'artefact_peer_assessment',
"view IN ($viewlist)",
"view IN ($viewlist) AND private = 0",
array(),
'',
'assessment,view'
......@@ -69,4 +70,9 @@ class LeapExportElementPeerassessment extends LeapExportElement {
),
);
}
public function get_entry_author() {
return get_string('importedassessment', 'artefact.peerassessment');
}
}
......@@ -33,6 +33,8 @@ class LeapImportPeerassessment extends LeapImportArtefactPlugin {
*/
private static $tempview = null;
private static $savetempview = false;
private static $tempblock = null;
private static $savetempblock = false;
public static function create_temporary_view($user) {
$time = db_format_timestamp(time());
......@@ -48,6 +50,15 @@ class LeapImportPeerassessment extends LeapImportArtefactPlugin {
return self::$tempview = insert_record('view', $viewdata, 'id', true);
}
public static function create_temporary_block() {
$blockdata = (object) array(
'blocktype' => 'peerassessment',
'title' => '--',
'configdata' => serialize(array('instructions' => '', 'retractable' => 0, 'retractedonload' => 0)),
'view' => self::$tempview,
);
return self::$tempblock = insert_record('block_instance', $blockdata, 'id', true);
}
/**
* Delete the temporary view
*/
......@@ -58,6 +69,9 @@ class LeapImportPeerassessment extends LeapImportArtefactPlugin {
set_field('view', 'title', $title, 'id', self::$tempview);
}
else {
if (!self::$savetempblock) {
delete_records('block_instance', 'id', self::$tempblock);
}
delete_records('view', 'id', self::$tempview);
}
}
......@@ -68,7 +82,6 @@ class LeapImportPeerassessment extends LeapImportArtefactPlugin {
if (PluginImportLeap::is_rdf_type($entry, $importer, 'entry')
&& $entry->xpath('mahara:artefactplugin[@mahara:type="peerassessment"]')) {
// Annotation may not have anything reflecting on it.
$strategies[] = array(
'strategy' => self::STRATEGY_IMPORT_AS_PEERASSESSMENT,
......@@ -174,8 +187,10 @@ class LeapImportPeerassessment extends LeapImportArtefactPlugin {
if (empty(self::$tempview)) {
self::create_temporary_view($config['owner']);
self::create_temporary_block();
}
$assessment->set('view', self::$tempview);
$assessment->set('block', self::$tempblock);
$assessment->set('tags', $content['tags']);
$assessment->commit();
$artefactmapping = array();
......@@ -201,11 +216,39 @@ class LeapImportPeerassessment extends LeapImportArtefactPlugin {
$importid = $importer->get('importertransport')->get('importid');
if ($entry_requests = get_records_select_array('import_entry_requests', 'importid = ? AND entrytype = ?', array($importid, 'peerassessment'))) {
foreach ($entry_requests as $entry_request) {
$entry = $importer->get_entry_by_id($entry_request->entryid);
$config = self::get_peerassessment_entry_data($entry, $importer, array());
$content = $config['content'];
$assessment = new ArtefactTypePeerassessment();
$assessment->set('title', $content['title']);
$assessment->set('description', $content['description']);
if ($content['ctime']) {
$assessment->set('ctime', $content['ctime']);
}
if ($content['mtime']) {
$assessment->set('mtime', $content['mtime']);
}
$assessment->set('owner', $config['owner']);
$assessment->set('allowcomments', (isset($config['allowcomments']) ? $config['allowcomments'] : true));
// Note, once the view is created, we'll need to come back to this peer assessment
// and populate the artefact_peerassessment.view column.
if ($content['authorname']) {
$assessment->set('authorname', $content['authorname']);
}
else {
$assessment->set('author', $content['author']);
}
if (empty(self::$tempview)) {
self::create_temporary_view($config['owner']);
self::create_temporary_block();
}
$assessment->set('view', self::$tempview);
$assessment->set('block', self::$tempblock);
$assessment->set('tags', $content['tags']);
$assessment->commit();
$assessmentid = self::create_artefact_from_request($importer, $entry_request);
$importer->add_artefactmapping($entry_request->entryid, $assessment->get('id'));
}
}
}
......@@ -250,11 +293,20 @@ class LeapImportPeerassessment extends LeapImportArtefactPlugin {
if ($viewid = $importer->get_viewid_imported_by_entryid($view_entry_request)) {
// Set the view on the peerassessment.
$peerassessment->set('view', $viewid);
$blockid = get_field('block_instance', 'id', 'blocktype', 'peerassessment', 'view', $viewid);
if ($blockid) {
$peerassessment->set('block', $blockid);
}
else {
self::$savetempblock = true;
set_field('block_instance', 'view', $viewid, 'id', self::$tempblock);
}
$peerassessment->commit();
}
else {
// Nothing to link this peerassessment to, so leave it in the temporary view.
self::$savetempview = true;
self::$savetempblock = true;
}
}
}
......@@ -275,11 +327,19 @@ class LeapImportPeerassessment extends LeapImportArtefactPlugin {
$view_entry_request = self::get_referent_entryid($entry, $importer);
if ($viewid = $importer->get_viewid_imported_by_entryid($view_entry_request)) {
$peerassessment->set('view', $viewid);
$blockid = get_field('block_instance', 'id', 'blocktype', 'peerassessment', 'view', $viewid);
if ($blockid) {
$peerassessment->set('block', $blockid);
}
else {
self::$savetempblock = true;
}
$peerassessment->commit();
}
else {
// Nothing to link this peerassessment to, so leave it in the temporary view.
self::$savetempview = true;
self::$savetempblock = true;
}
}
......
......@@ -58,3 +58,4 @@ $string['progress_verify'] = array(
"Give 1 verification to another person's peer assessment page",
"Give %s verifications to other people's peer assessment pages",
);
$string['importedassessment'] = "Imported peer assessment";
\ No newline at end of file
......@@ -90,7 +90,7 @@ class PluginArtefactPeerassessment extends PluginArtefact {
if (!$artefacts = get_column_sql("
SELECT assessment
FROM {artefact_peer_assessment}
WHERE view IN (" . join(',', array_map('intval', $viewids)) . ')', array())) {
WHERE private = 0 AND view IN (" . join(',', array_map('intval', $viewids)) . ')', array())) {
return array();
}
if ($attachments = get_column_sql('
......@@ -105,16 +105,27 @@ class PluginArtefactPeerassessment extends PluginArtefact {
JOIN {artefact_peer_assessment} apa ON apa.assessment = afe.resourceid
WHERE afe.resourcetype IN (?)
AND apa.view IN (" . join(',', array_map('intval', $viewids)) . ")
AND apa.private = 0
UNION
SELECT afe.fileid
FROM {artefact_file_embedded} afe
JOIN {artefact_peer_assessment} apa ON apa.block = afe.resourceid
WHERE afe.resourcetype in(?)
AND apa.view IN (" . join(',', array_map('intval', $viewids)) . ")"
AND apa.view IN (" . join(',', array_map('intval', $viewids)) . ")
AND apa.private = 0"
, array('assessment', 'peerinstruction'))) {
$artefacts = array_merge($artefacts, $embeds);
}
return $artefacts;
}
public static function exclude_artefacts_in_export($userid) {
$sql = " SELECT a.id
FROM {artefact} a
JOIN {artefact_peer_assessment} apa
ON a.id = apa.assessment
WHERE a.owner = ? AND a.artefacttype='peerassessment' AND apa.private = 1";
$artefacts = get_column_sql($sql, array($userid));
return $artefacts;
}
......@@ -369,9 +380,11 @@ class ArtefactTypePeerassessment extends ArtefactType {
*
* @param object $options Object of assessment options
* - defaults can be retrieved from get_assessment_options()
* @param object $versioning Object with data for the timeline view versions
* @param PluginExport $exporter object used when exporting the portfolios
* @return object $result Assessment data object
*/
public static function get_assessments($options, $versioning=null) {
public static function get_assessments($options, $versioning=null, $exporter=null) {
global $USER;
$allowedoptions = self::get_assessment_options();
// set the object's key/val pairs as variables
......@@ -487,7 +500,7 @@ class ArtefactTypePeerassessment extends ArtefactType {
}
$result->position = 'blockinstance';
self::build_html($result, $versioning);
self::build_html($result, $versioning, $exporter);
return $result;
}
......@@ -509,18 +522,23 @@ class ArtefactTypePeerassessment extends ArtefactType {
$assessment->view = $viewid;
$assessment->block = $blockversion->originalblockid;
$user = new User();
$user->find_by_id($assessment->author);
$assessment->username = $user->get('username');
$assessment->firstname = $user->get('firstname');
$assessment->lastname = $user->get('lastname');
$assessment->preferredname = $user->get('preferredname');
$assessment->email = $user->get('email');
$assessment->admin = $user->get('admin');
$assessment->staff = $user->get('staff');
$assessment->deleted = $user->get('deleted');
$assessment->profileicon = $user->get('profileicon');
if (!$assessment->private && !$assessment->author) {
// the assessment has been imported and is not link to an author
$assessment->authorname = get_string('importedassessment', 'artefact.peerassessment');
}
else {
$user = new User();
$user->find_by_id($assessment->author);
$assessment->username = $user->get('username');
$assessment->firstname = $user->get('firstname');
$assessment->lastname = $user->get('lastname');
$assessment->preferredname = $user->get('preferredname');
$assessment->email = $user->get('email');
$assessment->admin = $user->get('admin');
$assessment->staff = $user->get('staff');
$assessment->deleted = $user->get('deleted');
$assessment->profileicon = $user->get('profileicon');
}
$assessmentsversion[] = $assessment;
}
}
......@@ -607,7 +625,7 @@ class ArtefactTypePeerassessment extends ArtefactType {
}
}
public static function build_html(&$data, $versioning=null) {
public static function build_html(&$data, $versioning=null, $exporter=null) {
global $USER, $THEME;
$deletedmessage = array();
......@@ -664,7 +682,10 @@ class ArtefactTypePeerassessment extends ArtefactType {
$item->editlink = $smarty->fetch('artefact:peerassessment:editlink.tpl');
}
}
if ($exporter) {
// Don't export the author of the assessment
$item->author = null;
}
if ($item->author) {
if (isset($authors[$item->author])) {
$item->author = $authors[$item->author];
......
......@@ -321,7 +321,19 @@ abstract class PluginExport extends Plugin implements IPluginExport {
}
}
// check if there are any artefacts that shouldn't be exported for each artefact type
$artefactsnotincluded = array();
$plugins = plugins_installed('artefact');
foreach ($plugins as &$plugin) {
safe_require('artefact', $plugin->name);
$classname = generate_class_name('artefact', $plugin->name);
if (is_callable($classname . '::exclude_artefacts_in_export')) {
if ($artefacts = call_static_method($classname, 'exclude_artefacts_in_export', $userid)) {
$artefactsnotincluded = array_unique(array_merge($artefactsnotincluded, $artefacts));
}
}
}
$tmpartefacts = array_diff($tmpartefacts, $artefactsnotincluded);
$this->collections = array();
if (empty($this->views)) {
......
......@@ -311,7 +311,7 @@ class PluginImportLeap extends PluginImport {
*/
public function call_import_method_plugins($method) {
$installedplugins = array_map(function($a) { return $a->name; }, plugins_installed('artefact'));
$orderedimportplugins = array('internal', 'file', 'blog', 'resume', 'plans', 'annotation', 'comment');
$orderedimportplugins = array('internal', 'file', 'blog', 'resume', 'plans', 'annotation', 'comment', 'peerassessment');
foreach ($orderedimportplugins as $plugin) {
if (!in_array($plugin, $installedplugins)) {
continue;
......@@ -1016,10 +1016,10 @@ class PluginImportLeap extends PluginImport {
case self::STRATEGY_IMPORT_AS_COLLECTION:
require_once('collection.php');
$collectiondata = unserialize($entry_request->entrycontent);
if (isset($collectiondata['coverimage']) && !empty($collectiondata['coverimage'])) {
$collectiondata['coverimage'] = $this->artefactids[$collectiondata['coverimage']][0];
}
$collection = new Collection(0, $collectiondata);
$collection->commit();
$this->collectionids[$entry_request->entryid] = $collection->get('id');
......@@ -1282,7 +1282,7 @@ class PluginImportLeap extends PluginImport {
$rowscolssql = '';
for ($i=0; $i<count($columnids); $i++) {
$rowscolssql .= '(row = ' . ($i+1) . ' AND columns = ' . $columnids[$i+1] . ')';
$rowscolssql .= '("row" = ' . ($i + 1) . ' AND columns = ' . $columnids[$i + 1] . ')';
if ($i != (count($columnids)-1)) {
$rowscolssql .= ' OR ';
}
......@@ -2500,4 +2500,4 @@ abstract class LeapImportArtefactPlugin implements ILeapImportArtefactPlugin {
public static function cleanup(PluginImportLeap $importer) {
}
}
}
\ No newline at end of file
......@@ -2027,5 +2027,12 @@ function xmldb_core_upgrade($oldversion=0) {
}
}
if ($oldversion < 2020121700) {
log_debug('Remove not null restriction for "usr" field in "artefact_peer_assessment"');
$table = new XMLDBTable('artefact_peer_assessment');
$field = new XMLDBField('usr');
$field->setAttributes(XMLDB_TYPE_INTEGER, 10, null, false);
change_field_notnull($table, $field);
}
return $status;
}
......@@ -16,7 +16,7 @@ $config = new stdClass();
// See https://wiki.mahara.org/wiki/Developer_Area/Version_Numbering_Policy
// For upgrades on stable branches, increment the version by one. On master, use the date.
$config->version = 2020111600;
$config->version = 2020121700;
$config->series = '21.04';
$config->release = '21.04dev';
$config->minupgradefrom = 2017031605;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment