Moodle
  1. Moodle
  2. MDL-35762

Grade items show up in the gradebook for students who do not meet conditions for a conditional activity.

    Details

    • Testing Instructions:
      Hide

      Apologies to whoever has to test this.

      This requires testing repeated for 22_STABLE, 23_STABLE and master.

      1. Turn on conditional activities for your site. This is done via the site setting enableavailability

      2. Create 4 assignment in a course and set the conditions for access as follows:
      A No condition so it will always be visible.
      B a condition the student won't meet, set to display greyed out.
      C a condition the student won't meet, set to hide entirely.
      D a condition the student can meet such as setting their Yahoo ID, set to hide entirely.

      3. Log in as a student and do whatever you have to do to meet condition D above.

      4. Go to the user report (click on grades in the course) and check that the four activities are displayed or hidden appropriately ie either visible and a link, visible but not a link or hidden entirely.

      5. Log in as a teacher or admin and go to the student's user report. Check that all activities are visible and are links for you.

      6. Hide one of the visible activities on the course page. Go to the categories and items screen in the gradebook and unhide that same activity. ie you've just hidden the activity but unhidden the activities' grade.

      7. Log in as the student. Check that the activity is hidden on the course page. Check that it is displayed on the user report but it should NOT be a link to the activity.
      ------------------------------------------------------------------------------
      8. Turn off the site setting enableavailability.

      9. Log in as the student and go to the user report. Check that you don't get any errors plus that now you can see all of the activities that were hidden because of conditional availability conditions. Activities that were not shown because they are hidden (eye closed icon) should remain hidden.
      ------------------------------------------------------------------------------
      10. Log in as admin. Turn on the site setting enableavailability.

      11. Create a new course. Its easier to just make this from scratch than to edit the previous course.

      12. In section 1 create 4 activities:
      A a plain visible activity with no conditional access conditions.
      B an activity with a conditional access condition the student will not meet and set to "greyed out"
      C an activity with a conditional access condition the student will not meet and set to "hidden entirely"
      D an activity with no conditions but hidden on the course page

      13. In section 2 create 2 activities. Then hide section 2. This will hide both of these activities.

      14. Go to the categories and items screen and unhide one of the activities in section 2.

      15. Log in as a student and go to the course page.

      16. Check the visibility of the activities.

      17. Go to the user report.

      18. Check the visibility of the activities.

      ------------------------------------------------------------------------------
      Repeat the testing instructions from MDL-34931.

      Show
      Apologies to whoever has to test this. This requires testing repeated for 22_STABLE, 23_STABLE and master. 1. Turn on conditional activities for your site. This is done via the site setting enableavailability 2. Create 4 assignment in a course and set the conditions for access as follows: A No condition so it will always be visible. B a condition the student won't meet, set to display greyed out. C a condition the student won't meet, set to hide entirely. D a condition the student can meet such as setting their Yahoo ID, set to hide entirely. 3. Log in as a student and do whatever you have to do to meet condition D above. 4. Go to the user report (click on grades in the course) and check that the four activities are displayed or hidden appropriately ie either visible and a link, visible but not a link or hidden entirely. 5. Log in as a teacher or admin and go to the student's user report. Check that all activities are visible and are links for you. 6. Hide one of the visible activities on the course page. Go to the categories and items screen in the gradebook and unhide that same activity. ie you've just hidden the activity but unhidden the activities' grade. 7. Log in as the student. Check that the activity is hidden on the course page. Check that it is displayed on the user report but it should NOT be a link to the activity. ------------------------------------------------------------------------------ 8. Turn off the site setting enableavailability. 9. Log in as the student and go to the user report. Check that you don't get any errors plus that now you can see all of the activities that were hidden because of conditional availability conditions. Activities that were not shown because they are hidden (eye closed icon) should remain hidden. ------------------------------------------------------------------------------ 10. Log in as admin. Turn on the site setting enableavailability. 11. Create a new course. Its easier to just make this from scratch than to edit the previous course. 12. In section 1 create 4 activities: A a plain visible activity with no conditional access conditions. B an activity with a conditional access condition the student will not meet and set to "greyed out" C an activity with a conditional access condition the student will not meet and set to "hidden entirely" D an activity with no conditions but hidden on the course page 13. In section 2 create 2 activities. Then hide section 2. This will hide both of these activities. 14. Go to the categories and items screen and unhide one of the activities in section 2. 15. Log in as a student and go to the course page. 16. Check the visibility of the activities. 17. Go to the user report. 18. Check the visibility of the activities. ------------------------------------------------------------------------------ Repeat the testing instructions from MDL-34931 .
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-35762_fix
    • Rank:
      44512

      Description

      A teacher may create an assignment that they only wish certain users to view, for example, those who failed an assignment and need extra help. They may not want all users to see that there is an extra assignment in the course for those users, however, when viewing the gradebook the assignment is still be listed for all users. It may also confuse students who have achieved 100% for the course but can still not access it, making them question what they need to do to access it.

      IMO the item should be hidden from the gradebook if the condition is set to 'Hide activity entirely'.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          This looks very similar to MDL-29501, which was resolved a while ago.

          Is this relevant to all activities or only assignments?

          What gradebook page is showing the assignment and how?

          Show
          Michael de Raadt added a comment - This looks very similar to MDL-29501 , which was resolved a while ago. Is this relevant to all activities or only assignments? What gradebook page is showing the assignment and how?
          Hide
          Mark Nelson added a comment -

          That issue removed the link to the actual activity but still lists it. I was thinking it would be best to not display it it all, especially if the condition is set to 'Hide Entirely'. This is on the user report and affects all grade items.

          Show
          Mark Nelson added a comment - That issue removed the link to the actual activity but still lists it. I was thinking it would be best to not display it it all, especially if the condition is set to 'Hide Entirely'. This is on the user report and affects all grade items.
          Hide
          Michael de Raadt added a comment -

          Thanks for the extra info, Mark.

          Show
          Michael de Raadt added a comment - Thanks for the extra info, Mark.
          Hide
          Michael de Raadt added a comment -

          Sam: I've assigned this to you as you were involved in the linked issue. Feel free to reassign it.

          Show
          Michael de Raadt added a comment - Sam: I've assigned this to you as you were involved in the linked issue. Feel free to reassign it.
          Hide
          Sam Marshall added a comment -

          I think this is a gradebook bug. The gradebook should check whether an activity is available to the user as follows. Assume:

          $cm = $modinfo->get_cm($cmid)

          It sounds like the gradebook is already checking

          $cm->uservisible

          If this is false, then the item should not be a link, but it might still appear. On the main page, we still display items you cannot access if they have $cm->showavailability == 1 ('show greyed out'). However, in other places like the navigation, if $cm->uservisible is false, we do not display it at all.

          One or other of these approaches should be adopted for gradebook. I think checking showavailability might be right for this case. If so then the logic is:

          $cm->uservisible = true: show link as normal
          !$cm->uservisible && $cm->showavailability: show link greyed out
          !$cm->uservisible && !$cm->showavailability: do not show row at all

          Reassigning to whoever is in charge of gradebook (er, I hope, assuming I got that right in the interface) to make the decision - I assume the actual fix should be easy, although I don't know anything about gradebook code.

          Note that this is not a regression or anything - it's always been like that.

          Show
          Sam Marshall added a comment - I think this is a gradebook bug. The gradebook should check whether an activity is available to the user as follows. Assume: $cm = $modinfo->get_cm($cmid) It sounds like the gradebook is already checking $cm->uservisible If this is false, then the item should not be a link, but it might still appear. On the main page, we still display items you cannot access if they have $cm->showavailability == 1 ('show greyed out'). However, in other places like the navigation, if $cm->uservisible is false, we do not display it at all. One or other of these approaches should be adopted for gradebook. I think checking showavailability might be right for this case. If so then the logic is: $cm->uservisible = true: show link as normal !$cm->uservisible && $cm->showavailability: show link greyed out !$cm->uservisible && !$cm->showavailability: do not show row at all Reassigning to whoever is in charge of gradebook (er, I hope, assuming I got that right in the interface) to make the decision - I assume the actual fix should be easy, although I don't know anything about gradebook code. Note that this is not a regression or anything - it's always been like that.
          Hide
          Sam Marshall added a comment -

          (apparently I have to remove conditional activities from the list to get it to reassign, I can't just change the order...)

          Show
          Sam Marshall added a comment - (apparently I have to remove conditional activities from the list to get it to reassign, I can't just change the order...)
          Hide
          Andrew Davis added a comment -

          I dont seem to be able to replicate this. For me if a student doesnt meet the conditions, regardless of whether its set to show greyed out or hide entirely, the assignment doesnt appear in the user report when you log in as a student.

          Which is wrong but in the opposite way to what is described here.

          Show
          Andrew Davis added a comment - I dont seem to be able to replicate this. For me if a student doesnt meet the conditions, regardless of whether its set to show greyed out or hide entirely, the assignment doesnt appear in the user report when you log in as a student. Which is wrong but in the opposite way to what is described here.
          Hide
          Mark Nelson added a comment -

          Sam were you able to replicate this issue? If so, I think there is something weird going on with Andrew's install.

          Show
          Mark Nelson added a comment - Sam were you able to replicate this issue? If so, I think there is something weird going on with Andrew's install.
          Hide
          Andrew Davis added a comment - - edited

          Im attaching a zip file of screenshots. The important activities are two assignments a2 and old a2. One is mod_assign, the other is mod_assignment.

          Both should appear greyed out to the student but are being hidden entirely.

          Note that when taking the screenshots I was running this code (https://github.com/andyjdavis/moodle/compare/master...MDL-33117_grade_display). After switching back to master I still can't see the activities but the user report formatting is messed up.

          Show
          Andrew Davis added a comment - - edited Im attaching a zip file of screenshots. The important activities are two assignments a2 and old a2. One is mod_assign, the other is mod_assignment. Both should appear greyed out to the student but are being hidden entirely. Note that when taking the screenshots I was running this code ( https://github.com/andyjdavis/moodle/compare/master...MDL-33117_grade_display ). After switching back to master I still can't see the activities but the user report formatting is messed up.
          Hide
          Andrew Davis added a comment -

          Update: The activities appear if I replicate this set up in current integration. This is a new bug.

          Show
          Andrew Davis added a comment - Update: The activities appear if I replicate this set up in current integration. This is a new bug.
          Hide
          Sam Marshall added a comment -

          Apologies - I assumed this was an existing problem. If it's a new problem then it's a bit more serious than I had assumed.

          Show
          Sam Marshall added a comment - Apologies - I assumed this was an existing problem. If it's a new problem then it's a bit more serious than I had assumed.
          Hide
          Sam Marshall added a comment -

          Notes:

          1) It was reported against 2.2.5 and 2.3 so it shouldn't be a new problem.
          2) If the gradebook doesn't show lines at all when they are not uservisible (ie what Andrew is reporting happened), that's OK I should think.

          Show
          Sam Marshall added a comment - Notes: 1) It was reported against 2.2.5 and 2.3 so it shouldn't be a new problem. 2) If the gradebook doesn't show lines at all when they are not uservisible (ie what Andrew is reporting happened), that's OK I should think.
          Hide
          Sam Marshall added a comment -

          To confirm Andrew's findings: I am NOT able to reproduce this in current OU moodle dev version, which is based on 2.3.2.

          (I used OU moodle because it's so much easier to switch user, love our 'log in as example student' one-click block... but we have not customised gradebook so result should be consisted.)

          Show
          Sam Marshall added a comment - To confirm Andrew's findings: I am NOT able to reproduce this in current OU moodle dev version, which is based on 2.3.2. (I used OU moodle because it's so much easier to switch user, love our 'log in as example student' one-click block... but we have not customised gradebook so result should be consisted.)
          Hide
          Andrew Davis added a comment -

          I just pulled the latest changes into my master and rebased my fix for MDL-33117. I am now experiencing this bug in that branch. Whatever caused it was introduced into master in the last few days.

          Note: If the student doesn't meet the activity conditions and the activity is set to "show greyed out" it functions perfectly. If the activity is set to "hide activity entirely" it still appears greyed out.

          Just in case anyone is unclear on the problem here

          Show
          Andrew Davis added a comment - I just pulled the latest changes into my master and rebased my fix for MDL-33117 . I am now experiencing this bug in that branch. Whatever caused it was introduced into master in the last few days. Note: If the student doesn't meet the activity conditions and the activity is set to "show greyed out" it functions perfectly. If the activity is set to "hide activity entirely" it still appears greyed out. Just in case anyone is unclear on the problem here
          Hide
          Andrew Davis added a comment -

          Its possible this was introduced by MDL-34931. Attempting to definitely prove it was or was not now.

          Show
          Andrew Davis added a comment - Its possible this was introduced by MDL-34931 . Attempting to definitely prove it was or was not now.
          Hide
          Andrew Davis added a comment -

          I think I've proven that this was introduced by commit 5fee56d5c352708fba20d7caec9f5b4c6e133217 in MDL-34931. I created a branch of stable master, reverted that commit and the assignment that the student should not have been able to see disappeared.

          Show
          Andrew Davis added a comment - I think I've proven that this was introduced by commit 5fee56d5c352708fba20d7caec9f5b4c6e133217 in MDL-34931 . I created a branch of stable master, reverted that commit and the assignment that the student should not have been able to see disappeared.
          Hide
          Andrew Davis added a comment -

          Requesting a peer review. I'm going to do more testing on this and would encourage anyone who has some time to do the same.

          Show
          Andrew Davis added a comment - Requesting a peer review. I'm going to do more testing on this and would encourage anyone who has some time to do the same.
          Hide
          Frédéric Massart added a comment - - edited

          Hi Andrew,

          [Y] Syntax
          [-] Output
          [N] Whitespace
          [-] Language
          [-] Databases
          [Y] Testing
          [Y] Security
          [N] Documentation
          [Y] Git
          [Y] Sanity check

          • There is an extra whitespace in the if condition
          • The documentation a few lines above the condition should be updated.

          I have done some basic testing and either I do not understand how this is supposed to work, or it does not. Basically what I have done was having a hidden assignment, and an assignment hidden by conditional access. Both were hidden in the gradebook, even if I unhide them.

          From what I understood, the gradebook makes priority on the visibility of the grade and should display it the gradebook itself does not hide it. But here, we check if the activity is visible to the users, not the grade itself.

          By looking at the code, it appears that cm_info::$available detains the information we are looking for. I think not relating on the activity visibility was one of the reason I introduced the function is_access_restricted_by_group(), so that I could check if the activity was hidden because of that reason. Perhaps we could add a new function is_access_restricted_by_conditional_access(), but it is maybe not necessary.

          Cheers!

          /me sighs... pressed the wrong button

          Show
          Frédéric Massart added a comment - - edited Hi Andrew, [Y] Syntax [-] Output [N] Whitespace [-] Language [-] Databases [Y] Testing [Y] Security [N] Documentation [Y] Git [Y] Sanity check There is an extra whitespace in the if condition The documentation a few lines above the condition should be updated. I have done some basic testing and either I do not understand how this is supposed to work, or it does not. Basically what I have done was having a hidden assignment, and an assignment hidden by conditional access. Both were hidden in the gradebook, even if I unhide them. From what I understood, the gradebook makes priority on the visibility of the grade and should display it the gradebook itself does not hide it. But here, we check if the activity is visible to the users, not the grade itself. By looking at the code, it appears that cm_info::$available detains the information we are looking for. I think not relating on the activity visibility was one of the reason I introduced the function is_access_restricted_by_group(), so that I could check if the activity was hidden because of that reason. Perhaps we could add a new function is_access_restricted_by_conditional_access(), but it is maybe not necessary. Cheers! /me sighs... pressed the wrong button
          Hide
          Aparup Banerjee added a comment -

          reopened for Fred by Fred (request)

          Show
          Aparup Banerjee added a comment - reopened for Fred by Fred (request)
          Hide
          Andrew Davis added a comment -

          I've fixed the white space issue and updated the comment in code.

          I'm attaching a zip file contain some screenshots so you can see what I'm seeing. There are a bunch, hence the zip file. Ive named the screenshots and the activities in my test course so they're hopefully self explanatory. Some things to note:

          1) The activity "activity hidden on course but shown in GB" is hidden on the course page so it doesnt appear on the course page for the student. It has been made visible on the categories and items page so it appears on the user report for the student.

          2) "From what I understood, the gradebook makes priority on the visibility of the grade and should display it the gradebook itself does not hide it."
          I don't believe this to be true. Certainly if an activity is hidden on the course page but made visible in the gradebook it should appear in the gradebook. However, I believe that if an activity is hidden entirely by conditional availability that it should be hidden both on the course page and in the gradebook. The visible flag does not overrule conditional availability. <--- This is just my current understanding.

          Show
          Andrew Davis added a comment - I've fixed the white space issue and updated the comment in code. I'm attaching a zip file contain some screenshots so you can see what I'm seeing. There are a bunch, hence the zip file. Ive named the screenshots and the activities in my test course so they're hopefully self explanatory. Some things to note: 1) The activity "activity hidden on course but shown in GB" is hidden on the course page so it doesnt appear on the course page for the student. It has been made visible on the categories and items page so it appears on the user report for the student. 2) "From what I understood, the gradebook makes priority on the visibility of the grade and should display it the gradebook itself does not hide it." I don't believe this to be true. Certainly if an activity is hidden on the course page but made visible in the gradebook it should appear in the gradebook. However, I believe that if an activity is hidden entirely by conditional availability that it should be hidden both on the course page and in the gradebook. The visible flag does not overrule conditional availability. <--- This is just my current understanding.
          Hide
          Andrew Davis added a comment - - edited

          When I turn conditional access off at the site level I get the following error when accessing the user report.

          Notice: Use of undefined constant CONDITION_STUDENTVIEW_SHOW - assumed 'CONDITION_STUDENTVIEW_SHOW' in /home/andrew/Desktop/code/moodle/dev/master/grade/report/user/lib.php on line 357

          Also, it appears that an activity that is hidden on the course page but made visible in the gradebook will NOT appear in the user report if conditional access is disabled.

          Show
          Andrew Davis added a comment - - edited When I turn conditional access off at the site level I get the following error when accessing the user report. Notice: Use of undefined constant CONDITION_STUDENTVIEW_SHOW - assumed 'CONDITION_STUDENTVIEW_SHOW' in /home/andrew/Desktop/code/moodle/dev/master/grade/report/user/lib.php on line 357 Also, it appears that an activity that is hidden on the course page but made visible in the gradebook will NOT appear in the user report if conditional access is disabled.
          Hide
          Frédéric Massart added a comment -

          I think for testing purposes, we should also try with hidden sections.

          I'd personally go with the following:

          • Section A, visible
            • Activity 1 visible, no conditional access rules
            • Activity 2 visible, conditional access greying out
            • Activity 3 visible, conditional access hiding completely
            • Actibity 4 hidden, no conditional access rules
          • Section B, hidden
            • Activity 5, no conditional access rules (will be hidden by the section)
          1. Give grade on each activity
          2. As the graded student, make sure you CANNOT see the grades of 3, 4 and 5, but you can 1 and 2.
          3. Unhide 1, 2, 3, 4, 5 in the gradebook if they are
          4. As the graded student, make sure you CAN see the grades of 1, 2, 4 and 5, but not 3.

          Would this logic be correct?

          Show
          Frédéric Massart added a comment - I think for testing purposes, we should also try with hidden sections. I'd personally go with the following: Section A, visible Activity 1 visible, no conditional access rules Activity 2 visible, conditional access greying out Activity 3 visible, conditional access hiding completely Actibity 4 hidden, no conditional access rules Section B, hidden Activity 5, no conditional access rules (will be hidden by the section) Give grade on each activity As the graded student, make sure you CANNOT see the grades of 3, 4 and 5, but you can 1 and 2. Unhide 1, 2, 3, 4, 5 in the gradebook if they are As the graded student, make sure you CAN see the grades of 1, 2, 4 and 5, but not 3. Would this logic be correct?
          Hide
          Andrew Davis added a comment -

          Done a bunch of testing and I think I've got this worked out. I've also further expanded the testing instructions.

          Show
          Andrew Davis added a comment - Done a bunch of testing and I think I've got this worked out. I've also further expanded the testing instructions.
          Hide
          Andrew Davis added a comment -

          Putting this up for peer review again. Be aware that I've rebased so if you're going to check out the branch you'd probably be better off deleting your copy first then checking it out again.

          Show
          Andrew Davis added a comment - Putting this up for peer review again. Be aware that I've rebased so if you're going to check out the branch you'd probably be better off deleting your copy first then checking it out again.
          Hide
          Frédéric Massart added a comment - - edited

          Hi Andrew, will try to summarise what we discussed about, and maybe a few other things.

          • Apparently there are some random behaviours which depend on the grey'd out/hidden entirely property of conditional access on activity, even though conditional access does not have rules.
          • I noticed that if you hide a section, but unhide an activity in that section in the Gradebook, it is not displayed in the GB.

          Maybe to clarify the code, and avoid future possible regressions, that would be handy to create a new function within the module_info class to return if the activity has been hidden for that particular reason (A bit like is_user_access_restricted_by_group why not is_hidden_by_conditional_access). Doing so makes it easier to maintain the logic without having to edit the gradebook if for any reason there are new visibility options/scenarios. I'd also keep the is_user_access_restricted_by_group test in the gradebook as it clarifies the goal of this test.

          And, let me tell you that you have all my respect for working in the Gradebook that often .

          (I attached a test course which should contain all the possible scenarios, the 2 students were 's1' and 's2')

          Show
          Frédéric Massart added a comment - - edited Hi Andrew, will try to summarise what we discussed about, and maybe a few other things. Apparently there are some random behaviours which depend on the grey'd out/hidden entirely property of conditional access on activity, even though conditional access does not have rules. I noticed that if you hide a section, but unhide an activity in that section in the Gradebook, it is not displayed in the GB. Maybe to clarify the code, and avoid future possible regressions, that would be handy to create a new function within the module_info class to return if the activity has been hidden for that particular reason (A bit like is_user_access_restricted_by_group why not is_hidden_by_conditional_access ). Doing so makes it easier to maintain the logic without having to edit the gradebook if for any reason there are new visibility options/scenarios. I'd also keep the is_user_access_restricted_by_group test in the gradebook as it clarifies the goal of this test. And, let me tell you that you have all my respect for working in the Gradebook that often . (I attached a test course which should contain all the possible scenarios, the 2 students were 's1' and 's2')
          Hide
          Andrew Davis added a comment -

          The two bullet pointed behaviours you mentioned are almost certainly caused by the same bug. I'm creating a is_user_access_restricted_by_conditional_access() now. I should probably also add some unit tests while I'm at it.

          Show
          Andrew Davis added a comment - The two bullet pointed behaviours you mentioned are almost certainly caused by the same bug. I'm creating a is_user_access_restricted_by_conditional_access() now. I should probably also add some unit tests while I'm at it.
          Hide
          Andrew Davis added a comment -

          There is an updated version at https://github.com/andyjdavis/moodle/compare/master...MDL-35762_fix

          Fred is taking a look now.

          Remaining things I have to do:

          1) In is_user_access_restricted_by_conditional_access() is there a way to get the condition info other than reloading it?

          2) unit tests for is_user_access_restricted_by_conditional_access() and is_user_access_restricted_by_group()

          Show
          Andrew Davis added a comment - There is an updated version at https://github.com/andyjdavis/moodle/compare/master...MDL-35762_fix Fred is taking a look now. Remaining things I have to do: 1) In is_user_access_restricted_by_conditional_access() is there a way to get the condition info other than reloading it? 2) unit tests for is_user_access_restricted_by_conditional_access() and is_user_access_restricted_by_group()
          Hide
          Andrew Davis added a comment -

          Just pushed an updated version. All that's left to do is the unit tests.

          Show
          Andrew Davis added a comment - Just pushed an updated version. All that's left to do is the unit tests.
          Hide
          Andrew Davis added a comment -

          Unit tests are done. Putting this up for peer review.

          Show
          Andrew Davis added a comment - Unit tests are done. Putting this up for peer review.
          Hide
          Frédéric Massart added a comment -

          Hi Andrew,

          thanks for the unit tests, it's great! Hopefully we will have less regression created in the future, great job!

          I think this can go for integration now, I noticed a few fullstop missing at the end of some comments, but that's all.

          Also, I wonder if the right way of PHP Unit Testing those new methods is to manually overwrite the class variables of modinfo, or to change the settings in the course/activites and get modinfo to assign them correcty. Anyway, I think this issue has hang here for long enough and can jump to the next step, I was just expressing my thoughts.

          Cheers

          Show
          Frédéric Massart added a comment - Hi Andrew, thanks for the unit tests, it's great! Hopefully we will have less regression created in the future, great job! I think this can go for integration now, I noticed a few fullstop missing at the end of some comments, but that's all. Also, I wonder if the right way of PHP Unit Testing those new methods is to manually overwrite the class variables of modinfo, or to change the settings in the course/activites and get modinfo to assign them correcty. Anyway, I think this issue has hang here for long enough and can jump to the next step, I was just expressing my thoughts. Cheers
          Hide
          Andrew Davis added a comment -

          Rebased and sending for integration.

          re the unit tests it was a conscious decision to manually set the flags Vs setting up objects to have them automatically set to limit the amount of set up code required. It would also be just as valid to do all the set up.

          Show
          Andrew Davis added a comment - Rebased and sending for integration. re the unit tests it was a conscious decision to manually set the flags Vs setting up objects to have them automatically set to limit the amount of set up code required. It would also be just as valid to do all the set up.
          Hide
          Dan Poltawski 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
          Dan Poltawski 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 -

          Two comments:

          • added one extra commit on master to fix deprecated use of get_fast_modinfo() reset.
          • as far as this is a regression caused by MDL-34931 this has been backported to 22_STABLE and 23_STABLE. Note the phpunit tests have NOT been backported because the lack of assign generators in 23_STABLE and phpunit in 22_STABLE.

          So I'm amending testing-instructions to enforce them to pass explicitly under 22 and 23.

          Show
          Eloy Lafuente (stronk7) added a comment - Two comments: added one extra commit on master to fix deprecated use of get_fast_modinfo() reset. as far as this is a regression caused by MDL-34931 this has been backported to 22_STABLE and 23_STABLE. Note the phpunit tests have NOT been backported because the lack of assign generators in 23_STABLE and phpunit in 22_STABLE. So I'm amending testing-instructions to enforce them to pass explicitly under 22 and 23.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Adding the qa_test_required label because this sounds like a good candidate of multiple combinations and expected results in the gradebook, FYC.

          Show
          Eloy Lafuente (stronk7) added a comment - Adding the qa_test_required label because this sounds like a good candidate of multiple combinations and expected results in the gradebook, FYC.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (22, 23 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 & master), thanks!
          Hide
          Michael de Raadt added a comment -

          Test result: Success! (finally)

          Tested in 2.2, 2.3 and master.

          I was not able to test MDL-34931 in master as there was a problem with creating groups.

          Show
          Michael de Raadt added a comment - Test result: Success! (finally) Tested in 2.2, 2.3 and master. I was not able to test MDL-34931 in master as there was a problem with creating groups.
          Hide
          Michael de Raadt added a comment -

          In relation to other security issues, I don't believe this needs to be reported as a security issue.

          While there is some unnecessary information about activities being revealed to students it is definitely a serious bug, but while students cannot access those activities, I don't believe this is significant enough to report as a security issue to the entire community.

          Show
          Michael de Raadt added a comment - In relation to other security issues, I don't believe this needs to be reported as a security issue. While there is some unnecessary information about activities being revealed to students it is definitely a serious bug, but while students cannot access those activities, I don't believe this is significant enough to report as a security issue to the entire community.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: