From b94dcc42c0127db72dc0abc263605adbb1dd8976 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 25 Aug 2015 16:09:57 +0800 Subject: [PATCH 1/4] MDL-51179 Atto: Abort autosave if there are more recent changes to the files If the files in the draft area have been modified more recently than the timestamp of the draft - do not restore the files, or you will delete any newer files. We cannot really guess here - merging the 2 lists of files will be wrong. --- lib/editor/atto/autosave-ajax.php | 58 +++++++++++++++++++------------ 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/lib/editor/atto/autosave-ajax.php b/lib/editor/atto/autosave-ajax.php index fa45fd47d16..4e9e8378016 100644 --- a/lib/editor/atto/autosave-ajax.php +++ b/lib/editor/atto/autosave-ajax.php @@ -25,6 +25,7 @@ define('AJAX_SCRIPT', true); require_once(dirname(__FILE__) . '/../../../config.php'); +require_once($CFG->libdir . '/filestorage/file_storage.php'); $contextid = required_param('contextid', PARAM_INT); $elementid = required_param('elementid', PARAM_ALPHANUMEXT); @@ -114,32 +115,43 @@ if ($action === 'save') { $stale = $record->timemodified < $before; require_once($CFG->libdir . '/filelib.php'); - // This function copies all the files in one draft area, to another area (in this case it's - // another draft area). It also rewrites the text to @@PLUGINFILE@@ links. - $newdrafttext = file_save_draft_area_files($record->draftid, - $usercontext->id, - 'user', - 'draft', - $newdraftid, - array(), - $record->drafttext); + $fs = get_file_storage(); + $files = $fs->get_directory_files($usercontext->id, 'user', 'draft', $newdraftid, '/', true, true); - // Final rewrite to the new draft area (convert the @@PLUGINFILES@@ again). - $newdrafttext = file_rewrite_pluginfile_urls($newdrafttext, - 'draftfile.php', - $usercontext->id, - 'user', - 'draft', - $newdraftid); - $record->drafttext = $newdrafttext; + $lastfilemodified = 0; + foreach ($files as $file) { + $lastfilemodified = max($lastfilemodified, $file->get_timemodified()); + } + if ($record->timemodified < $lastfilemodified) { + $stale = true; + } - $record->pageinstance = $pageinstance; - $record->draftid = $newdraftid; - $record->timemodified = time(); - $DB->update_record('editor_atto_autosave', $record); - - // A response means the draft has been restored and here is the auto-saved text. if (!$stale) { + // This function copies all the files in one draft area, to another area (in this case it's + // another draft area). It also rewrites the text to @@PLUGINFILE@@ links. + $newdrafttext = file_save_draft_area_files($record->draftid, + $usercontext->id, + 'user', + 'draft', + $newdraftid, + array(), + $record->drafttext); + + // Final rewrite to the new draft area (convert the @@PLUGINFILES@@ again). + $newdrafttext = file_rewrite_pluginfile_urls($newdrafttext, + 'draftfile.php', + $usercontext->id, + 'user', + 'draft', + $newdraftid); + $record->drafttext = $newdrafttext; + + $record->pageinstance = $pageinstance; + $record->draftid = $newdraftid; + $record->timemodified = time(); + $DB->update_record('editor_atto_autosave', $record); + + // A response means the draft has been restored and here is the auto-saved text. $response['result'] = $record->drafttext; echo json_encode($response); } From 9314b26c1277842fecd0c1695e10eb3253b2d67d Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Wed, 26 Aug 2015 14:04:50 +0800 Subject: [PATCH 2/4] MDL-51179 Atto: When a resume is aborted - create a fresh draft for future saves. --- lib/editor/atto/autosave-ajax.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/editor/atto/autosave-ajax.php b/lib/editor/atto/autosave-ajax.php index 4e9e8378016..65915310a5e 100644 --- a/lib/editor/atto/autosave-ajax.php +++ b/lib/editor/atto/autosave-ajax.php @@ -154,6 +154,10 @@ if ($action === 'save') { // A response means the draft has been restored and here is the auto-saved text. $response['result'] = $record->drafttext; echo json_encode($response); + } else { + $DB->delete_records('editor_atto_autosave', array('id' => $record->id)); + + // No response means no error. } die(); } From db50734bd6477ee827e3510f05eb96a33dc1b722 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Fri, 28 Aug 2015 12:09:34 +0800 Subject: [PATCH 3/4] MDL-51179 Atto: Add a behat test for autosave --- lib/editor/atto/tests/behat/autosave.feature | 77 ++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 lib/editor/atto/tests/behat/autosave.feature diff --git a/lib/editor/atto/tests/behat/autosave.feature b/lib/editor/atto/tests/behat/autosave.feature new file mode 100644 index 00000000000..db4360088f6 --- /dev/null +++ b/lib/editor/atto/tests/behat/autosave.feature @@ -0,0 +1,77 @@ +@editor @editor_atto @atto @_file_upload +Feature: Atto Autosave + To reduce frustration, atto should save drafts of my work. + + Background: + Given the following "courses" exist: + | fullname | shortname | category | groupmode | + | Course 1 | C1 | 0 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | teacher2 | Teacher | 2 | teacher2@example.com | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | teacher2 | C1 | editingteacher | + And I log in as "admin" + And I navigate to "Atto toolbar settings" node in "Site administration > Plugins > Text editors > Atto HTML editor" + And I set the field "Autosave frequency" to "3" + And I set the field with xpath "//select[@name='s_editor_atto_autosavefrequency[u]']" to "seconds" + And I click on "Save changes" "button" + And I am on site homepage + And I follow "Course 1" + And I navigate to "Edit settings" node in "Course administration" + And I set the field with xpath "//select[@name='summary_editor[format]']" to "1" + And I click on "Save and display" "button" + And I log out + + @javascript + Scenario: Restore a draft + Given I log in as "teacher1" + And I follow "Course 1" + And I navigate to "Edit settings" node in "Course administration" + And I set the field "Course summary" to "This is my draft" + # Wait for the autosave + And I wait "5" seconds + And I log out + When I log in as "teacher1" + And I follow "Course 1" + And I navigate to "Edit settings" node in "Course administration" + # Wait for the autorestore + And I wait "2" seconds + Then I should see "This is my draft" + + @javascript + Scenario: Do not restore a draft if files have been modified + Given I log in as "teacher1" + And I follow "Course 1" + And I navigate to "Edit settings" node in "Course administration" + And I set the field "Course summary" to "This is my draft" + # Wait for the autosave + And I wait "5" seconds + And I log out + And I log in as "teacher2" + And I follow "Manage private files..." + And I upload "lib/editor/atto/tests/fixtures/moodle-logo.png" file to "Files" filemanager + And I click on "Save changes" "button" + And I click on "Dashboard" "link" in the "Navigation" "block" + And I follow "Course 1" + And I navigate to "Edit settings" node in "Course administration" + And I set the field "Course summary" to "

Image test

" + And I select the text in the "Course summary" Atto editor + And I click on "Image" "button" + And I click on "Browse repositories..." "button" + And I click on "Private files" "link" + And I click on "moodle-logo.png" "link" + And I click on "Select this file" "button" + And I set the field "Describe this image" to "It's the Moodle" + # Wait for the page to "settle". + And I wait until the page is ready + And I click on "Save image" "button" + And I click on "Save and display" "button" + And I log out + When I log in as "teacher1" + And I follow "Course 1" + And I navigate to "Edit settings" node in "Course administration" + Then I should not see "This is my draft" From 3dcdac00a16f062036476e31a494d2b941bff7fa Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 1 Sep 2015 12:23:00 +0800 Subject: [PATCH 4/4] MDL-51179 Atto: Extend autosave fix to cover text changes Added new functions to editor api - set/get_text so the original form text can be determined from an editor. When calling use_editor() you should first call set_text() with the text that will be inserted in the form element. There is also a new scheduled task for cleaning Atto autosave drafts. --- lib/adminlib.php | 1 + lib/deprecatedlib.php | 1 + .../classes/task/autosave_cleanup_task.php | 58 +++++++++++++++++++ lib/editor/atto/db/tasks.php | 44 ++++++++++++++ lib/editor/atto/lang/en/editor_atto.php | 1 + lib/editor/atto/lib.php | 4 +- lib/editor/atto/tests/behat/autosave.feature | 21 +++++++ lib/editor/atto/version.php | 2 +- lib/editorlib.php | 22 +++++++ lib/form/editor.php | 1 + lib/upgrade.txt | 3 + mod/data/field/textarea/field.class.php | 1 + mod/data/templates.php | 4 ++ question/behaviour/rendererbase.php | 1 + question/type/essay/renderer.php | 1 + 15 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 lib/editor/atto/classes/task/autosave_cleanup_task.php create mode 100644 lib/editor/atto/db/tasks.php diff --git a/lib/adminlib.php b/lib/adminlib.php index ef27f56716e..ec96ece61ce 100644 --- a/lib/adminlib.php +++ b/lib/adminlib.php @@ -2319,6 +2319,7 @@ class admin_setting_confightmleditor extends admin_setting_configtext { } $editor = editors_get_preferred_editor(FORMAT_HTML); + $editor->set_text($data); $editor->use_editor($this->get_id(), array('noclean'=>true)); return format_admin_setting($this, $this->visiblename, diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index 963c818c8ab..afa8c9bc767 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -1205,6 +1205,7 @@ function print_textarea($unused, $rows, $cols, $width, $height, $name, $value='' editors_head_setup(); $editor = editors_get_preferred_editor(FORMAT_HTML); + $editor->set_text($value); $editor->use_editor($id, array('legacy'=>true)); $str .= "\n".''; @@ -315,6 +316,7 @@ if ($mode == 'listtemplate'){ } $field = 'template'; +$editor->set_text($data->{$mode}); $editor->use_editor($field, $options); echo '
'; echo ''; @@ -327,6 +329,7 @@ if ($mode == 'listtemplate'){ echo '
'; $field = 'listtemplatefooter'; + $editor->set_text($data->listtemplatefooter); $editor->use_editor($field, $options); echo '
'; echo ''; @@ -338,6 +341,7 @@ if ($mode == 'listtemplate'){ echo '
'; $field = 'rsstitletemplate'; + $editor->set_text($data->rsstitletemplate); $editor->use_editor($field, $options); echo '
'; echo ''; diff --git a/question/behaviour/rendererbase.php b/question/behaviour/rendererbase.php index 0397da02797..0197329af57 100644 --- a/question/behaviour/rendererbase.php +++ b/question/behaviour/rendererbase.php @@ -82,6 +82,7 @@ abstract class qbehaviour_renderer extends plugin_renderer_base { $commenttext = format_text($commenttext, $commentformat, array('para' => false)); + $editor->set_text($commenttext); $editor->use_editor($id, array('context' => $options->context)); $commenteditor = html_writer::tag('div', html_writer::tag('textarea', s($commenttext), diff --git a/question/type/essay/renderer.php b/question/type/essay/renderer.php index 222e2a44410..f8db5516dec 100644 --- a/question/type/essay/renderer.php +++ b/question/type/essay/renderer.php @@ -236,6 +236,7 @@ class qtype_essay_format_editor_renderer extends plugin_renderer_base { list($draftitemid, $response) = $this->prepare_response_for_editing( $name, $step, $context); + $editor->set_text($response); $editor->use_editor($id, $this->get_editor_options($context), $this->get_filepicker_options($context, $draftitemid));