Moodle
  1. Moodle
  2. MDL-27314

Not possible to delete or regrade selected quiz attempts in separate groups mode

    Details

    • Testing Instructions:
      Hide

      1. Create a course with two groups, with a teacher and student in each.
      2. Edit/override the teacher role, so it does not have 'access all groups'.
      3. Create a quiz set to separate groups mode, and add some questions.
      4. Attempt it as some students from each group.
      5. As teacher, verify that you can delete quiz attempts from your group (using the overview and/or responses report).
      6. Try to hack the system using firebug: Find the checkbox like <input type="checkbox" value="238" name="attemptid[]"> and edit the attempt id to point to the attempt of a student not in your group, then click the delete button. The attempt should not be deleted. (It is just silently skipped, there is no error message.)

      Note that the code is different in 2.0, so the fix is different there. This really needs to be tested separately in 2.0 and more recent branches.

      Show
      1. Create a course with two groups, with a teacher and student in each. 2. Edit/override the teacher role, so it does not have 'access all groups'. 3. Create a quiz set to separate groups mode, and add some questions. 4. Attempt it as some students from each group. 5. As teacher, verify that you can delete quiz attempts from your group (using the overview and/or responses report). 6. Try to hack the system using firebug: Find the checkbox like <input type="checkbox" value="238" name="attemptid[]"> and edit the attempt id to point to the attempt of a student not in your group, then click the delete button. The attempt should not be deleted. (It is just silently skipped, there is no error message.) Note that the code is different in 2.0, so the fix is different there. This really needs to be tested separately in 2.0 and more recent branches.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      17006

      Description

      I have a class of 20 students, all of whom who took a quiz once.

      One of these students attempts needs to be deleted. So in the Results window I select that attempt, and then click on the [Delete selected attempts] button. The page reloads, but the attempt is still there. I've turned on Debug message to developer, but get no error messages.

      I've indicated the priority of this issues as a 'Blocker'. I suppose it's not the end of the world, but as a feature that worked fine in 1.9, it seems to me to be something that should work in the newer and better version 2.

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          Well, it works for me on my 2.0.2+ test site. So, to make any progress, we need more information about what is going wrong on your site.

          Can you try creating a new test course and quiz, attempting it as a student, and then deleting that attempt?

          Show
          Tim Hunt added a comment - Well, it works for me on my 2.0.2+ test site. So, to make any progress, we need more information about what is going wrong on your site. Can you try creating a new test course and quiz, attempting it as a student, and then deleting that attempt?
          Hide
          Tim Hunt added a comment -

          Aha! the bug is with separate groups mode, and it is effectively the same issue causing this, and MDL-27315.

          Show
          Tim Hunt added a comment - Aha! the bug is with separate groups mode, and it is effectively the same issue causing this, and MDL-27315 .
          Hide
          Peter Ruthven-Stuart added a comment -

          Tim, thank you for your quick response.

          I can confirm that this bug is related to 'separate groups mode'. I just created a quiz in a new course and:

          1) when the course groups mode was on, but in the quiz settings groups mode was off, I was able to both regrade and delete an attempt

          2) when the course groups mode was on, AND in the quiz groups mode was ON, I was neither able to regrade nor delete "selected attempts" (i.e. the buttons at the bottom of the results), BUT was able to regrade using the [Full regrade for 'group x'] button at the top of the results.

          Very much looking forward to a fix for this.

          Show
          Peter Ruthven-Stuart added a comment - Tim, thank you for your quick response. I can confirm that this bug is related to 'separate groups mode'. I just created a quiz in a new course and: 1) when the course groups mode was on, but in the quiz settings groups mode was off, I was able to both regrade and delete an attempt 2) when the course groups mode was on, AND in the quiz groups mode was ON, I was neither able to regrade nor delete "selected attempts" (i.e. the buttons at the bottom of the results), BUT was able to regrade using the [Full regrade for 'group x'] button at the top of the results. Very much looking forward to a fix for this.
          Hide
          Artem Andreev added a comment -
          Show
          Artem Andreev added a comment - Solution provided by Daniel Jann: http://moodle.org/mod/forum/discuss.php?d=182524#p803358
          Hide
          Koen Roggemans added a comment -

          It seems to be possible to delete the attempts without any problems for the primary site administrator only.

          Show
          Koen Roggemans added a comment - It seems to be possible to delete the attempts without any problems for the primary site administrator only.
          Hide
          Igor Sazonov added a comment -

          file /mod/quiz/report/attemptsreport.php

          function delete_selected_attempts

          ($allowed && !array_key_exists($attempt->userid, $allowed))

          replace to:

          ($allowed && !in_array($attempt->userid, $allowed))

          replace array_key_exists to in_array

          i hope bug will be closed =)

          Show
          Igor Sazonov added a comment - file /mod/quiz/report/attemptsreport.php function delete_selected_attempts ($allowed && !array_key_exists($attempt->userid, $allowed)) replace to: ($allowed && !in_array($attempt->userid, $allowed)) replace array_key_exists to in_array i hope bug will be closed =)
          Hide
          Tim Hunt added a comment -

          Thanks for the fix. I'm submitting it for integration.

          Show
          Tim Hunt added a comment - Thanks for the fix. I'm submitting it for integration.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          NOTE: The 22_STABLE has been fixed too by cherrypicking from master.

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks! NOTE: The 22_STABLE has been fixed too by cherrypicking from master.
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          Just been trying to test this now but have hit a road block.
          Setting up the course, groups, and quiz went fine.
          Students could attempt the quiz fine.
          Teacher viewing the overview and responses reports showed what was expected.
          But when as a teacher I tried to delete a quiz attempt I hit problems.
          Through the overview report everything worked as expected. I could delete attempts from students in my group and could not (through hacking) delete attempts from other groups.
          However through the responses report as suggested I hit big problems.

          In the responses report if I select one of the two attempts and click 'Delete selected attempts' I get a pop-up confirmation, and then upon clicking 'Yes' I get the following JS exception:

          Element of type FORM is not supported by the M.util.show_confirm_dialog function. Use A or INPUT

          After purging caches and attempting as both teachers in both Chrome and Firefox I still had no luck.
          Next thing I tried was disabling JS.
          Upon trying to delete from the responses report I got a different error. This time I was redirected to the following URL:

          http://integration.sam.local/mod/quiz/report.php%3E%3Cinput%20type=

          I checked the source and sure enough the form tag for the report was mangled:

          <form id="attemptsform" method="post" action="http://integration.sam.local/mod/quiz/report.php><input type="hidden" name="id" value="8" />

          That issue was a little easier to debug, mod/quiz/report/responses/responses_table.php line 63 is missing a double quote to close the action (annotations show MDL-20636 not this issue)
          I added the double quote and with JS disabled I could now delete an attempt from my group and through hacking the checkbox confirmed I could not delete an attempt from another group.
          I then enabled JS and attempted to delete the other attempt, however I still got the JS exception so more debugging required there.

          Failing this presently, that responses report needs fixing, however perhaps it's a separate issue? Will leave you to decide Tim. I you do open a new issue for the responses report let me know and I'll change this to a pass.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, Just been trying to test this now but have hit a road block. Setting up the course, groups, and quiz went fine. Students could attempt the quiz fine. Teacher viewing the overview and responses reports showed what was expected. But when as a teacher I tried to delete a quiz attempt I hit problems. Through the overview report everything worked as expected. I could delete attempts from students in my group and could not (through hacking) delete attempts from other groups. However through the responses report as suggested I hit big problems. In the responses report if I select one of the two attempts and click 'Delete selected attempts' I get a pop-up confirmation, and then upon clicking 'Yes' I get the following JS exception: Element of type FORM is not supported by the M.util.show_confirm_dialog function. Use A or INPUT After purging caches and attempting as both teachers in both Chrome and Firefox I still had no luck. Next thing I tried was disabling JS. Upon trying to delete from the responses report I got a different error. This time I was redirected to the following URL: http://integration.sam.local/mod/quiz/report.php%3E%3Cinput%20type= I checked the source and sure enough the form tag for the report was mangled: <form id="attemptsform" method="post" action="http://integration.sam.local/mod/quiz/report.php><input type="hidden" name="id" value="8" /> That issue was a little easier to debug, mod/quiz/report/responses/responses_table.php line 63 is missing a double quote to close the action (annotations show MDL-20636 not this issue) I added the double quote and with JS disabled I could now delete an attempt from my group and through hacking the checkbox confirmed I could not delete an attempt from another group. I then enabled JS and attempted to delete the other attempt, however I still got the JS exception so more debugging required there. Failing this presently, that responses report needs fixing, however perhaps it's a separate issue? Will leave you to decide Tim. I you do open a new issue for the responses report let me know and I'll change this to a pass. Cheers Sam
          Hide
          Tim Hunt added a comment -

          I will have a look and try to fix this today.

          Show
          Tim Hunt added a comment - I will have a look and try to fix this today.
          Hide
          Tim Hunt added a comment -

          The fix for MDL-30660 is needed to fix this properly.

          Show
          Tim Hunt added a comment - The fix for MDL-30660 is needed to fix this properly.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          To INTEGRATORS: MDL-30660 needs to be integrated before adding/integrating/testing this.

          Show
          Eloy Lafuente (stronk7) added a comment - To INTEGRATORS: MDL-30660 needs to be integrated before adding/integrating/testing this.
          Hide
          Tim Hunt added a comment -

          Right, I think I have finally fixed this. There are further commits to integrate on the 21, 22 and master branches. I made the commits on the same feature branches as above, that is

          Show
          Tim Hunt added a comment - Right, I think I have finally fixed this. There are further commits to integrate on the 21, 22 and master branches. I made the commits on the same feature branches as above, that is https://github.com/timhunt/moodle/compare/MOODLE_21_STABLE...MDL-27314_21 https://github.com/timhunt/moodle/compare/MOODLE_22_STABLE...MDL-27314_22 https://github.com/timhunt/moodle/compare/master...MDL-27314
          Hide
          Eloy Lafuente (stronk7) added a comment -

          To INTEGRATORS: Once MDL-30660 is integrated, the last commit on each of the branches above will be cherry-picked.

          Show
          Eloy Lafuente (stronk7) added a comment - To INTEGRATORS: Once MDL-30660 is integrated, the last commit on each of the branches above will be cherry-picked.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          I've just been testing this again and there is unfortunately one more problem.
          With JS enabled I cannot delete attempts from either the grades or response report (both work perfectly with JS disabled).
          I did a quick bit of testing and surely enough the comes because targetform = null within show_confirm_dialogue when processing the confirm action.
          A little bit more testing shows that if I rename the id input to something else using firebug and then trigger the confirm dialogue things work perfectly.
          Obviously node.ancestor suffers from the same problem noted as noted with the test method.

          I am testing using Firefox and the master branch.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, I've just been testing this again and there is unfortunately one more problem. With JS enabled I cannot delete attempts from either the grades or response report (both work perfectly with JS disabled). I did a quick bit of testing and surely enough the comes because targetform = null within show_confirm_dialogue when processing the confirm action. A little bit more testing shows that if I rename the id input to something else using firebug and then trigger the confirm dialogue things work perfectly. Obviously node.ancestor suffers from the same problem noted as noted with the test method. I am testing using Firefox and the master branch. Cheers Sam
          Hide
          Tim Hunt added a comment -

          Oh, drat! At one point while working on this I had edited YUI to investigate the YUI.selector.test bug. It looks like those changes were affecting this, even though I was sure I had tested them after reverting. Let me look again.

          Show
          Tim Hunt added a comment - Oh, drat! At one point while working on this I had edited YUI to investigate the YUI.selector.test bug. It looks like those changes were affecting this, even though I was sure I had tested them after reverting. Let me look again.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          JS deletion now working ok on 21, 22 and master (both overview and responses reports).

          Minor problem found in 20 (responses report). Tim is on it now, seems trivial to fix.

          Back in some hours.... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - JS deletion now working ok on 21, 22 and master (both overview and responses reports). Minor problem found in 20 (responses report). Tim is on it now, seems trivial to fix. Back in some hours.... ciao
          Hide
          Igor Sazonov added a comment -

          hey guys! do you need for a help? does my fix worked?

          Show
          Igor Sazonov added a comment - hey guys! do you need for a help? does my fix worked?
          Hide
          Tim Hunt added a comment -

          New commit at the end of the MDL-27314_20 branch. Hopefully, that is the final fix required.

          Show
          Tim Hunt added a comment - New commit at the end of the MDL-27314 _20 branch. Hopefully, that is the final fix required.
          Hide
          Tim Hunt added a comment -

          Igor, thanks for the offer. As we worked on this, we found several other problems, so in the end, we have ended up doing some more substantial changes that we hope fixes everything.

          Show
          Tim Hunt added a comment - Igor, thanks for the offer. As we worked on this, we found several other problems, so in the end, we have ended up doing some more substantial changes that we hope fixes everything.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Done, passing. Tested under 20, 21, 22 & master, the dialog and the cance/ok buttons are working as expected both in the overview and reponses reports.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Done, passing. Tested under 20, 21, 22 & master, the dialog and the cance/ok buttons are working as expected both in the overview and reponses reports. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yes, you did it!

          Now your code is part of the best weeklies released ever, many thanks!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yes, you did it! Now your code is part of the best weeklies released ever, many thanks! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: