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

          Peter Ruthven-Stuart created issue -
          Peter Ruthven-Stuart made changes -
          Field Original Value New Value
          Labels things_that_worked_in_1.9_but_not_in_2 usability
          Peter Ruthven-Stuart made changes -
          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.

          This was possible in 1.9.
          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.
          Priority Minor [ 4 ] Blocker [ 1 ]
          Labels things_that_worked_in_1.9_but_not_in_2 usability quiz things_that_worked_in_1.9_but_not_in_2 usability
          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?
          Tim Hunt made changes -
          Priority Blocker [ 1 ] Major [ 3 ]
          Labels quiz things_that_worked_in_1.9_but_not_in_2 usability
          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 .
          Tim Hunt made changes -
          Summary Not possible to delete a quiz attempt - no error message Not possible to delete or regrade selected quiz attempts in separate groups mode
          Fix Version/s STABLE backlog [ 10463 ]
          Priority Major [ 3 ] Critical [ 2 ]
          Labels triaged
          Tim Hunt made changes -
          Link This issue is duplicated by MDL-27315 [ 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.
          Peter Ruthven-Stuart made changes -
          Labels triaged things_that_worked_in_1.9_but_not_in_2 triaged
          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
          Anthony Borrow made changes -
          Link This issue is duplicated by MDL-26640 [ MDL-26640 ]
          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.
          Tim Hunt made changes -
          Status Open [ 1 ] Waiting for integration review [ 10010 ]
          Pull Master Diff URL https://github.com/timhunt/moodle/compare/master...MDL-27314
          Pull Master Branch MDL-27314
          Pull 2.0 Diff URL https://github.com/timhunt/moodle/compare/MOODLE_20_STABLE...MDL-27314_20
          Pull from Repository git://github.com/timhunt/moodle.git
          Pull 2.0 Branch MDL-27314_20
          Fix Version/s 2.0.7 [ 11451 ]
          Fix Version/s 2.1.4 [ 11452 ]
          Fix Version/s 2.2 [ 10656 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Testing Instructions 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.
          Pull 2.1 Branch MDL-27314_21
          Pull 2.1 Diff URL https://github.com/timhunt/moodle/compare/MOODLE_21_STABLE...MDL-27314_21
          Eloy Lafuente (stronk7) made changes -
          Fix Version/s 2.2.1 [ 11456 ]
          Fix Version/s 2.2 [ 10656 ]
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator stronk7
          Currently in integration Yes [ 10041 ]
          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.
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Affects Version/s 2.2 [ 10656 ]
          Affects Version/s 2.1.3 [ 11251 ]
          Affects Version/s 2.3 [ 10657 ]
          Sam Hemelryk made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Tester samhemelryk
          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
          Sam Hemelryk made changes -
          Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
          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.
          Tim Hunt made changes -
          Link This issue will be resolved by MDL-30660 [ MDL-30660 ]
          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.
          Tim Hunt made changes -
          Link This issue will help resolve MDL-29047 [ MDL-29047 ]
          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
          Tim Hunt made changes -
          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.
          Eloy Lafuente (stronk7) made changes -
          Status Problem during testing [ 10007 ] Integration review in progress [ 10004 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Sam Hemelryk made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          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
          Sam Hemelryk made changes -
          Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
          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.
          Eloy Lafuente (stronk7) made changes -
          Link This issue testing discovered MDL-30677 [ MDL-30677 ]
          Eloy Lafuente (stronk7) made changes -
          Status Problem during testing [ 10007 ] Integration review in progress [ 10004 ]
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Tester samhemelryk stronk7
          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
          Eloy Lafuente (stronk7) made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          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
          Eloy Lafuente (stronk7) made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 09/Dec/11

            People

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

              Dates

              • Created:
                Updated:
                Resolved: