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 2.4 Branch:
      Please cherry-pick.
    • Pull Master Branch:
      Please cherry-pick.
    • Rank:
      47159

      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.

        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: