Commit 7a7754ec authored by Son Nguyen's avatar Son Nguyen Committed by Aaron Wells

Handle embedded image deletion better. Bug 1489284

Refactor updating embedded images

Deals with the deleting of embedded images in a:
- blog
- forum post
- forum topic
- group description
- note
- page description

Also sorts out the problem where a note embedded item can be saved
with both 'editnote' and 'textbox' resourcetype. We only need one.

Also add 'static' to EmbeddedImage::methods()

behatnotneeded

Change-Id: Ife1f9dee5ffe9eae4468eadb8f46a16a0a2a9044
parent 2c8e91c0
......@@ -107,7 +107,6 @@ function delete_blog_submit(Pieform $form, $values) {
}
else {
$blog->delete();
EmbeddedImage::delete_embedded_images('blog', $blog->get('id'));
$SESSION->add_ok_msg(get_string('blogdeleted', 'artefact.blog'));
}
if ($institution) {
......
......@@ -206,8 +206,13 @@ class ArtefactTypeBlog extends ArtefactType {
return;
}
db_begin();
// Delete embedded images in the blog description
require_once('embeddedimage.php');
EmbeddedImage::delete_embedded_images('blog', $this->id);
// Delete the artefact and all children.
parent::delete();
db_commit();
}
/**
......
......@@ -287,6 +287,7 @@ class ArtefactTypeComment extends ArtefactType {
db_begin();
delete_records_select('artefact_comment_comment', 'artefact IN (' . $idstr . ')');
delete_records_select('artefact_file_embedded', 'resourcetype = ? AND resourceid IN (' . $idstr . ')', array('comment'));
parent::bulk_delete($artefactids);
db_commit();
}
......@@ -1316,6 +1317,8 @@ function delete_comment_submit(Pieform $form, $values) {
activity_occurred('feedback', $data, 'artefact', 'comment');
}
// Delete embedded images in the comment
require_once('embeddedimage.php');
EmbeddedImage::delete_embedded_images('comment', $comment->get('id'));
db_commit();
......
......@@ -168,7 +168,7 @@ function editnote_submit(Pieform $form, array $values) {
db_begin();
$artefact->set('title', $values['title']);
$newdescription = EmbeddedImage::prepare_embedded_images($values['description'], 'editnote', $artefact->get('id'), $artefact->get('group'));
$newdescription = EmbeddedImage::prepare_embedded_images($values['description'], 'textbox', $artefact->get('id'), $artefact->get('group'));
$artefact->set('description', $newdescription);
$artefact->set('tags', $values['tags']);
$artefact->set('allowcomments', (int) $values['allowcomments']);
......
......@@ -938,6 +938,23 @@ class ArtefactTypeHtml extends ArtefactType {
$this->set('description', $newdescription);
}
}
/**
* This function extends ArtefactType::delete() by deleting embedded images
*/
public function delete() {
if (empty($this->id)) {
return;
}
db_begin();
// Delete embedded images in the note
require_once('embeddedimage.php');
EmbeddedImage::delete_embedded_images('textbox', $this->id);
// Delete the artefact and all children.
parent::delete();
db_commit();
}
}
......
......@@ -238,7 +238,6 @@ function deletenote_submit(Pieform $form, array $values) {
$id = $data[$values['delete']]->id;
$note = new ArtefactTypeHtml($id);
$note->delete();
EmbeddedImage::delete_embedded_images('editnote', $id);
$SESSION->add_ok_msg(get_string('notedeleted', 'artefact.internal'));
redirect($baseurl);
}
......@@ -456,7 +456,7 @@ abstract class ArtefactType implements IArtefactType {
* If you just want the basic info,
* use {@link get_children_metadata} instead.
*
* @return array of instances.
* @return array of instances or false if no children.
*/
public function get_children_instances() {
if (!isset($this->childreninstances)) {
......@@ -480,7 +480,7 @@ abstract class ArtefactType implements IArtefactType {
* If you want instances, use {@link get_children_instances}
* but bear in mind this will have a performance impact.
*
* @return array
* @return array of false if no children
*/
public function get_children_metadata() {
if (!isset($this->childrenmetadata)) {
......
......@@ -90,6 +90,11 @@ class PluginBlocktypeText extends SystemBlocktype {
return $values;
}
public static function delete_instance($instance) {
require_once('embeddedimage.php');
EmbeddedImage::delete_embedded_images('text', $instance->get('id'));
}
public static function default_copy_type() {
return 'full';
}
......
......@@ -51,10 +51,8 @@ $smarty->assign('form', $form);
$smarty->display('group/delete.tpl');
function deletegroup_submit(Pieform $form, $values) {
require_once('embeddedimage.php');
global $SESSION, $USER, $groupid;
group_delete($groupid);
EmbeddedImage::delete_embedded_images('group', $groupid);
$SESSION->add_ok_msg(get_string('deletegroup', 'group'));
redirect('/group/mygroups.php');
}
......@@ -505,17 +505,16 @@ function editgroup_submit(Pieform $form, $values) {
db_begin();
$newvalues['description'] = EmbeddedImage::prepare_embedded_images($newvalues['description'], 'group', $group_data->id, $group_data->id);
if ($group_data->id) {
$newvalues['id'] = $group_data->id;
group_update((object)$newvalues);
}
else {
if (!$group_data->id) {
$newvalues['members'] = array($USER->get('id') => 'admin');
$group_data->id = group_create($newvalues);
$USER->reset_grouproles();
}
// Now update the description with any embedded image info
$newvalues['description'] = EmbeddedImage::prepare_embedded_images($newvalues['description'], 'group', $group_data->id, $group_data->id);
$newvalues['id'] = $group_data->id;
unset($newvalues['members']);
group_update((object)$newvalues);
$SESSION->add_ok_msg(get_string('groupsaved', 'group'));
......
......@@ -94,18 +94,20 @@ $form = pieform(array(
function deletepost_submit(Pieform $form, $values) {
global $SESSION, $USER;
$postid = $values['post'];
$objectionable = get_record_sql("SELECT fp.id
FROM {interaction_forum_post} fp
JOIN {objectionable} o
ON (o.objecttype = 'forum' AND o.objectid = fp.id)
WHERE fp.id = ?
AND o.resolvedby IS NULL
AND o.resolvedtime IS NULL", array($values['post']));
AND o.resolvedtime IS NULL", array($postid));
if ($objectionable !== false) {
// Trigger activity.
$data = new StdClass;
$data->postid = $values['post'];
$data->postid = $postid;
$data->message = '';
$data->reporter = $USER->get('id');
$data->ctime = time();
......@@ -116,8 +118,12 @@ function deletepost_submit(Pieform $form, $values) {
update_record(
'interaction_forum_post',
array('deleted' => 1),
array('id' => $values['post'])
array('id' => $postid)
);
// Delete embedded images in the forum post description
require_once('embeddedimage.php');
EmbeddedImage::delete_embedded_images('post', $postid);
$SESSION->add_ok_msg(get_string('deletepostsuccess', 'interaction.forum'));
// Figure out which parent record to redirect us to. If the parent record is deleted,
// keep moving up the chain until you find one that's not deleted.
......
......@@ -112,6 +112,8 @@ function deletetopic_submit(Pieform $form, $values) {
array('deleted' => 1),
array('id' => $topicid)
);
// Delete embedded images in the topic and its posts
require_once('embeddedimage.php');
EmbeddedImage::delete_embedded_images('topic', $topicid);
// mark relevant posts as deleted
update_record(
......
......@@ -824,6 +824,35 @@ class InteractionForumInstance extends InteractionInstance {
return 'forum';
}
public function delete() {
if (empty($this->id)) {
$this->dirty = false;
return;
}
db_begin();
// Delete embedded images in the forum description
require_once('embeddedimage.php');
EmbeddedImage::delete_embedded_images('forum', $this->id);
// Delete the interaction instance
parent::delete();
db_commit();
}
public function commit() {
if (empty($this->dirty)) {
return;
}
db_begin();
parent::commit();
// Update embedded images in the forum description
require_once('embeddedimage.php');
$newdescription = EmbeddedImage::prepare_embedded_images($this->description, 'forum', $this->id, $this->group);
set_field('interaction_instance', 'description', $newdescription, 'id', $this->id);
db_commit();
}
public function interaction_remove_user($userid) {
delete_records('interaction_forum_moderator', 'forum', $this->id, 'user', $userid);
delete_records('interaction_forum_subscription_forum', 'forum', $this->id, 'user', $userid);
......
......@@ -282,14 +282,12 @@ function edit_interaction_validation(Pieform $form, $values) {
}
function edit_interaction_submit(Pieform $form, $values) {
require_once('embeddedimage.php');
safe_require('interaction', $values['plugin']);
$classname = generate_interaction_instance_class_name($values['plugin']);
$instance = new $classname($values['id']);
$instance->set('creator', $values['creator']);
$instance->set('title', $values['title']);
$newdescription = EmbeddedImage::prepare_embedded_images($values['description'], 'forum', $instance->get('id'), $instance->get('group'));
$instance->set('description', $newdescription);
$instance->set('description', $values['description']);
if (empty($values['id'])) {
$instance->set('group', $values['group']);
}
......
......@@ -4234,5 +4234,32 @@ function xmldb_core_upgrade($oldversion=0) {
}
}
if ($oldversion < 2015092909) {
log_debug('Need to consolidate "textbox" and "editnote" embedded resource types as they are in fact the same thing');
if ($records = get_records_sql_array('SELECT * FROM {artefact_file_embedded} WHERE resourcetype IN (?, ?)', array('editnote','textbox'))) {
$newrecords = array();
// Turn the results into something easier to check against
foreach ($records as $k => $v) {
$newrecords[$v->resourcetype . '_' . $v->resourceid . '_' . $v->fileid] = $v;
}
foreach ($newrecords as $nk => $nv) {
// need to sort out the 'editnote' options
if (preg_match('/^editnote_(.*)$/', $nk, $match)) {
// Check to see if there is a corresponding 'textbox' one - if not we need to make one
if (!array_key_exists('textbox_' . $match[1], $newrecords)) {
insert_record('artefact_file_embedded', (object) array(
'fileid' => $nv->fileid,
'resourcetype' => 'textbox',
'resourceid' => $nv->resourceid,
));
}
// now delete the 'editnote' one
delete_records('artefact_file_embedded', 'id', $nv->id);
}
}
}
}
return $status;
}
......@@ -27,7 +27,7 @@ class EmbeddedImage {
* @param int $groupid The id of the group the resource is in if applicable
* @return string The updated $fieldvalue
*/
public function prepare_embedded_images($fieldvalue, $resourcetype, $resourceid, $groupid = NULL) {
public static function prepare_embedded_images($fieldvalue, $resourcetype, $resourceid, $groupid = NULL) {
if (empty($fieldvalue) || empty($resourcetype) || empty($resourceid)) {
return $fieldvalue;
......@@ -113,7 +113,6 @@ class EmbeddedImage {
}
$image->parentNode->replaceChild($imgnode, $image);
}
self::remove_embedded_images($resourcetype, $resourceid, $publicimages);
// we only want the fragments inside the body tag created by new DOMDocument
......@@ -136,48 +135,42 @@ class EmbeddedImage {
* @param int $resourceid The id of the resourcetype
* @return void
*/
public function delete_embedded_images($resourcetype, $resourceid) {
public static function delete_embedded_images($resourcetype, $resourceid) {
self::remove_embedded_images($resourcetype, $resourceid);
if ($resourcetype == 'forum') {
// we deleted embedded forum image above, now delete any embedded child topic and post images for that forum
$topicids = get_records_array('interaction_forum_topic', 'forum', $resourceid, 'id DESC', 'id');
foreach ($topicids as $id) {
// note recursion to remove posts associated with topic
self::delete_embedded_images('topic', $id->id);
if ($topicids = get_records_array('interaction_forum_topic', 'forum', $resourceid, 'id DESC', 'id')) {
foreach ($topicids as $id) {
// note recursion to remove posts associated with topic
self::delete_embedded_images('topic', $id->id);
}
}
}
else if ($resourcetype == 'topic') {
// we deleted embedded topic image above, now delete any embedded child post images for that topic
$postids = get_records_array('interaction_forum_post', 'topic', $resourceid, 'id DESC', 'id');
foreach ($postids as $id) {
self::remove_embedded_images('post', $id->id);
}
}
else if ($resourcetype == 'blog') {
// we deleted blog image above, now delete any embedded blogpost images for that blog
if ($blogpostids = get_records_array('artefact', 'parent', $resourceid, 'id DESC', 'id')) {
foreach ($blogpostids as $id) {
self::remove_embedded_images('blogpost', $id->id);
if ($postids = get_records_array('interaction_forum_post', 'topic', $resourceid, 'id DESC', 'id')) {
foreach ($postids as $id) {
self::remove_embedded_images('post', $id->id);
}
}
}
}
public function can_see_embedded_image($fileid, $resourcetype, $resourceid) {
public static function can_see_embedded_image($fileid, $resourcetype, $resourceid) {
$imgispublic = get_field('artefact_file_embedded', 'id', 'fileid', $fileid, 'resourcetype', $resourcetype, 'resourceid', $resourceid);
return $imgispublic !== false;
}
function remove_embedded_images($resourcetype, $resourceid, $publicimages = NULL) {
static function remove_embedded_images($resourcetype, $resourceid, $publicimages = NULL) {
$existingpublicimages = get_records_select_array('artefact_file_embedded', "resourcetype = ? AND resourceid = ?", array($resourcetype, $resourceid));
if (!$existingpublicimages) {
return;
}
foreach ($existingpublicimages as $img) {
if (empty($publicimages) || !in_array($img->fileid, $publicimages)) {
delete_records('artefact_file_embedded', 'fileid', $img->fileid);
delete_records('artefact_file_embedded', 'fileid', $img->fileid, 'resourceid', $img->resourceid);
}
}
}
......@@ -190,7 +183,7 @@ class EmbeddedImage {
* @param string $resourceid
* @return mixed
*/
public function rewrite_embedded_image_urls_from_import($text, array $artefactids, $resourcetype=null, $resourceid=null) {
public static function rewrite_embedded_image_urls_from_import($text, array $artefactids, $resourcetype=null, $resourceid=null) {
$resourcestr = (!empty($resourcetype) && !empty($resourceid)) ?
"&$resourcetype=$resourceid"
: '';
......
......@@ -768,6 +768,10 @@ function group_delete($groupid, $shortname=null, $institution=null, $notifymembe
delete_records('group_member_request', 'group', $group->id);
delete_records('view_access', 'group', $group->id);
// Delete embedded images in the group description
require_once('embeddedimage.php');
EmbeddedImage::delete_embedded_images('group', $group->id);
// Delete views owned by the group
require_once(get_config('libroot') . 'view.php');
foreach (get_column('view', 'id', 'group', $group->id) as $viewid) {
......
......@@ -16,7 +16,7 @@ $config = new stdClass();
// See https://wiki.mahara.org/index.php/Developer_Area/Version_Numbering_Policy
// For upgrades on stable branches, increment the version by one. On master, use the date.
$config->version = 2015092908;
$config->version = 2015092909;
$config->series = '15.10';
$config->release = '15.10rc3testing';
$config->minupgradefrom = 2009022600;
......
......@@ -868,6 +868,8 @@ class View {
// but unlock its artefacts just in case.
ArtefactType::update_locked($this->owner);
}
require_once('embeddedimage.php');
EmbeddedImage::delete_embedded_images('description', $this->id);
$this->deleted = true;
db_commit();
}
......
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