Moodle
  1. Moodle
  2. MDL-38935

Quiz autosave does not save Essay responses

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide
      1. Create a quiz with at least one essay question.
      2. Do steps 1-5 of the testing instructions for MDL-38538, concentrating on the essay question.

      (You can also use Firebug to verify that the contents of the editor are sent to the server in the Autosave post request.)

      Show
      Create a quiz with at least one essay question. Do steps 1-5 of the testing instructions for MDL-38538 , concentrating on the essay question. (You can also use Firebug to verify that the contents of the editor are sent to the server in the Autosave post request.)
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      This follows on from MDL-38538. We need to work harder to get those responses out of TinyMCE.

      While doing work here, also:
      1. Ignore changes to scrollpos.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Tim Hunt added a comment -

            Submitting for integration. That was easier than I expected.

            Show
            Tim Hunt added a comment - Submitting for integration. That was easier than I expected.
            Hide
            Damyon Wiese added a comment -

            Hi Tim,

            Whilst I agree with the change for set_timeout in phpunit - this needs to be done in it's own issue so the history can be tracked. This change also detects several tests that are changing this value which need to be fixed before this can be integrated. Please split that change into it's own issue and remove the change from this branch.

            FYI - Running on master I get these fails with this patch:

            1) core_textlib_testcase::test_parse_charset
            Warning: max_execution_time was changed.
             
            /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/util.php:220
            /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/advanced_testcase.php:118
             
            To re-run:
             ./vendor/bin/phpunit core_textlib_testcase lib/tests/textlib_test.php
             
            2) qubaid_condition_test::test_qubaid_list_one_join
            Warning: max_execution_time was changed.
             
            /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/util.php:220
            /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/advanced_testcase.php:118
             
            To re-run:
             ./vendor/bin/phpunit qubaid_condition_test question/engine/tests/datalib_test.php
             
            3) mod_lti_locallib_testcase::test_split_custom_parameters
            Warning: max_execution_time was changed.
             
            /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/util.php:220
            /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/basic_testcase.php:76
             
            To re-run:
             ./vendor/bin/phpunit mod_lti_locallib_testcase mod/lti/tests/locallib_test.php
            

            The rest of this patch looks good and is fine for integration once the above is addressed.

            Thanks, Damyon

            Show
            Damyon Wiese added a comment - Hi Tim, Whilst I agree with the change for set_timeout in phpunit - this needs to be done in it's own issue so the history can be tracked. This change also detects several tests that are changing this value which need to be fixed before this can be integrated. Please split that change into it's own issue and remove the change from this branch. FYI - Running on master I get these fails with this patch: 1) core_textlib_testcase::test_parse_charset Warning: max_execution_time was changed.   /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/util.php:220 /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/advanced_testcase.php:118   To re-run: ./vendor/bin/phpunit core_textlib_testcase lib/tests/textlib_test.php   2) qubaid_condition_test::test_qubaid_list_one_join Warning: max_execution_time was changed.   /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/util.php:220 /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/advanced_testcase.php:118   To re-run: ./vendor/bin/phpunit qubaid_condition_test question/engine/tests/datalib_test.php   3) mod_lti_locallib_testcase::test_split_custom_parameters Warning: max_execution_time was changed.   /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/util.php:220 /home/damyonw/Documents/Moodle/integration/master/moodle/lib/phpunit/classes/basic_testcase.php:76   To re-run: ./vendor/bin/phpunit mod_lti_locallib_testcase mod/lti/tests/locallib_test.php The rest of this patch looks good and is fine for integration once the above is addressed. Thanks, Damyon
            Hide
            Tim Hunt added a comment -

            Sorry, Damyon, the set-timeout thing is a separate issue, MDL-38912 to be precise. I course, I needed those changes in my workspace in order to run the unit tests, and I must have been careless with git commit -a. Sorry, I will fix this soon.

            Show
            Tim Hunt added a comment - Sorry, Damyon, the set-timeout thing is a separate issue, MDL-38912 to be precise. I course, I needed those changes in my workspace in order to run the unit tests, and I must have been careless with git commit -a. Sorry, I will fix this soon.
            Hide
            Tim Hunt added a comment -

            OK, commit fixed. Sorry for the confusing Damyon.

            Show
            Tim Hunt added a comment - OK, commit fixed. Sorry for the confusing Damyon.
            Hide
            Damyon Wiese added a comment -

            Sorry - for the delay - this has been integrated for master now - unit tests passed.

            Also - only I can apologise for the confusing Damyon

            Show
            Damyon Wiese added a comment - Sorry - for the delay - this has been integrated for master now - unit tests passed. Also - only I can apologise for the confusing Damyon
            Hide
            Tim Hunt added a comment -

            LOL. Thanks Damyon. (Obviously, that was a typo for confusion.)

            Show
            Tim Hunt added a comment - LOL. Thanks Damyon. (Obviously, that was a typo for confusion.)
            Hide
            Dan Poltawski added a comment -

            It works.

            It seemsed like it took a long time to do the autosave (I had it configured to 1mins and it seeeme to me to be longer than that). Does it reset its counter when it sees changes? (I think switching between tabs caused the field to have a change detected and wondered if that did that).

            Show
            Dan Poltawski added a comment - It works. It seemsed like it took a long time to do the autosave (I had it configured to 1mins and it seeeme to me to be longer than that). Does it reset its counter when it sees changes? (I think switching between tabs caused the field to have a change detected and wondered if that did that).
            Hide
            Tim Hunt added a comment -

            It starts a timer the first time it detects a change. The timer is not reset on subsequent edits.

            It does seem that TinyMCE reports a change when it loses focus. I don't know why it does that, but I don't think it is a big problem, so I do not propose to fight with it to avoid saving in that case.

            Show
            Tim Hunt added a comment - It starts a timer the first time it detects a change. The timer is not reset on subsequent edits. It does seem that TinyMCE reports a change when it loses focus. I don't know why it does that, but I don't think it is a big problem, so I do not propose to fight with it to avoid saving in that case.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Your awesome contributions are now part of Moodle, your fav LMS out there.

            Closing this as fixed.

            Many thanks for all the hard work, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Your awesome contributions are now part of Moodle, your fav LMS out there. Closing this as fixed. Many thanks for all the hard work, ciao

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: