Moodle
  1. Moodle
  2. MDL-36095

Section availability display to teachers is not consistent with activity availability

    Details

    • Testing Instructions:
      Hide

      (0. Availability must be enabled in server settings.)

      1. Create a new course with default settings. Enrol a test user as student in the course.
      2. In the first section, create an activity set to be 'available until...' some time in 2003.
      3. Edit settings for the second section to roughly the same ('available until...' 2003). Set the availability information to 'display greyed-out' (note: the default is different for annoying reasons, basically it is not the form default, but because the section already exists in database with the 0 value).
      4. Check the availability message displayed (to admin) against the activity and the section.
      - The two messages should be consistent, along the lines of:
      Restricted: 'Available until 16 October 2003.'

      5. In the third section, add an activity. This time set the 'available from' date to the future.
      6. Edit settings for the fourth section, again to set 'available from' to the future and turn on the 'show greyed-out'.
      - Again the two messages should be consistent.

      7. Log in as the test student.
      - The first activity and section should be totally hidden even though 'show greyed-out' was turned on. (Note: This is a special case for 'available until' - there are comments explaining this in the code. Because 'available until' is normally used in order to reduce clutter from the page, it doesn't make sense to have it appear in that case as students do not need to know that something used to be available. Anyway, it might be weird, but let's be consistent between activities and sections.)
      - The second activity and section should both show with the 'Available from 17 October 2014.' message. (Note this is a slightly shorter message than the one that shows to staff. The distinction is that the staff messages appear even when you personally have access to it, while the student ones only ever appear when appropriate to that student.)

      Show
      (0. Availability must be enabled in server settings.) 1. Create a new course with default settings. Enrol a test user as student in the course. 2. In the first section, create an activity set to be 'available until...' some time in 2003. 3. Edit settings for the second section to roughly the same ('available until...' 2003). Set the availability information to 'display greyed-out' (note: the default is different for annoying reasons, basically it is not the form default, but because the section already exists in database with the 0 value). 4. Check the availability message displayed (to admin) against the activity and the section. - The two messages should be consistent, along the lines of: Restricted: 'Available until 16 October 2003.' 5. In the third section, add an activity. This time set the 'available from' date to the future. 6. Edit settings for the fourth section, again to set 'available from' to the future and turn on the 'show greyed-out'. - Again the two messages should be consistent. 7. Log in as the test student. - The first activity and section should be totally hidden even though 'show greyed-out' was turned on. (Note: This is a special case for 'available until' - there are comments explaining this in the code. Because 'available until' is normally used in order to reduce clutter from the page, it doesn't make sense to have it appear in that case as students do not need to know that something used to be available. Anyway, it might be weird, but let's be consistent between activities and sections.) - The second activity and section should both show with the 'Available from 17 October 2014.' message. (Note this is a slightly shorter message than the one that shows to staff. The distinction is that the staff messages appear even when you personally have access to it, while the student ones only ever appear when appropriate to that student.)
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-36095-m24
    • Pull Master Branch:
      MDL-36095-master
    • Rank:
      44857

      Description

      In section summary teacher can define available from and available to.
      If the date 'available from' is reached the section is available for students. If the date 'available to' is set this is not shown to teachers. Students get no access from this date, but teachers can't see that the section isn't anymore available for students. Teachers see section normaly as available.

        Issue Links

          Activity

          Hide
          Sam Marshall added a comment -

          Confirmed on qa.moodle.net. The 'available from' date is displayed, but an 'available until' date (in the past) is not shown.

          Show
          Sam Marshall added a comment - Confirmed on qa.moodle.net. The 'available from' date is displayed, but an 'available until' date (in the past) is not shown.
          Hide
          Sam Marshall added a comment -

          Note: The 'available until' date IS shown in the equivalent situation for a single activity, so this is definitely inconsistent.

          Show
          Sam Marshall added a comment - Note: The 'available until' date IS shown in the equivalent situation for a single activity, so this is definitely inconsistent.
          Hide
          Sam Marshall added a comment -

          I have made the following changes to improve consistency:

          1) Copied the logic for activity availability message to the part where it displays section availability message. There are supposed to be different messages for students/staff, and this wasn't done the same way for sections. For students, the 'available until' message intentionally never shows.

          2) When you edit section settings, the default (if not turned on yet) availability dates are now set to midnight. This is for consistency, activities already have this behaviour. Using midnight dates makes the dates look better to students as they omit the time (if you're in the same timezone).

          3) Changed it to hide sections entirely if they are not available and have no explanatory text, even if it is set to 'show greyed-out', same as it does for activities. (See note in test script for explanation of this behaviour. There are also comments in code. It's odd, but I think it should be consistent.)

          Not entirely related but I also removed an unnecessary setDefault line in the edit section form which didn't do anything because that item is always set by data anyway.

          Will now submit for code review.

          Show
          Sam Marshall added a comment - I have made the following changes to improve consistency: 1) Copied the logic for activity availability message to the part where it displays section availability message. There are supposed to be different messages for students/staff, and this wasn't done the same way for sections. For students, the 'available until' message intentionally never shows. 2) When you edit section settings, the default (if not turned on yet) availability dates are now set to midnight. This is for consistency, activities already have this behaviour. Using midnight dates makes the dates look better to students as they omit the time (if you're in the same timezone). 3) Changed it to hide sections entirely if they are not available and have no explanatory text, even if it is set to 'show greyed-out', same as it does for activities. (See note in test script for explanation of this behaviour. There are also comments in code. It's odd, but I think it should be consistent.) Not entirely related but I also removed an unnecessary setDefault line in the edit section form which didn't do anything because that item is always set by data anyway. Will now submit for code review.
          Hide
          Sam Marshall added a comment -

          Please could somebody review this. Notes:

          1) There is some weird behaviour, mentioned - this is intended, it should be consistent with activities. If we want to change it, that can be a separate issue for later.

          2) The test script doesn't include it because I think it uses the same code, but this does appear to work for section-on-own-page view as well as the default view.

          3) I have only done a patch for master at the moment. If the review passes, I would expect to do one for 2.3 as well since this is a bugfix. (Conditional availability for sections is a 2.3 feature so there is no need for 2.2 patch.)

          Show
          Sam Marshall added a comment - Please could somebody review this. Notes: 1) There is some weird behaviour, mentioned - this is intended, it should be consistent with activities. If we want to change it, that can be a separate issue for later. 2) The test script doesn't include it because I think it uses the same code, but this does appear to work for section-on-own-page view as well as the default view. 3) I have only done a patch for master at the moment. If the review passes, I would expect to do one for 2.3 as well since this is a bugfix. (Conditional availability for sections is a 2.3 feature so there is no need for 2.2 patch.)
          Hide
          Ralf Hilgenstock added a comment -

          Thanks. Really quick reaction and patch.

          Show
          Ralf Hilgenstock added a comment - Thanks. Really quick reaction and patch.
          Hide
          Dan Poltawski added a comment -

          Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..

          Show
          Dan Poltawski added a comment - Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..
          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
          Dan Poltawski added a comment -

          Sending this back for peer review as it isn't yet in state for integration (needs new branches etc)

          Show
          Dan Poltawski added a comment - Sending this back for peer review as it isn't yet in state for integration (needs new branches etc)
          Hide
          Damyon Wiese added a comment -

          Thanks Sam,

          This patch is almost perfect. I found one minor white space issue:

          if($fullinfo) {
          

          Needs a space after the if.

          Can you please fix this issue and then add branches to backport this? (I also had to resolve a minor conflict when I rebased it onto the latest master).

          Full checklist:
          [Y] Syntax
          [Y] Output
          [N] Whitespace - (One minor issue)
          [Y] Language
          [-] Databases
          [Y] Testing
          [-] Security
          [-] Documentation
          [N] Git - Just needs rebasing and backporting
          [Y] Sanity check

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Thanks Sam, This patch is almost perfect. I found one minor white space issue: if ($fullinfo) { Needs a space after the if. Can you please fix this issue and then add branches to backport this? (I also had to resolve a minor conflict when I rebased it onto the latest master). Full checklist: [Y] Syntax [Y] Output [N] Whitespace - (One minor issue) [Y] Language [-] Databases [Y] Testing [-] Security [-] Documentation [N] Git - Just needs rebasing and backporting [Y] Sanity check Regards, Damyon
          Hide
          Sam Marshall added a comment -

          Thanks for review! To update - I am back at work today, and hope to do the correction & backport on this today or tomorrow.

          Show
          Sam Marshall added a comment - Thanks for review! To update - I am back at work today, and hope to do the correction & backport on this today or tomorrow.
          Hide
          Sam Marshall added a comment -

          Have now rebased (a small part of the changes is no longer necessary in 2.4 and master because somebody else did a similar thing) and backported. Just checking it again now...

          Show
          Sam Marshall added a comment - Have now rebased (a small part of the changes is no longer necessary in 2.4 and master because somebody else did a similar thing) and backported. Just checking it again now...
          Hide
          Sam Marshall added a comment -

          I ran through the test again on master after fixing the issue raised and doing the backport, and it seems OK, so submitting for integration

          Note: The 2.3 patch is basically the same as the one I submitted originally, except for whitespace change. Master and 2.4 ones, which are identical to each other, remove a change about default value for showavailability which is no longer needed.

          Show
          Sam Marshall added a comment - I ran through the test again on master after fixing the issue raised and doing the backport, and it seems OK, so submitting for integration Note: The 2.3 patch is basically the same as the one I submitted originally, except for whitespace change. Master and 2.4 ones, which are identical to each other, remove a change about default value for showavailability which is no longer needed.
          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 Hemelryk added a comment -

          Thanks Sam, changes look spot on and have been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Sam, changes look spot on and have been integrated now.
          Hide
          Jason Fowler added a comment -

          All fine, thanks Sam

          Show
          Jason Fowler added a comment - All fine, thanks Sam
          Hide
          Dan Poltawski added a comment -

          Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

          Show
          Dan Poltawski added a comment - Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!
          Hide
          Frédéric Massart added a comment -

          I was redirected here while debugging MDL-41893. Can I have someone to confirm that an activity/section should "disappear" when it becomes unavailable. Even though it is set to be "grey'd out"?

          Show
          Frédéric Massart added a comment - I was redirected here while debugging MDL-41893 . Can I have someone to confirm that an activity/section should "disappear" when it becomes unavailable. Even though it is set to be "grey'd out"?
          Hide
          Sam Marshall added a comment -

          Hi Fred.

          The special case here is specific to 'Available until' and not to any other mechanism that controls whether something is unavailable. For every other mechanism, it should be greyed out.

          It is this use case, which was raised as a requirement when we originally developed the feature (waaaaaaaay back). You have an activity which is available from 1 July until 7 July. You would like students to know about the upcoming activity (so you choose 'show greyed out') so that they can see when it is coming and plan to carry out the activity. But after the activity has finished you would like it to completely disappear because it's just cluttering up the page: if they missed it, they missed it. Because there is only one setting for whether it's visible greyed out or not, there is this special case for 'available until'.

          To put it another way, the logic is that with 'available until', there is no point having it visible to students after availability ends, because they will never be able to access it. With the other settings such as 'available from', there is a point to showing the info because students can see it's coming and plan to come back at the right time to do it.

          I've written this about activities. Regarding sections these are just supposed to be consistent with activities.

          I don't know how important this use case is, and whether it's worth the slightly confusing behaviour or not!

          Show
          Sam Marshall added a comment - Hi Fred. The special case here is specific to 'Available until' and not to any other mechanism that controls whether something is unavailable. For every other mechanism, it should be greyed out. It is this use case, which was raised as a requirement when we originally developed the feature (waaaaaaaay back). You have an activity which is available from 1 July until 7 July. You would like students to know about the upcoming activity (so you choose 'show greyed out') so that they can see when it is coming and plan to carry out the activity. But after the activity has finished you would like it to completely disappear because it's just cluttering up the page: if they missed it, they missed it. Because there is only one setting for whether it's visible greyed out or not, there is this special case for 'available until'. To put it another way, the logic is that with 'available until', there is no point having it visible to students after availability ends, because they will never be able to access it. With the other settings such as 'available from', there is a point to showing the info because students can see it's coming and plan to come back at the right time to do it. I've written this about activities. Regarding sections these are just supposed to be consistent with activities. I don't know how important this use case is, and whether it's worth the slightly confusing behaviour or not!
          Hide
          Frédéric Massart added a comment -

          Thanks for clarifying this Sam. I don't know how either if it's a confusing behaviour. Perhaps we should document this somewhere.

          Show
          Frédéric Massart added a comment - Thanks for clarifying this Sam. I don't know how either if it's a confusing behaviour. Perhaps we should document this somewhere.
          Hide
          Dan Poltawski added a comment -

          Maybe it would be less confusing it is was called 'greyed before available' or something like that?

          Show
          Dan Poltawski added a comment - Maybe it would be less confusing it is was called 'greyed before available' or something like that?
          Hide
          Séverin Terrier added a comment -

          Yes, "Greyed before available" seems fine for me, better than the actual sentence...

          Show
          Séverin Terrier added a comment - Yes, "Greyed before available" seems fine for me, better than the actual sentence...

            People

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

              Dates

              • Created:
                Updated:
                Resolved: