Moodle
  1. Moodle
  2. MDL-26165

When unlimited retakes of quiz is allowed, confirmation message implies answers can no longer be changed.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.1, 2.1, 2.2
    • Fix Version/s: 2.1.1
    • Component/s: Quiz
    • Labels:
      None
    • Testing Instructions:
      Hide

      1. Create a quiz.
      2. Log in as a student and attempt the quiz.

      There will be an are-your-sure pop-up for both the Start attempt and Submit all and finish buttons. Are these clear enough?

      Show
      1. Create a quiz. 2. Log in as a student and attempt the quiz. There will be an are-your-sure pop-up for both the Start attempt and Submit all and finish buttons. Are these clear enough?
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      15701

      Description

      When a quiz is set to unlimited retakes, when the quiz is completed, after "Submit all and finish" is clicked, a confirmation warning message appears that says: "...you will no longer be able to change your answers."
      This misleading warning scares the students into not finishing the quiz attempt to get the needed feedback. On an unlimited retake quiz, no further confirmation is needed when quiz is done, or at least if there is one, it should not be confusing.

        Issue Links

          Activity

          Hide
          Charles Fulton added a comment -

          I've tested a patch which shows a different confirmation message (attached) if the quiz has unlimited attempts or if the user hasn't yet exhausted their attempts.

          Show
          Charles Fulton added a comment - I've tested a patch which shows a different confirmation message (attached) if the quiz has unlimited attempts or if the user hasn't yet exhausted their attempts.
          Show
          Charles Fulton added a comment - I've published the patch here: https://github.com/mackensen/moodle/commit/136667bd8e9b4ff9efb0c3ca474827d1a839d7ee .
          Hide
          Tim Hunt added a comment -

          I'm afraid I don't really like the change. The more words you have in an "Are you sure" message, the less likely anyone is to read an understand it.

          So I think we should be aiming for something shorter, while still being clear. I would also like to use the same message in all situations, if possible.

          Also, some technical comments on your patch:
          1. Don't put comments like MDL-123 in the code. It is just clutter. If you need that information, use git blame.
          2. You commit message is not in the right format. You must right the issue number with a -, or it will not automatically be made into a link. You are not specific which messages is being improved.
          3. A very trivial change has added two new local variables. There must be a neater way to write that code.

          Show
          Tim Hunt added a comment - I'm afraid I don't really like the change. The more words you have in an "Are you sure" message, the less likely anyone is to read an understand it. So I think we should be aiming for something shorter, while still being clear. I would also like to use the same message in all situations, if possible. Also, some technical comments on your patch: 1. Don't put comments like MDL-123 in the code. It is just clutter. If you need that information, use git blame. 2. You commit message is not in the right format. You must right the issue number with a -, or it will not automatically be made into a link. You are not specific which messages is being improved. 3. A very trivial change has added two new local variables. There must be a neater way to write that code.
          Hide
          Charles Fulton added a comment -

          Thanks for responding, Tim. I think we do better to err on the side of an informative message. The users who don't read messages won't be affected either way; the users who do will appreciate the information. As for the local variables; this was my thinking:
          1. I didn't want to make two calls to the same class variable twice in the same if statement.
          2. I didn't want to split up the add_confirm_action() logic.

          Anyway, I've rewritten the patch to address the stylistic concerns and to eliminate one of the local variables. I don't see how the other one can be eliminated without an evil looking ternary function, and above all code like this needs to be human-readable. New patch is here: https://github.com/mackensen/moodle/commit/465fc3c0e1e31e7f4c859052494d56eb0d5a17de.

          Show
          Charles Fulton added a comment - Thanks for responding, Tim. I think we do better to err on the side of an informative message. The users who don't read messages won't be affected either way; the users who do will appreciate the information. As for the local variables; this was my thinking: 1. I didn't want to make two calls to the same class variable twice in the same if statement. 2. I didn't want to split up the add_confirm_action() logic. Anyway, I've rewritten the patch to address the stylistic concerns and to eliminate one of the local variables. I don't see how the other one can be eliminated without an evil looking ternary function, and above all code like this needs to be human-readable. New patch is here: https://github.com/mackensen/moodle/commit/465fc3c0e1e31e7f4c859052494d56eb0d5a17de .
          Hide
          Tim Hunt added a comment -

          Here is my version of a simplified and non-confusing submit confirmation. Comments please.

          Show
          Tim Hunt added a comment - Here is my version of a simplified and non-confusing submit confirmation. Comments please.
          Hide
          Tim Hunt added a comment -

          Note that the change for this bug relies on the change in MDL-28195 - you need to apply that patch first.

          I also made a similar change to the start attempt are-you-sure. (I changed the 'Yes' button to 'Start attempt'.)

          Comments please. I will submit this for integration after MDL-28195 is approved.

          Show
          Tim Hunt added a comment - Note that the change for this bug relies on the change in MDL-28195 - you need to apply that patch first. I also made a similar change to the start attempt are-you-sure. (I changed the 'Yes' button to 'Start attempt'.) Comments please. I will submit this for integration after MDL-28195 is approved.
          Hide
          Charles Fulton added a comment -

          This works for me (and I also like the change in MDL-28195).

          Show
          Charles Fulton added a comment - This works for me (and I also like the change in MDL-28195 ).
          Hide
          Tim Hunt added a comment -

          Rebased, due to changes in MDL-28195. That has been approved, at least in principle, so I am submitting this for integration too now.

          Please can we have this on both the 2.1 and master branches.

          Show
          Tim Hunt added a comment - Rebased, due to changes in MDL-28195 . That has been approved, at least in principle, so I am submitting this for integration too now. Please can we have this on both the 2.1 and master branches.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks (master and 21)!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks (master and 21)!
          Hide
          Andrew Davis added a comment -

          Looks like a good change to me

          Show
          Andrew Davis added a comment - Looks like a good change to me
          Hide
          Sam Hemelryk added a comment -

          Congratulations - this fix has just been released in the weeklies.

          Show
          Sam Hemelryk added a comment - Congratulations - this fix has just been released in the weeklies.

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: