Commit baac44f1 authored by Robert Lyon's avatar Robert Lyon
Browse files

Bug 1594579: Copy view artefacts only once



Rather than copy the same artefact once per page we should only copy
it once per copy of page(s) transaction. Eg if we are to copy a
collection of 5 pages and they all have a block pointing to the same
image we should copy that image only once not 5 times.

behatnotneeded - behat file to come

Change-Id: Iecdde53515cdd9d5ee02918252b486aa0f662fab
Signed-off-by: Robert Lyon's avatarRobert Lyon <robertl@catalyst.net.nz>
parent 33b484bf
...@@ -601,13 +601,13 @@ class ArtefactTypeBlog extends ArtefactType { ...@@ -601,13 +601,13 @@ class ArtefactTypeBlog extends ArtefactType {
$groupid = $view->get('group'); $groupid = $view->get('group');
$institution = $view->get('institution'); $institution = $view->get('institution');
if ($groupid || $institution) { if ($groupid || $institution) {
$SESSION->add_ok_msg(get_string('copiedblogpoststonewjournal', 'collection')); $SESSION->add_msg_once(get_string('copiedblogpoststonewjournal', 'collection'), 'ok', true, 'messages');
} }
else { else {
try { try {
$user = get_user($view->get('owner')); $user = get_user($view->get('owner'));
set_account_preference($user->id, 'multipleblogs', 1); set_account_preference($user->id, 'multipleblogs', 1);
$SESSION->add_ok_msg(get_string('copiedblogpoststonewjournal', 'collection')); $SESSION->add_msg_once(get_string('copiedblogpoststonewjournal', 'collection'), 'ok', true, 'messages');
} }
catch (Exception $e) { catch (Exception $e) {
$SESSION->add_error_msg(get_string('unabletosetmultipleblogs', 'error', $user->username, $viewid, get_config('wwwroot') . 'account/index.php'), false); $SESSION->add_error_msg(get_string('unabletosetmultipleblogs', 'error', $user->username, $viewid, get_config('wwwroot') . 'account/index.php'), false);
......
...@@ -227,6 +227,31 @@ class Session { ...@@ -227,6 +227,31 @@ class Session {
$this->__unset($key); $this->__unset($key);
} }
/**
* Checks that a successful message is only added once
*
* @param string $message The message to add
* @param boolean $escape Whether to HTML escape the message
* @param string $placement Place for messages to appear on page (See render_messages()
* for information about placement options)
*/
public function add_msg_once($message, $type, $escape=true, $placement='messages') {
$this->ensure_session();
if ($escape) {
$message = self::escape_message($message);
}
$msgs = $this->get('messages');
foreach ($msgs as $msg) {
if ($msg ['msg'] == $message && $msg['type'] == $type && $msg['placement'] == $placement) {
// msg exists
$this->ro_session();
return;
}
}
$typestr = 'add_' . $type . '_msg';
$this->$typestr($message, $escape=true, $placement='messages');
$this->ro_session();
}
/** /**
* Adds a message that indicates something was successful * Adds a message that indicates something was successful
* *
......
...@@ -511,13 +511,13 @@ class User { ...@@ -511,13 +511,13 @@ class User {
if (is_null($systemprofileviewid)) { if (is_null($systemprofileviewid)) {
$systemprofileviewid = get_field('view', 'id', 'institution', 'mahara', 'template', View::SITE_TEMPLATE, 'type', 'profile'); $systemprofileviewid = get_field('view', 'id', 'institution', 'mahara', 'template', View::SITE_TEMPLATE, 'type', 'profile');
} }
$artefactcopies = array();
list($view) = View::create_from_template(array( list($view) = View::create_from_template(array(
'owner' => $this->get('id'), 'owner' => $this->get('id'),
'title' => get_field('view', 'title', 'id', $systemprofileviewid), 'title' => get_field('view', 'title', 'id', $systemprofileviewid),
'description' => get_string('profiledescription'), 'description' => get_string('profiledescription'),
'type' => 'profile', 'type' => 'profile',
), $systemprofileviewid, $this->get('id'), false); ), $systemprofileviewid, $this->get('id'), false, false, $artefactcopies);
// Set view access // Set view access
$access = array( $access = array(
...@@ -565,13 +565,13 @@ class User { ...@@ -565,13 +565,13 @@ class User {
if (is_null($systemdashboardviewid)) { if (is_null($systemdashboardviewid)) {
$systemdashboardviewid = get_field('view', 'id', 'institution', 'mahara', 'template', View::SITE_TEMPLATE, 'type', 'dashboard'); $systemdashboardviewid = get_field('view', 'id', 'institution', 'mahara', 'template', View::SITE_TEMPLATE, 'type', 'dashboard');
} }
$artefactcopies = array();
list($view) = View::create_from_template(array( list($view) = View::create_from_template(array(
'owner' => $this->get('id'), 'owner' => $this->get('id'),
'title' => get_field('view', 'title', 'id', $systemdashboardviewid), 'title' => get_field('view', 'title', 'id', $systemdashboardviewid),
'description' => get_string('dashboarddescription'), 'description' => get_string('dashboarddescription'),
'type' => 'dashboard', 'type' => 'dashboard',
), $systemdashboardviewid, $this->get('id'), false); ), $systemdashboardviewid, $this->get('id'), false, false, $artefactcopies);
db_commit(); db_commit();
...@@ -1315,13 +1315,14 @@ class User { ...@@ -1315,13 +1315,14 @@ class User {
} }
db_begin(); db_begin();
$artefactcopies = array();
foreach ($templateids as $tid) { foreach ($templateids as $tid) {
View::create_from_template(array( View::create_from_template(array(
'owner' => $this->get('id'), 'owner' => $this->get('id'),
'title' => $views[$tid]->title, 'title' => $views[$tid]->title,
'description' => $views[$tid]->description, 'description' => $views[$tid]->description,
'type' => $views[$tid]->type == 'profile' && $checkviewaccess ? 'portfolio' : $views[$tid]->type, 'type' => $views[$tid]->type == 'profile' && $checkviewaccess ? 'portfolio' : $views[$tid]->type,
), $tid, $this->get('id'), $checkviewaccess); ), $tid, $this->get('id'), $checkviewaccess, false, $artefactcopies);
} }
db_commit(); db_commit();
} }
......
...@@ -281,6 +281,7 @@ class Collection { ...@@ -281,6 +281,7 @@ class Collection {
$views = $colltemplate->get('views'); $views = $colltemplate->get('views');
$copyviews = array(); $copyviews = array();
$artefactcopies = array();
foreach ($views['views'] as $v) { foreach ($views['views'] as $v) {
$values = array( $values = array(
'new' => true, 'new' => true,
...@@ -289,7 +290,7 @@ class Collection { ...@@ -289,7 +290,7 @@ class Collection {
'institution' => isset($data->institution) ? $data->institution : null, 'institution' => isset($data->institution) ? $data->institution : null,
'usetemplate' => $v->view 'usetemplate' => $v->view
); );
list($view, $template, $copystatus) = View::create_from_template($values, $v->view, $userid, $checkaccess, $titlefromtemplate); list($view, $template, $copystatus) = View::create_from_template($values, $v->view, $userid, $checkaccess, $titlefromtemplate, $artefactcopies);
if (isset($copystatus['quotaexceeded'])) { if (isset($copystatus['quotaexceeded'])) {
$SESSION->clear('messages'); $SESSION->clear('messages');
return array(null, $colltemplate, array('quotaexceeded' => true)); return array(null, $colltemplate, array('quotaexceeded' => true));
......
...@@ -462,7 +462,7 @@ function group_create($data) { ...@@ -462,7 +462,7 @@ function group_create($data) {
} }
// Copy views for the new group // Copy views for the new group
$templates = get_column('view_autocreate_grouptype', 'view', 'grouptype', $data['grouptype']); $artefactcopies = array();
$templates = get_records_sql_array(" $templates = get_records_sql_array("
SELECT v.id, v.title, v.description SELECT v.id, v.title, v.description
FROM {view} v FROM {view} v
...@@ -477,7 +477,7 @@ function group_create($data) { ...@@ -477,7 +477,7 @@ function group_create($data) {
'group' => $id, 'group' => $id,
'title' => $template->title, 'title' => $template->title,
'description' => $template->description, 'description' => $template->description,
), $template->id, null, false); ), $template->id, null, false, false, $artefactcopies);
$view->set_access(array(array( $view->set_access(array(array(
'type' => 'group', 'type' => 'group',
'id' => $id, 'id' => $id,
...@@ -511,7 +511,7 @@ function group_create($data) { ...@@ -511,7 +511,7 @@ function group_create($data) {
'title' => $template->get('title'), 'title' => $template->get('title'),
'description' => $template->get('description'), 'description' => $template->get('description'),
'type' => 'grouphomepage', 'type' => 'grouphomepage',
), $t->id, 0, false); ), $t->id, 0, false, false, $artefactcopies);
} }
else { else {
throw new NotFoundException("group_create: group homepage is not found"); throw new NotFoundException("group_create: group homepage is not found");
...@@ -2812,6 +2812,7 @@ function group_copy($groupid, $return) { ...@@ -2812,6 +2812,7 @@ function group_copy($groupid, $return) {
} }
*/ */
// Copy views for the new group // Copy views for the new group
$artefactcopies = array();
$templates = get_records_sql_array(" $templates = get_records_sql_array("
SELECT v.id, v.title, v.description, v.type SELECT v.id, v.title, v.description, v.type
FROM {view} v FROM {view} v
...@@ -2825,7 +2826,7 @@ function group_copy($groupid, $return) { ...@@ -2825,7 +2826,7 @@ function group_copy($groupid, $return) {
'group' => $new_groupid, 'group' => $new_groupid,
'title' => $template->title, 'title' => $template->title,
'description' => $template->description, 'description' => $template->description,
), $template->id, null, false); ), $template->id, null, false, false, $artefactcopies);
if ($template->type == 'grouphomepage') { if ($template->type == 'grouphomepage') {
$duplicate_homepage = $view; $duplicate_homepage = $view;
......
...@@ -349,13 +349,15 @@ class View { ...@@ -349,13 +349,15 @@ class View {
* @param int $userid The user who has issued the command to create the * @param int $userid The user who has issued the command to create the
* view. See View::_create * view. See View::_create
* @param int $checkaccess Whether to check that the user can see the view before copying it * @param int $checkaccess Whether to check that the user can see the view before copying it
* @param bool $titlefromtemplate Use the default title supplied by template
* @param array $artefactcopies The mapping between old artefact ids and new ones (created in blockinstance copy)
* @return array A list consisting of the new view, the template view and * @return array A list consisting of the new view, the template view and
* information about the copy - i.e. how many blocks and * information about the copy - i.e. how many blocks and
* artefacts were copied * artefacts were copied
* @throws SystemException under various circumstances, see the source for * @throws SystemException under various circumstances, see the source for
* more information * more information
*/ */
public static function create_from_template($viewdata, $templateid, $userid=null, $checkaccess=true, $titlefromtemplate=false) { public static function create_from_template($viewdata, $templateid, $userid=null, $checkaccess=true, $titlefromtemplate=false, &$artefactcopies) {
if (is_null($userid)) { if (is_null($userid)) {
global $USER; global $USER;
$userid = $USER->get('id'); $userid = $USER->get('id');
...@@ -401,7 +403,7 @@ class View { ...@@ -401,7 +403,7 @@ class View {
$view->urlid = self::new_urlid($view->urlid, (object)$viewdata); $view->urlid = self::new_urlid($view->urlid, (object)$viewdata);
try { try {
$copystatus = $view->copy_contents($template); $copystatus = $view->copy_contents($template, $artefactcopies);
} }
catch (QuotaExceededException $e) { catch (QuotaExceededException $e) {
db_rollback(); db_rollback();
...@@ -5446,8 +5448,8 @@ class View { ...@@ -5446,8 +5448,8 @@ class View {
} }
public function copy_contents($template) { public function copy_contents($template, &$artefactcopies) {
$artefactcopies = array(); // Correspondence between original artefact ids and id of the copy
$this->set('numrows', $template->get('numrows')); $this->set('numrows', $template->get('numrows'));
$this->set('layout', $template->get('layout')); $this->set('layout', $template->get('layout'));
if ($template->get('owner') == 0 if ($template->get('owner') == 0
...@@ -6473,7 +6475,8 @@ function createview_submit(Pieform $form, $values) { ...@@ -6473,7 +6475,8 @@ function createview_submit(Pieform $form, $values) {
else if (isset($values['usetemplate'])) { else if (isset($values['usetemplate'])) {
$templateid = $values['usetemplate']; $templateid = $values['usetemplate'];
unset($values['usetemplate']); unset($values['usetemplate']);
list($view, $template, $copystatus) = View::create_from_template($values, $templateid); $artefactcopies = array();
list($view, $template, $copystatus) = View::create_from_template($values, $templateid, null, true, false, $artefactcopies);
if (isset($copystatus['quotaexceeded'])) { if (isset($copystatus['quotaexceeded'])) {
$SESSION->add_error_msg(get_string('viewcopywouldexceedquota', 'view')); $SESSION->add_error_msg(get_string('viewcopywouldexceedquota', 'view'));
redirect(get_config('wwwroot') . 'view/choosetemplate.php'); redirect(get_config('wwwroot') . 'view/choosetemplate.php');
...@@ -6488,7 +6491,8 @@ function createview_submit(Pieform $form, $values) { ...@@ -6488,7 +6491,8 @@ function createview_submit(Pieform $form, $values) {
// Use the site default portfolio page to create a new page // Use the site default portfolio page to create a new page
$sitedefaultviewid = get_field('view', 'id', 'institution', 'mahara', 'template', View::SITE_TEMPLATE, 'type', 'portfolio'); $sitedefaultviewid = get_field('view', 'id', 'institution', 'mahara', 'template', View::SITE_TEMPLATE, 'type', 'portfolio');
if (!empty($sitedefaultviewid)) { if (!empty($sitedefaultviewid)) {
list($view, $template, $copystatus) = View::create_from_template($values, $sitedefaultviewid); $artefactcopies = array();
list($view, $template, $copystatus) = View::create_from_template($values, $sitedefaultviewid, null, true, false, $artefactcopies);
if (isset($copystatus['quotaexceeded'])) { if (isset($copystatus['quotaexceeded'])) {
$SESSION->add_error_msg(get_string('viewcreatewouldexceedquota', 'view')); $SESSION->add_error_msg(get_string('viewcreatewouldexceedquota', 'view'));
redirect(get_config('wwwroot') . 'view/index.php'); redirect(get_config('wwwroot') . 'view/index.php');
...@@ -6548,7 +6552,8 @@ function copyview($id, $istemplate = false, $groupid = null, $collectionid = nul ...@@ -6548,7 +6552,8 @@ function copyview($id, $istemplate = false, $groupid = null, $collectionid = nul
redirect(get_config('wwwroot') . 'collection/edit.php?copy=1&id=' . $collection->get('id')); redirect(get_config('wwwroot') . 'collection/edit.php?copy=1&id=' . $collection->get('id'));
} }
else { else {
list($view, $template, $copystatus) = View::create_from_template($values, $id); $artefactcopies = array();
list($view, $template, $copystatus) = View::create_from_template($values, $id, null, true, false, $artefactcopies);
if (isset($copystatus['quotaexceeded'])) { if (isset($copystatus['quotaexceeded'])) {
$SESSION->add_error_msg(get_string('viewcopywouldexceedquota', 'view')); $SESSION->add_error_msg(get_string('viewcopywouldexceedquota', 'view'));
redirect(get_config('wwwroot') . 'view/view.php?id=' . $id); redirect(get_config('wwwroot') . 'view/view.php?id=' . $id);
......
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