Moodle
  1. Moodle
  2. MDL-37514

Quiz is not automatically submitted in preview mode when time is up

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.4
    • Fix Version/s: 2.3.7, 2.4.4
    • Component/s: Quiz
    • Labels:
    • Testing Instructions:
      Hide

      (difficulty: easy)

      1) Create a quiz
      2) Set a timelimit about 20-30 seconds.
      3) Add couple of questions to quiz
      4) Preview the quiz as teacher or admin
      5) Quiz should be submitted automatically when time is up.
      6) Now attempt the quiz as a student. Make sure the quiz automatically submits for them too.

      Show
      (difficulty: easy) 1) Create a quiz 2) Set a timelimit about 20-30 seconds. 3) Add couple of questions to quiz 4) Preview the quiz as teacher or admin 5) Quiz should be submitted automatically when time is up. 6) Now attempt the quiz as a student. Make sure the quiz automatically submits for them too.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      Please cherry-pick.

      Description

      With the latest Moodle update(2.3.4), when teacher is previewing the quiz and quiz has been set to submit automatically open attempts, then it does'nt do that when time is up.
      If you enter with(or as) a student account then it does submit when time is up.
      So probably something is wrong with the quiz preview mode for teacher.

      Steps to reproduce:
      1) Create a quiz
      2) Set a timelimit about 20-30 seconds.
      3) When time expires set Open attempts are submitted automatically.
      4) Add couple of questions to quiz
      5) Preview the quiz as teacher or admin
      6) Quiz should be submitted automatically when time is up.

      I tried to reproduce this at demo.moodle.org but there is still 2.3.2 running. This happens with 2.3.4.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Tõnis Tartes added a comment -

            It's probably not a big problem for students, but teachers might think that quiz is broken somehow.

            Show
            Tõnis Tartes added a comment - It's probably not a big problem for students, but teachers might think that quiz is broken somehow.
            Hide
            Tim Hunt added a comment -

            Matt, is this related to MDL-35717?

            Show
            Tim Hunt added a comment - Matt, is this related to MDL-35717 ?
            Hide
            Matt Petro added a comment -

            Right, the behavior changed in MDL-35717. As of 2.3.4 the countdown timer is displayed in a preview, but there is no submit when it gets to zero. If this is causing confusion then maybe we should popup an alert that says the time expired, or change the timer text from 0:0:0 to "Times up!".

            There was a bit on inconsistency in 2.3.3 between the javascript countdown timer which closes the preview, and the cron job which ignores previews. I was trying to straighten that out.

            Show
            Matt Petro added a comment - Right, the behavior changed in MDL-35717 . As of 2.3.4 the countdown timer is displayed in a preview, but there is no submit when it gets to zero. If this is causing confusion then maybe we should popup an alert that says the time expired, or change the timer text from 0:0:0 to "Times up!". There was a bit on inconsistency in 2.3.3 between the javascript countdown timer which closes the preview, and the cron job which ignores previews. I was trying to straighten that out.
            Hide
            Nikunj Thakkar added a comment - - edited

            I have checked the code for version 2.5.This issue is because it is set disabled by default for preview.Although if it is needed,Let me know I will update the code.

            Show
            Nikunj Thakkar added a comment - - edited I have checked the code for version 2.5.This issue is because it is set disabled by default for preview.Although if it is needed,Let me know I will update the code.
            Hide
            Tim Hunt added a comment -

            Right, there are two issues here:

            1. What behaviour is most helpful for teachers? Probably the only way to try to answer that is to ask in the quiz forum: https://moodle.org/mod/forum/view.php?id=737.

            2. If we decide that the quiz should auto-submit, how should we change the code? If you are planning to apply as a GSoC student, then making a patch that implements that demonstrates your ability to write code, whether or not it ends up being integrated, so you can still make a patch.

            Show
            Tim Hunt added a comment - Right, there are two issues here: 1. What behaviour is most helpful for teachers? Probably the only way to try to answer that is to ask in the quiz forum: https://moodle.org/mod/forum/view.php?id=737 . 2. If we decide that the quiz should auto-submit, how should we change the code? If you are planning to apply as a GSoC student, then making a patch that implements that demonstrates your ability to write code, whether or not it ends up being integrated, so you can still make a patch.
            Hide
            Nikunj Thakkar added a comment - - edited

            Yes I am planning to apply for GSoC.I will make the patch and submit here.

            Show
            Nikunj Thakkar added a comment - - edited Yes I am planning to apply for GSoC.I will make the patch and submit here.
            Hide
            Nikunj Thakkar added a comment -

            Dear Sir,

            Here is the link of patch file.I have made some changes in the code and now its working as per the requirement.

            https://github.com/nikunjness/moodle/commit/ff8239fc4c610c1401268f24e0b5925b076de117

            Let me know if is it sufficient or not?

            Show
            Nikunj Thakkar added a comment - Dear Sir, Here is the link of patch file.I have made some changes in the code and now its working as per the requirement. https://github.com/nikunjness/moodle/commit/ff8239fc4c610c1401268f24e0b5925b076de117 Let me know if is it sufficient or not?
            Hide
            Tim Hunt added a comment -

            Thanks for working on this. You are on the right lines, but what you have done is not complete yet.

            Did you test? (How did you test?) Hint: You have only solved part of the problem so far.

            Second, to comment on the format of the patch:

            1. You should not comment out code and leave it in the code-base. That just leaves a mess for the future. Do not worry about deleting old code. If someone wants to find it again, then can use git to get the old version back.

            2. There is a standard way to write commit messages for Moodle, so that they are easy to read. See http://docs.moodle.org/dev/Commit_cheat_sheet#Provide_clear_commit_messages
            To see why this is a good thing, type the command
            git log --oneline --no-merges
            and note that the consistent format makes it easier to read.

            Show
            Tim Hunt added a comment - Thanks for working on this. You are on the right lines, but what you have done is not complete yet. Did you test? (How did you test?) Hint: You have only solved part of the problem so far. Second, to comment on the format of the patch: 1. You should not comment out code and leave it in the code-base. That just leaves a mess for the future. Do not worry about deleting old code. If someone wants to find it again, then can use git to get the old version back. 2. There is a standard way to write commit messages for Moodle, so that they are easy to read. See http://docs.moodle.org/dev/Commit_cheat_sheet#Provide_clear_commit_messages To see why this is a good thing, type the command git log --oneline --no-merges and note that the consistent format makes it easier to read.
            Hide
            Nikunj Thakkar added a comment -

            Thank you for your guidance.It will be helpful to me.I have tested it on my local machine and after removing those lines quiz was getting submitted in preview mode.There needs to be an option for making it available after preview and I think I am missing this part.I will work on it and let you know about the progress.

            Show
            Nikunj Thakkar added a comment - Thank you for your guidance.It will be helpful to me.I have tested it on my local machine and after removing those lines quiz was getting submitted in preview mode.There needs to be an option for making it available after preview and I think I am missing this part.I will work on it and let you know about the progress.
            Hide
            Nikunj Thakkar added a comment -

            Dear sir,

            I have worked on it and figured out why this is happening.I have tried to solve it out but got stuck in code.It will be very kind of you if you help me a little to solve this problem.

            Show
            Nikunj Thakkar added a comment - Dear sir, I have worked on it and figured out why this is happening.I have tried to solve it out but got stuck in code.It will be very kind of you if you help me a little to solve this problem.
            Hide
            Tim Hunt added a comment -

            Have you tried attempting a timed quiz as a student, to see what happens? You need to ask, why does it work for students, but not for teachers.

            Show
            Tim Hunt added a comment - Have you tried attempting a timed quiz as a student, to see what happens? You need to ask, why does it work for students, but not for teachers.
            Hide
            Nikunj Thakkar added a comment -

            Yes I have tried.It is because for preview user we are creating an object of accessmanager having permissions of ignoring time limits (i.e $canignoretimelimit).I have tried to change it in rule access base but it was still not working.We need to change this but unable to do so.
            You can check it here https://github.com/nikunjness/moodle/blob/master/mod/quiz/accessmanager.php
            At line 56.

            Show
            Nikunj Thakkar added a comment - Yes I have tried.It is because for preview user we are creating an object of accessmanager having permissions of ignoring time limits (i.e $canignoretimelimit).I have tried to change it in rule access base but it was still not working.We need to change this but unable to do so. You can check it here https://github.com/nikunjness/moodle/blob/master/mod/quiz/accessmanager.php At line 56.
            Hide
            Tim Hunt added a comment -

            Well, what I was trying to get at is that what actually submits the quiz automatically is some JavaScript code. Have you looked at that?

            Show
            Tim Hunt added a comment - Well, what I was trying to get at is that what actually submits the quiz automatically is some JavaScript code. Have you looked at that?
            Hide
            Nikunj Thakkar added a comment -

            Sorry for too late reply.I will check it out and report to you.I am having exams now so unable to dig the code right now.But I will do it as soon as I get some time to look.Thank you for your guidance.

            Show
            Nikunj Thakkar added a comment - Sorry for too late reply.I will check it out and report to you.I am having exams now so unable to dig the code right now.But I will do it as soon as I get some time to look.Thank you for your guidance.
            Hide
            Alex Leontiev added a comment - - edited

            It's probably true, that bug is almost solved by you, guys.
            I just wanted to tell that I confirm it too, and I'm trying to work on it right now by reading mod/quiz/attempt.php and looking on javascript on page.

            Show
            Alex Leontiev added a comment - - edited It's probably true, that bug is almost solved by you, guys. I just wanted to tell that I confirm it too, and I'm trying to work on it right now by reading mod/quiz/attempt.php and looking on javascript on page.
            Hide
            Tim Hunt added a comment -

            I'll give you a hint, the JavaScript you are looking for is in mod/quiz/module.js

            Show
            Tim Hunt added a comment - I'll give you a hint, the JavaScript you are looking for is in mod/quiz/module.js
            Hide
            Alex Leontiev added a comment - - edited

            Thanks for the hint, Tim. I've even figured that in fact Moodle is caching javascripts, so putting debug statements in mod/quiz/moodle.js is of no use, unless you turn off the caching.

            Starting to love Moodle

            Show
            Alex Leontiev added a comment - - edited Thanks for the hint, Tim. I've even figured that in fact Moodle is caching javascripts, so putting debug statements in mod/quiz/moodle.js is of no use, unless you turn off the caching. Starting to love Moodle
            Hide
            Alex Leontiev added a comment -

            Tim, I don't know whether it's what we are looking for or not, but when I'm starting to take quiz as admin (in attempt mode, I swear), at some point variable at M.mod_quiz.timer.preview is becoming true (although it probably shouldn't, according to my understanding). Hence, when time is up in an update function in moodle.js we are getting into (M.mod_quiz.timer.preview && secondsleft < 0), not in (secondsleft<0), as we'd like to.

            I don't know what happens at student's view, though, since I have only one user in my Moodle at this moment.

            Show
            Alex Leontiev added a comment - Tim, I don't know whether it's what we are looking for or not, but when I'm starting to take quiz as admin (in attempt mode, I swear), at some point variable at M.mod_quiz.timer.preview is becoming true (although it probably shouldn't, according to my understanding). Hence, when time is up in an update function in moodle.js we are getting into (M.mod_quiz.timer.preview && secondsleft < 0), not in (secondsleft<0), as we'd like to. I don't know what happens at student's view, though, since I have only one user in my Moodle at this moment.
            Hide
            Tim Hunt added a comment -

            1. For doing development, you need to set some Moodle config options:

            • Set Debugging to developer level, and display debug messages on.
            • Theme designer mode on.
            • Cache lang strings off.
            • Cache JavaScript off.
            • Cache filtered text off.

            Also, it is worth creating some extra users in your test Moodel site, so you can log in as a Student or Teacher. (Admin -> Users -> Add new user ...)

            M.mod_quiz.timer.preview should be true if you are admin or teacher, but it should not prevent submission.

            I know there is a lot to learn (this is one of the reasons we ask you to fix a bug.)

            Show
            Tim Hunt added a comment - 1. For doing development, you need to set some Moodle config options: Set Debugging to developer level, and display debug messages on. Theme designer mode on. Cache lang strings off. Cache JavaScript off. Cache filtered text off. Also, it is worth creating some extra users in your test Moodel site, so you can log in as a Student or Teacher. (Admin -> Users -> Add new user ...) M.mod_quiz.timer.preview should be true if you are admin or teacher, but it should not prevent submission. I know there is a lot to learn (this is one of the reasons we ask you to fix a bug.)
            Hide
            Nikunj Thakkar added a comment -

            I have already applied above steps when you mentioned it in your post.But still getting same problem.I have also deleted the JavaScript code which was preventing the time to stop after time out.I think its little bit tricky and we have to figure out something that can't be seen by giving one or two tries.Still working on it and definitely try to fix this.

            Show
            Nikunj Thakkar added a comment - I have already applied above steps when you mentioned it in your post.But still getting same problem.I have also deleted the JavaScript code which was preventing the time to stop after time out.I think its little bit tricky and we have to figure out something that can't be seen by giving one or two tries.Still working on it and definitely try to fix this.
            Hide
            Alex Leontiev added a comment -

            Well, I think preview=true is really the thing that prevents submission. In that if clause in "update" (M.mod_quiz.timer.preview && secondsleft < 0) - there's a return statement there. So, if we get there, we don't get further

            Show
            Alex Leontiev added a comment - Well, I think preview=true is really the thing that prevents submission. In that if clause in "update" (M.mod_quiz.timer.preview && secondsleft < 0) - there's a return statement there. So, if we get there, we don't get further
            Hide
            Nikunj Thakkar added a comment -

            Alex,I have deleted that part of code from the file and tried to submit the quiz as it is happening for normal user but it is not getting submitted as expected.

            Show
            Nikunj Thakkar added a comment - Alex,I have deleted that part of code from the file and tried to submit the quiz as it is happening for normal user but it is not getting submitted as expected.
            Hide
            Alex Leontiev added a comment -

            Tim, when I manually set M.mod_quiz.timer.preview=false on each update it DOES submit! Even for admin!

            Show
            Alex Leontiev added a comment - Tim, when I manually set M.mod_quiz.timer.preview=false on each update it DOES submit! Even for admin!
            Hide
            Alex Leontiev added a comment - - edited

            Nikunj, please take a look at my module.js,
            located at http://people.cs.nctu.edu.tw/~inp9822058/module.js

            there are two if's there, we should get at second one to submit quiz on time, instead we get in first one, because preview is set to true, and there's return in that
            first if

            Show
            Alex Leontiev added a comment - - edited Nikunj, please take a look at my module.js, located at http://people.cs.nctu.edu.tw/~inp9822058/module.js there are two if's there, we should get at second one to submit quiz on time, instead we get in first one, because preview is set to true, and there's return in that first if
            Hide
            Alex Leontiev added a comment - - edited

            in my opinion, the very reason of the bug is that when we are creating $attemptobj for admin in attempt.php, preview, $attemptobj->attempt->preview gets true. I think, that's the problem.

            Because of this quiz does not become submitted on time.

            I have a first-draft patch, ready to submit, but I'd like to talk to some more experienced person first. Can I?

            Show
            Alex Leontiev added a comment - - edited in my opinion, the very reason of the bug is that when we are creating $attemptobj for admin in attempt.php, preview, $attemptobj->attempt->preview gets true. I think, that's the problem. Because of this quiz does not become submitted on time. I have a first-draft patch, ready to submit, but I'd like to talk to some more experienced person first. Can I?
            Hide
            Nikunj Thakkar added a comment -

            is it working now? If so then you should submit the patch.Ask Tim if you want to confirm further.

            Show
            Nikunj Thakkar added a comment - is it working now? If so then you should submit the patch.Ask Tim if you want to confirm further.
            Hide
            Alex Leontiev added a comment -

            I cannot find Tim at this point. As soon, as he'll appear, I'll let him know.

            Show
            Alex Leontiev added a comment - I cannot find Tim at this point. As soon, as he'll appear, I'll let him know.
            Hide
            Tim Hunt added a comment -

            The quiz has two modes:
            preview = 0: A real quiz attempt. This is what happens for students.
            preview = 1: A preview attempt. This is not a real attempt. It does not show up in the quiz reports, and it is likely to be automatically deleted at any time.

            The point of previews is to let teachers get a rough impression of how things will look to students, but without polluting the reports etc.

            This information is stored in the quiz_attempts.preview column in the database, it is a property of the quiz_attempt class in the code, and we pass it to JavaScript.

            So, the fact that M.mod_quiz.timer.preview gets set to true for Admins and Teachers is a good thing. It lets the JavaScript know what type of attempt this is, if applicable. (Actually it is controlled by the mod/quiz:preview capability, not by specific roles.)

            So, M.mod_quiz.timer.preview being true, it is the test
            if (M.mod_quiz.timer.preview && secondsleft < 0) {
            that is a problem.

            Show
            Tim Hunt added a comment - The quiz has two modes: preview = 0: A real quiz attempt. This is what happens for students. preview = 1: A preview attempt. This is not a real attempt. It does not show up in the quiz reports, and it is likely to be automatically deleted at any time. The point of previews is to let teachers get a rough impression of how things will look to students, but without polluting the reports etc. This information is stored in the quiz_attempts.preview column in the database, it is a property of the quiz_attempt class in the code, and we pass it to JavaScript. So, the fact that M.mod_quiz.timer.preview gets set to true for Admins and Teachers is a good thing. It lets the JavaScript know what type of attempt this is, if applicable. (Actually it is controlled by the mod/quiz:preview capability, not by specific roles.) So, M.mod_quiz.timer.preview being true, it is the test if (M.mod_quiz.timer.preview && secondsleft < 0) { that is a problem.
            Hide
            Alex Leontiev added a comment -

            https://github.com/nailbiter/moodle/commit/88a24295cc85d65677639a614b01b6348b77d2f2

            here's my attempt to fix. Please, let me know if there's something wrong with it.

            Show
            Alex Leontiev added a comment - https://github.com/nailbiter/moodle/commit/88a24295cc85d65677639a614b01b6348b77d2f2 here's my attempt to fix. Please, let me know if there's something wrong with it.
            Hide
            Alex Leontiev added a comment -

            https://github.com/nailbiter/moodle/compare/MOODLE_23_STABLE...wip-MDL-37514

            Peer review would be very helpful, for I don't know how the correct behaviour in all use cases should look like

            Show
            Alex Leontiev added a comment - https://github.com/nailbiter/moodle/compare/MOODLE_23_STABLE...wip-MDL-37514 Peer review would be very helpful, for I don't know how the correct behaviour in all use cases should look like
            Hide
            Tim Hunt added a comment -

            0. An undocumented github feature that makes the change easier to review is to add ?w=0 at the end of the URL. https://github.com/nailbiter/moodle/compare/MOODLE_23_STABLE...wip-MDL-37514?w=0

            1. I thought that code checker checked .js files, anyway, you still have some codeing style violations.

            2. I don't think there is any point to the different display Y.one('#quiz-time-left').setContent('0:00:00'); or Y.one('#quiz-time-left').setContent(M.str.quiz.timesup); depending on whether this is a preview or not. Would you like to amend the commit again?

            3. In the commit comment, you say "I does not seems like a accidental bug, rather it looks like misunderstanding." You are correct about that.

            4. This is not common criticism, but I think the commit comment is too long!

            • The first line of the commit comment is good. (Not sure I would mention module.js, might be worth editing it to include the word preview?)
            • The testing instructions should be here in this issue, not be in the commit comment.
            Show
            Tim Hunt added a comment - 0. An undocumented github feature that makes the change easier to review is to add ?w=0 at the end of the URL. https://github.com/nailbiter/moodle/compare/MOODLE_23_STABLE...wip-MDL-37514?w=0 1. I thought that code checker checked .js files, anyway, you still have some codeing style violations. 2. I don't think there is any point to the different display Y.one('#quiz-time-left').setContent('0:00:00'); or Y.one('#quiz-time-left').setContent(M.str.quiz.timesup); depending on whether this is a preview or not. Would you like to amend the commit again? 3. In the commit comment, you say "I does not seems like a accidental bug, rather it looks like misunderstanding." You are correct about that. 4. This is not common criticism, but I think the commit comment is too long! The first line of the commit comment is good. (Not sure I would mention module.js, might be worth editing it to include the word preview?) The testing instructions should be here in this issue, not be in the commit comment.
            Hide
            Alex Leontiev added a comment -

            Hope, this attempt will be better
            https://github.com/nailbiter/moodle/compare/MOODLE_23_STABLE...wip-MDL-37514?w=0

            Again, as I'm a novice developer, please help me with the peer review!

            Testing instructions:
            (difficulty: easy, requires teacher/admin account)
            1) Create a quiz
            2) Set a timelimit about 20-30 seconds.
            3) When time expires set "Open attempts are submitted automatically".
            4) Add couple of questions to quiz
            5) Preview the quiz as teacher or admin
            6) Quiz should be submitted automatically when time is up.

            Please, check also the correct behaviour in case when "Open attempts are submitted automatically" is not set - I'm not sure what
            the behavior in that case should be.

            Show
            Alex Leontiev added a comment - Hope, this attempt will be better https://github.com/nailbiter/moodle/compare/MOODLE_23_STABLE...wip-MDL-37514?w=0 Again, as I'm a novice developer, please help me with the peer review! Testing instructions: (difficulty: easy, requires teacher/admin account) 1) Create a quiz 2) Set a timelimit about 20-30 seconds. 3) When time expires set "Open attempts are submitted automatically". 4) Add couple of questions to quiz 5) Preview the quiz as teacher or admin 6) Quiz should be submitted automatically when time is up. Please, check also the correct behaviour in case when "Open attempts are submitted automatically" is not set - I'm not sure what the behavior in that case should be.
            Hide
            Tim Hunt added a comment -

            Great. That patch is now spot on. Submitting for integration.

            To INTEGRATORS. Please cherry-pick this fix to 2.4 STABLE.

            (The change in behaviour from 2.3 -> 2.4 was a mistake, so don't worry about a minor UI change on the stable branch. Every bug fix is a UI change after all!)

            Show
            Tim Hunt added a comment - Great. That patch is now spot on. Submitting for integration. To INTEGRATORS. Please cherry-pick this fix to 2.4 STABLE. (The change in behaviour from 2.3 -> 2.4 was a mistake, so don't worry about a minor UI change on the stable branch. Every bug fix is a UI change after all!)
            Hide
            Tim Hunt added a comment -

            OK, correction. This fix is needed in all three branches 2.3, 2.4 & master. It should cherry-pick cleanly.

            Show
            Tim Hunt added a comment - OK, correction. This fix is needed in all three branches 2.3, 2.4 & master. It should cherry-pick cleanly.
            Hide
            Dan Poltawski added a comment -

            Integrated to master, 24 and 23 - thanks!

            (It didn't cherry-pick cleanly )

            Show
            Dan Poltawski added a comment - Integrated to master, 24 and 23 - thanks! (It didn't cherry-pick cleanly )
            Hide
            Andrew Davis added a comment -

            Seems to be working as it should. Passing.

            Show
            Andrew Davis added a comment - Seems to be working as it should. Passing.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            I feel myself really alone tonight! So was time to push your fixes upstream!

            "Lest we forget. We will remember them."

            Thanks and ciao!

            Show
            Eloy Lafuente (stronk7) added a comment - I feel myself really alone tonight! So was time to push your fixes upstream! "Lest we forget. We will remember them." Thanks and ciao!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: