Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.0, 2.3
    • Fix Version/s: 2.3
    • Component/s: Quiz
    • Labels:
      None
    • Environment:
      All
    • Testing Instructions:
      Hide

      1. Create a quiz with several pages.

      2. Start attempting the quiz as a student. Half way through, stop, noting which page you are currently on.

      3. Go back in and continue the attempt. Make sure you are automatically taken back to the page you were on.

      4. Finish off the attempt and submit. Make sure there are no regressions.

      Show
      1. Create a quiz with several pages. 2. Start attempting the quiz as a student. Half way through, stop, noting which page you are currently on. 3. Go back in and continue the attempt. Make sure you are automatically taken back to the page you were on. 4. Finish off the attempt and submit. Make sure there are no regressions.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      It would be good if the attempt could remember the page the student was last on so that if the student comes back to continue the attempt he is still on the same page.

      Very minor and low priority.

        Gliffy Diagrams

          Issue Links

            Activity

            Anonymous created issue -
            Michael Blake made changes -
            Field Original Value New Value
            Reporter anonymous [ anonymous ]
            Tim Hunt made changes -
            Status In Progress [ 3 ] Open [ 1 ]
            Martin Dougiamas made changes -
            Workflow jira [ 13735 ] MDL Workflow [ 41250 ]
            Martin Dougiamas made changes -
            Workflow MDL Workflow [ 41250 ] MDL Full Workflow [ 69719 ]
            Hide
            Charles Fulton added a comment -

            I'll throw some code out here just to get things started: https://github.com/mackensen/moodle/compare/master...MDL-3054. I admit that I don't know the appropriate way to bump a version number.

            I think this makes sense as default quiz behaviour; should it be teacher-configurable?

            Show
            Charles Fulton added a comment - I'll throw some code out here just to get things started: https://github.com/mackensen/moodle/compare/master...MDL-3054 . I admit that I don't know the appropriate way to bump a version number. I think this makes sense as default quiz behaviour; should it be teacher-configurable?
            Hide
            Tim Hunt added a comment -

            Proof of concept is easy (although I note that you have added one extra DB query to every page-view of attempt.php, so not that easy to get right). What is needed is a complete solution that dots all the is and crosses all the ts.

            • I would always start new attempts from page 1 (or is it 0), so don't do that change in startattempt.php
            • In the DB definition, I would put the currentpage column next to layout.
            • You don't yet have any code to go to the current page when the attempt is resumed.
            Show
            Tim Hunt added a comment - Proof of concept is easy (although I note that you have added one extra DB query to every page-view of attempt.php, so not that easy to get right). What is needed is a complete solution that dots all the is and crosses all the ts. I would always start new attempts from page 1 (or is it 0), so don't do that change in startattempt.php In the DB definition, I would put the currentpage column next to layout. You don't yet have any code to go to the current page when the attempt is resumed.
            Hide
            Charles Fulton added a comment -

            @Tim thanks for responding. New attempts start at page 0 and this code doesn't change that. The line in startattempt.php does resume attempts from where the user left off (at least when I tested it). I'll take another look at attempt.php and try to make that more efficient.

            Show
            Charles Fulton added a comment - @Tim thanks for responding. New attempts start at page 0 and this code doesn't change that. The line in startattempt.php does resume attempts from where the user left off (at least when I tested it). I'll take another look at attempt.php and try to make that more efficient.
            Hide
            Tim Hunt added a comment -

            Ah, I was misreading that change in startattempt.php. However, you are assuming that students always get to attempt.php from startattempt.php. This is not try. Try searching the code for "->attempt_url(" and "attempt.php".

            Show
            Tim Hunt added a comment - Ah, I was misreading that change in startattempt.php. However, you are assuming that students always get to attempt.php from startattempt.php. This is not try. Try searching the code for "->attempt_url(" and "attempt.php".
            Hide
            Charles Fulton added a comment -

            Amended and updated. The only other point of entry I see for a student is review.php, and I would think with a review the student would want to start at the beginning of the quiz.

            Show
            Charles Fulton added a comment - Amended and updated. The only other point of entry I see for a student is review.php, and I would think with a review the student would want to start at the beginning of the quiz.
            Hide
            Tim Hunt added a comment -

            I will try to look tomorrow, but not promises.

            Show
            Tim Hunt added a comment - I will try to look tomorrow, but not promises.
            Tim Hunt made changes -
            Status Open [ 1 ] Peer review in progress [ 10013 ]
            Hide
            Tim Hunt added a comment -

            I think we are getting there, but more review comments:

            1. In mod/quiz/attempt.php, better to use set_field than update_record.

            2. In startattempt.php, I think a page= in the URL should override the one stored in the DB, so change the default for optional_param to -1, and then only do your new line of code if $page is -1 (which will be the most common case).

            3. I just fixed a bug that altered mod/quiz/version.php, so once this weekly is out, you will have to rebase and fix merge conflicts.

            Show
            Tim Hunt added a comment - I think we are getting there, but more review comments: 1. In mod/quiz/attempt.php, better to use set_field than update_record. 2. In startattempt.php, I think a page= in the URL should override the one stored in the DB, so change the default for optional_param to -1, and then only do your new line of code if $page is -1 (which will be the most common case). 3. I just fixed a bug that altered mod/quiz/version.php, so once this weekly is out, you will have to rebase and fix merge conflicts.
            Tim Hunt made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Hide
            Charles Fulton added a comment -

            Rebased, merged and amended, with the suggested fixes.

            Show
            Charles Fulton added a comment - Rebased, merged and amended, with the suggested fixes.
            Hide
            Tim Hunt added a comment -

            One final thing:

            if ($attemptobj->get_currentpage() !== $page) {

            I think that should be !=

            Show
            Tim Hunt added a comment - One final thing: if ($attemptobj->get_currentpage() !== $page) { I think that should be !=
            Hide
            Charles Fulton added a comment -

            Makes sense to me. Amended.

            Show
            Charles Fulton added a comment - Makes sense to me. Amended.
            Hide
            Tim Hunt added a comment -

            Thanks for sticking with it Charles. Submitting for integration now.

            Show
            Tim Hunt added a comment - Thanks for sticking with it Charles. Submitting for integration now.
            Tim Hunt made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Pull Master Diff URL https://github.com/mackensen/moodle/compare/master...MDL-3054
            Pull Master Branch MDL-3054
            Pull from Repository git://github.com/mackensen/moodle.git
            Fix Version/s 2.3 [ 10657 ]
            Peer reviewer timhunt
            Testing Instructions 1. Create a quiz with several pages.

            2. Start attempting the quiz as a student. Half way through, stop, noting which page you are currently on.

            3. Go back in and continue the attempt. Make sure you are automatically taken back to the page you were on.

            4. Finish off the attempt and submit. Make sure there are no regressions.
            Tim Hunt made changes -
            Assignee Tim Hunt [ timhunt ] Charles Fulton [ cfulton ]
            Tim Hunt made changes -
            Link This issue will help resolve MDL-11047 [ MDL-11047 ]
            Eloy Lafuente (stronk7) made changes -
            Currently in integration Yes [ 10041 ]
            Aparup Banerjee made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator nebgor
            Hide
            Aparup Banerjee added a comment -

            Thanks guys, this has been integrated into master after a quick whitespace cleanup.

            Show
            Aparup Banerjee added a comment - Thanks guys, this has been integrated into master after a quick whitespace cleanup.
            Aparup Banerjee made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Affects Version/s 2.3 [ 10657 ]
            Aparup Banerjee made changes -
            Labels docs_required
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Plz, amend this ASAP!

            Number of errors: 1
            Column currentpage of table quiz_attempts difference found in unsigned: false !== true
            

            aka, install says signed, upgrade says unsigned!

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Plz, amend this ASAP! Number of errors: 1 Column currentpage of table quiz_attempts difference found in unsigned: false !== true aka, install says signed, upgrade says unsigned! Ciao
            Hide
            Aparup Banerjee added a comment -

            gr, I've got this https://github.com/nebgor/moodle/compare/integration-master...MDL-3054 , git://github.com/nebgor/moodle.git branch MDL-3054

            I'm assuming that $currentpage isn't negative ever (from perusing the code) but i may be wrong Tim.

            tester: we may have to revert this if not fixed.

            Show
            Aparup Banerjee added a comment - gr, I've got this https://github.com/nebgor/moodle/compare/integration-master...MDL-3054 , git://github.com/nebgor/moodle.git branch MDL-3054 I'm assuming that $currentpage isn't negative ever (from perusing the code) but i may be wrong Tim. tester: we may have to revert this if not fixed.
            Hide
            Tim Hunt added a comment -

            No need to revert, I will fix it today.

            We really need to get rid of the whole unsigned thing completely.

            Show
            Tim Hunt added a comment - No need to revert, I will fix it today. We really need to get rid of the whole unsigned thing completely.
            Hide
            Tim Hunt added a comment -

            Apu, +1 for your fix. Thanks.

            Show
            Tim Hunt added a comment - Apu, +1 for your fix. Thanks.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Apu's commit has been integrated, and CI DB checks are happy now, so this can be normally tested. Thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - Apu's commit has been integrated, and CI DB checks are happy now, so this can be normally tested. Thanks!
            Jason Fowler made changes -
            Tester phalacee
            Jason Fowler made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            Jason Fowler added a comment -

            Works nicely

            Show
            Jason Fowler added a comment - Works nicely
            Jason Fowler made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            Eloy Lafuente (stronk7) added a comment -

            This is now available in the git and cvs repositories.

            Consider the responsibility of your fingerprints engraved there for future generations!

            Thanks for the work, closing, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - This is now available in the git and cvs repositories. Consider the responsibility of your fingerprints engraved there for future generations! Thanks for the work, closing, ciao
            Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 19/Jan/12
            Aparup Banerjee made changes -
            Link This issue will be resolved by MDL-31319 [ MDL-31319 ]
            Hide
            Chris Collman added a comment -

            Bless all for work on this. Know it is off the beaten path that social constructionist want to take

            Show
            Chris Collman added a comment - Bless all for work on this. Know it is off the beaten path that social constructionist want to take
            Hide
            Mary Cooch added a comment -

            Documented in 2.3 here http://docs.moodle.org/23/en/Using_Quiz so removing docs label.

            Show
            Mary Cooch added a comment - Documented in 2.3 here http://docs.moodle.org/23/en/Using_Quiz so removing docs label.
            Mary Cooch made changes -
            Labels docs_required
            Tim Hunt made changes -
            Link This issue caused a regression MDL-34451 [ MDL-34451 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: