Moodle
  1. Moodle
  2. MDL-32535

Regression: course completion report download shows {$a} unused placeholder when activities auto completion is set

    Details

    • Testing Instructions:
      Hide

      1. Restore the attached backup file as a new course.

      2. Turn editing on. Using source view or by inspecting the item, confirm the alt text for:
      a. The icon next to the first quiz. This should read 'The system marks this item complete according to conditions: The answer is 42'
      b. The icon next to the label. This should read 'Students can manually mark this item complete: Manual completion'

      3. Turn editing off. Enrol a test account into the course as a student. Log in as the test account.

      4. Confirm the alt text for:
      a. Icon next to the first quiz. 'Not completed: The answer is 42'
      b. Icon next to label. 'Not completed: Manual completion. Select to mark as complete.'

      5. Click the tickbox next to label to mark it complete. Confirm the new alt text: 'Completed: Manual completion. Select to mark as not complete.'

      6. Do the first quiz (it doesn't matter if you get it right or wrong). Answer it and click 'Submit all and finish'. Return to course page. Quiz should be ticked (current icon = red tick). Confirm alt text 'Completed: The answer is 42'

      7. Do the second quiz. Get it right. Return to course page - quiz should have green tick. Confirm alt text 'Completed: The answer is 42 (pass grade) (achieved pass grade)'
      NOTE: Confusing text here is because I put '(pass grade)' at end of activity name, sorry.

      8. Redo the second quiz and get it wrong this time. Quiz should have black X. Confirm alt text 'Completed: The answer is 42 (pass grade) (did not achieve pass grade)'

      9. Log back in as admin. Go to Reports / Activity completion. Save as .csv. Content should be:

      ,"Email address","The answer is 42","","The answer is 42 (pass grade)","","Manual completion",""
      "User 1","u1@x.x","Completed","Thursday, 19 April 2012, 1:52 PM","Completed (did not achieve pass grade)","Thursday, 19 April 2012, 2:40 PM","Not completed",""
      

      (Note that the status text 'Completed', 'Completed (did not achieve pass grade)', and 'Not completed' here do not have the activity names, which is the correct behaviour that this issue is initially about.)

      Show
      1. Restore the attached backup file as a new course. 2. Turn editing on. Using source view or by inspecting the item, confirm the alt text for: a. The icon next to the first quiz. This should read 'The system marks this item complete according to conditions: The answer is 42' b. The icon next to the label. This should read 'Students can manually mark this item complete: Manual completion' 3. Turn editing off. Enrol a test account into the course as a student. Log in as the test account. 4. Confirm the alt text for: a. Icon next to the first quiz. 'Not completed: The answer is 42' b. Icon next to label. 'Not completed: Manual completion. Select to mark as complete.' 5. Click the tickbox next to label to mark it complete. Confirm the new alt text: 'Completed: Manual completion. Select to mark as not complete.' 6. Do the first quiz (it doesn't matter if you get it right or wrong). Answer it and click 'Submit all and finish'. Return to course page. Quiz should be ticked (current icon = red tick). Confirm alt text 'Completed: The answer is 42' 7. Do the second quiz. Get it right. Return to course page - quiz should have green tick. Confirm alt text 'Completed: The answer is 42 (pass grade) (achieved pass grade)' NOTE: Confusing text here is because I put '(pass grade)' at end of activity name, sorry. 8. Redo the second quiz and get it wrong this time. Quiz should have black X. Confirm alt text 'Completed: The answer is 42 (pass grade) (did not achieve pass grade)' 9. Log back in as admin. Go to Reports / Activity completion. Save as .csv. Content should be: , "Email address" , "The answer is 42" , ""," The answer is 42 (pass grade) "," "," Manual completion "," " "User 1" , "u1@x.x" , "Completed" , "Thursday, 19 April 2012, 1:52 PM" , "Completed (did not achieve pass grade)" , "Thursday, 19 April 2012, 2:40 PM" , "Not completed" ,"" (Note that the status text 'Completed', 'Completed (did not achieve pass grade)', and 'Not completed' here do not have the activity names, which is the correct behaviour that this issue is initially about.)
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-32535-master
    • Rank:
      39441

      Description

      The recent changes in strings completion-alt-auto-n and completion-alt-auto-y introduced some regressions in course completion report download, where now a unused "{$a}" placeholder is shown. such as "Completed: {$a}" or "Not completed: {$a}".

      A sample portion of the code shows we are not using the placeholder:
      $describe=get_string('completion-alt-auto-'.$completiontype,'completion');

      In in report/completion/index.php the following lines need to be changed:

      • 607
      • 636
      • 650
      • 675

        Issue Links

          Activity

          Hide
          Sam Marshall added a comment -

          Regression was caused by MDL-30817 (which incidentally wasn't my code). Looking into it...

          Show
          Sam Marshall added a comment - Regression was caused by MDL-30817 (which incidentally wasn't my code). Looking into it...
          Hide
          Sam Marshall added a comment -

          OK, there are two issues here:

          1) The course completion report (this is course completion, not activity completion, so not really my problem but I will fix it anyway) was inappropriately using strings intended for alt text for a different purpose. The activity completion report (which is my problem) does the same thing.

          2) The strings for alt text were not fully changed (the 'fail/pass' ones were missed out, a couple others in editing mode).

          I think we need to duplicate the strings for use with course completion report (and if anywhere else wants them) as a generic description when not used as alt text. (Note that this is not literally duplicating the strings, the strings are different as they don't have the {$a} in.)

          I am writing a test script but this will only cover activity completion and not course completion because I am not actually familiar with that. The change should be pretty safe as I search/replaced the string names, but if somebody is able to add to the test script, that would be helpful.

          Show
          Sam Marshall added a comment - OK, there are two issues here: 1) The course completion report (this is course completion, not activity completion, so not really my problem but I will fix it anyway) was inappropriately using strings intended for alt text for a different purpose. The activity completion report (which is my problem) does the same thing. 2) The strings for alt text were not fully changed (the 'fail/pass' ones were missed out, a couple others in editing mode). I think we need to duplicate the strings for use with course completion report (and if anywhere else wants them) as a generic description when not used as alt text. (Note that this is not literally duplicating the strings, the strings are different as they don't have the {$a} in.) I am writing a test script but this will only cover activity completion and not course completion because I am not actually familiar with that. The change should be pretty safe as I search/replaced the string names, but if somebody is able to add to the test script, that would be helpful.
          Hide
          Sam Marshall added a comment -

          Think this fix (and test script included) pretty much covers it...

          Show
          Sam Marshall added a comment - Think this fix (and test script included) pretty much covers it...
          Hide
          Andrea Bicciolo added a comment - - edited

          Sam, thanks for your amazing fast fix

          Show
          Andrea Bicciolo added a comment - - edited Sam, thanks for your amazing fast fix
          Hide
          Sam Christy added a comment -

          Thank you Andrea for forwarding me to this bug report from my post: http://moodle.org/mod/forum/discuss.php?d=201486

          Thank you Sam for providing a patch for the problem, I've just applied it to my own installation and it has resolved the problem.

          Show
          Sam Christy added a comment - Thank you Andrea for forwarding me to this bug report from my post: http://moodle.org/mod/forum/discuss.php?d=201486 Thank you Sam for providing a patch for the problem, I've just applied it to my own installation and it has resolved the problem.
          Hide
          Andrea Bicciolo added a comment -

          We followed test instructions an everything performed as expectd, "$a" is no more visible in course completion report. Sending to integration review.

          Show
          Andrea Bicciolo added a comment - We followed test instructions an everything performed as expectd, "$a" is no more visible in course completion report. Sending to integration review.
          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
          Sam Marshall added a comment -

          Rebased (master branch only, 2.2 is probably stable anyway) - no conflicts

          Show
          Sam Marshall added a comment - Rebased (master branch only, 2.2 is probably stable anyway) - no conflicts
          Hide
          Dan Poltawski added a comment -

          Hi Sam,

          Should this fix be backported to 2.1 (seems to cherr-pick cleanly)

          Show
          Dan Poltawski added a comment - Hi Sam, Should this fix be backported to 2.1 (seems to cherr-pick cleanly)
          Hide
          Sam Marshall added a comment -

          Dan: I didn't think of that because it was reported against 2.x - but yes I think it should since it was caused by MDL-30817 which went into 2.1. (I had a look at the commit for that, it seems same on all branches too.)

          Show
          Sam Marshall added a comment - Dan: I didn't think of that because it was reported against 2.x - but yes I think it should since it was caused by MDL-30817 which went into 2.1. (I had a look at the commit for that, it seems same on all branches too.)
          Hide
          Dan Poltawski added a comment -

          Integrated, thanks

          Show
          Dan Poltawski added a comment - Integrated, thanks
          Hide
          Frédéric Massart added a comment -

          Once completed the alt texts and icons (completion-alt-auto-pass, completion-alt-auto-fail) for the quizzes did not come up as expected. The completion-alt-auto-y was used instead.

          The same wrong texts are present in the CSV export.

          Everything else worked just fine.

          This is just me but why are the alt attributes more verbose than the title ones? Why aren't they similar?

          Show
          Frédéric Massart added a comment - Once completed the alt texts and icons (completion-alt-auto-pass, completion-alt-auto-fail) for the quizzes did not come up as expected. The completion-alt-auto-y was used instead. The same wrong texts are present in the CSV export. Everything else worked just fine. This is just me but why are the alt attributes more verbose than the title ones? Why aren't they similar?
          Hide
          Sam Marshall added a comment -

          Regarding pass grade not working - looking at it.

          Regarding alt/title: The alt attributes are there for accessibility reasons and are primarily intended for people using screen readers. Title attributes are there for sighted people who hover over the item and want to know what it does. Often they are indeed the same but there can be differences because typically, titles are there to describe what happens if you click something, whereas alt text is there to describe the meaning of the thing as well.

          In this case, for example, title text is:

          'Mark as complete: [activity]'.

          Alt text is:

          'Not completed:

          {activity}

          . Select to mark as complete.'

          So the alt text tells you the current state first (which a sighted person could see just by looking that the icon is not ticked) and then tells you that you can click, whoops, 'select' to mark it complete. The title only has to tell you the second part.

          Show
          Sam Marshall added a comment - Regarding pass grade not working - looking at it. Regarding alt/title: The alt attributes are there for accessibility reasons and are primarily intended for people using screen readers. Title attributes are there for sighted people who hover over the item and want to know what it does. Often they are indeed the same but there can be differences because typically, titles are there to describe what happens if you click something, whereas alt text is there to describe the meaning of the thing as well. In this case, for example, title text is: 'Mark as complete: [activity] '. Alt text is: 'Not completed: {activity} . Select to mark as complete.' So the alt text tells you the current state first (which a sighted person could see just by looking that the icon is not ticked) and then tells you that you can click, whoops, 'select' to mark it complete. The title only has to tell you the second part.
          Hide
          Sam Marshall added a comment -

          I just tested this again using my branch. Specifically I did a shortened version of the test:

          1. Restore the attached backup file as a new course.

          3. Turn editing off. Enrol a test account into the course as a student. Log in as the test account.

          7. Do the second quiz. Get it right. Return to course page - quiz should have green tick. Confirm alt text 'Completed: The answer is 42 (pass grade) (achieved pass grade)'
          NOTE: Confusing text here is because I put '(pass grade)' at end of activity name, sorry.

          9. Log back in as admin. Go to Reports / Activity completion. Save as .csv.

          Before doing the test, I went to admin and 'purge all caches' (this was necessary because I didn't have a version update to clear caches, I don't think it should have been needed on test system).

          When I did this on my system using this branch, the icon for the second quiz correctly appeared as a green tick (not a default red one) and the alt text was as described in the test case, and the csv file had 'Completed (achieved pass grade)'. So I think it is working. Are you saying that isn't the case for you?

          Show
          Sam Marshall added a comment - I just tested this again using my branch. Specifically I did a shortened version of the test: 1. Restore the attached backup file as a new course. 3. Turn editing off. Enrol a test account into the course as a student. Log in as the test account. 7. Do the second quiz. Get it right. Return to course page - quiz should have green tick. Confirm alt text 'Completed: The answer is 42 (pass grade) (achieved pass grade)' NOTE: Confusing text here is because I put '(pass grade)' at end of activity name, sorry. 9. Log back in as admin. Go to Reports / Activity completion. Save as .csv. Before doing the test, I went to admin and 'purge all caches' (this was necessary because I didn't have a version update to clear caches, I don't think it should have been needed on test system). When I did this on my system using this branch, the icon for the second quiz correctly appeared as a green tick (not a default red one) and the alt text was as described in the test case, and the csv file had 'Completed (achieved pass grade)'. So I think it is working. Are you saying that isn't the case for you?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm giving this one more testing iteration...

          Show
          Eloy Lafuente (stronk7) added a comment - I'm giving this one more testing iteration...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Tested under 21_STABLE and master. Everything went ok (one small difference was found in the .csv file, but was verified with SamM).

          So passing. Thanks and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Tested under 21_STABLE and master. Everything went ok (one small difference was found in the .csv file, but was verified with SamM). So passing. Thanks and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          U P S T R E A M I Z E D !

          Many thanks for the hard work, closing this as fixed.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao
          Hide
          Frédéric Massart added a comment -

          Hi guys,

          My bad, I should have mentioned that (for some obscure reason) I could not load the backup. I manually recreated it but I was probably wrong somewhere. Sorry about all that! Although, realising this this morning, I imported and tested once more. It appears that when succeeding the first quiz on the first attempt, the icon is a red tick. I know that this has been integrated, but I thought that you might be interested in knowing that minor little issue.

          Cheers!

          Show
          Frédéric Massart added a comment - Hi guys, My bad, I should have mentioned that (for some obscure reason) I could not load the backup. I manually recreated it but I was probably wrong somewhere. Sorry about all that! Although, realising this this morning, I imported and tested once more. It appears that when succeeding the first quiz on the first attempt, the icon is a red tick. I know that this has been integrated, but I thought that you might be interested in knowing that minor little issue. Cheers!
          Hide
          Andrea Bicciolo added a comment -

          About the alt text 'Not completed:

          {activity}

          . Select to mark as complete' in the report, in my opinion the sentence is misleading. In the course completion report AFAIK there is no way to select an activity to mark it as complete. Or I'm wrong somewhere?

          Show
          Andrea Bicciolo added a comment - About the alt text 'Not completed: {activity} . Select to mark as complete' in the report, in my opinion the sentence is misleading. In the course completion report AFAIK there is no way to select an activity to mark it as complete. Or I'm wrong somewhere?
          Hide
          Sam Marshall added a comment -

          Andrea: Does it use that alt text that you quoted in the activity completion report? I thought it had different alt text in the report, but maybe not.

          If it does use that alt text, I agree that would be another bug, please file new issue if you want and I'll fix it...

          Show
          Sam Marshall added a comment - Andrea: Does it use that alt text that you quoted in the activity completion report? I thought it had different alt text in the report, but maybe not. If it does use that alt text, I agree that would be another bug, please file new issue if you want and I'll fix it...
          Hide
          Andrea Bicciolo added a comment -

          Sam,
          checked again using latest lang pack and moodle versions, you are right, alt text in the report appears appropriate, I apologize for the false alarm (blush).

          Show
          Andrea Bicciolo added a comment - Sam, checked again using latest lang pack and moodle versions, you are right, alt text in the report appears appropriate, I apologize for the false alarm (blush).
          Hide
          Sam Marshall added a comment -

          thanks. and yay, I've had enough of fiddling with these strings

          Show
          Sam Marshall added a comment - thanks. and yay, I've had enough of fiddling with these strings
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Fred, but testing instructions (point 6) say exactly that one red tick is expected, isn't it?

          Show
          Eloy Lafuente (stronk7) added a comment - Fred, but testing instructions (point 6) say exactly that one red tick is expected, isn't it?

            People

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

              Dates

              • Created:
                Updated:
                Resolved: